diff --git a/core/block_access_list_creation.go b/core/block_access_list_creation.go index eaf92cb618..cd87f3726d 100644 --- a/core/block_access_list_creation.go +++ b/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 - balIdx uint16 - accessListBuilder *bal.AccessListBuilder + accessList *bal.ConstructionBlockAccessList - // mutations and state reads from currently-executing bal index - idxMutations *bal.StateDiff - idxReads bal.StateAccesses + // the access list index that changes are currently being recorded into + balIdx uint16 } // 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, - OnColdStorageRead: balTracer.OnColdStorageRead, + OnStorageRead: balTracer.OnStorageRead, + 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.Apply(0, a.idxMutations, a.idxReads) - a.accessListBuilder = bal.NewAccessListBuilder() + a.accessList.FinalisePendingChanges(0) 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.Apply(a.balIdx, a.idxMutations, a.idxReads) - a.accessListBuilder = bal.NewAccessListBuilder() + a.accessList.FinalisePendingChanges(a.balIdx) 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.Apply(a.balIdx, a.idxMutations, a.idxReads) - a.accessListBuilder = bal.NewAccessListBuilder() + a.accessList.FinalisePendingChanges(a.balIdx) } 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) { - a.accessListBuilder.StorageRead(addr, key) +func (a *BlockAccessListTracer) OnStorageRead(addr common.Address, key common.Hash) { + a.accessList.StorageRead(addr, key) } -func (a *BlockAccessListTracer) OnColdAccountRead(addr common.Address) { - a.accessListBuilder.AccountRead(addr) +func (a *BlockAccessListTracer) OnAcountRead(addr common.Address) { + 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) } diff --git a/core/blockchain.go b/core/blockchain.go index af789d7ffe..57b39b9f57 100644 --- a/core/blockchain.go +++ b/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() diff --git a/core/parallel_state_processor.go b/core/parallel_state_processor.go index 96c69cbf81..f406562db4 100644 --- a/core/parallel_state_processor.go +++ b/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 } diff --git a/core/types/bal/bal.go b/core/types/bal/bal.go index 0fc8f18931..9f49ebce96 100644 --- a/core/types/bal/bal.go +++ b/core/types/bal/bal.go @@ -25,33 +25,9 @@ import ( "maps" ) -/* -BAL Building rework - -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 { +// idxAccessListBuilder is responsible for producing the state accesses and +// reads recorded within the scope of a single index in the access list. +type idxAccessListBuilder 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 { - return &AccessListBuilder{ +func newAccessListBuilder() *idxAccessListBuilder { + 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 *AccessListBuilder) SelfDestruct(address common.Address) { +func (c *idxAccessListBuilder) 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"` } diff --git a/miner/worker.go b/miner/worker.go index 12a3c6055c..aa05acde82 100644 --- a/miner/worker.go +++ b/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 }