PARTIAL sync: coreth PR #1206: sentinel err tx apis cleanup

consensus/dummy/consensus.go

@@ -23,7 +23,16 @@ import (
 	"github.com/ava-labs/subnet-evm/utils"
 )
 
-var errUnclesUnsupported = errors.New("uncles unsupported")
+var (
+	errUnclesUnsupported                   = errors.New("uncles unsupported")
+	ErrInvalidBlockGasCost                 = errors.New("invalid blockGasCost")
+	errInvalidExcessBlobGasBeforeCancun    = errors.New("invalid excessBlobGas before cancun")
+	errInvalidBlobGasUsedBeforeCancun      = errors.New("invalid blobGasUsed before cancun")
+	errInvalidParentBeaconRootBeforeCancun = errors.New("invalid parentBeaconRoot before cancun")
+	errMissingParentBeaconRoot             = errors.New("header is missing beaconRoot")
+	errNonEmptyParentBeaconRoot            = errors.New("invalid non-empty parentBeaconRoot")
+	errBlobsNotEnabled                     = errors.New("blobs not enabled on avalanche networks")
+)
 
 type Mode struct {
 	ModeSkipHeader   bool
@@ -170,24 +179,24 @@ func (eng *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header *
 	if !cancun {
 		switch {
 		case header.ExcessBlobGas != nil:
-			return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *header.ExcessBlobGas)
+			return fmt.Errorf("%w: have %d, expected nil", errInvalidExcessBlobGasBeforeCancun, *header.ExcessBlobGas)
 		case header.BlobGasUsed != nil:
-			return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *header.BlobGasUsed)
+			return fmt.Errorf("%w: have %d, expected nil", errInvalidBlobGasUsedBeforeCancun, *header.BlobGasUsed)
 		case header.ParentBeaconRoot != nil:
-			return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected nil", *header.ParentBeaconRoot)
+			return fmt.Errorf("%w: have %#x, expected nil", errInvalidParentBeaconRootBeforeCancun, *header.ParentBeaconRoot)
 		}
 	} else {
 		if header.ParentBeaconRoot == nil {
-			return errors.New("header is missing beaconRoot")
+			return errMissingParentBeaconRoot
 		}
 		if *header.ParentBeaconRoot != (common.Hash{}) {
-			return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected empty", *header.ParentBeaconRoot)
+			return fmt.Errorf("%w: have %#x, expected empty", errNonEmptyParentBeaconRoot, *header.ParentBeaconRoot)
 		}
 		if err := eip4844.VerifyEIP4844Header(parent, header); err != nil {
 			return err
 		}
 		if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures BlobGasUsed is non-nil
-			return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *header.BlobGasUsed)
+			return fmt.Errorf("%w: used %d blob gas, expected 0", errBlobsNotEnabled, *header.BlobGasUsed)
 		}
 	}
 	return nil
@@ -245,7 +254,7 @@ func (eng *DummyEngine) Finalize(chain consensus.ChainHeaderReader, block *types
 		timestamp,
 	)
 	if !utils.BigEqual(blockGasCost, expectedBlockGasCost) {
-		return fmt.Errorf("invalid blockGasCost: have %d, want %d", blockGasCost, expectedBlockGasCost)
+		return fmt.Errorf("%w: have %d, want %d", ErrInvalidBlockGasCost, blockGasCost, expectedBlockGasCost)
 	}
 	if config.IsSubnetEVM(timestamp) {
 		// Verify the block fee was paid.

plugin/evm/customheader/block_gas_cost.go

@@ -22,7 +22,10 @@ var (
 	errBlockGasCostNil = errors.New("block gas cost is nil")
 	errNoGasUsed       = errors.New("no gas used")
 
-	ErrInsufficientBlockGas = errors.New("insufficient gas to cover the block cost")
+	ErrInsufficientBlockGas                     = errors.New("insufficient gas to cover the block cost")
+	errInvalidExtraStateChangeContribution      = errors.New("invalid extra state change contribution")
+	errInvalidBaseFeeApricotPhase4              = errors.New("invalid base fee in apricot phase 4")
+	errInvalidRequiredBlockGasCostApricotPhase4 = errors.New("invalid block gas cost in apricot phase 4")
 )
 
 // BlockGasCost calculates the required block gas cost based on the parent
@@ -135,10 +138,10 @@ func VerifyBlockFee(
 	extraStateChangeContribution *big.Int,
 ) error {
 	if baseFee == nil || baseFee.Sign() <= 0 {
-		return fmt.Errorf("invalid base fee (%d) in apricot phase 4", baseFee)
+		return fmt.Errorf("%w: %d", errInvalidBaseFeeApricotPhase4, baseFee)
 	}
 	if requiredBlockGasCost == nil || !requiredBlockGasCost.IsUint64() {
-		return fmt.Errorf("invalid block gas cost (%d) in apricot phase 4", requiredBlockGasCost)
+		return fmt.Errorf("%w: %d", errInvalidRequiredBlockGasCostApricotPhase4, requiredBlockGasCost)
 	}
 	// If the required block gas cost is 0, we don't need to verify the block fee
 	if requiredBlockGasCost.Sign() == 0 {
@@ -154,7 +157,7 @@ func VerifyBlockFee(
 	// Add in the external contribution
 	if extraStateChangeContribution != nil {
 		if extraStateChangeContribution.Cmp(common.Big0) < 0 {
-			return fmt.Errorf("invalid extra state change contribution: %d", extraStateChangeContribution)
+			return fmt.Errorf("%w: %d", errInvalidExtraStateChangeContribution, extraStateChangeContribution)
 		}
 		totalBlockFee.Add(totalBlockFee, extraStateChangeContribution)
 	}

plugin/evm/customheader/block_gas_cost_test.go

@@ -331,7 +331,7 @@ func TestVerifyBlockFee(t *testing.T) {
 		txs                    []*types.Transaction
 		receipts               []*types.Receipt
 		extraStateContribution *big.Int
-		shouldErr              bool
+		expectedErr            error
 	}{
 		"tx only base fee": {
 			baseFee:            big.NewInt(100),
@@ -344,7 +344,7 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 1000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              true,
+			expectedErr:            ErrInsufficientBlockGas,
 		},
 		"tx covers exactly block fee": {
 			baseFee:            big.NewInt(100),
@@ -357,7 +357,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 100_000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              false,
 		},
 		"txs share block fee": {
 			baseFee:            big.NewInt(100),
@@ -372,7 +371,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 100_000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              false,
 		},
 		"txs split block fee": {
 			baseFee:            big.NewInt(100),
@@ -387,7 +385,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 100_000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              false,
 		},
 		"split block fee with extra state contribution": {
 			baseFee:            big.NewInt(100),
@@ -400,7 +397,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 100_000},
 			},
 			extraStateContribution: big.NewInt(5_000_000),
-			shouldErr:              false,
 		},
 		"extra state contribution insufficient": {
 			baseFee:                big.NewInt(100),
@@ -409,7 +405,7 @@ func TestVerifyBlockFee(t *testing.T) {
 			txs:                    nil,
 			receipts:               nil,
 			extraStateContribution: big.NewInt(9_999_999),
-			shouldErr:              true,
+			expectedErr:            ErrInsufficientBlockGas,
 		},
 		"negative extra state contribution": {
 			baseFee:                big.NewInt(100),
@@ -418,7 +414,7 @@ func TestVerifyBlockFee(t *testing.T) {
 			txs:                    nil,
 			receipts:               nil,
 			extraStateContribution: big.NewInt(-1),
-			shouldErr:              true,
+			expectedErr:            errInvalidExtraStateChangeContribution,
 		},
 		"extra state contribution covers block fee": {
 			baseFee:                big.NewInt(100),
@@ -427,7 +423,6 @@ func TestVerifyBlockFee(t *testing.T) {
 			txs:                    nil,
 			receipts:               nil,
 			extraStateContribution: big.NewInt(10_000_000),
-			shouldErr:              false,
 		},
 		"extra state contribution covers more than block fee": {
 			baseFee:                big.NewInt(100),
@@ -436,7 +431,6 @@ func TestVerifyBlockFee(t *testing.T) {
 			txs:                    nil,
 			receipts:               nil,
 			extraStateContribution: big.NewInt(10_000_001),
-			shouldErr:              false,
 		},
 		"tx only base fee after full time window": {
 			baseFee:            big.NewInt(100),
@@ -449,7 +443,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 1000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              false,
 		},
 		"tx only base fee after large time window": {
 			baseFee:            big.NewInt(100),
@@ -462,7 +455,6 @@ func TestVerifyBlockFee(t *testing.T) {
 				{GasUsed: 1000},
 			},
 			extraStateContribution: nil,
-			shouldErr:              false,
 		},
 		"zero block gas cost": {
 			baseFee:                big.NewInt(100),
@@ -484,15 +476,8 @@ func TestVerifyBlockFee(t *testing.T) {
 			)
 			bigBlockGasCost := new(big.Int).SetUint64(blockGasCost)
 
-			if err := VerifyBlockFee(test.baseFee, bigBlockGasCost, test.txs, test.receipts, test.extraStateContribution); err != nil {
-				if !test.shouldErr {
-					t.Fatalf("Unexpected error: %s", err)
-				}
-			} else {
-				if test.shouldErr {
-					t.Fatal("Should have failed verification")
-				}
-			}
+			err := VerifyBlockFee(test.baseFee, bigBlockGasCost, test.txs, test.receipts, test.extraStateContribution)
+			require.ErrorIs(t, err, test.expectedErr)
 		})
 	}
 }

plugin/evm/vm.go

@@ -131,16 +131,21 @@ var (
 )
 
 var (
-	errEmptyBlock                    = errors.New("empty block")
-	errUnsupportedFXs                = errors.New("unsupported feature extensions")
-	errInvalidBlock                  = errors.New("invalid block")
-	errInvalidNonce                  = errors.New("invalid nonce")
-	errUnclesUnsupported             = errors.New("uncles unsupported")
-	errNilBaseFeeSubnetEVM           = errors.New("nil base fee is invalid after subnetEVM")
-	errNilBlockGasCostSubnetEVM      = errors.New("nil blockGasCost is invalid after subnetEVM")
-	errInvalidHeaderPredicateResults = errors.New("invalid header predicate results")
-	errInitializingLogger            = errors.New("failed to initialize logger")
-	errShuttingDownVM                = errors.New("shutting down VM")
+	errEmptyBlock                        = errors.New("empty block")
+	errUnsupportedFXs                    = errors.New("unsupported feature extensions")
+	errNilBaseFeeSubnetEVM               = errors.New("nil base fee is invalid after subnetEVM")
+	errNilBlockGasCostSubnetEVM          = errors.New("nil blockGasCost is invalid after subnetEVM")
+	errInvalidBlock                      = errors.New("invalid block")
+	errInvalidNonce                      = errors.New("invalid nonce")
+	errUnclesUnsupported                 = errors.New("uncles unsupported")
+	errInvalidHeaderPredicateResults     = errors.New("invalid header predicate results")
+	errInitializingLogger                = errors.New("failed to initialize logger")
+	errShuttingDownVM                    = errors.New("shutting down VM")
+	errFirewoodPruningRequired           = errors.New("pruning must be enabled for Firewood")
+	errFirewoodSnapshotCacheDisabled     = errors.New("snapshot cache must be disabled for Firewood")
+	errFirewoodOfflinePruningUnsupported = errors.New("offline pruning is not supported for Firewood")
+	errFirewoodStateSyncUnsupported      = errors.New("state sync is not yet supported for Firewood")
+	errPathStateUnsupported              = errors.New("path state scheme is not supported")
 )
 
 // legacyApiNames maps pre geth v1.10.20 api names to their updated counterparts.
@@ -385,22 +390,22 @@ func (vm *VM) Initialize(
 		log.Warn("This is untested in production, use at your own risk")
 		// Firewood only supports pruning for now.
 		if !vm.config.Pruning {
-			return errors.New("Pruning must be enabled for Firewood")
+			return errFirewoodPruningRequired
 		}
 		// Firewood does not support iterators, so the snapshot cannot be constructed
 		if vm.config.SnapshotCache > 0 {
-			return errors.New("Snapshot cache must be disabled for Firewood")
+			return errFirewoodSnapshotCacheDisabled
 		}
 		if vm.config.OfflinePruning {
-			return errors.New("Offline pruning is not supported for Firewood")
+			return errFirewoodOfflinePruningUnsupported
 		}
 		if vm.config.StateSyncEnabled {
-			return errors.New("State sync is not yet supported for Firewood")
+			return errFirewoodStateSyncUnsupported
 		}
 	}
 	if vm.ethConfig.StateScheme == rawdb.PathScheme {
 		log.Error("Path state scheme is not supported. Please use HashDB or Firewood state schemes instead")
-		return errors.New("Path state scheme is not supported")
+		return errPathStateUnsupported
 	}
 
 	// Create directory for offline pruning

plugin/evm/vm_test.go

@@ -1632,9 +1632,8 @@ func testUncleBlock(t *testing.T, scheme string) {
 	if _, err := vm1.ParseBlock(context.Background(), vm2BlkC.Bytes()); err != nil {
 		t.Fatalf("VM1 errored parsing blkC: %s", err)
 	}
-	if _, err := vm1.ParseBlock(context.Background(), uncleBlock.Bytes()); !errors.Is(err, errUnclesUnsupported) {
-		t.Fatalf("VM1 should have failed with %q but got %q", errUnclesUnsupported, err.Error())
-	}
+	_, err = vm1.ParseBlock(context.Background(), uncleBlock.Bytes())
+	require.ErrorIs(t, err, errUnclesUnsupported)
 }
 
 // Regression test to ensure that a VM that is not able to parse a block that

plugin/evm/wrapped_block.go

@@ -40,7 +40,20 @@ var (
 	errTotalIntrinsicGasCostExceedsClaimed = errors.New("total intrinsic gas cost is greater than claimed gas used")
 )
 
-// wrappedBlock implements the snowman.Block interface by wrapping a libevm block
+// Sentinel errors for header validation in this file
+var (
+	errInvalidExcessBlobGasBeforeCancun    = errors.New("invalid excessBlobGas before cancun")
+	errInvalidBlobGasUsedBeforeCancun      = errors.New("invalid blobGasUsed before cancun")
+	errInvalidParentBeaconRootBeforeCancun = errors.New("invalid parentBeaconRoot before cancun")
+	errMissingExcessBlobGas                = errors.New("header is missing excessBlobGas")
+	errMissingBlobGasUsed                  = errors.New("header is missing blobGasUsed")
+	errMissingParentBeaconRoot             = errors.New("header is missing parentBeaconRoot")
+	errParentBeaconRootNonEmpty            = errors.New("invalid non-empty parentBeaconRoot")
+	errBlobGasUsedNilInCancun              = errors.New("blob gas used must not be nil in Cancun")
+	errBlobsNotEnabled                     = errors.New("blobs not enabled on avalanche networks")
+)
+
+// wrappedBlock implements the snowman.wrappedBlock interface
 type wrappedBlock struct {
 	id       ids.ID
 	ethBlock *types.Block
@@ -376,31 +389,31 @@ func (b *wrappedBlock) syntacticVerify() error {
 	// Verify the existence / non-existence of excessBlobGas
 	cancun := rules.IsCancun
 	if !cancun && ethHeader.ExcessBlobGas != nil {
-		return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *ethHeader.ExcessBlobGas)
+		return fmt.Errorf("%w: have %d, expected nil", errInvalidExcessBlobGasBeforeCancun, *ethHeader.ExcessBlobGas)
 	}
 	if !cancun && ethHeader.BlobGasUsed != nil {
-		return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *ethHeader.BlobGasUsed)
+		return fmt.Errorf("%w: have %d, expected nil", errInvalidBlobGasUsedBeforeCancun, *ethHeader.BlobGasUsed)
 	}
 	if cancun && ethHeader.ExcessBlobGas == nil {
-		return errors.New("header is missing excessBlobGas")
+		return errMissingExcessBlobGas
 	}
 	if cancun && ethHeader.BlobGasUsed == nil {
-		return errors.New("header is missing blobGasUsed")
+		return errMissingBlobGasUsed
 	}
 	if !cancun && ethHeader.ParentBeaconRoot != nil {
-		return fmt.Errorf("invalid parentBeaconRoot: have %x, expected nil", *ethHeader.ParentBeaconRoot)
+		return fmt.Errorf("%w: have %x, expected nil", errInvalidParentBeaconRootBeforeCancun, *ethHeader.ParentBeaconRoot)
 	}
 	if cancun {
 		switch {
 		case ethHeader.ParentBeaconRoot == nil:
-			return errors.New("header is missing parentBeaconRoot")
+			return errMissingParentBeaconRoot
 		case *ethHeader.ParentBeaconRoot != (common.Hash{}):
-			return fmt.Errorf("invalid parentBeaconRoot: have %x, expected empty hash", ethHeader.ParentBeaconRoot)
+			return fmt.Errorf("%w: have %x, expected empty hash", errParentBeaconRootNonEmpty, ethHeader.ParentBeaconRoot)
 		}
 		if ethHeader.BlobGasUsed == nil {
-			return errors.New("blob gas used must not be nil in Cancun")
+			return errBlobGasUsedNilInCancun
 		} else if *ethHeader.BlobGasUsed > 0 {
-			return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *ethHeader.BlobGasUsed)
+			return fmt.Errorf("%w: used %d blob gas, expected 0", errBlobsNotEnabled, *ethHeader.BlobGasUsed)
 		}
 	}