Merge branch 'develop' into fix/legacy-isemver

cannon/mipsevm/evm.go

@@ -25,7 +25,6 @@ import (
 
 var (
 	StepBytes4                      = crypto.Keccak256([]byte("step(bytes,bytes)"))[:4]
-	CheatBytes4                     = crypto.Keccak256([]byte("cheat(uint256,bytes32,bytes32,uint256)"))[:4]
 	LoadKeccak256PreimagePartBytes4 = crypto.Keccak256([]byte("loadKeccak256PreimagePart(uint256,bytes)"))[:4]
 	LoadLocalDataBytes4             = crypto.Keccak256([]byte("loadLocalData(uint256,bytes32,uint256,uint256)"))[:4]
 )

op-node/flags/flags.go

@@ -243,15 +243,15 @@ var (
 			"Available networks: %s", strings.Join(chaincfg.BetaAvailableNetworks(), ", ")),
 		EnvVars: prefixEnvVars("BETA_EXTRA_NETWORKS"),
 	}
-	BetaRollupHalt = &cli.StringFlag{
-		Name:    "beta.rollup.halt",
-		Usage:   "Beta feature: opt-in option to halt on incompatible protocol version requirements of the given level (major/minor/patch/none), as signaled onchain in L1",
-		EnvVars: prefixEnvVars("BETA_ROLLUP_HALT"),
+	RollupHalt = &cli.StringFlag{
+		Name:    "rollup.halt",
+		Usage:   "Opt-in option to halt on incompatible protocol version requirements of the given level (major/minor/patch/none), as signaled onchain in L1",
+		EnvVars: prefixEnvVars("ROLLUP_HALT"),
 	}
-	BetaRollupLoadProtocolVersions = &cli.BoolFlag{
-		Name:    "beta.rollup.load-protocol-versions",
-		Usage:   "Beta feature: load protocol versions from the superchain L1 ProtocolVersions contract (if available), and report in logs and metrics",
-		EnvVars: prefixEnvVars("BETA_ROLLUP_LOAD_PROTOCOL_VERSIONS"),
+	RollupLoadProtocolVersions = &cli.BoolFlag{
+		Name:    "rollup.load-protocol-versions",
+		Usage:   "Load protocol versions from the superchain L1 ProtocolVersions contract (if available), and report in logs and metrics",
+		EnvVars: prefixEnvVars("ROLLUP_LOAD_PROTOCOL_VERSIONS"),
 	}
 	CanyonOverrideFlag = &cli.Uint64Flag{
 		Name:   "override.canyon",
@@ -300,8 +300,8 @@ var optionalFlags = []cli.Flag{
 	L2EngineSyncEnabled,
 	SkipSyncStartCheck,
 	BetaExtraNetworks,
-	BetaRollupHalt,
-	BetaRollupLoadProtocolVersions,
+	RollupHalt,
+	RollupLoadProtocolVersions,
 	CanyonOverrideFlag,
 }
 

op-node/flags/flags_test.go

 package flags
 
 import (
+	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli/v2"
 )
@@ -29,3 +31,23 @@ func TestUniqueFlags(t *testing.T) {
 		seenCLI[name] = struct{}{}
 	}
 }
+
+// TestBetaFlags test that all flags starting with "beta." have "BETA_" in the env var, and vice versa.
+func TestBetaFlags(t *testing.T) {
+	for _, flag := range Flags {
+		envFlag, ok := flag.(interface {
+			GetEnvVars() []string
+		})
+		if !ok || len(envFlag.GetEnvVars()) == 0 { // skip flags without env-var support
+			continue
+		}
+		name := flag.Names()[0]
+		envName := envFlag.GetEnvVars()[0]
+		if strings.HasPrefix(name, "beta.") {
+			assert.Contains(t, envName, "BETA_", "%q flag must contain BETA in env var to match \"beta.\" flag name", name)
+		}
+		if strings.Contains(envName, "BETA_") {
+			assert.True(t, strings.HasPrefix(name, "beta."), "%q flag must start with \"beta.\" in flag name to match \"BETA_\" env var", name)
+		}
+	}
+}

op-node/service.go

@@ -36,7 +36,7 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {
 		return nil, err
 	}
 
-	if !ctx.Bool(flags.BetaRollupLoadProtocolVersions.Name) {
+	if !ctx.Bool(flags.RollupLoadProtocolVersions.Name) {
 		log.Info("Not opted in to ProtocolVersions signal loading, disabling ProtocolVersions contract now.")
 		rollupConfig.ProtocolVersionsAddress = common.Address{}
 	}
@@ -66,7 +66,7 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {
 
 	syncConfig := NewSyncConfig(ctx)
 
-	haltOption := ctx.String(flags.BetaRollupHalt.Name)
+	haltOption := ctx.String(flags.RollupHalt.Name)
 	if haltOption == "none" {
 		haltOption = ""
 	}

packages/contracts-bedrock/.gas-snapshot

@@ -532,8 +532,8 @@ OptimistTest:test_tokenURI_returnsCorrectTokenURI_succeeds() (gas: 195908)
 OptimistTest:test_transferFrom_soulbound_reverts() (gas: 75512)
 PreimageOracle_Test:test_keccak256PreimageKey_succeeds() (gas: 319)
 PreimageOracle_Test:test_loadKeccak256PreimagePart_outOfBoundsOffset_reverts() (gas: 8993)
-PreimageOracle_Test:test_loadKeccak256PreimagePart_succeeds() (gas: 76098)
-PreimageOracle_Test:test_loadLocalData_onePart_succeeds() (gas: 75840)
+PreimageOracle_Test:test_loadKeccak256PreimagePart_succeeds() (gas: 76076)
+PreimageOracle_Test:test_loadLocalData_onePart_succeeds() (gas: 75818)
 PreimageOracle_Test:test_loadLocalData_outOfBoundsOffset_reverts() (gas: 8803)
 ProtocolVersions_Initialize_Test:test_initialize_events_succeeds() (gas: 52175)
 ProtocolVersions_Initialize_Test:test_initialize_values_succeeds() (gas: 32301)

packages/contracts-bedrock/scripts/Deploy.s.sol

@@ -47,11 +47,6 @@ import "src/libraries/DisputeTypes.sol";
 contract Deploy is Deployer {
     DeployConfig cfg;
 
-    /// @notice The create2 salt used for deployment of the contract implementations.
-    ///         Using this helps to reduce config across networks as the implementation
-    ///         addresses will be the same across networks when deployed with create2.
-    bytes32 constant IMPL_SALT = keccak256(bytes("ether's phoenix"));
-
     /// @notice The name of the script, used to ensure the right deploy artifacts
     ///         are used.
     function name() public pure override returns (string memory name_) {
@@ -92,6 +87,13 @@ contract Deploy is Deployer {
         transferDisputeGameFactoryOwnership();
     }
 
+    /// @notice The create2 salt used for deployment of the contract implementations.
+    ///         Using this helps to reduce config across networks as the implementation
+    ///         addresses will be the same across networks when deployed with create2.
+    function implSalt() public returns (bytes32) {
+        return keccak256(bytes(vm.envOr("IMPL_SALT", string("ether's phoenix"))));
+    }
+
     /// @notice Modifier that wraps a function in broadcasting.
     modifier broadcast() {
         vm.startBroadcast();
@@ -327,7 +329,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the L1CrossDomainMessenger
     function deployL1CrossDomainMessenger() public broadcast returns (address addr_) {
-        L1CrossDomainMessenger messenger = new L1CrossDomainMessenger{ salt: IMPL_SALT }();
+        L1CrossDomainMessenger messenger = new L1CrossDomainMessenger{ salt: implSalt() }();
 
         require(address(messenger.PORTAL()) == address(0));
         require(address(messenger.portal()) == address(0));
@@ -343,7 +345,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the OptimismPortal
     function deployOptimismPortal() public broadcast returns (address addr_) {
-        OptimismPortal portal = new OptimismPortal{ salt: IMPL_SALT }();
+        OptimismPortal portal = new OptimismPortal{ salt: implSalt() }();
 
         require(address(portal.L2_ORACLE()) == address(0));
         require(portal.GUARDIAN() == address(0));
@@ -358,7 +360,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the L2OutputOracle
     function deployL2OutputOracle() public broadcast returns (address addr_) {
-        L2OutputOracle oracle = new L2OutputOracle{ salt: IMPL_SALT }({
+        L2OutputOracle oracle = new L2OutputOracle{ salt: implSalt() }({
             _submissionInterval: cfg.l2OutputOracleSubmissionInterval(),
             _l2BlockTime: cfg.l2BlockTime(),
             _finalizationPeriodSeconds: cfg.finalizationPeriodSeconds()
@@ -385,7 +387,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the OptimismMintableERC20Factory
     function deployOptimismMintableERC20Factory() public broadcast returns (address addr_) {
-        OptimismMintableERC20Factory factory = new OptimismMintableERC20Factory{ salt: IMPL_SALT }();
+        OptimismMintableERC20Factory factory = new OptimismMintableERC20Factory{ salt: implSalt() }();
 
         require(factory.BRIDGE() == address(0));
         require(factory.bridge() == address(0));
@@ -398,7 +400,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the DisputeGameFactory
     function deployDisputeGameFactory() public onlyDevnet broadcast returns (address addr_) {
-        DisputeGameFactory factory = new DisputeGameFactory{ salt: IMPL_SALT }();
+        DisputeGameFactory factory = new DisputeGameFactory{ salt: implSalt() }();
         save("DisputeGameFactory", address(factory));
         console.log("DisputeGameFactory deployed at %s", address(factory));
 
@@ -407,7 +409,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the BlockOracle
     function deployBlockOracle() public onlyDevnet broadcast returns (address addr_) {
-        BlockOracle oracle = new BlockOracle{ salt: IMPL_SALT }();
+        BlockOracle oracle = new BlockOracle{ salt: implSalt() }();
         save("BlockOracle", address(oracle));
         console.log("BlockOracle deployed at %s", address(oracle));
 
@@ -416,7 +418,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the ProtocolVersions
     function deployProtocolVersions() public onlyTestnetOrDevnet broadcast returns (address addr_) {
-        ProtocolVersions versions = new ProtocolVersions{ salt: IMPL_SALT }();
+        ProtocolVersions versions = new ProtocolVersions{ salt: implSalt() }();
         save("ProtocolVersions", address(versions));
         console.log("ProtocolVersions deployed at %s", address(versions));
 
@@ -425,7 +427,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the PreimageOracle
     function deployPreimageOracle() public onlyDevnet broadcast returns (address addr_) {
-        PreimageOracle preimageOracle = new PreimageOracle{ salt: IMPL_SALT }();
+        PreimageOracle preimageOracle = new PreimageOracle{ salt: implSalt() }();
         save("PreimageOracle", address(preimageOracle));
         console.log("PreimageOracle deployed at %s", address(preimageOracle));
 
@@ -434,7 +436,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy Mips
     function deployMips() public onlyDevnet broadcast returns (address addr_) {
-        MIPS mips = new MIPS{ salt: IMPL_SALT }(IPreimageOracle(mustGetAddress("PreimageOracle")));
+        MIPS mips = new MIPS{ salt: implSalt() }(IPreimageOracle(mustGetAddress("PreimageOracle")));
         save("Mips", address(mips));
         console.log("MIPS deployed at %s", address(mips));
 
@@ -443,7 +445,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the SystemConfig
     function deploySystemConfig() public broadcast returns (address addr_) {
-        SystemConfig config = new SystemConfig{ salt: IMPL_SALT }();
+        SystemConfig config = new SystemConfig{ salt: implSalt() }();
 
         require(config.owner() == address(0xdEaD));
         require(config.overhead() == 0);
@@ -476,7 +478,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the L1StandardBridge
     function deployL1StandardBridge() public broadcast returns (address addr_) {
-        L1StandardBridge bridge = new L1StandardBridge{ salt: IMPL_SALT }();
+        L1StandardBridge bridge = new L1StandardBridge{ salt: implSalt() }();
 
         require(address(bridge.MESSENGER()) == address(0));
         require(address(bridge.messenger()) == address(0));
@@ -491,7 +493,7 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the L1ERC721Bridge
     function deployL1ERC721Bridge() public broadcast returns (address addr_) {
-        L1ERC721Bridge bridge = new L1ERC721Bridge{ salt: IMPL_SALT }();
+        L1ERC721Bridge bridge = new L1ERC721Bridge{ salt: implSalt() }();
 
         require(address(bridge.MESSENGER()) == address(0));
         require(bridge.OTHER_BRIDGE() == Predeploys.L2_ERC721_BRIDGE);

packages/contracts-bedrock/src/cannon/PreimageOracle.sol

@@ -31,17 +31,6 @@ contract PreimageOracle is IPreimageOracle {
         dat_ = preimageParts[_key][_offset];
     }
 
-    /// TODO(CLI-4104):
-    /// we need to mix-in the ID of the dispute for local-type keys to avoid collisions,
-    /// and restrict local pre-image insertion to the dispute-managing contract.
-    /// For now we permit anyone to write any pre-image unchecked, to make testing easy.
-    /// This method is DANGEROUS. And NOT FOR PRODUCTION.
-    function cheat(uint256 partOffset, bytes32 key, bytes32 part, uint256 size) external {
-        preimagePartOk[key][partOffset] = true;
-        preimageParts[key][partOffset] = part;
-        preimageLengths[key] = size;
-    }
-
     /// @inheritdoc IPreimageOracle
     function loadLocalData(
         uint256 _ident,

specs/fault-proof.md

@@ -94,7 +94,7 @@ and avoids unnecessary contract redeployments per dispute.
 
 This global store contract should be non-upgradeable.
 
-Since `keccak256` is a safe 32-byte hash input, the first bit is overwritten with a `2` to derive the key,
+Since `keccak256` is a safe 32-byte hash input, the first byte is overwritten with a `2` to derive the key,
 while keeping the rest of the key "readable" (matching the original hash).
 
 #### Type `3`: Global generic key
@@ -130,7 +130,7 @@ as there is no way to revert it to the original commitment without knowing said
 
 #### Type `4-128`: reserved range
 
-Range start and and both inclusive.
+Range start and end both inclusive.
 
 This range of key types is reserved for future usage by the core protocol.
 E.g. version changes, contract migrations, chain-data, additional core features, etc.
@@ -302,7 +302,7 @@ Using these data-sources, the derivation pipeline is processed till we hit one o
 
 - `EOF`: when we run out of L1 data, the L2 chain will not change further, and the epilogue can start.
 - Eager epilogue condition: depending on the type of claim to verify,
-  if the L2 result irreversible (i.e. no later L1 inputs can override it),
+  if the L2 result is irreversible (i.e. no later L1 inputs can override it),
   the processing may end early when the result is ready.
   E.g. when asserting state at a specific L2 block, rather than the very tip of the L2 chain.