Use variables for errors.

Improve error wording

op-node/rollup/types.go

@@ -12,6 +12,27 @@ import (
 	"github.com/ethereum-optimism/optimism/op-node/eth"
 )
 
+var (
+	ErrBlockTimeZero                 = errors.New("block time cannot be 0")
+	ErrMissingChannelTimeout         = errors.New("channel timeout must be set, this should cover at least a L1 block time")
+	ErrInvalidSeqWindowSize          = errors.New("sequencing window size must at least be 2")
+	ErrMissingGenesisL1Hash          = errors.New("genesis L1 hash cannot be empty")
+	ErrMissingGenesisL2Hash          = errors.New("genesis L2 hash cannot be empty")
+	ErrGenesisHashesSame             = errors.New("achievement get! rollup inception: L1 and L2 genesis cannot be the same")
+	ErrMissingGenesisL2Time          = errors.New("missing L2 genesis time")
+	ErrMissingBatcherAddr            = errors.New("missing genesis system config batcher address")
+	ErrMissingOverhead               = errors.New("missing genesis system config overhead")
+	ErrMissingScalar                 = errors.New("missing genesis system config scalar")
+	ErrMissingGasLimit               = errors.New("missing genesis system config gas limit")
+	ErrMissingBatchInboxAddress      = errors.New("missing batch inbox address")
+	ErrMissingDepositContractAddress = errors.New("missing deposit contract address")
+	ErrMissingL1ChainID              = errors.New("L1 chain ID must not be nil")
+	ErrMissingL2ChainID              = errors.New("L2 chain ID must not be nil")
+	ErrChainIDsSame                  = errors.New("L1 and L2 chain IDs must be different")
+	ErrL1ChainIDNotPositive          = errors.New("L1 chain ID must be non-zero and positive")
+	ErrL2ChainIDNotPositive          = errors.New("L2 chain ID must be non-zero and positive")
+)
+
 type Genesis struct {
 	// The L1 block that the rollup starts *after* (no derived transactions)
 	L1 eth.BlockID `json:"l1"`
@@ -147,58 +168,58 @@ func (cfg *Config) CheckL2GenesisBlockHash(ctx context.Context, client L2Client)
 // Check verifies that the given configuration makes sense
 func (cfg *Config) Check() error {
 	if cfg.BlockTime == 0 {
-		return fmt.Errorf("block time cannot be 0, got %d", cfg.BlockTime)
+		return ErrBlockTimeZero
 	}
 	if cfg.ChannelTimeout == 0 {
-		return fmt.Errorf("channel timeout must be set, this should cover at least a L1 block time")
+		return ErrMissingChannelTimeout
 	}
 	if cfg.SeqWindowSize < 2 {
-		return fmt.Errorf("sequencing window size must at least be 2, got %d", cfg.SeqWindowSize)
+		return ErrInvalidSeqWindowSize
 	}
 	if cfg.Genesis.L1.Hash == (common.Hash{}) {
-		return errors.New("genesis l1 hash cannot be empty")
+		return ErrMissingGenesisL1Hash
 	}
 	if cfg.Genesis.L2.Hash == (common.Hash{}) {
-		return errors.New("genesis l2 hash cannot be empty")
+		return ErrMissingGenesisL2Hash
 	}
 	if cfg.Genesis.L2.Hash == cfg.Genesis.L1.Hash {
-		return errors.New("achievement get! rollup inception: L1 and L2 genesis cannot be the same")
+		return ErrGenesisHashesSame
 	}
 	if cfg.Genesis.L2Time == 0 {
-		return errors.New("missing L2 genesis time")
+		return ErrMissingGenesisL2Time
 	}
 	if cfg.Genesis.SystemConfig.BatcherAddr == (common.Address{}) {
-		return errors.New("missing genesis system config batcher address")
+		return ErrMissingBatcherAddr
 	}
 	if cfg.Genesis.SystemConfig.Overhead == (eth.Bytes32{}) {
-		return errors.New("missing genesis system config overhead")
+		return ErrMissingOverhead
 	}
 	if cfg.Genesis.SystemConfig.Scalar == (eth.Bytes32{}) {
-		return errors.New("missing genesis system config scalar")
+		return ErrMissingScalar
 	}
 	if cfg.Genesis.SystemConfig.GasLimit == 0 {
-		return errors.New("missing genesis system config gas limit")
+		return ErrMissingGasLimit
 	}
 	if cfg.BatchInboxAddress == (common.Address{}) {
-		return errors.New("missing batch inbox address")
+		return ErrMissingBatchInboxAddress
 	}
 	if cfg.DepositContractAddress == (common.Address{}) {
-		return errors.New("missing deposit contract address")
+		return ErrMissingDepositContractAddress
 	}
 	if cfg.L1ChainID == nil {
-		return errors.New("l1 chain ID must not be nil")
+		return ErrMissingL1ChainID
 	}
 	if cfg.L2ChainID == nil {
-		return errors.New("l2 chain ID must not be nil")
+		return ErrMissingL2ChainID
 	}
 	if cfg.L1ChainID.Cmp(cfg.L2ChainID) == 0 {
-		return errors.New("l1 and l2 chain IDs must be different")
+		return ErrChainIDsSame
 	}
 	if cfg.L1ChainID.Sign() < 1 {
-		return errors.New("l1 chain ID must be positive")
+		return ErrL1ChainIDNotPositive
 	}
 	if cfg.L2ChainID.Sign() < 1 {
-		return errors.New("l2 chain ID must be positive")
+		return ErrL2ChainIDNotPositive
 	}
 	return nil
 }

op-node/rollup/types_test.go

@@ -217,112 +217,112 @@ func TestConfig_Check(t *testing.T) {
 	tests := []struct {
 		name        string
 		modifier    func(cfg *Config)
-		expectedErr string
+		expectedErr error
 	}{
 		{
 			name:        "BlockTimeZero",
 			modifier:    func(cfg *Config) { cfg.BlockTime = 0 },
-			expectedErr: "block time cannot be 0",
+			expectedErr: ErrBlockTimeZero,
 		},
 		{
 			name:        "ChannelTimeoutZero",
 			modifier:    func(cfg *Config) { cfg.ChannelTimeout = 0 },
-			expectedErr: "channel timeout must be set",
+			expectedErr: ErrMissingChannelTimeout,
 		},
 		{
 			name:        "SeqWindowSizeZero",
 			modifier:    func(cfg *Config) { cfg.SeqWindowSize = 0 },
-			expectedErr: "sequencing window size must at least be 2",
+			expectedErr: ErrInvalidSeqWindowSize,
 		},
 		{
 			name:        "SeqWindowSizeOne",
 			modifier:    func(cfg *Config) { cfg.SeqWindowSize = 1 },
-			expectedErr: "sequencing window size must at least be 2",
+			expectedErr: ErrInvalidSeqWindowSize,
 		},
 		{
 			name:        "NoL1Genesis",
 			modifier:    func(cfg *Config) { cfg.Genesis.L1.Hash = common.Hash{} },
-			expectedErr: "genesis l1 hash cannot be empty",
+			expectedErr: ErrMissingGenesisL1Hash,
 		},
 		{
 			name:        "NoL2Genesis",
 			modifier:    func(cfg *Config) { cfg.Genesis.L2.Hash = common.Hash{} },
-			expectedErr: "genesis l2 hash cannot be empty",
+			expectedErr: ErrMissingGenesisL2Hash,
 		},
 		{
 			name:        "GenesisHashesEqual",
 			modifier:    func(cfg *Config) { cfg.Genesis.L2.Hash = cfg.Genesis.L1.Hash },
-			expectedErr: "L1 and L2 genesis cannot be the same",
+			expectedErr: ErrGenesisHashesSame,
 		},
 		{
 			name:        "GenesisL2TimeZero",
 			modifier:    func(cfg *Config) { cfg.Genesis.L2Time = 0 },
-			expectedErr: "missing L2 genesis time",
+			expectedErr: ErrMissingGenesisL2Time,
 		},
 		{
 			name:        "NoBatcherAddr",
 			modifier:    func(cfg *Config) { cfg.Genesis.SystemConfig.BatcherAddr = common.Address{} },
-			expectedErr: "missing genesis system config batcher address",
+			expectedErr: ErrMissingBatcherAddr,
 		},
 		{
 			name:        "NoOverhead",
 			modifier:    func(cfg *Config) { cfg.Genesis.SystemConfig.Overhead = eth.Bytes32{} },
-			expectedErr: "missing genesis system config overhead",
+			expectedErr: ErrMissingOverhead,
 		},
 		{
 			name:        "NoScalar",
 			modifier:    func(cfg *Config) { cfg.Genesis.SystemConfig.Scalar = eth.Bytes32{} },
-			expectedErr: "missing genesis system config scalar",
+			expectedErr: ErrMissingScalar,
 		},
 		{
 			name:        "NoGasLimit",
 			modifier:    func(cfg *Config) { cfg.Genesis.SystemConfig.GasLimit = 0 },
-			expectedErr: "missing genesis system config gas limit",
+			expectedErr: ErrMissingGasLimit,
 		},
 		{
 			name:        "NoBatchInboxAddress",
 			modifier:    func(cfg *Config) { cfg.BatchInboxAddress = common.Address{} },
-			expectedErr: "missing batch inbox address",
+			expectedErr: ErrMissingBatchInboxAddress,
 		},
 		{
 			name:        "NoDepositContractAddress",
 			modifier:    func(cfg *Config) { cfg.DepositContractAddress = common.Address{} },
-			expectedErr: "missing deposit contract address",
+			expectedErr: ErrMissingDepositContractAddress,
 		},
 		{
 			name:        "NoL1ChainId",
 			modifier:    func(cfg *Config) { cfg.L1ChainID = nil },
-			expectedErr: "l1 chain ID must not be nil",
+			expectedErr: ErrMissingL1ChainID,
 		},
 		{
 			name:        "NoL2ChainId",
 			modifier:    func(cfg *Config) { cfg.L2ChainID = nil },
-			expectedErr: "l2 chain ID must not be nil",
+			expectedErr: ErrMissingL2ChainID,
 		},
 		{
 			name:        "ChainIDsEqual",
 			modifier:    func(cfg *Config) { cfg.L2ChainID = cfg.L1ChainID },
-			expectedErr: "l1 and l2 chain IDs must be different",
+			expectedErr: ErrChainIDsSame,
 		},
 		{
 			name:        "L1ChainIdNegative",
 			modifier:    func(cfg *Config) { cfg.L1ChainID = big.NewInt(-1) },
-			expectedErr: "l1 chain ID must be positive",
+			expectedErr: ErrL1ChainIDNotPositive,
 		},
 		{
 			name:        "L1ChainIdZero",
 			modifier:    func(cfg *Config) { cfg.L1ChainID = big.NewInt(0) },
-			expectedErr: "l1 chain ID must be positive",
+			expectedErr: ErrL1ChainIDNotPositive,
 		},
 		{
 			name:        "L2ChainIdNegative",
 			modifier:    func(cfg *Config) { cfg.L2ChainID = big.NewInt(-1) },
-			expectedErr: "l2 chain ID must be positive",
+			expectedErr: ErrL2ChainIDNotPositive,
 		},
 		{
 			name:        "L2ChainIdZero",
 			modifier:    func(cfg *Config) { cfg.L2ChainID = big.NewInt(0) },
-			expectedErr: "l2 chain ID must be positive",
+			expectedErr: ErrL2ChainIDNotPositive,
 		},
 	}
 	for _, test := range tests {
@@ -330,7 +330,7 @@ func TestConfig_Check(t *testing.T) {
 			cfg := randConfig()
 			test.modifier(cfg)
 			err := cfg.Check()
-			assert.ErrorContains(t, err, test.expectedErr)
+			assert.Same(t, err, test.expectedErr)
 		})
 	}
 }