refactor: move access list index mutations/read construction within the ConstructionAccessList and don't expose it publicly

core/block_access_list_creation.go

@@ -23,22 +23,16 @@ type BlockAccessListTracer struct {
 	// scopes are in the proceeding indices.
 	// When an execution scope terminates in a non-reverting fashion, the changes are
 	// merged into the access list of the parent scope.
-	blockTxCount      int
+	accessList *bal.ConstructionBlockAccessList
-	accessList        *bal.ConstructionBlockAccessList
-	balIdx            uint16
-	accessListBuilder *bal.AccessListBuilder
 
-	// mutations and state reads from currently-executing bal index
+	// the access list index that changes are currently being recorded into
-	idxMutations *bal.StateDiff
+	balIdx uint16
-	idxReads     bal.StateAccesses
 }
 
 // NewBlockAccessListTracer returns an BlockAccessListTracer and a set of hooks
-func NewBlockAccessListTracer(startIdx int) (*BlockAccessListTracer, *tracing.Hooks) {
+func NewBlockAccessListTracer() (*BlockAccessListTracer, *tracing.Hooks) {
 	balTracer := &BlockAccessListTracer{
 		accessList: bal.NewConstructionBlockAccessList(),
-		//balIdx:            uint16(startIdx),
-		accessListBuilder: bal.NewAccessListBuilder(),
 	}
 	hooks := &tracing.Hooks{
 		OnBlockFinalization:  balTracer.OnBlockFinalization,
@@ -50,8 +44,8 @@ func NewBlockAccessListTracer(startIdx int) (*BlockAccessListTracer, *tracing.Ho
 		OnBalanceChange:      balTracer.OnBalanceChange,
 		OnNonceChangeV2:      balTracer.OnNonceChange,
 		OnStorageChange:      balTracer.OnStorageChange,
-		OnColdAccountRead:    balTracer.OnColdAccountRead,
+		OnStorageRead:        balTracer.OnStorageRead,
-		OnColdStorageRead:    balTracer.OnColdStorageRead,
+		OnAccountRead:        balTracer.OnAcountRead,
 		OnSelfDestructChange: balTracer.OnSelfDestruct,
 	}
 	wrappedHooks, err := tracing.WrapWithJournal(hooks)
@@ -69,72 +63,57 @@ func (a *BlockAccessListTracer) AccessList() *bal.ConstructionBlockAccessList {
 }
 
 func (a *BlockAccessListTracer) OnPreTxExecutionDone() {
-	a.idxMutations, a.idxReads = a.accessListBuilder.FinaliseIdxChanges()
+	a.accessList.FinalisePendingChanges(0)
-	a.accessList.Apply(0, a.idxMutations, a.idxReads)
-	a.accessListBuilder = bal.NewAccessListBuilder()
 	a.balIdx++
 }
 
-// TODO: I don't like that AccessList and this do slightly different things,
-// and that they mutate the access list builder (not apparent in the naming of the methods)
-//
-// ^ idea: add Finalize() which returns the diff/accesses, also accumulating them in the BAL.
-// AccessList just returns the constructed BAL.
-func (a *BlockAccessListTracer) IdxChanges() (*bal.StateDiff, bal.StateAccesses) {
-	return a.idxMutations, a.idxReads
-}
-
 func (a *BlockAccessListTracer) TxEndHook(receipt *types.Receipt, err error) {
-	a.idxMutations, a.idxReads = a.accessListBuilder.FinaliseIdxChanges()
+	a.accessList.FinalisePendingChanges(a.balIdx)
-	a.accessList.Apply(a.balIdx, a.idxMutations, a.idxReads)
-	a.accessListBuilder = bal.NewAccessListBuilder()
 	a.balIdx++
 }
 
 func (a *BlockAccessListTracer) OnEnter(depth int, typ byte, from common.Address, to common.Address, input []byte, gas uint64, value *big.Int) {
-	a.accessListBuilder.EnterScope()
+	a.accessList.EnterScope()
 }
 
 func (a *BlockAccessListTracer) OnExit(depth int, output []byte, gasUsed uint64, err error, reverted bool) {
-	a.accessListBuilder.ExitScope(reverted)
+	a.accessList.ExitScope(reverted)
 }
 
 func (a *BlockAccessListTracer) OnCodeChange(addr common.Address, prevCodeHash common.Hash, prevCode []byte, codeHash common.Hash, code []byte, reason tracing.CodeChangeReason) {
 	// TODO: if we don't have this equality check, some tests fail.  should be investigated.
 	// probably the tracer shouldn't invoke code change if the code didn't actually change tho.
 	if prevCodeHash != codeHash {
-		a.accessListBuilder.CodeChange(addr, prevCode, code)
+		a.accessList.CodeChange(addr, prevCode, code)
 	}
 }
 
 func (a *BlockAccessListTracer) OnSelfDestruct(addr common.Address) {
-	a.accessListBuilder.SelfDestruct(addr)
+	a.accessList.SelfDestruct(addr)
 }
 
 func (a *BlockAccessListTracer) OnBlockFinalization() {
-	a.idxMutations, a.idxReads = a.accessListBuilder.FinaliseIdxChanges()
+	a.accessList.FinalisePendingChanges(a.balIdx)
-	a.accessList.Apply(a.balIdx, a.idxMutations, a.idxReads)
-	a.accessListBuilder = bal.NewAccessListBuilder()
 }
 
 func (a *BlockAccessListTracer) OnBalanceChange(addr common.Address, prevBalance, newBalance *big.Int, _ tracing.BalanceChangeReason) {
 	newU256 := new(uint256.Int).SetBytes(newBalance.Bytes())
 	prevU256 := new(uint256.Int).SetBytes(prevBalance.Bytes())
-	a.accessListBuilder.BalanceChange(addr, prevU256, newU256)
+	a.accessList.BalanceChange(addr, prevU256, newU256)
 }
 
 func (a *BlockAccessListTracer) OnNonceChange(addr common.Address, prev uint64, new uint64, reason tracing.NonceChangeReason) {
-	a.accessListBuilder.NonceChange(addr, prev, new)
+	a.accessList.NonceChange(addr, prev, new)
 }
 
-func (a *BlockAccessListTracer) OnColdStorageRead(addr common.Address, key common.Hash) {
+func (a *BlockAccessListTracer) OnStorageRead(addr common.Address, key common.Hash) {
-	a.accessListBuilder.StorageRead(addr, key)
+	a.accessList.StorageRead(addr, key)
 }
 
-func (a *BlockAccessListTracer) OnColdAccountRead(addr common.Address) {
+func (a *BlockAccessListTracer) OnAcountRead(addr common.Address) {
-	a.accessListBuilder.AccountRead(addr)
+	a.accessList.AccountRead(addr)
 }
 
 func (a *BlockAccessListTracer) OnStorageChange(addr common.Address, slot common.Hash, prev common.Hash, new common.Hash) {
-	a.accessListBuilder.StorageWrite(addr, slot, prev, new)
+	a.accessList.StorageWrite(addr, slot, prev, new)
 }

core/blockchain.go

@@ -2150,7 +2150,7 @@ func (bc *BlockChain) ProcessBlock(parentRoot common.Hash, block *types.Block, s
 		var balTracer *BlockAccessListTracer
 		// Process block using the parent state as reference point
 		if constructBALForTesting {
-			balTracer, bc.cfg.VmConfig.Tracer = NewBlockAccessListTracer(0)
+			balTracer, bc.cfg.VmConfig.Tracer = NewBlockAccessListTracer()
 		}
 		// Process block using the parent state as reference point
 		pstart := time.Now()

core/parallel_state_processor.go

@@ -57,7 +57,7 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR
 	tPostprocessStart := time.Now()
 	header := block.Header()
 
-	balTracer, hooks := NewBlockAccessListTracer(len(block.Transactions()) + 1)
+	balTracer, hooks := NewBlockAccessListTracer()
 	tracingStateDB := state.NewHookedState(postTxState, hooks)
 	context := NewEVMBlockContext(header, p.chain, nil)
 	postTxState.SetAccessListIndex(len(block.Transactions()) + 1)
@@ -125,7 +125,7 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR
 	postTxState.Finalise(true)
 
 	balTracer.OnBlockFinalization()
-	diff, stateReads := balTracer.IdxChanges()
+	diff, stateReads := balTracer.accessList.FinalizedIdxChanges()
 	allStateReads.Merge(stateReads)
 
 	balIdx := len(block.Transactions()) + 1
@@ -248,7 +248,7 @@ func (p *ParallelStateProcessor) calcAndVerifyRoot(preState *state.StateDB, bloc
 // execTx executes single transaction returning a result which includes state accessed/modified
 func (p *ParallelStateProcessor) execTx(block *types.Block, tx *types.Transaction, txIdx int, db *state.StateDB, signer types.Signer) *txExecResult {
 	header := block.Header()
-	balTracer, hooks := NewBlockAccessListTracer(txIdx + 1)
+	balTracer, hooks := NewBlockAccessListTracer()
 	tracingStateDB := state.NewHookedState(db, hooks)
 	context := NewEVMBlockContext(header, p.chain, nil)
 
@@ -278,7 +278,7 @@ func (p *ParallelStateProcessor) execTx(block *types.Block, tx *types.Transactio
 		return &txExecResult{err: err}
 	}
 
-	diff, accesses := balTracer.IdxChanges()
+	diff, accesses := balTracer.accessList.FinalizedIdxChanges()
 	if err := db.BlockAccessList().ValidateStateDiff(txIdx+1, diff); err != nil {
 		return &txExecResult{err: err}
 	}
@@ -317,7 +317,7 @@ func (p *ParallelStateProcessor) Process(block *types.Block, statedb *state.Stat
 	alReader := state.NewBALReader(block, statedb)
 	statedb.SetBlockAccessList(alReader)
 
-	balTracer, hooks := NewBlockAccessListTracer(0)
+	balTracer, hooks := NewBlockAccessListTracer()
 	tracingStateDB := state.NewHookedState(statedb, hooks)
 	// TODO: figure out exactly why we need to set the hooks on the TracingStateDB and the vm.Config
 	cfg.Tracer = hooks
@@ -335,7 +335,7 @@ func (p *ParallelStateProcessor) Process(block *types.Block, statedb *state.Stat
 	// TODO: weird that I have to manually call finalize here
 	balTracer.OnPreTxExecutionDone()
 
-	diff, stateReads := balTracer.IdxChanges()
+	diff, stateReads := balTracer.accessList.FinalizedIdxChanges()
 	if err := statedb.BlockAccessList().ValidateStateDiff(0, diff); err != nil {
 		return nil, err
 	}

core/types/bal/bal.go

@@ -25,33 +25,9 @@ import (
 	"maps"
 )
 
-/*
+// idxAccessListBuilder is responsible for producing the state accesses and
-BAL Building rework
+// reads recorded within the scope of a single index in the access list.
-
+type idxAccessListBuilder struct {
-type BALBuilder
-* hold state for the current execution context:
-	* the state mutations that have already been finalized (previous completed txs)
-    * state reads that have been finalized
-	* the pending state reads/mutations of the current tx
-
-pending state:
-	* a stack (pushing/popping as new execution frames are entered/exited),
-      each item is a map (address -> accountStateAndModifications{})
-finalized state:
-	* the ConstructionBlockAccessList type (sans the pending state stuff that I have added there
-
-Verification Path:
-* only validate single "transition" at a time:
-	* only need the component which collects pending state and finalizes it for one step.
-
-TLDR:
-* break the pending state into its own struct, out of ConstructionBlockAccessList
-* create a 'BALBuilder' type that encompasses the 'finalized' ConstructionBlockAccessList and pending state
-* ensure that this new model fits nicely with the BAL validation code path
-*/
-
-// TODO: maybe rename this to StateDiffBuilder (?)
-type AccessListBuilder struct {
 	// stores the tx-prestate values of any account/storage values which were modified
 	//
 	// TODO: it's a bit unfortunate that the prestate.StorageWrites is reused here to
@@ -64,8 +40,8 @@ type AccessListBuilder struct {
 	accessesStack []map[common.Address]*constructionAccountAccess
 }
 
-func NewAccessListBuilder() *AccessListBuilder {
+func newAccessListBuilder() *idxAccessListBuilder {
-	return &AccessListBuilder{
+	return &idxAccessListBuilder{
 		make(map[common.Address]*AccountState),
 		[]map[common.Address]*constructionAccountAccess{
 			make(map[common.Address]*constructionAccountAccess),
@@ -73,7 +49,7 @@ func NewAccessListBuilder() *AccessListBuilder {
 	}
 }
 
-func (c *AccessListBuilder) StorageRead(address common.Address, key common.Hash) {
+func (c *idxAccessListBuilder) storageRead(address common.Address, key common.Hash) {
 	if _, ok := c.accessesStack[len(c.accessesStack)-1][address]; !ok {
 		c.accessesStack[len(c.accessesStack)-1][address] = &constructionAccountAccess{}
 	}
@@ -81,13 +57,13 @@ func (c *AccessListBuilder) StorageRead(address common.Address, key common.Hash)
 	acctAccesses.StorageRead(key)
 }
 
-func (c *AccessListBuilder) AccountRead(address common.Address) {
+func (c *idxAccessListBuilder) accountRead(address common.Address) {
 	if _, ok := c.accessesStack[len(c.accessesStack)-1][address]; !ok {
 		c.accessesStack[len(c.accessesStack)-1][address] = &constructionAccountAccess{}
 	}
 }
 
-func (c *AccessListBuilder) StorageWrite(address common.Address, key, prevVal, newVal common.Hash) {
+func (c *idxAccessListBuilder) storageWrite(address common.Address, key, prevVal, newVal common.Hash) {
 	if _, ok := c.prestates[address]; !ok {
 		c.prestates[address] = &AccountState{}
 	}
@@ -105,7 +81,7 @@ func (c *AccessListBuilder) StorageWrite(address common.Address, key, prevVal, n
 	acctAccesses.StorageWrite(key, prevVal, newVal)
 }
 
-func (c *AccessListBuilder) BalanceChange(address common.Address, prev, cur *uint256.Int) {
+func (c *idxAccessListBuilder) balanceChange(address common.Address, prev, cur *uint256.Int) {
 	if _, ok := c.prestates[address]; !ok {
 		c.prestates[address] = &AccountState{}
 	}
@@ -119,7 +95,7 @@ func (c *AccessListBuilder) BalanceChange(address common.Address, prev, cur *uin
 	acctAccesses.BalanceChange(cur)
 }
 
-func (c *AccessListBuilder) CodeChange(address common.Address, prev, cur []byte) {
+func (c *idxAccessListBuilder) codeChange(address common.Address, prev, cur []byte) {
 	// auth unset and selfdestruct pass code change as 'nil'
 	// however, internally in the access list accumulation of state changes,
 	// a nil field on an account means that it was never modified in the block.
@@ -141,8 +117,7 @@ func (c *AccessListBuilder) CodeChange(address common.Address, prev, cur []byte)
 	acctAccesses.CodeChange(cur)
 }
 
-// TODO: rename this hook to "deleted" (CREATE2 + initcode + CALL empties account case)
+func (c *idxAccessListBuilder) selfDestruct(address common.Address) {
-func (c *AccessListBuilder) SelfDestruct(address common.Address) {
 	// convert all the account storage writes to reads, preserve the existing reads
 	if _, ok := c.accessesStack[len(c.accessesStack)-1][address]; !ok {
 		// TODO: figure out exactly which situations cause this case
@@ -168,7 +143,7 @@ func (c *AccessListBuilder) SelfDestruct(address common.Address) {
 	*/
 }
 
-func (c *AccessListBuilder) NonceChange(address common.Address, prev, cur uint64) {
+func (c *idxAccessListBuilder) nonceChange(address common.Address, prev, cur uint64) {
 	if _, ok := c.prestates[address]; !ok {
 		c.prestates[address] = &AccountState{}
 	}
@@ -182,11 +157,11 @@ func (c *AccessListBuilder) NonceChange(address common.Address, prev, cur uint64
 	acctAccesses.NonceChange(cur)
 }
 
-func (c *AccessListBuilder) EnterScope() {
+func (c *idxAccessListBuilder) enterScope() {
 	c.accessesStack = append(c.accessesStack, make(map[common.Address]*constructionAccountAccess))
 }
 
-func (c *AccessListBuilder) ExitScope(reverted bool) {
+func (c *idxAccessListBuilder) exitScope(reverted bool) {
 	// all storage writes in the child scope are converted into reads
 	// if there were no storage writes, the account is reported in the BAL as a read (if it wasn't already in the BAL and/or mutated previously)
 	childAccessList := c.accessesStack[len(c.accessesStack)-1]
@@ -207,7 +182,10 @@ func (c *AccessListBuilder) ExitScope(reverted bool) {
 	c.accessesStack = c.accessesStack[:len(c.accessesStack)-1]
 }
 
-func (a *AccessListBuilder) FinaliseIdxChanges() (*StateDiff, StateAccesses) {
+// finalise returns the net state mutations at the access list index as well as
+// state which was accessed.  The idxAccessListBuilder instance should be discarded
+// after calling finalise.
+func (a *idxAccessListBuilder) finalise() (*StateDiff, StateAccesses) {
 	diff := &StateDiff{make(map[common.Address]*AccountState)}
 	stateAccesses := make(StateAccesses)
 
@@ -257,7 +235,10 @@ func (a *AccessListBuilder) FinaliseIdxChanges() (*StateDiff, StateAccesses) {
 	return diff, stateAccesses
 }
 
-func (c *ConstructionBlockAccessList) Apply(idx uint16, diff *StateDiff, accesses StateAccesses) {
+func (c *ConstructionBlockAccessList) FinalisePendingChanges(idx uint16) {
+	diff, accesses := c.nonFinalizedAccessList.finalise()
+	c.nonFinalizedAccessList = newAccessListBuilder()
+
 	for addr, stateDiff := range diff.Mutations {
 		acctChanges, ok := c.Accounts[addr]
 		if !ok {
@@ -323,13 +304,44 @@ func (c *ConstructionBlockAccessList) Apply(idx uint16, diff *StateDiff, accesse
 			acctAccess.StorageReads[key] = struct{}{}
 		}
 	}
+	c.lastFinalizedMutations = diff
+	c.lastFinalizedAccesses = accesses
+}
+
+func (c *ConstructionBlockAccessList) StorageRead(address common.Address, key common.Hash) {
+	c.nonFinalizedAccessList.storageRead(address, key)
+}
+func (c *ConstructionBlockAccessList) AccountRead(address common.Address) {
+	c.nonFinalizedAccessList.accountRead(address)
+}
+func (c *ConstructionBlockAccessList) StorageWrite(address common.Address, key, prevVal, newVal common.Hash) {
+	c.nonFinalizedAccessList.storageWrite(address, key, prevVal, newVal)
+}
+func (c *ConstructionBlockAccessList) BalanceChange(address common.Address, prev, cur *uint256.Int) {
+	c.nonFinalizedAccessList.balanceChange(address, prev, cur)
+}
+func (c *ConstructionBlockAccessList) NonceChange(address common.Address, prev, cur uint64) {
+	c.nonFinalizedAccessList.nonceChange(address, prev, cur)
+}
+func (c *ConstructionBlockAccessList) CodeChange(address common.Address, prev, cur []byte) {
+	c.nonFinalizedAccessList.codeChange(address, prev, cur)
+}
+func (c *ConstructionBlockAccessList) SelfDestruct(address common.Address) {
+	c.nonFinalizedAccessList.selfDestruct(address)
+}
+
+func (c *ConstructionBlockAccessList) EnterScope() {
+	c.nonFinalizedAccessList.enterScope()
+}
+func (c *ConstructionBlockAccessList) ExitScope(reverted bool) {
+	c.nonFinalizedAccessList.exitScope(reverted)
 }
 
 // TODO: the BalReader Validation method should accept the computed values as
 // a index/StateDiff/StateAccesses trio.
 
 // BAL tracer maintains a ConstructionBlockAccessList.
-// For each BAL index, it instantiates an AccessListBuilder and
+// For each BAL index, it instantiates an idxAccessListBuilder and
 // appends the result to the access list where appropriate
 
 // ---- below is the actual code written before my idea sketch above ----
@@ -508,14 +520,20 @@ func NewConstructionAccountAccesses() *ConstructionAccountAccesses {
 // in execution (account addresses and storage keys).
 type ConstructionBlockAccessList struct {
 	Accounts map[common.Address]*ConstructionAccountAccesses
-	curIdx   uint16
+
+	nonFinalizedAccessList *idxAccessListBuilder
+
+	lastFinalizedMutations *StateDiff
+	lastFinalizedAccesses  StateAccesses
 }
 
 // NewConstructionBlockAccessList instantiates an empty access list.
 func NewConstructionBlockAccessList() *ConstructionBlockAccessList {
 	return &ConstructionBlockAccessList{
 		make(map[common.Address]*ConstructionAccountAccesses),
-		0,
+		newAccessListBuilder(),
+		nil,
+		nil,
 	}
 }
 
@@ -551,6 +569,10 @@ func (c *ConstructionBlockAccessList) Copy() *ConstructionBlockAccessList {
 	return res
 }
 
+func (c *ConstructionBlockAccessList) FinalizedIdxChanges() (*StateDiff, StateAccesses) {
+	return c.lastFinalizedMutations, c.lastFinalizedAccesses
+}
+
 type StateDiff struct {
 	Mutations map[common.Address]*AccountState `json:"Mutations,omitempty"`
 }

miner/worker.go

@@ -300,7 +300,7 @@ func (miner *Miner) makeEnv(parent *types.Header, header *types.Header, coinbase
 	var hookedState vm.StateDB = sdb
 	var vmConfig vm.Config
 	if miner.chainConfig.IsAmsterdam(header.Number, header.Time) {
-		alTracer, hooks = core.NewBlockAccessListTracer(0)
+		alTracer, hooks = core.NewBlockAccessListTracer()
 		hookedState = state.NewHookedState(sdb, hooks)
 		vmConfig.Tracer = hooks
 	}