minor refactor. instrument the bal tracer with some debug logging

core/block_access_list_tracer.go

@@ -28,6 +28,7 @@ func NewBlockAccessListTracer() (*BlockAccessListTracer, *tracing.Hooks) {
 		OnBlockFinalization:  balTracer.OnBlockFinalization,
 		OnPreTxExecutionDone: balTracer.OnPreTxExecutionDone,
 		OnTxEnd:              balTracer.TxEndHook,
+		OnTxStart:            balTracer.TxStartHook,
 		OnEnter:              balTracer.OnEnter,
 		OnExit:               balTracer.OnExit,
 		OnCodeChangeV2:       balTracer.OnCodeChange,
@@ -54,6 +55,10 @@ func (a *BlockAccessListTracer) OnPreTxExecutionDone() {
 	a.balIdx++
 }
 
+func (a *BlockAccessListTracer) TxStartHook(vm *tracing.VMContext, tx *types.Transaction, from common.Address) {
+	a.builder.EnterTx(tx.Hash())
+}
+
 func (a *BlockAccessListTracer) TxEndHook(receipt *types.Receipt, err error) {
 	a.builder.FinaliseIdxChanges(a.balIdx)
 	a.balIdx++

core/blockchain.go

@@ -2020,8 +2020,8 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 		return nil, err
 	}
 
-	accessList := state.NewBALReader(block, reader)
+	stateReader := state.NewBALReader(block, reader)
-	stateTransition, err := state.NewBALStateTransition(accessList, bc.statedb, parentRoot)
+	stateTransition, err := state.NewBALStateTransition(stateReader, bc.statedb, parentRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -2030,7 +2030,7 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block *
 		return nil, err
 	}
 
-	statedb.SetBlockAccessList(accessList)
+	statedb.SetBlockAccessList(stateReader)
 
 	if bc.logger != nil && bc.logger.OnBlockStart != nil {
 		bc.logger.OnBlockStart(tracing.BlockEvent{

core/parallel_state_processor.go

@@ -286,20 +286,17 @@ func (p *ParallelStateProcessor) execTx(block *types.Block, tx *types.Transactio
 // Process performs EVM execution and state root computation for a block which is known
 // to contain an access list.
 func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *state.BALStateTransition, statedb *state.StateDB, cfg vm.Config) (*ProcessResultWithMetrics, error) {
-	//fmt.Println("Parallel Process")
 	var (
-		header = block.Header()
+		header           = block.Header()
-		resCh  = make(chan *ProcessResultWithMetrics)
+		resCh            = make(chan *ProcessResultWithMetrics)
-		signer = types.MakeSigner(p.chainConfig(), header.Number, header.Time)
+		signer           = types.MakeSigner(p.chainConfig(), header.Number, header.Time)
-	)
-
-	txResCh := make(chan txExecResult)
-	pStart := time.Now()
-	var (
-		tPreprocess      time.Duration // time to create a set of prestates for parallel transaction execution
-		tExecStart       time.Time
 		rootCalcResultCh = make(chan stateRootCalculationResult)
 		context          vm.BlockContext
+		txResCh          = make(chan txExecResult)
+
+		pStart      = time.Now()
+		tExecStart  time.Time
+		tPreprocess time.Duration // time to create a set of prestates for parallel transaction execution
 	)
 
 	balTracer, hooks := NewBlockAccessListTracer()
@@ -317,7 +314,6 @@ func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *st
 		ProcessParentBlockHash(block.ParentHash(), evm)
 	}
 
-	// TODO: weird that I have to manually call finalize here
 	balTracer.OnPreTxExecutionDone()
 
 	diff, stateReads := balTracer.builder.FinalizedIdxChanges()
@@ -328,13 +324,9 @@ func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *st
 	// compute the post-tx state prestate (before applying final block system calls and eip-4895 withdrawals)
 	// the post-tx state transition is verified by resultHandler
 	postTxState := statedb.Copy()
-
 	tPreprocess = time.Since(pStart)
 
 	// execute transactions and state root calculation in parallel
-
-	// TODO: figure out how to funnel the state reads from the bal tracer through to the post-block-exec state/slot read
-	// validation
 	tExecStart = time.Now()
 	go p.resultHandler(block, stateReads, postTxState, tExecStart, txResCh, rootCalcResultCh, resCh)
 	var workers errgroup.Group
@@ -355,7 +347,7 @@ func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *st
 	if res.ProcessResult.Error != nil {
 		return nil, res.ProcessResult.Error
 	}
+	// TODO: remove preprocess metric ?
 	res.PreProcessTime = tPreprocess
-	//	res.PreProcessLoadTime = tPreprocessLoad
 	return res, nil
 }

core/state/bal_reader.go

@@ -9,11 +9,15 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/holiman/uint256"
 	"sync"
-	"time"
 )
 
 // TODO: probably unnecessary to cache the resolved state object here as it will already be in the db cache?
 // ^ experiment with the performance of keeping this as-is vs just using the db cache.
+
+// prestateResolver asynchronously fetches the prestate state accounts of addresses
+// which are reported as modified in EIP-7928 access lists in order to produce the full
+// updated state account (including fields that weren't modified in the BAL) for the
+// state root update
 type prestateResolver struct {
 	inProgress map[common.Address]chan struct{}
 	resolved   sync.Map
@@ -21,7 +25,9 @@ type prestateResolver struct {
 	cancel     func()
 }
 
-func (p *prestateResolver) resolve(r Reader, addrs []common.Address) {
+// schedule begins the retrieval of a set of state accounts running on
+// a background goroutine.
+func (p *prestateResolver) schedule(r Reader, addrs []common.Address) {
 	p.inProgress = make(map[common.Address]chan struct{})
 	p.ctx, p.cancel = context.WithCancel(context.Background())
 
@@ -29,6 +35,8 @@ func (p *prestateResolver) resolve(r Reader, addrs []common.Address) {
 		p.inProgress[addr] = make(chan struct{})
 	}
 
+	// TODO: probably we can retrieve these on a single go-routine
+	// the transaction execution will also load them
 	for _, addr := range addrs {
 		resolveAddr := addr
 		go func() {
@@ -52,6 +60,8 @@ func (p *prestateResolver) stop() {
 	p.cancel()
 }
 
+// account returns the state account for the given address, blocking if it is
+// still being resolved from disk.
 func (p *prestateResolver) account(addr common.Address) *types.StateAccount {
 	if _, ok := p.inProgress[addr]; !ok {
 		return nil
@@ -118,7 +128,7 @@ func NewBALReader(block *types.Block, reader Reader) *BALReader {
 	for _, acctDiff := range *block.Body().AccessList {
 		r.accesses[acctDiff.Address] = &acctDiff
 	}
-	r.prestateReader.resolve(reader, r.ModifiedAccounts())
+	r.prestateReader.schedule(reader, r.ModifiedAccounts())
 	return r
 }
 
@@ -158,35 +168,10 @@ func (r *BALReader) ValidateStateReads(allReads bal.StateAccesses) error {
 		}
 	}
 
-	// TODO: where do we validate that the storage read/write sets are distinct?
-
 	return nil
 }
 
-func (r *BALReader) AccessedState() (res map[common.Address]map[common.Hash]struct{}) {
+// changesAt returns all state changes occurring at the given index.
-	res = make(map[common.Address]map[common.Hash]struct{})
-	for addr, accesses := range r.accesses {
-		if len(accesses.StorageReads) > 0 {
-			res[addr] = make(map[common.Hash]struct{})
-			for _, slot := range accesses.StorageReads {
-				res[addr][slot] = struct{}{}
-			}
-		} else if len(accesses.BalanceChanges) == 0 && len(accesses.NonceChanges) == 0 && len(accesses.StorageChanges) == 0 && len(accesses.CodeChanges) == 0 {
-			res[addr] = make(map[common.Hash]struct{})
-		}
-	}
-	return
-}
-
-// TODO: it feels weird that this modifies the prestate instance. However, it's needed because it will
-// subsequently be used in Commit.
-func (r *BALReader) StateRoot(prestate *StateDB) (root common.Hash, prestateLoadTime time.Duration, rootUpdateTime time.Duration) {
-	root = prestate.IntermediateRoot(true)
-	// TODO: fix the metrics calculation here
-	return root, prestateLoadTime, rootUpdateTime
-}
-
-// changesAt returns all state changes at the given index.
 func (r *BALReader) changesAt(idx int) *bal.StateDiff {
 	res := &bal.StateDiff{make(map[common.Address]*bal.AccountMutations)}
 	for addr, _ := range r.accesses {
@@ -208,6 +193,8 @@ func (r *BALReader) accountChangesAt(addr common.Address, idx int) *bal.AccountM
 
 	var res bal.AccountMutations
 
+	// TODO: remove the reverse iteration here to clean the code up
+
 	for i := len(acct.BalanceChanges) - 1; i >= 0; i-- {
 		if acct.BalanceChanges[i].TxIdx == uint16(idx) {
 			res.Balance = acct.BalanceChanges[i].Balance
@@ -277,7 +264,8 @@ func (r *BALReader) readAccount(db *StateDB, addr common.Address, idx int) *stat
 	return r.initObjFromDiff(db, addr, prestate, diff)
 }
 
-// readAccountDiff returns the accumulated state changes of an account up through idx.
+// readAccountDiff returns the accumulated state changes of an account up
+// through, and including the given index.
 func (r *BALReader) readAccountDiff(addr common.Address, idx int) *bal.AccountMutations {
 	diff, exist := r.accesses[addr]
 	if !exist {

core/state/bal_state_transition.go

@@ -25,7 +25,7 @@ type BALStateTransition struct {
 	stateTrie  Trie
 	parentRoot common.Hash
 
-	// the state root of the block
+	// the computed state root of the block
 	rootHash common.Hash
 	// the state modifications performed by the block
 	diffs map[common.Address]*bal.AccountMutations
@@ -77,7 +77,7 @@ type BALStateTransitionMetrics struct {
 func NewBALStateTransition(accessList *BALReader, db Database, parentRoot common.Hash) (*BALStateTransition, error) {
 	reader, err := db.Reader(parentRoot)
 	if err != nil {
-		panic("OH FUCK")
+		return nil, err
 	}
 	stateTrie, err := db.OpenTrie(parentRoot)
 	if err != nil {
@@ -102,7 +102,6 @@ func NewBALStateTransition(accessList *BALReader, db Database, parentRoot common
 	}, nil
 }
 
-// TODO: make use of this method return the error from IntermediateRoot or Commit
 func (s *BALStateTransition) Error() error {
 	return s.err
 }

core/state/database.go

@@ -81,7 +81,7 @@ type Trie interface {
 	// be returned.
 	GetAccount(address common.Address) (*types.StateAccount, error)
 
-	// PrefetchAccount attempts to resolve specific accounts from the database
+	// PrefetchAccount attempts to schedule specific accounts from the database
 	// to accelerate subsequent trie operations.
 	PrefetchAccount([]common.Address) error
 
@@ -90,7 +90,7 @@ type Trie interface {
 	// a trie.MissingNodeError is returned.
 	GetStorage(addr common.Address, key []byte) ([]byte, error)
 
-	// PrefetchStorage attempts to resolve specific storage slots from the database
+	// PrefetchStorage attempts to schedule specific storage slots from the database
 	// to accelerate subsequent trie operations.
 	PrefetchStorage(addr common.Address, keys [][]byte) error
 

core/state/iterator.go

@@ -29,7 +29,7 @@ import (
 
 // nodeIterator is an iterator to traverse the entire state trie post-order,
 // including all of the contract code and contract state tries. Preimage is
-// required in order to resolve the contract address.
+// required in order to schedule the contract address.
 type nodeIterator struct {
 	state *StateDB // State being iterated
 	tr    Trie     // Primary account trie for traversal

core/state/reader.go

@@ -315,7 +315,7 @@ func (r *trieReader) Storage(addr common.Address, key common.Hash) (common.Hash,
 			root, ok := r.subRoots[addr]
 
 			// The storage slot is accessed without account caching. It's unexpected
-			// behavior but try to resolve the account first anyway.
+			// behavior but try to schedule the account first anyway.
 			if !ok {
 				_, err := r.account(addr)
 				if err != nil {
@@ -448,14 +448,14 @@ func newReaderWithCache(reader Reader) *readerWithCache {
 //
 // An error will be returned if the state is corrupted in the underlying reader.
 func (r *readerWithCache) account(addr common.Address) (*types.StateAccount, bool, error) {
-	// Try to resolve the requested account in the local cache
+	// Try to schedule the requested account in the local cache
 	r.accountLock.RLock()
 	acct, ok := r.accounts[addr]
 	r.accountLock.RUnlock()
 	if ok {
 		return acct, true, nil
 	}
-	// Try to resolve the requested account from the underlying reader
+	// Try to schedule the requested account from the underlying reader
 	acct, err := r.Reader.Account(addr)
 	if err != nil {
 		return nil, false, err
@@ -484,7 +484,7 @@ func (r *readerWithCache) storage(addr common.Address, slot common.Hash) (common
 		ok     bool
 		bucket = &r.storageBuckets[addr[0]&0x0f]
 	)
-	// Try to resolve the requested storage slot in the local cache
+	// Try to schedule the requested storage slot in the local cache
 	bucket.lock.RLock()
 	slots, ok := bucket.storages[addr]
 	if ok {
@@ -494,7 +494,7 @@ func (r *readerWithCache) storage(addr common.Address, slot common.Hash) (common
 	if ok {
 		return value, true, nil
 	}
-	// Try to resolve the requested storage slot from the underlying reader
+	// Try to schedule the requested storage slot from the underlying reader
 	value, err := r.Reader.Storage(addr, slot)
 	if err != nil {
 		return common.Hash{}, false, err

core/types/bal/bal.go

@@ -21,7 +21,9 @@ import (
 	"encoding/json"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/holiman/uint256"
+	"log/slog"
 	"maps"
+	"os"
 )
 
 // idxAccessListBuilder is responsible for producing the state accesses and
@@ -36,14 +38,18 @@ type idxAccessListBuilder struct {
 	// and terminating a frame merges the accesses/modifications into the
 	// intermediate access list of the calling frame.
 	accessesStack []map[common.Address]*constructionAccountAccess
+
+	// context logger for instrumenting debug logging
+	logger *slog.Logger
 }
 
-func newAccessListBuilder() *idxAccessListBuilder {
+func newAccessListBuilder(logger *slog.Logger) *idxAccessListBuilder {
 	return &idxAccessListBuilder{
 		make(map[common.Address]*accountIdxPrestate),
 		[]map[common.Address]*constructionAccountAccess{
 			make(map[common.Address]*constructionAccountAccess),
 		},
+		logger,
 	}
 }
 
@@ -175,6 +181,8 @@ func (c *idxAccessListBuilder) exitScope(evmErr bool) {
 		}
 	}
 
+	c.logger.Info("parent access list", "depth", len(c.accessesStack), "access list", parentAccessList)
+
 	c.accessesStack = c.accessesStack[:len(c.accessesStack)-1]
 }
 
@@ -194,6 +202,10 @@ func (a *idxAccessListBuilder) finalise() (*StateDiff, StateAccesses) {
 			access.balance = nil
 		}
 
+		if addr == common.HexToAddress("6e2e9c4d90be192a84d25ed58f1a38261cd3bc15") {
+			//fmt.Printf("access code is %x\n", access.code)
+			//fmt.Printf("prestate code is %x\n", a.prestates[addr].code)
+		}
 		if access.code != nil && bytes.Equal(access.code, a.prestates[addr].code) {
 			access.code = nil
 		}
@@ -231,12 +243,20 @@ func (a *idxAccessListBuilder) finalise() (*StateDiff, StateAccesses) {
 	return diff, stateAccesses
 }
 
+func (c *AccessListBuilder) EnterTx(txHash common.Hash) {
+	logger := slog.New(slog.DiscardHandler)
+	if txHash == common.HexToHash("0xf5df8d7a86856fc2e562abd47260237ec01adf2f7b46bc9fcb08485e73b77c14") {
+		logger = slog.New(slog.NewTextHandler(os.Stdout, nil))
+	}
+	c.idxBuilder = newAccessListBuilder(logger)
+}
+
 // FinaliseIdxChanges records all pending state mutations/accesses in the
 // access list at the given index.  The set of pending state mutations/accesse are
 // then emptied.
 func (c *AccessListBuilder) FinaliseIdxChanges(idx uint16) {
 	pendingDiff, pendingAccesses := c.idxBuilder.finalise()
-	c.idxBuilder = newAccessListBuilder()
+	c.idxBuilder = newAccessListBuilder(slog.New(slog.DiscardHandler))
 
 	// if any of the newly-written storage slots were previously
 	// accessed, they must be removed from the accessed state set.
@@ -485,15 +505,18 @@ type AccessListBuilder struct {
 
 	lastFinalizedMutations *StateDiff
 	lastFinalizedAccesses  StateAccesses
+	logger                 *slog.Logger
 }
 
 // NewAccessListBuilder instantiates an empty access list.
 func NewAccessListBuilder() *AccessListBuilder {
+	logger := slog.New(slog.DiscardHandler)
 	return &AccessListBuilder{
 		make(map[common.Address]*ConstructionAccountAccesses),
-		newAccessListBuilder(),
+		newAccessListBuilder(logger),
 		nil,
 		nil,
+		logger,
 	}
 }
 

core/types/bal/bal_encoding.go

@@ -176,8 +176,6 @@ func (e *AccountAccess) validate() error {
 			return err
 		}
 	}
-	// test case ideas: keys in both read/writes, duplicate keys in either read/writes
-	// ensure that the read and write key sets are distinct
 	readKeys := make(map[common.Hash]struct{})
 	writeKeys := make(map[common.Hash]struct{})
 	for _, readKey := range e.StorageReads {
@@ -221,7 +219,8 @@ func (e *AccountAccess) validate() error {
 		return errors.New("nonce changes not in ascending order by tx index")
 	}
 
-	// Convert code change
+	// validate that code changes could plausibly be correct (none exceed
+	// max code size of a contract)
 	for _, codeChange := range e.CodeChanges {
 		if len(codeChange.Code) > params.MaxCodeSize {
 			return fmt.Errorf("code change contained oversized code")