From 8a685a9dc590eb99689249e662d7e83b1aca3205 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 18 Apr 2024 22:33:33 -0600 Subject: [PATCH] contracts-bedrock: ecotone gas config support Updates the gas config on the `SystemConfig` to be ecotone first. This resolves chain operator ux issues with setting the gas config, rendering the additional tooling for calculating the encoded scalar version obsolete. This version of the system config is technically backwards compatible with pre-ecotone, but it requires a follow up tx to call the old `setGasConfig` method. The old gas config method was updated such that it cannot be used to set ecotone style config to prevent footguns. A new method `setGasConfigEcotone` should be used instead. --- .../scripts/ChainAssertions.sol | 8 ++- .../contracts-bedrock/scripts/Deploy.s.sol | 4 +- .../scripts/DeployConfig.s.sol | 5 ++ .../scripts/go-ffi/differential-testing.go | 28 +++++++++ .../contracts-bedrock/src/L1/SystemConfig.sol | 50 ++++++++++++--- .../test/L1/SystemConfig.t.sol | 61 +++++++++++++++++-- .../test/setup/FFIInterface.sol | 21 +++++++ 7 files changed, 157 insertions(+), 20 deletions(-) diff --git a/packages/contracts-bedrock/scripts/ChainAssertions.sol b/packages/contracts-bedrock/scripts/ChainAssertions.sol index be95d777c4ce..56dd84403850 100644 --- a/packages/contracts-bedrock/scripts/ChainAssertions.sol +++ b/packages/contracts-bedrock/scripts/ChainAssertions.sol @@ -71,8 +71,8 @@ library ChainAssertions { if (_isProxy) { require(config.owner() == _cfg.finalSystemOwner()); - require(config.overhead() == _cfg.gasPriceOracleOverhead()); - require(config.scalar() == _cfg.gasPriceOracleScalar()); + require(config.basefeeScalar() == _cfg.basefeeScalar()); + require(config.blobbasefeeScalar() == _cfg.blobbasefeeScalar()); require(config.batcherHash() == bytes32(uint256(uint160(_cfg.batchSenderAddress())))); require(config.gasLimit() == uint64(_cfg.l2GenesisBlockGasLimit())); require(config.unsafeBlockSigner() == _cfg.p2pSequencerAddress()); @@ -98,7 +98,9 @@ library ChainAssertions { } else { require(config.owner() == address(0xdead)); require(config.overhead() == 0); - require(config.scalar() == 0); + require(config.scalar() == uint256(0x01) << 248); // version 1 + require(config.basefeeScalar() == 0); + require(config.blobbasefeeScalar() == 0); require(config.batcherHash() == bytes32(0)); require(config.gasLimit() == 1); require(config.unsafeBlockSigner() == address(0)); diff --git a/packages/contracts-bedrock/scripts/Deploy.s.sol b/packages/contracts-bedrock/scripts/Deploy.s.sol index ae251fbb2947..02963f4953c5 100644 --- a/packages/contracts-bedrock/scripts/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/Deploy.s.sol @@ -985,8 +985,8 @@ contract Deploy is Deployer { SystemConfig.initialize, ( cfg.finalSystemOwner(), - cfg.gasPriceOracleOverhead(), - cfg.gasPriceOracleScalar(), + cfg.basefeeScalar(), + cfg.blobbasefeeScalar(), batcherHash, uint64(cfg.l2GenesisBlockGasLimit()), cfg.p2pSequencerAddress(), diff --git a/packages/contracts-bedrock/scripts/DeployConfig.s.sol b/packages/contracts-bedrock/scripts/DeployConfig.s.sol index 3eb94f54bccb..2233cdddb4b7 100644 --- a/packages/contracts-bedrock/scripts/DeployConfig.s.sol +++ b/packages/contracts-bedrock/scripts/DeployConfig.s.sol @@ -48,6 +48,8 @@ contract DeployConfig is Script { uint256 public l2GenesisBlockGasLimit; uint256 public gasPriceOracleOverhead; uint256 public gasPriceOracleScalar; + uint32 public basefeeScalar; + uint32 public blobbasefeeScalar; bool public enableGovernance; uint256 public eip1559Denominator; uint256 public eip1559Elasticity; @@ -121,6 +123,9 @@ contract DeployConfig is Script { l2GenesisBlockGasLimit = stdJson.readUint(_json, "$.l2GenesisBlockGasLimit"); gasPriceOracleOverhead = stdJson.readUint(_json, "$.gasPriceOracleOverhead"); gasPriceOracleScalar = stdJson.readUint(_json, "$.gasPriceOracleScalar"); + basefeeScalar = uint32(stdJson.readUint(_json, "$.gasPriceOracleBaseFeeScalar")); + blobbasefeeScalar = uint32(stdJson.readUint(_json, "$.gasPriceOracleBlobBaseFeeScalar")); + enableGovernance = stdJson.readBool(_json, "$.enableGovernance"); eip1559Denominator = stdJson.readUint(_json, "$.eip1559Denominator"); eip1559Elasticity = stdJson.readUint(_json, "$.eip1559Elasticity"); diff --git a/packages/contracts-bedrock/scripts/go-ffi/differential-testing.go b/packages/contracts-bedrock/scripts/go-ffi/differential-testing.go index ebbf889d758b..e53395234d1c 100644 --- a/packages/contracts-bedrock/scripts/go-ffi/differential-testing.go +++ b/packages/contracts-bedrock/scripts/go-ffi/differential-testing.go @@ -10,6 +10,7 @@ import ( "github.com/ethereum-optimism/optimism/cannon/mipsevm" "github.com/ethereum-optimism/optimism/op-bindings/predeploys" "github.com/ethereum-optimism/optimism/op-chain-ops/crossdomain" + "github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -35,6 +36,8 @@ var ( {Type: fixedBytes}, } + uint32Type, _ = abi.NewType("uint32", "", nil) + // Decoded nonce tuple (nonce, version) decodedNonce, _ = abi.NewType("tuple", "DecodedNonce", []abi.ArgumentMarshaling{ {Name: "nonce", Type: "uint256"}, @@ -44,6 +47,12 @@ var ( {Name: "encodedNonce", Type: decodedNonce}, } + // Decoded ecotone scalars (uint32, uint32) + decodedScalars = abi.Arguments{ + {Name: "basefeeScalar", Type: uint32Type}, + {Name: "blobbasefeeScalar", Type: uint32Type}, + } + // WithdrawalHash slot tuple (bytes32, bytes32) withdrawalSlot, _ = abi.NewType("tuple", "SlotHash", []abi.ArgumentMarshaling{ {Name: "withdrawalHash", Type: "bytes32"}, @@ -361,6 +370,25 @@ func DiffTestUtils() { packed, err := cannonMemoryProofArgs.Pack(&output) checkErr(err, "Error encoding output") fmt.Print(hexutil.Encode(packed[32:])) + case "encodeScalarEcotone": + basefeeScalar, err := strconv.ParseUint(args[1], 10, 32) + checkErr(err, "Error decocding basefeeScalar") + blobbasefeeScalar, err := strconv.ParseUint(args[2], 10, 32) + checkErr(err, "Error decocding blobbasefeeScalar") + + encoded := eth.EncodeScalar(eth.EcostoneScalars{ + BaseFeeScalar: uint32(basefeeScalar), + BlobBaseFeeScalar: uint32(blobbasefeeScalar), + }) + fmt.Print(hexutil.Encode(encoded[:])) + case "decodeScalarEcotone": + scalar := common.HexToHash(args[1]) + scalars, err := eth.DecodeScalar([32]byte(scalar[:])) + checkErr(err, "Error decoding scalar") + + packed, err := decodedScalars.Pack(scalars.BaseFeeScalar, scalars.BlobBaseFeeScalar) + checkErr(err, "Error encoding output") + fmt.Print(hexutil.Encode(packed)) default: panic(fmt.Errorf("Unknown command: %s", args[0])) } diff --git a/packages/contracts-bedrock/src/L1/SystemConfig.sol b/packages/contracts-bedrock/src/L1/SystemConfig.sol index 9e4d94dc18a4..aea698dbf743 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfig.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfig.sol @@ -85,9 +85,12 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { uint8 internal constant GAS_PAYING_TOKEN_DECIMALS = 18; /// @notice Fixed L2 gas overhead. Used as part of the L2 fee calculation. + /// Deprecated since the Ecotone network upgrade uint256 public overhead; /// @notice Dynamic L2 gas overhead. Used as part of the L2 fee calculation. + /// The most significant byte is used to determine the version since the + /// Ecotone network upgrade. uint256 public scalar; /// @notice Identifier for the batcher. @@ -98,6 +101,12 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { /// @notice L2 block gas limit. uint64 public gasLimit; + /// @notice Basefee scalar value. Part of the L2 fee calculation since the Ecotone network upgrade. + uint32 public basefeeScalar; + + /// @notice Blobbasefee scalar value. Part of the L2 fee calculation since the Ecotone network upgrade. + uint32 public blobbasefeeScalar; + /// @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. @@ -110,8 +119,8 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data); /// @notice Semantic version. - /// @custom:semver 2.1.0 - string public constant version = "2.1.0"; + /// @custom:semver 2.2.0 + string public constant version = "2.2.0"; /// @notice Constructs the SystemConfig contract. Cannot set /// the owner to `address(0)` due to the Ownable contract's @@ -122,8 +131,8 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { Storage.setUint(START_BLOCK_SLOT, type(uint256).max); initialize({ _owner: address(0xdEaD), - _overhead: 0, - _scalar: 0, + _basefeeScalar: 0, + _blobbasefeeScalar: 0, _batcherHash: bytes32(0), _gasLimit: 1, _unsafeBlockSigner: address(0), @@ -151,8 +160,8 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { /// @notice Initializer. /// The resource config must be set before the require check. /// @param _owner Initial owner of the contract. - /// @param _overhead Initial overhead value. - /// @param _scalar Initial scalar value. + /// @param _basefeeScalar Initial basefee scalar value. + /// @param _blobbasefeeScalar Initial blobbasefee scalar value. /// @param _batcherHash Initial batcher hash. /// @param _gasLimit Initial gas limit. /// @param _unsafeBlockSigner Initial unsafe block signer address. @@ -162,8 +171,8 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { /// @param _addresses Set of L1 contract addresses. These should be the proxies. function initialize( address _owner, - uint256 _overhead, - uint256 _scalar, + uint32 _basefeeScalar, + uint32 _blobbasefeeScalar, bytes32 _batcherHash, uint64 _gasLimit, address _unsafeBlockSigner, @@ -179,7 +188,7 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { // These are set in ascending order of their UpdateTypes. _setBatcherHash(_batcherHash); - _setGasConfig({ _overhead: _overhead, _scalar: _scalar }); + _setGasConfigEcotone({ _basefeeScalar: _basefeeScalar, _blobbasefeeScalar: _blobbasefeeScalar }); _setGasLimit(_gasLimit); Storage.setAddress(UNSAFE_BLOCK_SIGNER_SLOT, _unsafeBlockSigner); @@ -333,6 +342,7 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { } /// @notice Updates gas config. Can only be called by the owner. + /// Deprecated in favor of setGasConfigEcotone since the Ecotone upgrade. /// @param _overhead New overhead value. /// @param _scalar New scalar value. function setGasConfig(uint256 _overhead, uint256 _scalar) external onlyOwner { @@ -343,6 +353,8 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { /// @param _overhead New overhead value. /// @param _scalar New scalar value. function _setGasConfig(uint256 _overhead, uint256 _scalar) internal { + require((uint256(0xff) << 248) & _scalar == 0, "SystemConfig: scalar exceeds max."); + overhead = _overhead; scalar = _scalar; @@ -350,6 +362,26 @@ contract SystemConfig is OwnableUpgradeable, ISemver, IGasToken { emit ConfigUpdate(VERSION, UpdateType.GAS_CONFIG, data); } + /// @notice Updates gas config as of the Ecotone upgrade. Can only be called by the owner. + /// @param _basefeeScalar New basefeeScalar value. + /// @param _blobbasefeeScalar New blobbasefeeScalar value. + function setGasConfigEcotone(uint32 _basefeeScalar, uint32 _blobbasefeeScalar) external onlyOwner { + _setGasConfigEcotone(_basefeeScalar, _blobbasefeeScalar); + } + + /// @notice Internal function for updating the fee scalars as of the Ecotone upgrade. + /// @param _basefeeScalar New basefeeScalar value. + /// @param _blobbasefeeScalar New blobbasefeeScalar value. + function _setGasConfigEcotone(uint32 _basefeeScalar, uint32 _blobbasefeeScalar) internal { + basefeeScalar = _basefeeScalar; + blobbasefeeScalar = _blobbasefeeScalar; + + scalar = (uint256(0x01) << 248) | (uint256(_blobbasefeeScalar) << 32) | _basefeeScalar; + + bytes memory data = abi.encode(overhead, scalar); + emit ConfigUpdate(VERSION, UpdateType.GAS_CONFIG, data); + } + /// @notice Updates the L2 gas limit. Can only be called by the owner. /// @param _gasLimit New gas limit. function setGasLimit(uint64 _gasLimit) external onlyOwner { diff --git a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol index 727867a4be0d..d182c3d71e82 100644 --- a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol @@ -33,6 +33,8 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init { address unsafeBlockSigner; address systemConfigImpl; address optimismMintableERC20Factory; + uint32 basefeeScalar; + uint32 blobbasefeeScalar; function setUp() public virtual override { super.setUp(); @@ -40,6 +42,8 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init { owner = deploy.cfg().finalSystemOwner(); overhead = deploy.cfg().gasPriceOracleOverhead(); scalar = deploy.cfg().gasPriceOracleScalar(); + basefeeScalar = deploy.cfg().basefeeScalar(); + blobbasefeeScalar = deploy.cfg().blobbasefeeScalar(); batcherHash = bytes32(uint256(uint160(deploy.cfg().batchSenderAddress()))); gasLimit = uint64(deploy.cfg().l2GenesisBlockGasLimit()); unsafeBlockSigner = deploy.cfg().p2pSequencerAddress(); @@ -56,6 +60,8 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init { assertEq(impl.batcherHash(), bytes32(0)); assertEq(impl.gasLimit(), 1); assertEq(impl.unsafeBlockSigner(), address(0)); + assertEq(impl.basefeeScalar(), 0); + assertEq(impl.blobbasefeeScalar(), 0); ResourceMetering.ResourceConfig memory actual = impl.resourceConfig(); assertEq(actual.maxResourceLimit, 1); assertEq(actual.elasticityMultiplier, 1); @@ -86,6 +92,8 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init { assertEq(systemConfig.batcherHash(), batcherHash); assertEq(systemConfig.gasLimit(), gasLimit); assertEq(systemConfig.unsafeBlockSigner(), unsafeBlockSigner); + assertEq(systemConfig.basefeeScalar(), basefeeScalar); + assertEq(systemConfig.blobbasefeeScalar(), blobbasefeeScalar); // Depends on `initialize` being called with defaults ResourceMetering.ResourceConfig memory rcfg = Constants.DEFAULT_RESOURCE_CONFIG(); ResourceMetering.ResourceConfig memory actual = systemConfig.resourceConfig(); @@ -127,8 +135,8 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Initialize_Test { vm.expectRevert("SystemConfig: gas limit too low"); systemConfig.initialize({ _owner: alice, - _overhead: 2100, - _scalar: 1000000, + _basefeeScalar: basefeeScalar, + _blobbasefeeScalar: blobbasefeeScalar, _batcherHash: bytes32(hex"abcd"), _gasLimit: minimumGasLimit - 1, _unsafeBlockSigner: address(1), @@ -157,8 +165,8 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Initialize_Test { vm.prank(systemConfig.owner()); systemConfig.initialize({ _owner: alice, - _overhead: 2100, - _scalar: 1000000, + _basefeeScalar: basefeeScalar, + _blobbasefeeScalar: blobbasefeeScalar, _batcherHash: bytes32(hex"abcd"), _gasLimit: gasLimit, _unsafeBlockSigner: address(1), @@ -188,8 +196,8 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Initialize_Test { vm.prank(systemConfig.owner()); systemConfig.initialize({ _owner: alice, - _overhead: 2100, - _scalar: 1000000, + _basefeeScalar: basefeeScalar, + _blobbasefeeScalar: blobbasefeeScalar, _batcherHash: bytes32(hex"abcd"), _gasLimit: gasLimit, _unsafeBlockSigner: address(1), @@ -379,6 +387,24 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init { systemConfig.setGasConfig(0, 0); } + /// @notice Ensures that `setGasConfig` reverts if version byte is set. + function test_setGasConfig_badValues_reverts() external { + vm.prank(systemConfig.owner()); + vm.expectRevert("SystemConfig: scalar exceeds max."); + systemConfig.setGasConfig({ + _overhead: 0, + _scalar: type(uint256).max + }); + } + + function test_setGasConfigEcotone_notOwner_reverts() external { + vm.expectRevert("Ownable: caller is not the owner"); + systemConfig.setGasConfigEcotone({ + _basefeeScalar: 0, + _blobbasefeeScalar: 0 + }); + } + /// @dev Tests that `setGasLimit` reverts if the caller is not the owner. function test_setGasLimit_notOwner_reverts() external { vm.expectRevert("Ownable: caller is not the owner"); @@ -486,6 +512,29 @@ contract SystemConfig_Setters_Test is SystemConfig_Init { assertEq(systemConfig.scalar(), newScalar); } + function testFuzz_setGasConfigEcotone_succeeds(uint32 _basefeeScalar, uint32 _blobbasefeeScalar) external { + bytes32 encoded = ffi.encodeScalarEcotone({ + _basefeeScalar: _basefeeScalar, + _blobbasefeeScalar: _blobbasefeeScalar + }); + + vm.expectEmit(address(systemConfig)); + emit ConfigUpdate(0, SystemConfig.UpdateType.GAS_CONFIG, abi.encode(systemConfig.overhead(), encoded)); + + vm.prank(systemConfig.owner()); + systemConfig.setGasConfigEcotone({ + _basefeeScalar: _basefeeScalar, + _blobbasefeeScalar: _blobbasefeeScalar + }); + assertEq(systemConfig.basefeeScalar(), _basefeeScalar); + assertEq(systemConfig.blobbasefeeScalar(), _blobbasefeeScalar); + assertEq(systemConfig.scalar(), uint256(encoded)); + + (uint32 basefeeScalar, uint32 blobbbasefeeScalar) = ffi.decodeScalarEcotone(encoded); + assertEq(uint256(basefeeScalar), uint256(_basefeeScalar)); + assertEq(uint256(blobbbasefeeScalar), uint256(_blobbasefeeScalar)); + } + /// @dev Tests that `setGasLimit` updates the gas limit successfully. function testFuzz_setGasLimit_succeeds(uint64 newGasLimit) external { uint64 minimumGasLimit = systemConfig.minimumGasLimit(); diff --git a/packages/contracts-bedrock/test/setup/FFIInterface.sol b/packages/contracts-bedrock/test/setup/FFIInterface.sol index 34f509633ff5..e7cca3c8335d 100644 --- a/packages/contracts-bedrock/test/setup/FFIInterface.sol +++ b/packages/contracts-bedrock/test/setup/FFIInterface.sol @@ -243,4 +243,25 @@ contract FFIInterface { (bytes32 memRoot, bytes memory proof) = abi.decode(result, (bytes32, bytes)); return (memRoot, proof); } + + function encodeScalarEcotone(uint32 _basefeeScalar, uint32 _blobbasefeeScalar) external returns (bytes32) { + string[] memory cmds = new string[](5); + cmds[0] = "scripts/go-ffi/go-ffi"; + cmds[1] = "diff"; + cmds[2] = "encodeScalarEcotone"; + cmds[3] = vm.toString(_basefeeScalar); + cmds[4] = vm.toString(_blobbasefeeScalar); + bytes memory result = vm.ffi(cmds); + return abi.decode(result, (bytes32)); + } + + function decodeScalarEcotone(bytes32 _scalar) external returns (uint32, uint32) { + string[] memory cmds = new string[](4); + cmds[0] = "scripts/go-ffi/go-ffi"; + cmds[1] = "diff"; + cmds[2] = "decodeScalarEcotone"; + cmds[3] = vm.toString(_scalar); + bytes memory result = vm.ffi(cmds); + return abi.decode(result, (uint32, uint32)); + } }