From d163f06283da68db5aef3a0563d47e9296523150 Mon Sep 17 00:00:00 2001 From: qu0b Date: Sun, 1 Feb 2026 14:45:03 +0000 Subject: [PATCH] state/bal: fix multiple bugs in BAL state transition Fix several bugs in the BAL (Block Access List) state transition that caused state root mismatches between the block building path and the BAL validation path: 1. setError inverted condition: `if s.err != nil` should be `if s.err == nil`. This caused all errors to be silently swallowed, matching StateDB.setError. 2. IntermediateRoot ignores deleteEmptyObjects: The building path calls StateDB.IntermediateRoot(true) which deletes empty accounts (EIP-158), but the BAL path discarded this parameter. Now properly deletes empty accounts when deleteEmptyObjects is true. 3. parallel_state_processor hardcoded false for deleteEmptyObjects: Should pass config.IsEIP158(block.Number()) like block_validator does. 4. Non-deterministic ModifiedAccounts ordering: Map iteration produced random address ordering. Now sorted deterministically. 5. loadOriginStorages deadlock: On Storage() error, goroutine returned without sending to channel, causing pendingStorageCount to never reach 0. Now always sends to channel regardless of error. 6. Data race on metrics.StateHash: Removed racy per-goroutine writes to s.metrics.StateHash that were overwritten by the final state trie hash. --- core/parallel_state_processor.go | 2 +- core/state/bal_reader.go | 10 ++++++++-- core/state/bal_state_transition.go | 26 +++++++++++++++++--------- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/core/parallel_state_processor.go b/core/parallel_state_processor.go index 6a9c91750f..4cad00bfbf 100644 --- a/core/parallel_state_processor.go +++ b/core/parallel_state_processor.go @@ -230,7 +230,7 @@ type stateRootCalculationResult struct { func (p *ParallelStateProcessor) calcAndVerifyRoot(preState *state.StateDB, block *types.Block, stateTransition *state.BALStateTransition, resCh chan stateRootCalculationResult) { // calculate and apply the block state modifications //root, prestateLoadTime, rootCalcTime := preState.BlockAccessList().StateRoot(preState) - root := stateTransition.IntermediateRoot(false) + root := stateTransition.IntermediateRoot(p.chainConfig().IsEIP158(block.Number())) res := stateRootCalculationResult{ // TODO: I think we can remove the root from this struct diff --git a/core/state/bal_reader.go b/core/state/bal_reader.go index c03788796b..66b162ef5c 100644 --- a/core/state/bal_reader.go +++ b/core/state/bal_reader.go @@ -3,13 +3,15 @@ package state import ( "context" "fmt" + "slices" + "sync" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/types/bal" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" "github.com/holiman/uint256" - "sync" ) // TODO: probably unnecessary to cache the resolved state object here as it will already be in the db cache? @@ -133,13 +135,17 @@ func NewBALReader(block *types.Block, reader Reader) *BALReader { return r } -// ModifiedAccounts returns a list of all accounts with mutations in the access list +// ModifiedAccounts returns a deterministically-ordered list of all accounts +// with mutations in the access list. func (r *BALReader) ModifiedAccounts() (res []common.Address) { for addr, access := range r.accesses { if len(access.NonceChanges) != 0 || len(access.CodeChanges) != 0 || len(access.StorageChanges) != 0 || len(access.BalanceChanges) != 0 { res = append(res, addr) } } + slices.SortFunc(res, func(a, b common.Address) int { + return a.Cmp(b) + }) return res } diff --git a/core/state/bal_state_transition.go b/core/state/bal_state_transition.go index 4ebe710557..1d8c1f73a6 100644 --- a/core/state/bal_state_transition.go +++ b/core/state/bal_state_transition.go @@ -107,7 +107,7 @@ func (s *BALStateTransition) Error() error { } func (s *BALStateTransition) setError(err error) { - if s.err != nil { + if s.err == nil { s.err = err } } @@ -390,7 +390,6 @@ func (s *BALStateTransition) loadOriginStorages() { val, err := s.reader.Storage(addr, common.Hash(storageKey)) if err != nil { s.setError(err) - return } originStoragesCh <- &originStorage{ addr, @@ -418,7 +417,7 @@ func (s *BALStateTransition) loadOriginStorages() { // IntermediateRoot applies block state mutations and computes the updated state // trie root. -func (s *BALStateTransition) IntermediateRoot(_ bool) common.Hash { +func (s *BALStateTransition) IntermediateRoot(deleteEmptyObjects bool) common.Hash { if s.rootHash != (common.Hash{}) { return s.rootHash } @@ -502,9 +501,7 @@ func (s *BALStateTransition) IntermediateRoot(_ bool) common.Hash { } } - hashStart := time.Now() tr.Hash() - s.metrics.StateHash = time.Since(hashStart) } }() } @@ -548,11 +545,22 @@ func (s *BALStateTransition) IntermediateRoot(_ bool) common.Hash { return common.Hash{} } } - if err := s.stateTrie.UpdateAccount(mutatedAddr, acct, len(code)); err != nil { - s.setError(err) - return common.Hash{} + // Delete empty accounts when deleteEmptyObjects is set, matching + // the behavior of StateDB.Finalise which deletes accounts where + // nonce == 0, balance == 0, and codeHash == emptyCodeHash. + if deleteEmptyObjects && acct.Nonce == 0 && acct.Balance.IsZero() && common.BytesToHash(acct.CodeHash) == types.EmptyCodeHash { + if err := s.stateTrie.DeleteAccount(mutatedAddr); err != nil { + s.setError(err) + return common.Hash{} + } + s.deletions[mutatedAddr] = struct{}{} + } else { + if err := s.stateTrie.UpdateAccount(mutatedAddr, acct, len(code)); err != nil { + s.setError(err) + return common.Hash{} + } + s.postStates[mutatedAddr] = acct } - s.postStates[mutatedAddr] = acct } }