diff --git a/core/block_access_list_tracer.go b/core/block_access_list_tracer.go index fbadc168f3..18a918f6df 100644 --- a/core/block_access_list_tracer.go +++ b/core/block_access_list_tracer.go @@ -17,6 +17,18 @@ type BlockAccessListTracer struct { // the access list index that changes are currently being recorded into balIdx uint16 + + // the number of system calls that have been invoked, used when building + // an access list to determine if the system calls being executed are + // before/after the block transactions. + sysCallCount int + + // true if the tracer is processing post-tx state changes. in this case + // we won't record the final index after the end of the second post-tx + // system contract but after the finalization of the block. + // This is because we have EIP-4895 withdrawals which are processed after the + // last system contracts execute and must be included in the BAL. + isPostTx bool } // NewBlockAccessListTracer returns an BlockAccessListTracer and a set of hooks @@ -26,7 +38,6 @@ func NewBlockAccessListTracer() (*BlockAccessListTracer, *tracing.Hooks) { } hooks := &tracing.Hooks{ OnBlockFinalization: balTracer.OnBlockFinalization, - OnPreTxExecutionDone: balTracer.OnPreTxExecutionDone, OnTxEnd: balTracer.TxEndHook, OnTxStart: balTracer.TxStartHook, OnEnter: balTracer.OnEnter, @@ -38,11 +49,16 @@ func NewBlockAccessListTracer() (*BlockAccessListTracer, *tracing.Hooks) { OnStorageRead: balTracer.OnStorageRead, OnAccountRead: balTracer.OnAcountRead, OnSelfDestructChange: balTracer.OnSelfDestruct, + OnSystemCallEnd: balTracer.OnSystemCallEnd, } wrappedHooks, _ := tracing.WrapWithJournal(hooks) return balTracer, wrappedHooks } +func (a *BlockAccessListTracer) SetPostTx() { + a.isPostTx = true +} + // AccessList returns the constructed access list. // It is assumed that this is only called after all the block state changes // have been executed and the block has been finalized. @@ -50,9 +66,15 @@ func (a *BlockAccessListTracer) AccessList() *bal.AccessListBuilder { return a.builder } -func (a *BlockAccessListTracer) OnPreTxExecutionDone() { - a.builder.FinaliseIdxChanges(0) - a.balIdx++ +func (a *BlockAccessListTracer) OnSystemCallEnd() { + if a.isPostTx { + return + } + a.sysCallCount++ + if a.sysCallCount == 2 { + a.builder.FinaliseIdxChanges(a.balIdx) + a.balIdx++ + } } func (a *BlockAccessListTracer) TxStartHook(vm *tracing.VMContext, tx *types.Transaction, from common.Address) { diff --git a/core/blockchain.go b/core/blockchain.go index b54a3a2632..050f0b57bd 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2231,7 +2231,6 @@ func (bc *BlockChain) ProcessBlock(parentRoot common.Hash, block *types.Block, s defer func() { bc.cfg.VmConfig.Tracer = nil }() - } // 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 d6e5f24fc7..6a9c91750f 100644 --- a/core/parallel_state_processor.go +++ b/core/parallel_state_processor.go @@ -52,9 +52,12 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR header := block.Header() balTracer, hooks := NewBlockAccessListTracer() + balTracer.SetPostTx() + tracingStateDB := state.NewHookedState(postTxState, hooks) context := NewEVMBlockContext(header, p.chain, nil) - postTxState.SetAccessListIndex(len(block.Transactions()) + 1) + lastBALIdx := len(block.Transactions()) + 1 + postTxState.SetAccessListIndex(lastBALIdx) cfg := vm.Config{ Tracer: hooks, @@ -122,6 +125,8 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR diff, stateReads := balTracer.builder.FinalizedIdxChanges() allStateReads.Merge(stateReads) + // TODO: if there is a failure, we need to print out the detailed logs explaining why the BAL validation failed + // but logs are disabled when we are running tests to prevent a ton of output balIdx := len(block.Transactions()) + 1 if !postTxState.BlockAccessList().ValidateStateDiff(balIdx, diff) { return &ProcessResultWithMetrics{ @@ -129,9 +134,9 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR } } - if err := postTxState.BlockAccessList().ValidateStateReads(*allStateReads); err != nil { + if !postTxState.BlockAccessList().ValidateStateReads(lastBALIdx, *allStateReads) { return &ProcessResultWithMetrics{ - ProcessResult: &ProcessResult{Error: err}, + ProcessResult: &ProcessResult{Error: fmt.Errorf("BAL validation failure")}, } } @@ -315,8 +320,6 @@ func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *st ProcessParentBlockHash(block.ParentHash(), evm) } - balTracer.OnPreTxExecutionDone() - diff, stateReads := balTracer.builder.FinalizedIdxChanges() if !statedb.BlockAccessList().ValidateStateDiff(0, diff) { return nil, fmt.Errorf("BAL validation failure") diff --git a/core/state/bal_reader.go b/core/state/bal_reader.go index 1cd0491db6..3d76c290d6 100644 --- a/core/state/bal_reader.go +++ b/core/state/bal_reader.go @@ -143,7 +143,42 @@ func (r *BALReader) ModifiedAccounts() (res []common.Address) { return res } -func (r *BALReader) ValidateStateReads(computedReads bal.StateAccesses) error { +func logReadsDiff(idx int, address common.Address, computedReads map[common.Hash]struct{}, expectedReads []*bal.EncodedStorage) { + expectedReadsMap := make(map[common.Hash]struct{}) + for _, er := range expectedReads { + expectedReadsMap[er.ToHash()] = struct{}{} + } + + allReads := make(map[common.Hash]struct{}) + + for er := range expectedReadsMap { + allReads[er] = struct{}{} + } + for cr := range computedReads { + allReads[cr] = struct{}{} + } + + var missingExpected, missingComputed []common.Hash + + for storage := range allReads { + _, hasComputed := computedReads[storage] + _, hasExpected := expectedReadsMap[storage] + if hasComputed && !hasExpected { + missingExpected = append(missingExpected, storage) + } + if !hasComputed && hasExpected { + missingComputed = append(missingComputed, storage) + } + } + if len(missingExpected) > 0 { + log.Error("read storage slots which were not reported in the BAL", "index", idx, "address", address, missingExpected) + } + if len(missingComputed) > 0 { + log.Error("did not read storage slots which were reported in the BAL", "index", idx, "address", address, missingComputed) + } +} + +func (r *BALReader) ValidateStateReads(idx int, computedReads bal.StateAccesses) bool { // 1. remove any slots from 'allReads' which were written // 2. validate that the read set in the BAL matches 'allReads' exactly for addr, reads := range computedReads { @@ -154,22 +189,25 @@ func (r *BALReader) ValidateStateReads(computedReads bal.StateAccesses) error { } } if _, ok := r.accesses[addr]; !ok { - return fmt.Errorf("account %x was accessed during execution but is not present in the access list", addr) + log.Error(fmt.Sprintf("account %x was accessed during execution but is not present in the access list", addr)) + return false } expectedReads := r.accesses[addr].StorageReads if len(reads) != len(expectedReads) { - return fmt.Errorf("mismatch between the number of computed reads and number of expected reads") + logReadsDiff(idx, addr, reads, expectedReads) + return false } for _, slot := range expectedReads { if _, ok := reads[slot.ToHash()]; !ok { - return fmt.Errorf("expected read is missing from BAL") + log.Error("expected read is missing from BAL") + return false } } } - return nil + return true } // changesAt returns all state changes occurring at the given index. diff --git a/eth/api_debug.go b/eth/api_debug.go index 698c0c3bfc..e90a3d869f 100644 --- a/eth/api_debug.go +++ b/eth/api_debug.go @@ -546,5 +546,8 @@ func (api *DebugAPI) GetBlockAccessList(number rpc.BlockNumberOrHash) (interface if block == nil { return nil, fmt.Errorf("block not found") } + if block.Body().AccessList == nil { + return nil, nil + } return block.Body().AccessList.StringableRepresentation(), nil } diff --git a/tests/block_test.go b/tests/block_test.go index 9bc77b4c90..16ad424513 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -214,7 +214,6 @@ func execBlockTest(t *testing.T, bt *testMatcher, test *BlockTest, buildAndVerif for _, snapshot := range snapshotConf { for _, dbscheme := range dbschemeConf { - //tracer := logger.NewJSONLogger(&logger.Config{}, os.Stdout) if err := bt.checkFailure(t, test.Run(snapshot, dbscheme, false, buildAndVerifyBAL, nil, nil)); err != nil { t.Errorf("test with config {snapshotter:%v, scheme:%v} failed: %v", snapshot, dbscheme, err) return