This looks like an usual design, not consistent with our code - IMO this static logic does not belong here but can be accessed through MainnetPrecompiledContracts::byzantium and MainnetPrecompiledContracts::osaka. The duty to configure these things should be on the caller (configuration) not the instance itself.

Let me know what you think 9b34bf9

Do you mean static factory pattern is an unusual design or something specific to this?

The usage of static factory follows the design already in place for some precompiles e.g AltBN128AddPrecompiledContract.byzantium

I think it's pretty conventional for the static factory to live with the class it's constructing and be responsible for configuring the "osaka" version of it.

If you look through MainnetProtocolSpecs, a lot of fork configuration is done by wiring in a fork-specific version of a class, which extends the previous one, e.g. GasCalculator.

The difference here is we wire in a static factory instead of a whole new version of the class, since it's only one value that is changed.

However, if there is a performance issue with the extra check for prague though, wiring in e.g. OsakaBigIntegerModularExponentiationPrecompiledContract is more code but might be easier to JIT compile (we can check profiling to confirm).

Let me know what you think 9b34bf9

No objections in general, but I think my approach is more conventional, at least in Besu.
No configuration currently exists in MainnetPrecompiledContracts, it's just wiring. That's mostly the case for MainnetProtocolSpecs too, with some exceptions. Most config happens in the fork-specific classes I believe.

Do you mean static factory pattern is an unusual design or something specific to this?

Not that it's unusual but it looks a bit weird like this IMO. So you have 2 places you have to modify if you want to add a new fork even if such fork only lowers the limits, MainnetPrecompiledContracts and BigIntegerModularExponentiationPrecompiledContract with a corresponding static method.

The usage of static factory follows the design already in place for some precompiles e.g AltBN128AddPrecompiledContract.byzantium

Right... I missed this case. I only looked at populateForPrague and they are all using constructors. I think the one of the problems is not being consistent, as there are 2 different approaches in parallel.

If you look through MainnetProtocolSpecs, a lot of fork configuration is done by wiring in a fork-specific version of a class, which extends the previous one, e.g. GasCalculator.

Not sure I understand the example - that actually seems to prove my point. So you have all of the different parts constructed in MainnetProtocolSpecs and MainnetEVMs not in ProtocolSpec neither in EVM - these simply provide the constructor, not the specific static constructor for the fork, and MainnetProtocolSpecs and MainnetEVMs instantiate with the right parameters. If we had static factories for every fork in every class it would be a lot of boiler plate.

However, if there is a performance issue with the extra check for prague though, wiring in e.g. OsakaBigIntegerModularExponentiationPrecompiledContract is more code but might be easier to JIT compile (we can check profiling to confirm).

Yes, I think you are referring to the size of the method. It impacts JIT compilation for sure. Though these are configutation methods - they are not hot - so performance is not really a concern here.

I also think that the statics here degrade the testability of BigIntegerModularExponentiationPrecompiledContract because now there's no way to inject dependencies into the class for testing since all constructors are private. The first temptation would be to package-private the constructor for testing which makes it even worse.

On a separate segway, not sure why MainnetPrecompiledContracts is an interface since it's clearly a factory class 😆

Need to have a unit test for this

yep, this was first draft for the devnet, planning to productionise next. Good reminder 👍

nit: why using underscores in variable names

This is to match the spec - controversial in java conventions perhaps, but I find it's really useful if the spec and code values match for future reference.

nit: same as https://github.com/hyperledger/besu/pull/8632/files#r2099915626