Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve discrepancy between on-chain and off-chain elasticity_multiplier values. #11149

Closed
geoknee opened this issue Jul 15, 2024 · 3 comments
Closed

Comments

@geoknee
Copy link
Contributor

geoknee commented Jul 15, 2024

Off chain

op-geth uses a static elasticity multiplier set in the chain config. See here and here.

This value is currently hardcoded to 6.

On chain

We also store an elasticityMultiplier inside the ResourceConfig struct:

/// value.
struct ResourceConfig {
uint32 maxResourceLimit;
uint8 elasticityMultiplier;
uint8 baseFeeMaxChangeDenominator;

in the SytemConfig contract:

/// @notice The configuration for the deposit fee market.
/// Used by the OptimismPortal to meter the cost of buying L2 gas on L1.
/// Set as internal with a getter so that the struct is returned instead of a tuple.
ResourceMetering.ResourceConfig internal _resourceConfig;

On OP Mainnet, the parameter is actually set to 10. See this etherscan link.. I believe this is a historical / out-of-date value.

Moreover, we are actually reading and validating this parameter in the superchain registry (see here), since we have specified that 10 is the standard value.

From what I can tell this parameter is not being read by the off-chain software. Here's a hint that this was once the plan:

/// @notice An internal setter for the resource config.
/// Ensures that the config is sane before storing it by checking for invariants.
/// In the future, this method may emit an event that the `op-node` picks up
/// for when the resource config is changed.
/// @param _config The new resource config.
function _setResourceConfig(ResourceMetering.ResourceConfig memory _config) internal {

I propose that we should either:

  1. Complete that plan so that the elasticity parameter can be changed on chain and picked up off chain
  2. Remove the on chain parameter in the next contracts release to prevent confusion.
@sebastianst
Copy link
Member

If we can't do 1. in Granite or Holocene, it could be worth it doing 2. for the next contracts release to avoid confusion until we add it back?

@geoknee geoknee changed the title Resolve discrepancy between on-chain and off-chain elasticity_multipliervalues. Resolve discrepancy between on-chain and off-chain elasticity_multiplier values. Jul 15, 2024
@tynes
Copy link
Contributor

tynes commented Jul 15, 2024

The ResourceConfig is specific to the deposit tx fee market. It is not used to configure the L2 fee market. Both are separate gas markets with different basefees. Its incredibly complex to maintain an EIP1559 fee market on chain, the state space is just too large to test thoroughly and ensure quality UX (for example out of gas issues happen randomly for end users). I am a big fan of ethereum-optimism/specs#82 which replaces the on chain EIP1559 fee market with a queue, which would remove the oog issues, make deposits cheaper and remove the risk that deposits use up more gas than the L2 gas limit if the system is configured incorrectly.

@geoknee
Copy link
Contributor Author

geoknee commented Jul 15, 2024

Thanks @mark. I wonder if there are any documentation improvements we can make in the short term to prevent confusion on this issue? Otherwise, it looks like there isn't any action necessary after all and this ticket can be closed.

@geoknee geoknee closed this as completed Jul 15, 2024
@geoknee geoknee closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants