Merge branch 'develop' into optional-governance

op-bindings/bindings/registry.go

@@ -2,15 +2,20 @@ package bindings
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/ethereum-optimism/optimism/op-bindings/solc"
 	"github.com/ethereum/go-ethereum/common"
 )
 
+// layouts respresents the set of storage layouts. It is populated in an init function.
 var layouts = make(map[string]*solc.StorageLayout)
 
+// deployedBytecodes represents the set of deployed bytecodes. It is populated
+// in an init function.
 var deployedBytecodes = make(map[string]string)
 
+// GetStorageLayout returns the storage layout of a contract by name.
 func GetStorageLayout(name string) (*solc.StorageLayout, error) {
 	layout := layouts[name]
 	if layout == nil {
@@ -19,11 +24,36 @@ func GetStorageLayout(name string) (*solc.StorageLayout, error) {
 	return layout, nil
 }
 
+// GetDeployedBytecode returns the deployed bytecode of a contract by name.
 func GetDeployedBytecode(name string) ([]byte, error) {
 	bc := deployedBytecodes[name]
 	if bc == "" {
 		return nil, fmt.Errorf("%s: deployed bytecode not found", name)
 	}
 
+	if !isHex(bc) {
+		return nil, fmt.Errorf("%s: invalid deployed bytecode", name)
+	}
+
 	return common.FromHex(bc), nil
 }
+
+// isHexCharacter returns bool of c being a valid hexadecimal.
+func isHexCharacter(c byte) bool {
+	return ('0' <= c && c <= '9') || ('a' <= c && c <= 'f') || ('A' <= c && c <= 'F')
+}
+
+// isHex validates whether each byte is valid hexadecimal string.
+func isHex(str string) bool {
+	if len(str)%2 != 0 {
+		return false
+	}
+	str = strings.TrimPrefix(str, "0x")
+
+	for _, c := range []byte(str) {
+		if !isHexCharacter(c) {
+			return false
+		}
+	}
+	return true
+}

op-e2e/system_test.go

@@ -816,7 +816,7 @@ func TestSystemDenseTopology(t *testing.T) {
 		params, err := p2p.GetPeerScoreParams("light", 2)
 		require.NoError(t, err)
 		node.P2P = &p2p.Config{
-			PeerScoring:    params,
+			PeerScoring:    &params,
 			BanningEnabled: false,
 		}
 	}

op-node/p2p/cli/load_config.go

@@ -98,7 +98,7 @@ func loadTopicScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64
 		if err != nil {
 			return err
 		}
-		conf.TopicScoring = topicScoreParams
+		conf.TopicScoring = &topicScoreParams
 	}
 
 	return nil
@@ -114,7 +114,7 @@ func loadPeerScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64)
 		if err != nil {
 			return err
 		}
-		conf.PeerScoring = peerScoreParams
+		conf.PeerScoring = &peerScoreParams
 	}
 
 	return nil

op-node/p2p/config.go

@@ -63,8 +63,8 @@ type Config struct {
 	AltSync bool
 
 	// Pubsub Scoring Parameters
-	PeerScoring  pubsub.PeerScoreParams
-	TopicScoring pubsub.TopicScoreParams
+	PeerScoring  *pubsub.PeerScoreParams
+	TopicScoring *pubsub.TopicScoreParams
 
 	// Whether to ban peers based on their [PeerScoring] score. Should be negative.
 	BanningEnabled bool
@@ -135,7 +135,7 @@ func (conf *Config) Disabled() bool {
 }
 
 func (conf *Config) PeerScoringParams() *pubsub.PeerScoreParams {
-	return &conf.PeerScoring
+	return conf.PeerScoring
 }
 
 func (conf *Config) BanPeers() bool {
@@ -151,7 +151,7 @@ func (conf *Config) BanDuration() time.Duration {
 }
 
 func (conf *Config) TopicScoringParams() *pubsub.TopicScoreParams {
-	return &conf.TopicScoring
+	return conf.TopicScoring
 }
 
 func (conf *Config) ReqRespSyncEnabled() bool {

op-node/p2p/gossip.go

@@ -443,10 +443,7 @@ func JoinGossip(p2pCtx context.Context, self peer.ID, topicScoreParams *pubsub.T
 	}
 	go LogTopicEvents(p2pCtx, log.New("topic", "blocks"), blocksTopicEvents)
 
-	// A [TimeInMeshQuantum] value of 0 means the topic score is disabled.
-	// If we passed a topicScoreParams with [TimeInMeshQuantum] set to 0,
-	// libp2p errors since the params will be rejected.
-	if topicScoreParams != nil && topicScoreParams.TimeInMeshQuantum != 0 {
+	if topicScoreParams != nil {
 		if err = blocksTopic.SetScoreParams(topicScoreParams); err != nil {
 			return nil, fmt.Errorf("failed to set topic score params: %w", err)
 		}

op-node/p2p/host.go

@@ -146,7 +146,15 @@ func (conf *Config) Host(log log.Logger, reporter metrics.Reporter, metrics Host
 	}
 
 	peerScoreParams := conf.PeerScoringParams()
-	ps, err := store.NewExtendedPeerstore(context.Background(), log, clock.SystemClock, basePs, conf.Store, peerScoreParams.RetainScore)
+	var scoreRetention time.Duration
+	if peerScoreParams != nil {
+		// Use the same retention period as gossip will if available
+		scoreRetention = peerScoreParams.RetainScore
+	} else {
+		// Disable score GC if peer scoring is disabled
+		scoreRetention = 0
+	}
+	ps, err := store.NewExtendedPeerstore(context.Background(), log, clock.SystemClock, basePs, conf.Store, scoreRetention)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open extended peerstore: %w", err)
 	}

op-node/p2p/peer_scores.go

@@ -12,14 +12,13 @@ func ConfigurePeerScoring(gossipConf GossipSetupConfigurables, scorer Scorer, lo
 	peerScoreParams := gossipConf.PeerScoringParams()
 	peerScoreThresholds := NewPeerScoreThresholds()
 	opts := []pubsub.Option{}
-	// Check the app specific score since libp2p doesn't export it's [validate] function :/
-	if peerScoreParams != nil && peerScoreParams.AppSpecificScore != nil {
+	if peerScoreParams != nil {
 		opts = []pubsub.Option{
 			pubsub.WithPeerScore(peerScoreParams, &peerScoreThresholds),
 			pubsub.WithPeerScoreInspect(scorer.SnapshotHook(), peerScoreInspectFrequency),
 		}
 	} else {
-		log.Warn("Proceeding with no peer scoring...\nMissing AppSpecificScore in peer scoring params")
+		log.Info("Peer scoring disabled")
 	}
 	return opts
 }

op-node/p2p/peer_scores_test.go

@@ -102,7 +102,7 @@ func newGossipSubs(testSuite *PeerScoresTestSuite, ctx context.Context, hosts []
 			&rollup.Config{L2ChainID: big.NewInt(123)},
 			extPeerStore, testSuite.mockMetricer, logger)
 		opts = append(opts, ConfigurePeerScoring(&Config{
-			PeerScoring: pubsub.PeerScoreParams{
+			PeerScoring: &pubsub.PeerScoreParams{
 				AppSpecificScore: func(p peer.ID) float64 {
 					if p == hosts[0].ID() {
 						return -1000

op-node/rollup/types.go

@@ -142,7 +142,7 @@ func (cfg *Config) CheckL1ChainID(ctx context.Context, client L1Client) error {
 		return err
 	}
 	if cfg.L1ChainID.Cmp(id) != 0 {
-		return fmt.Errorf("incorrect L1 RPC chain id %d, expected %d", cfg.L1ChainID, id)
+		return fmt.Errorf("incorrect L1 RPC chain id %d, expected %d", id, cfg.L1ChainID)
 	}
 	return nil
 }
@@ -154,7 +154,7 @@ func (cfg *Config) CheckL1GenesisBlockHash(ctx context.Context, client L1Client)
 		return err
 	}
 	if l1GenesisBlockRef.Hash != cfg.Genesis.L1.Hash {
-		return fmt.Errorf("incorrect L1 genesis block hash %d, expected %d", cfg.Genesis.L1.Hash, l1GenesisBlockRef.Hash)
+		return fmt.Errorf("incorrect L1 genesis block hash %s, expected %s", l1GenesisBlockRef.Hash, cfg.Genesis.L1.Hash)
 	}
 	return nil
 }
@@ -171,7 +171,7 @@ func (cfg *Config) CheckL2ChainID(ctx context.Context, client L2Client) error {
 		return err
 	}
 	if cfg.L2ChainID.Cmp(id) != 0 {
-		return fmt.Errorf("incorrect L2 RPC chain id, expected from config %d, obtained from client %d", cfg.L2ChainID, id)
+		return fmt.Errorf("incorrect L2 RPC chain id %d, expected %d", id, cfg.L2ChainID)
 	}
 	return nil
 }
@@ -183,7 +183,7 @@ func (cfg *Config) CheckL2GenesisBlockHash(ctx context.Context, client L2Client)
 		return err
 	}
 	if l2GenesisBlockRef.Hash != cfg.Genesis.L2.Hash {
-		return fmt.Errorf("incorrect L2 genesis block hash %d, expected %d", cfg.Genesis.L2.Hash, l2GenesisBlockRef.Hash)
+		return fmt.Errorf("incorrect L2 genesis block hash %s, expected %s", l2GenesisBlockRef.Hash, cfg.Genesis.L2.Hash)
 	}
 	return nil
 }

packages/contracts-bedrock/.gas-snapshot

@@ -27,11 +27,11 @@ CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10597)
 CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34883)
 DeployerWhitelist_Test:test_owner_succeeds() (gas: 7582)
 DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395)
-DisputeGameFactory_Test:test_owner_succeeds() (gas: 7582)
-DisputeGameFactory_Test:test_setImplementation_notOwner_reverts() (gas: 11191)
-DisputeGameFactory_Test:test_setImplementation_succeeds() (gas: 38765)
+DisputeGameFactory_Test:test_owner_succeeds() (gas: 7627)
+DisputeGameFactory_Test:test_setImplementation_notOwner_reverts() (gas: 11077)
+DisputeGameFactory_Test:test_setImplementation_succeeds() (gas: 38320)
 DisputeGameFactory_Test:test_transferOwnership_notOwner_reverts() (gas: 10979)
-DisputeGameFactory_Test:test_transferOwnership_succeeds() (gas: 13180)
+DisputeGameFactory_Test:test_transferOwnership_succeeds() (gas: 13225)
 FeeVault_Test:test_constructor_succeeds() (gas: 18185)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352135)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2950342)

packages/contracts-bedrock/.storage-layout

@@ -253,11 +253,12 @@
 ➡ contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory
 =======================
 
-| Name         | Type                                            | Slot | Offset | Bytes | Contract                                                    |
-|--------------|-------------------------------------------------|------|--------|-------|-------------------------------------------------------------|
-| _owner       | address                                         | 0    | 0      | 20    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
-| gameImpls    | mapping(enum GameType => contract IDisputeGame) | 1    | 0      | 32    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
-| disputeGames | mapping(Hash => contract IDisputeGame)          | 2    | 0      | 32    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
+| Name            | Type                                       | Slot | Offset | Bytes | Contract                                                    |
+|-----------------|--------------------------------------------|------|--------|-------|-------------------------------------------------------------|
+| _owner          | address                                    | 0    | 0      | 20    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
+| gameImpls       | mapping(GameType => contract IDisputeGame) | 1    | 0      | 32    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
+| disputeGames    | mapping(Hash => contract IDisputeGame)     | 2    | 0      | 32    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
+| disputeGameList | contract IDisputeGame[]                    | 3    | 0      | 32    | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
 
 =======================
 ➡ contracts/dispute/BondManager.sol:BondManager

packages/contracts-bedrock/contracts/dispute/BondManager.sol

@@ -83,7 +83,7 @@ contract BondManager is IBondManager {
 
         IDisputeGame caller = IDisputeGame(msg.sender);
         IDisputeGame game = DISPUTE_GAME_FACTORY.games(
-            GameType.ATTESTATION,
+            GameTypes.ATTESTATION,
             caller.rootClaim(),
             caller.extraData()
         );
@@ -108,7 +108,7 @@ contract BondManager is IBondManager {
 
         IDisputeGame caller = IDisputeGame(msg.sender);
         IDisputeGame game = DISPUTE_GAME_FACTORY.games(
-            GameType.ATTESTATION,
+            GameTypes.ATTESTATION,
             caller.rootClaim(),
             caller.extraData()
         );

packages/contracts-bedrock/contracts/dispute/DisputeGameFactory.sol

@@ -32,6 +32,13 @@ contract DisputeGameFactory is Ownable, IDisputeGameFactory {
      */
     mapping(Hash => IDisputeGame) internal disputeGames;
 
+    /**
+     * @notice An append-only array of disputeGames that have been created.
+     * @dev This accessor is used by offchain game solvers to efficiently
+     *      track dispute games
+     */
+    IDisputeGame[] public disputeGameList;
+
     /**
      * @notice Constructs a new DisputeGameFactory contract.
      * @param _owner The owner of the contract.
@@ -40,6 +47,13 @@ contract DisputeGameFactory is Ownable, IDisputeGameFactory {
         transferOwnership(_owner);
     }
 
+    /**
+     * @inheritdoc IDisputeGameFactory
+     */
+    function gameCount() external view returns (uint256 _gameCount) {
+        _gameCount = disputeGameList.length;
+    }
+
     /**
      * @inheritdoc IDisputeGameFactory
      */
@@ -81,6 +95,7 @@ contract DisputeGameFactory is Ownable, IDisputeGameFactory {
 
         // Store the dispute game in the mapping & emit the `DisputeGameCreated` event.
         disputeGames[uuid] = proxy;
+        disputeGameList.push(proxy);
         emit DisputeGameCreated(address(proxy), gameType, rootClaim);
     }
 

packages/contracts-bedrock/contracts/dispute/interfaces/IDisputeGameFactory.sol

@@ -29,6 +29,12 @@ interface IDisputeGameFactory {
      */
     event ImplementationSet(address indexed impl, GameType indexed gameType);
 
+    /**
+     * @notice the total number of dispute games created by this factory.
+     * @return _gameCount The total number of dispute games created by this factory.
+     */
+    function gameCount() external view returns (uint256 _gameCount);
+
     /**
      * @notice `games` queries an internal a mapping that maps the hash of
      *         `gameType ++ rootClaim ++ extraData` to the deployed `DisputeGame` clone.

packages/contracts-bedrock/contracts/libraries/DisputeTypes.sol

@@ -57,6 +57,11 @@ type Clock is uint256;
  */
 type Position is uint256;
 
+/**
+ * @notice A `GameType` represents the type of game being played.
+ */
+type GameType is uint8;
+
 /**
  * @notice The current status of the dispute game.
  */
@@ -70,13 +75,22 @@ enum GameStatus {
 }
 
 /**
- * @notice The type of proof system being used.
+ * @title GameTypes
+ * @notice A library that defines the IDs of games that can be played.
  */
-enum GameType {
-    // The game will use a `IDisputeGame` implementation that utilizes fault proofs.
-    FAULT,
-    // The game will use a `IDisputeGame` implementation that utilizes validity proofs.
-    VALIDITY,
-    // The game will use a `IDisputeGame` implementation that utilizes attestation proofs.
-    ATTESTATION
+library GameTypes {
+    /**
+     * @dev The game will use a `IDisputeGame` implementation that utilizes fault proofs.
+     */
+    GameType internal constant FAULT = GameType.wrap(0);
+
+    /**
+     * @dev The game will use a `IDisputeGame` implementation that utilizes validity proofs.
+     */
+    GameType internal constant VALIDITY = GameType.wrap(1);
+
+    /**
+     * @dev The game will use a `IDisputeGame` implementation that utilizes attestation proofs.
+     */
+    GameType internal constant ATTESTATION = GameType.wrap(2);
 }

packages/contracts-bedrock/contracts/test/BondManager.t.sol

@@ -208,7 +208,7 @@ contract BondManager_Test is Test {
         {
             rootClaim = Claim.wrap(bytes32(""));
             MockAttestationDisputeGame implementation = new MockAttestationDisputeGame();
-            GameType gt = GameType.ATTESTATION;
+            GameType gt = GameTypes.ATTESTATION;
             factory.setImplementation(gt, IDisputeGame(address(implementation)));
             vm.expectEmit(false, true, true, false);
             emit DisputeGameCreated(address(0), gt, rootClaim);
@@ -261,7 +261,7 @@ contract BondManager_Test is Test {
         {
             rootClaim = Claim.wrap(bytes32(""));
             MockAttestationDisputeGame implementation = new MockAttestationDisputeGame();
-            GameType gt = GameType.ATTESTATION;
+            GameType gt = GameTypes.ATTESTATION;
             factory.setImplementation(gt, IDisputeGame(address(implementation)));
             vm.expectEmit(false, true, true, false);
             emit DisputeGameCreated(address(0), gt, rootClaim);
@@ -418,7 +418,7 @@ contract MockAttestationDisputeGame {
     }
 
     function gameType() external pure returns (GameType _gameType) {
-        return GameType.ATTESTATION;
+        return GameTypes.ATTESTATION;
     }
 
     function rootClaim() external view returns (Claim _rootClaim) {