Skip to content

Commit

Permalink
contracts-bedrock: ecotone gas config support
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tynes
tynes committed May 10, 2024
1 parent 1a20fd9 commit 8a685a9
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 20 deletions.
8 changes: 5 additions & 3 deletions packages/contracts-bedrock/scripts/ChainAssertions.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/scripts/Deploy.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
5 changes: 5 additions & 0 deletions packages/contracts-bedrock/scripts/DeployConfig.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
28 changes: 28 additions & 0 deletions packages/contracts-bedrock/scripts/go-ffi/differential-testing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"},
Expand All @@ -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"},
Expand Down Expand Up @@ -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]))
}
Expand Down
50 changes: 41 additions & 9 deletions packages/contracts-bedrock/src/L1/SystemConfig.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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),
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand All @@ -343,13 +353,35 @@ 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;

bytes memory data = abi.encode(_overhead, _scalar);
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 {
Expand Down
61 changes: 55 additions & 6 deletions packages/contracts-bedrock/test/L1/SystemConfig.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,17 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init {
address unsafeBlockSigner;
address systemConfigImpl;
address optimismMintableERC20Factory;
uint32 basefeeScalar;
uint32 blobbasefeeScalar;

function setUp() public virtual override {
super.setUp();
batchInbox = deploy.cfg().batchInboxAddress();
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();
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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();
Expand Down
21 changes: 21 additions & 0 deletions packages/contracts-bedrock/test/setup/FFIInterface.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

0 comments on commit 8a685a9

Please sign in to comment.