Changes for BAL parallel execution

core/block_validator.go

@@ -19,11 +19,10 @@ package core
 import (
 	"errors"
 	"fmt"
-	"github.com/ethereum/go-ethereum/core/types/bal"
-
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/core/types/bal"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/trie"
 )
@@ -122,13 +121,23 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 	return nil
 }
 
-func (v *BlockValidator) ValidateStateWithDiff(block *types.Block, prestate *state.StateDB, res *ProcessResult, diff *bal.StateDiff, stateless bool) error {
-	if res == nil {
-		return errors.New("nil ProcessResult value")
-	}
+func (v *BlockValidator) ValidateStateWithDiff(block *types.Block, prestate *state.StateDB, resCh chan *ProcessResult, diff *bal.StateDiff, stateless bool) (*ProcessResult, error) {
+	// Validate the state root against the received state root and throw
+	// an error if they don't match.
 	header := block.Header()
+	prestate.ApplyDiff(diff)
+	root := prestate.IntermediateRoot(v.config.IsEIP158(header.Number))
+
+	res := <-resCh
+	if res.Error != nil {
+		return nil, res.Error
+	}
+	if header.Root != root {
+		return res, fmt.Errorf("invalid merkle root (remote: %x local: %x) dberr: %w", header.Root, root, prestate.Error())
+	}
+
 	if block.GasUsed() != res.GasUsed {
-		return fmt.Errorf("invalid gas used (remote: %d local: %d)", block.GasUsed(), res.GasUsed)
+		return res, fmt.Errorf("invalid gas used (remote: %d local: %d)", block.GasUsed(), res.GasUsed)
 	}
 	// Validate the received block's bloom with the one derived from the generated receipts.
 	// For valid blocks this should always validate to true.
@@ -138,34 +147,29 @@ func (v *BlockValidator) ValidateStateWithDiff(block *types.Block, prestate *sta
 	// everything.
 	rbloom := types.MergeBloom(res.Receipts)
 	if rbloom != header.Bloom {
-		return fmt.Errorf("invalid bloom (remote: %x  local: %x)", header.Bloom, rbloom)
+		return res, fmt.Errorf("invalid bloom (remote: %x  local: %x)", header.Bloom, rbloom)
 	}
 	// In stateless mode, return early because the receipt and state root are not
 	// provided through the witness, rather the cross validator needs to return it.
 	if stateless {
-		return nil
+		return res, nil
 	}
 	// The receipt Trie's root (R = (Tr [[H1, R1], ... [Hn, Rn]]))
 	receiptSha := types.DeriveSha(res.Receipts, trie.NewStackTrie(nil))
 	if receiptSha != header.ReceiptHash {
-		return fmt.Errorf("invalid receipt root hash (remote: %x local: %x)", header.ReceiptHash, receiptSha)
+		return res, fmt.Errorf("invalid receipt root hash (remote: %x local: %x)", header.ReceiptHash, receiptSha)
 	}
 	// Validate the parsed requests match the expected header value.
 	if header.RequestsHash != nil {
 		reqhash := types.CalcRequestsHash(res.Requests)
 		if reqhash != *header.RequestsHash {
-			return fmt.Errorf("invalid requests hash (remote: %x local: %x)", *header.RequestsHash, reqhash)
+			return res, fmt.Errorf("invalid requests hash (remote: %x local: %x)", *header.RequestsHash, reqhash)
 		}
 	} else if res.Requests != nil {
-		return errors.New("block has requests before prague fork")
+		return res, errors.New("block has requests before prague fork")
 	}
-	// Validate the state root against the received state root and throw
-	// an error if they don't match.
-	prestate.ApplyDiff(diff)
-	if root := prestate.IntermediateRoot(v.config.IsEIP158(header.Number)); header.Root != root {
-		return fmt.Errorf("invalid merkle root (remote: %x local: %x) dberr: %w", header.Root, root, prestate.Error())
-	}
-	return nil
+
+	return res, nil
 }
 
 // ValidateState validates the various changes that happen after a state transition,

core/blockchain.go

@@ -2060,16 +2060,21 @@ func (bc *BlockChain) processBlock(parentRoot common.Hash, block *types.Block, s
 		pstart := time.Now()
 		var diff *bal.StateDiff
 		var prestate *state.StateDB
-		prestate, diff, res, err = bc.processor.ProcessWithAccessList(block, statedb, bc.cfg.VmConfig, block.Body().AccessList)
+		var resCh chan *ProcessResult
+		prestate, diff, resCh, err = bc.processor.ProcessWithAccessList(block, statedb, bc.cfg.VmConfig, block.Body().AccessList)
 		if err != nil {
-			bc.reportBlock(block, res, err)
+			// TODO: okay to pass nil here as execution result?
+			bc.reportBlock(block, nil, err)
 			return nil, err
 		}
 		ptime = time.Since(pstart)
 
 		vstart := time.Now()
-		if err := bc.validator.ValidateStateWithDiff(block, prestate, res, diff, false); err != nil {
-			bc.reportBlock(block, res, err)
+		var err error
+		res, err = bc.validator.ValidateStateWithDiff(block, prestate, resCh, diff, false)
+		if err != nil {
+			// TODO: okay to pass nil here as execution result?
+			bc.reportBlock(block, nil, err)
 			return nil, err
 		}
 		vtime = time.Since(vstart)

core/state/statedb.go

@@ -122,6 +122,12 @@ type StateDB struct {
 	txIndex int
 	sender  common.Address
 
+	// block access list modifications will be recorded with this index.
+	// 0 - state access before transaction execution
+	// 1 -> len(block txs) - state access of each transaction
+	// len(block txs) + 1 - state access after transaction execution.
+	balIndex int
+
 	logs    map[common.Hash][]*types.Log
 	logSize uint
 
@@ -215,7 +221,7 @@ func NewWithReader(root common.Hash, db Database, reader Reader) (*StateDB, erro
 func (s *StateDB) GetStateDiff() *bal.StateDiff {
 	// TODO: seems very dangerous to return the direct diff, because the caller will modify it.
 	// However, the alternative is probably not great either?
-	return s.diff
+	return s.diff.Copy()
 }
 
 // StartPrefetcher initializes a new trie prefetcher to pull in nodes from the
@@ -800,35 +806,36 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool, balPost *bal.StateDiff) (pos
 			continue
 		}
 		if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) {
-			if obj.selfDestructed {
-				// an object that was not created in the current transaction
-				// can only become empty and be removed if only the balance
-				// was non-zero before, the account was the target of a create
-				// whose initcode called SENDALL leaving the account empty at
-				// the end of the transaction.
-
-				// an account with a preexisting balance will not have its nonce set to 1 upon being the target of a CREATE
-
-				// TODO: why is the nonce set here (thus making empty() false)
-
-				// TODO: for testing purposes we should probably have tests that create/destroy the same account multiple times via this same edge-case with create2
-				if s.constructionBAL != nil {
-					s.constructionBAL.BalanceChange(uint16(s.txIndex), obj.address, uint256.NewInt(0))
-				} else if balPost != nil {
-					balDiff, ok := balPost.Mutations[obj.address]
-					if !ok {
-						panic("account not found in bal post mutations")
-					}
-					if balDiff.Nonce != nil || balDiff.Code != nil || balDiff.StorageWrites != nil || balDiff.Balance == nil || !new(uint256.Int).SetBytes(balDiff.Balance[:]).IsZero() {
-						panic("crap")
-					}
-				}
+			// an object that was not created in the current transaction
+			// can only become empty and be removed if only the balance
+			// was non-zero before, the account was the target of a create
+			// whose initcode called SENDALL leaving the account empty at
+			// the end of the transaction.
+
+			// an account with a preexisting balance will not have its nonce set to 1 upon being the target of a CREATE
 
-				// overrite any previous state (only storage reads possible here)
-				postState := bal.NewEmptyAccountState()
-				postState.Balance = &bal.Balance{}
-				post.Mutations[obj.address] = postState
+			// TODO: why is the nonce set here (thus making empty() false)
+
+			// TODO: for testing purposes we should probably have tests that create/destroy the same account multiple times via this same edge-case with create2
+			if s.constructionBAL != nil {
+				s.constructionBAL.BalanceChange(uint16(s.balIndex), obj.address, uint256.NewInt(0))
+			} else if balPost != nil {
+				balDiff, ok := balPost.Mutations[obj.address]
+				if !ok {
+					panic("account not found in bal post mutations")
+				}
+				if balDiff.Nonce != nil || balDiff.Code != nil || balDiff.StorageWrites != nil || balDiff.Balance == nil || !new(uint256.Int).SetBytes(balDiff.Balance[:]).IsZero() {
+					panic("crap")
+				}
 			}
+
+			// TODO: only enable recording of post-state optionally
+			// TODO: only record this post-cancun, when an account can only become empty as a result of sendall
+			// overrite any previous state (only storage reads possible here)
+			postState := bal.NewEmptyAccountState()
+			postState.Balance = &bal.Balance{}
+			post.Mutations[obj.address] = postState
+
 			delete(s.stateObjects, obj.address)
 			s.markDelete(addr)
 			// We need to maintain account deletions explicitly (will remain
@@ -844,56 +851,48 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool, balPost *bal.StateDiff) (pos
 
 				// for addresses that changed balance, add the post-change value
 				if obj.Balance().Cmp(obj.txPreBalance) != 0 {
-					s.constructionBAL.BalanceChange(uint16(s.txIndex), obj.address, obj.Balance())
+					s.constructionBAL.BalanceChange(uint16(s.balIndex), obj.address, obj.Balance())
 				}
 
-				// include nonces for any contracts which incremented them.
-				// newly-delegated contracts are not included
-				// consider contract creation execution which executes a
-
-				// only include nonce changes in the BAL if:
-				// * the object was a contract
-				// OR
-				// * the object was the tx sender, is a delegated EOA, and had its nonce bumped by more than 1
-				if s.sender == obj.address {
-					if obj.Nonce() != obj.txPreNonce+1 {
-						s.constructionBAL.NonceChange(obj.address, uint16(s.txIndex), obj.Nonce())
-					}
-				} else if obj.Nonce() != obj.txPreNonce {
-					s.constructionBAL.NonceChange(obj.address, uint16(s.txIndex), obj.Nonce())
+				if obj.Nonce() != obj.txPreNonce {
+					s.constructionBAL.NonceChange(obj.address, uint16(s.balIndex), obj.Nonce())
 				}
 
 				// TODO: newContract will be set regardless of whether the creation initcode succeeded
 				// ensure that if an initcode was run, the object was deleted before here.
 
 				// include code of created contracts
 				// Delegations are not included because they can be statically inferred from the tx and its prestate.
-				if obj.newContract && len(obj.code) != 0 && !obj.isDelegated() {
-					s.constructionBAL.CodeChange(obj.address, uint16(s.txIndex), obj.code)
+				if obj.dirtyCode {
+					// TODO: this flag is only reset upon commit.  However, we want to know if the tx changed since the beginning of the transaction
+					s.constructionBAL.CodeChange(obj.address, uint16(s.balIndex), obj.code)
 				}
 			} else if balPost != nil {
-				// TODO: compute a bal.StateDiff from the finalized objects and do the comparison outside this func
-				// I will also call finalise after performing pre-tx-execution operations (withdrawals + beacon root)
-				// and in addition call it after post-tx system contracts have executed.
-				accountDiff, ok := balPost.Mutations[obj.address]
-				if !ok {
-					panic("TODO return error here, bad block")
-				}
-				if obj.newContract && (accountDiff.Code == nil || bytes.Compare(accountDiff.Code, obj.code) != 0) {
-					panic("TODO return error here, bad block")
-				}
-				if common.BytesToHash(obj.CodeHash()) != types.EmptyCodeHash && obj.Nonce() != obj.txPreNonce {
-					if obj.isDelegated() || accountDiff.Nonce == nil || *accountDiff.Nonce != obj.Nonce() {
+				panic("assumed dead code path")
+				/*
+					// TODO: compute a bal.StateDiff from the finalized objects and do the comparison outside this func
+					// I will also call finalise after performing pre-tx-execution operations (withdrawals + beacon root)
+					// and in addition call it after post-tx system contracts have executed.
+					accountDiff, ok := balPost.Mutations[obj.address]
+					if !ok {
 						panic("TODO return error here, bad block")
 					}
-				}
-				if !obj.Balance().Eq(obj.txPreBalance) && (accountDiff.Balance == nil || !obj.Balance().Eq(new(uint256.Int).SetBytes((*accountDiff.Balance)[:]))) {
-					panic("TODO return error here, bad block")
-				}
+					if obj.newContract && (accountDiff.Code == nil || bytes.Compare(accountDiff.Code, obj.code) != 0) {
+						panic("TODO return error here, bad block")
+					}
+					if common.BytesToHash(obj.CodeHash()) != types.EmptyCodeHash && obj.Nonce() != obj.txPreNonce {
+						if obj.isDelegated() || accountDiff.Nonce == nil || *accountDiff.Nonce != obj.Nonce() {
+							panic("TODO return error here, bad block")
+						}
+					}
+					if !obj.Balance().Eq(obj.txPreBalance) && (accountDiff.Balance == nil || !obj.Balance().Eq(new(uint256.Int).SetBytes((*accountDiff.Balance)[:]))) {
+						panic("TODO return error here, bad block")
+					}
+				*/
 			}
 
 			var accountPost bal.AccountState
-			if obj.newContract {
+			if obj.dirtyCode {
 				accountPost.Code = bytes.Clone(obj.code)
 			}
 			if obj.Nonce() != obj.txPreNonce {
@@ -912,18 +911,22 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool, balPost *bal.StateDiff) (pos
 			// compute bal storage mutations after finalisation
 			if s.constructionBAL != nil {
 				for key, val := range obj.pendingStorage {
-					s.constructionBAL.StorageWrite(uint16(s.txIndex), obj.address, key, val)
+					//TODO: this is wrong and will include the storage kv multiple times even if it is only modified once in the block.  move this logic into the state object's finalise method
+					s.constructionBAL.StorageWrite(uint16(s.balIndex), obj.address, key, val)
 				}
 			} else if balPost != nil {
-				accountDiff, _ := balPost.Mutations[obj.address]
-				if len(obj.pendingStorage) > 0 {
-					if len(accountDiff.StorageWrites) != len(obj.pendingStorage) {
-						panic("TODO return error here, bad block")
-					}
-					if !maps.Equal(accountDiff.StorageWrites, obj.pendingStorage) {
-						panic("TODO return error here, bad block")
+				panic("assumed dead code path")
+				/*
+					accountDiff, _ := balPost.Mutations[obj.address]
+					if len(obj.pendingStorage) > 0 {
+						if len(accountDiff.StorageWrites) != len(obj.pendingStorage) {
+							panic("TODO return error here, bad block")
+						}
+						if !maps.Equal(accountDiff.StorageWrites, obj.pendingStorage) {
+							panic("TODO return error here, bad block")
+						}
 					}
-				}
+				*/
 			}
 
 			if len(obj.pendingStorage) > 0 {
@@ -1125,6 +1128,11 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
 func (s *StateDB) SetTxContext(thash common.Hash, ti int) {
 	s.thash = thash
 	s.txIndex = ti
+	s.balIndex = ti + 1
+}
+
+func (s *StateDB) SetAccessListIndex(idx int) {
+	s.balIndex = idx
 }
 
 // TODO: combine this into set tx context