chore(all): final small changes to the libevm branch

accounts/abi/bind/bind_extra_test.go

@@ -18,11 +18,12 @@ import (
 	"github.com/ava-labs/libevm/common"
 	"github.com/ava-labs/libevm/core/types"
 	"github.com/ava-labs/libevm/crypto"
+	"github.com/stretchr/testify/require"
 )
 
 // TestGetSenderNativeAssetCall checks that the NativeAssetCall proxies the
 // caller address This behavior is disabled on the network and is only to test
-// previous behavior. Note the test uses ApricotPhase2Config.
+// previous behavior. Note the test uses [params.TestApricotPhase2Config].
 func TestGetSenderNativeAssetCall(t *testing.T) {
 	// pragma solidity >=0.8.0 <0.9.0;
 	// contract GetSenderNativeAssetCall {
@@ -34,7 +35,7 @@ func TestGetSenderNativeAssetCall(t *testing.T) {
 	// 			_sender = msg.sender;
 	// 	}
 	// }
-	rawABI := `[
+	const rawABI = `[
 			{
 					"inputs": [],
 				"name": "getSender",
@@ -51,17 +52,17 @@ func TestGetSenderNativeAssetCall(t *testing.T) {
 			}
 	]`
 	bytecode := common.FromHex(`6080604052348015600f57600080fd5b506101608061001f6000396000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c806350c36a521461003b5780635e01eb5a14610045575b600080fd5b610043610063565b005b61004d6100a5565b60405161005a919061010f565b60405180910390f35b336000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550565b60008060009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b60006100f9826100ce565b9050919050565b610109816100ee565b82525050565b60006020820190506101246000830184610100565b9291505056fea26469706673582212209023ce54f38e749b58f44e8da750354578080ce16df95037b7305ed7e480c36d64736f6c634300081b0033`)
-	setSenderMethodName := "setSender"
-	getSenderMethodName := "getSender"
+	const setSenderMethodName = "setSender"
+	const getSenderMethodName = "getSender"
 
 	parsedABI, err := abi.JSON(bytes.NewReader([]byte(rawABI)))
-	if err != nil {
-		t.Fatalf("Failed to parse ABI: %v", err)
-	}
+	require.NoError(t, err, "Failed to parse ABI")
 
 	// Generate a new random account and a funded simulator
-	key, _ := crypto.GenerateKey()
-	auth, _ := bind.NewKeyedTransactorWithChainID(key, big.NewInt(1337))
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err, "Failed to generate key")
+	auth, err := bind.NewKeyedTransactorWithChainID(key, big.NewInt(1337))
+	require.NoError(t, err, "Failed to create transactor")
 	alloc := types.GenesisAlloc{auth.From: {Balance: big.NewInt(1000000000000000000)}}
 	atApricotPhase2 := func(nodeConf *node.Config, ethConf *ethconfig.Config) {
 		chainConfig := *params.TestApricotPhase2Config
@@ -73,13 +74,14 @@ func TestGetSenderNativeAssetCall(t *testing.T) {
 		Backend: b,
 		Client:  b.Client(),
 	}
-	defer sim.Close()
+	t.Cleanup(func() {
+		err = sim.Close()
+		require.NoError(t, err, "Failed to close simulator")
+	})
 
 	// Deploy the get/setSender contract
 	_, _, interactor, err := bind.DeployContract(auth, parsedABI, bytecode, sim)
-	if err != nil {
-		t.Fatalf("Failed to deploy interactor contract: %v", err)
-	}
+	require.NoError(t, err, "Failed to deploy interactor contract")
 	sim.Commit(false)
 
 	// Setting NativeAssetCall in the transact opts will proxy the call through
@@ -91,21 +93,15 @@ func TestGetSenderNativeAssetCall(t *testing.T) {
 			AssetAmount: big.NewInt(0),
 		},
 	}
-	if _, err := interactor.Transact(opts, setSenderMethodName); err != nil {
-		t.Fatalf("Failed to set sender: %v", err)
-	}
+	_, err = interactor.Transact(opts, setSenderMethodName)
+	require.NoError(t, err, "Failed to set sender")
 	sim.Commit(true)
 
-	var results []interface{}
-	if err := interactor.Call(nil, &results, getSenderMethodName); err != nil {
-		t.Fatalf("Failed to get sender: %v", err)
-	}
-	if len(results) != 1 {
-		t.Fatalf("Expected one result, got %d", len(results))
-	}
-	if addr, ok := results[0].(common.Address); !ok {
-		t.Fatalf("Expected address, got %T", results[0])
-	} else if addr != auth.From {
-		t.Fatalf("Address mismatch: have '%v'", addr)
-	}
+	var results []any
+	err = interactor.Call(nil, &results, getSenderMethodName)
+	require.NoError(t, err, "Failed to get sender")
+	require.Len(t, results, 1)
+	addr, ok := results[0].(common.Address)
+	require.Truef(t, ok, "Expected %T, got %T", common.Address{}, results[0])
+	require.Equal(t, addr, auth.From, "Address mismatch")
 }

consensus/dummy/consensus.go

@@ -148,19 +148,19 @@ func NewFullFaker() *DummyEngine {
 	}
 }
 
-func verifyHeaderGasFields(configExtra *extras.ChainConfig, header *types.Header, parent *types.Header) error {
-	if err := customheader.VerifyGasUsed(configExtra, parent, header); err != nil {
+func verifyHeaderGasFields(config *extras.ChainConfig, header *types.Header, parent *types.Header) error {
+	if err := customheader.VerifyGasUsed(config, parent, header); err != nil {
 		return err
 	}
-	if err := customheader.VerifyGasLimit(configExtra, parent, header); err != nil {
+	if err := customheader.VerifyGasLimit(config, parent, header); err != nil {
 		return err
 	}
-	if err := customheader.VerifyExtraPrefix(configExtra, parent, header); err != nil {
+	if err := customheader.VerifyExtraPrefix(config, parent, header); err != nil {
 		return err
 	}
 
 	// Verify header.BaseFee matches the expected value.
-	expectedBaseFee, err := customheader.BaseFee(configExtra, parent, header.Time)
+	expectedBaseFee, err := customheader.BaseFee(config, parent, header.Time)
 	if err != nil {
 		return fmt.Errorf("failed to calculate base fee: %w", err)
 	}
@@ -172,7 +172,7 @@ func verifyHeaderGasFields(configExtra *extras.ChainConfig, header *types.Header
 
 	// Enforce BlockGasCost constraints
 	expectedBlockGasCost := customheader.BlockGasCost(
-		configExtra,
+		config,
 		parent,
 		header.Time,
 	)
@@ -181,7 +181,7 @@ func verifyHeaderGasFields(configExtra *extras.ChainConfig, header *types.Header
 	}
 
 	// Verify ExtDataGasUsed not present before AP4
-	if !configExtra.IsApricotPhase4(header.Time) {
+	if !config.IsApricotPhase4(header.Time) {
 		if headerExtra.ExtDataGasUsed != nil {
 			return fmt.Errorf("invalid extDataGasUsed before fork: have %d, want <nil>", headerExtra.ExtDataGasUsed)
 		}
@@ -377,19 +377,19 @@ func (eng *DummyEngine) Finalize(chain consensus.ChainHeaderReader, block *types
 		}
 	}
 
-	configExtra := params.GetExtra(chain.Config())
+	config := params.GetExtra(chain.Config())
 	timestamp := block.Time()
 	// Verify the BlockGasCost set in the header matches the expected value.
 	blockGasCost := customtypes.BlockGasCost(block)
 	expectedBlockGasCost := customheader.BlockGasCost(
-		configExtra,
+		config,
 		parent,
 		timestamp,
 	)
 	if !utils.BigEqual(blockGasCost, expectedBlockGasCost) {
 		return fmt.Errorf("invalid blockGasCost: have %d, want %d", blockGasCost, expectedBlockGasCost)
 	}
-	if configExtra.IsApricotPhase4(timestamp) {
+	if config.IsApricotPhase4(timestamp) {
 		// Validate extDataGasUsed and BlockGasCost match expectations
 		//
 		// NOTE: This is a duplicate check of what is already performed in
@@ -440,10 +440,9 @@ func (eng *DummyEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, h
 		header.Time,
 	)
 	if configExtra.IsApricotPhase4(header.Time) {
-		ext := customtypes.GetHeaderExtra(header)
-		ext.ExtDataGasUsed = extDataGasUsed
-		if ext.ExtDataGasUsed == nil {
-			ext.ExtDataGasUsed = new(big.Int).Set(common.Big0)
+		headerExtra.ExtDataGasUsed = extDataGasUsed
+		if headerExtra.ExtDataGasUsed == nil {
+			headerExtra.ExtDataGasUsed = new(big.Int)
 		}
 
 		// Verify that this block covers the block fee.

core/blockchain.go

@@ -67,13 +67,15 @@ import (
 
 // ====== If resolving merge conflicts ======
 //
-// All calls to metrics.NewRegistered*() have been replaced with
-// metrics.GetOrRegister*() and this package's corresponding libevm package
-// imported above. Together these ensure that the metric here is the same as the
-// one with the same name in libevm.
+// All calls to metrics.NewRegistered*() for metrics also defined in libevm/core have been
+// replaced either with:
+//   - metrics.GetOrRegister*() to get a metric already registered in libevm/core, or register it
+//     here otherwise
+//   - [getOrOverrideAsRegisteredCounter] to get a metric already registered in libevm/core
+//     only if it is a [metrics.Counter]. If it is not, the metric is unregistered and registered
+//     as a [metrics.Counter] here.
 //
-// Note, however, those that have had their types overridden as
-// [metrics.Counter].
+// These replacements ensure the same metrics are shared between the two packages.
 var (
 	accountReadTimer         = getOrOverrideAsRegisteredCounter("chain/account/reads", nil)
 	accountHashTimer         = getOrOverrideAsRegisteredCounter("chain/account/hashes", nil)
@@ -195,7 +197,7 @@ func (c *CacheConfig) triedbConfig() *triedb.Config {
 		config.DBOverride = hashdb.Config{
 			CleanCacheSize:                  c.TrieCleanLimit * 1024 * 1024,
 			StatsPrefix:                     trieCleanCacheStatsNamespace,
-			ReferenceRootAtomicallyOnUpdate: true, // Automatically reference root nodes when an update is made
+			ReferenceRootAtomicallyOnUpdate: true,
 		}.BackendConstructor
 	}
 	if c.StateScheme == rawdb.PathScheme {
@@ -1172,8 +1174,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, parentRoot common.
 	}
 
 	// Commit all cached state changes into underlying memory database.
-	var err error
-	_, err = bc.commitWithSnap(block, parentRoot, state)
+	_, err := bc.commitWithSnap(block, parentRoot, state)
 	if err != nil {
 		return err
 	}
@@ -1695,8 +1696,8 @@ func (bc *BlockChain) reprocessBlock(parent *types.Block, current *types.Block)
 func (bc *BlockChain) commitWithSnap(
 	current *types.Block, parentRoot common.Hash, statedb *state.StateDB,
 ) (common.Hash, error) {
-	// blockHashes must be passed through Commit since snapshots are based on the
-	// block hash.
+	// blockHashes must be passed through [state.StateDB]'s Commit since snapshots
+	// are based on the block hash.
 	blockHashes := snapshot.WithBlockHashes(current.Hash(), current.ParentHash())
 	root, err := statedb.Commit(current.NumberU64(), bc.chainConfig.IsEIP158(current.Number()), blockHashes)
 	if err != nil {
@@ -1851,7 +1852,7 @@ func (bc *BlockChain) reprocessState(current *types.Block, reexec uint64) error
 		// Flatten snapshot if initialized, holding a reference to the state root until the next block
 		// is processed.
 		if err := bc.flattenSnapshot(func() error {
-			if previousRoot != (common.Hash{}) {
+			if previousRoot != (common.Hash{}) && previousRoot != root {
 				triedb.Dereference(previousRoot)
 			}
 			previousRoot = root

core/blockchain_ext.go

@@ -5,9 +5,9 @@ package core
 import "github.com/ava-labs/libevm/metrics"
 
 // getOrOverrideAsRegisteredCounter searches for a metric already registered
-// with`name`. If a metric is found and it is a Counter, it is returned. If a
-// metric is found and it is not a Counter, it is unregistered and replaced with
-// a new registered Counter. If no metric is found, a new Counter is constructed
+// with `name`. If a metric is found and it is a [metrics.Counter], it is returned. If a
+// metric is found and it is not a [metrics.Counter], it is unregistered and replaced with
+// a new registered [metrics.Counter]. If no metric is found, a new [metrics.Counter] is constructed
 // and registered.
 //
 // This is necessary for a metric defined in libevm with the same name but a
@@ -17,11 +17,10 @@ func getOrOverrideAsRegisteredCounter(name string, r metrics.Registry) metrics.C
 		r = metrics.DefaultRegistry
 	}
 
-	switch c := r.GetOrRegister(name, metrics.NewCounter).(type) {
-	case metrics.Counter:
+	if c, ok := r.GetOrRegister(name, metrics.NewCounter).(metrics.Counter); ok {
 		return c
-	default: // `name` must have already been registered to be any other type
-		r.Unregister(name)
-		return metrics.NewRegisteredCounter(name, r)
 	}
+	// `name` must have already been registered to be any other type
+	r.Unregister(name)
+	return metrics.NewRegisteredCounter(name, r)
 }

core/blockchain_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/ava-labs/coreth/core/state"
 	"github.com/ava-labs/coreth/core/state/pruner"
 	"github.com/ava-labs/coreth/params"
-	"github.com/ava-labs/coreth/params/extras"
 	"github.com/ava-labs/coreth/plugin/evm/customrawdb"
 	"github.com/ava-labs/coreth/plugin/evm/upgrade/ap3"
 	"github.com/ava-labs/libevm/common"
@@ -425,11 +424,8 @@ func TestUngracefulAsyncShutdown(t *testing.T) {
 	// Ensure that key1 has some funds in the genesis block.
 	genesisBalance := big.NewInt(1000000)
 	gspec := &Genesis{
-		Config: params.WithExtra(
-			&params.ChainConfig{HomesteadBlock: new(big.Int)},
-			&extras.ChainConfig{},
-		),
-		Alloc: types.GenesisAlloc{addr1: {Balance: genesisBalance}},
+		Config: &params.ChainConfig{HomesteadBlock: new(big.Int)},
+		Alloc:  types.GenesisAlloc{addr1: {Balance: genesisBalance}},
 	}
 
 	blockchain, err := create(chainDB, gspec, common.Hash{})

core/chain_makers.go

@@ -373,12 +373,12 @@ func GenerateChainWithGenesis(genesis *Genesis, engine consensus.Engine, n int,
 func (cm *chainMaker) makeHeader(parent *types.Block, gap uint64, state *state.StateDB, engine consensus.Engine) *types.Header {
 	time := parent.Time() + gap // block time is fixed at [gap] seconds
 
-	configExtra := params.GetExtra(cm.config)
-	gasLimit, err := header.GasLimit(configExtra, parent.Header(), time)
+	config := params.GetExtra(cm.config)
+	gasLimit, err := header.GasLimit(config, parent.Header(), time)
 	if err != nil {
 		panic(err)
 	}
-	baseFee, err := header.BaseFee(configExtra, parent.Header(), time)
+	baseFee, err := header.BaseFee(config, parent.Header(), time)
 	if err != nil {
 		panic(err)
 	}

core/evm.go

@@ -49,14 +49,14 @@ func init() {
 
 type hooks struct{}
 
-// OverrideNewEVMArgs is a hook that is called back when a new EVM is created.
+// OverrideNewEVMArgs is a hook that is called in [vm.NewEVM].
 // It allows for the modification of the EVM arguments before the EVM is created.
 // Specifically, we set Random to be the same as Difficulty since Shanghai.
 // This allows using the same jump table as upstream.
 // Then we set Difficulty to 0 as it is post Merge in upstream.
 // Additionally we wrap the StateDB with the appropriate StateDB wrapper,
 // which is used in coreth to process historical pre-AP1 blocks with the
-// GetCommittedState method as it was historically.
+// [StateDbAP1.GetCommittedState] method as it was historically.
 func (hooks) OverrideNewEVMArgs(args *vm.NewEVMArgs) *vm.NewEVMArgs {
 	rules := args.ChainConfig.Rules(args.BlockContext.BlockNumber, params.IsMergeTODO, args.BlockContext.Time)
 	args.StateDB = wrapStateDB(rules, args.StateDB)
@@ -79,7 +79,7 @@ func wrapStateDB(rules params.Rules, db vm.StateDB) vm.StateDB {
 	if params.GetRulesExtra(rules).IsApricotPhase1 {
 		db = &StateDbAP1{db.(extstate.VmStateDB)}
 	}
-	return &extstate.StateDB{VmStateDB: db.(extstate.VmStateDB)}
+	return extstate.New(db.(extstate.VmStateDB))
 }
 
 type StateDbAP1 struct {
@@ -121,7 +121,6 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
 	if header.ExcessBlobGas != nil {
 		blobBaseFee = eip4844.CalcBlobFee(*header.ExcessBlobGas)
 	}
-
 	return vm.BlockContext{
 		CanTransfer: CanTransfer,
 		Transfer:    Transfer,
@@ -144,14 +143,14 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
 // NewEVMBlockContextWithPredicateResults creates a new context for use in the
 // EVM with an override for the predicate results. The miner uses this to pass
 // predicate results to the EVM when header.Extra is not fully formed yet.
-func NewEVMBlockContextWithPredicateResults(rules extras.AvalancheRules, header *types.Header, chain ChainContext, author *common.Address, predicateResults []byte) vm.BlockContext {
+func NewEVMBlockContextWithPredicateResults(rules extras.AvalancheRules, header *types.Header, chain ChainContext, author *common.Address, predicateBytes []byte) vm.BlockContext {
 	blockCtx := NewEVMBlockContext(header, chain, author)
 	// Note this only sets the block context, which is the hand-off point for
 	// the EVM. The actual header is not modified.
 	blockCtx.Header.Extra = customheader.SetPredicateBytesInExtra(
 		rules,
 		bytes.Clone(header.Extra),
-		predicateResults,
+		predicateBytes,
 	)
 	return blockCtx
 }