big changes, everything up in the air. add a detailed comment to drive the evolution of the code design

core/block_access_list_creation.go

@@ -1,7 +1,6 @@
 package core
 
 import (
-	"fmt"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/tracing"
 	"github.com/ethereum/go-ethereum/core/types"
@@ -10,6 +9,12 @@ import (
 	"math/big"
 )
 
+type accountPrestate struct {
+	balance *uint256.Int
+	nonce   *uint64
+	code    []byte
+}
+
 // BlockAccessListTracer constructs an EIP-7928 block access list from the
 // execution of a block
 type BlockAccessListTracer struct {
@@ -18,18 +23,14 @@ 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.
-	callAccessLists []*bal.ConstructionBlockAccessList
+	accessList *bal.ConstructionBlockAccessList
 	txIdx      uint16
-
-	// if non-nil, it's the address of the account which just self-destructed.
-	// reset at the end of the call-scope which self-destructed.
-	selfdestructedAccount *common.Address
 }
 
 // NewBlockAccessListTracer returns an BlockAccessListTracer and a set of hooks
 func NewBlockAccessListTracer(startIdx int) (*BlockAccessListTracer, *tracing.Hooks) {
 	balTracer := &BlockAccessListTracer{
-		callAccessLists: []*bal.ConstructionBlockAccessList{bal.NewConstructionBlockAccessList()},
+		accessList: bal.NewConstructionBlockAccessList(),
 		txIdx:      uint16(startIdx),
 	}
 	hooks := &tracing.Hooks{
@@ -44,12 +45,16 @@ func NewBlockAccessListTracer(startIdx int) (*BlockAccessListTracer, *tracing.Ho
 		OnColdAccountRead: balTracer.OnColdAccountRead,
 		OnColdStorageRead: balTracer.OnColdStorageRead,
 	}
-	return balTracer, hooks
+	wrappedHooks, err := tracing.WrapWithJournal(hooks)
+	if err != nil {
+		panic(err) // TODO: ....
+	}
+	return balTracer, wrappedHooks
 }
 
 // AccessList returns the constructed access list
 func (a *BlockAccessListTracer) AccessList() *bal.ConstructionBlockAccessList {
-	return a.callAccessLists[0]
+	return a.accessList
 }
 
 func (a *BlockAccessListTracer) TxEndHook(receipt *types.Receipt, err error) {
@@ -67,15 +72,11 @@ func (a *BlockAccessListTracer) OnEnter(depth int, typ byte, from common.Address
 }
 
 func (a *BlockAccessListTracer) OnExit(depth int, output []byte, gasUsed uint64, err error, reverted bool) {
-	// any self-destructed accounts must have been created in the same transaction
+	// TODO: handle self-destructed accounts here...
-	// so there is no difference between the pre/post tx state of those accounts
+
-	if a.selfdestructedAccount != nil {
-		delete(a.callAccessLists[len(a.callAccessLists)-1].Accounts, *a.selfdestructedAccount)
-	}
 	parentAccessList := a.callAccessLists[len(a.callAccessLists)-2]
 	scopeAccessList := a.callAccessLists[len(a.callAccessLists)-1]
 	if reverted {
-		fmt.Printf("reverted \n%s\n", scopeAccessList.ToEncodingObj().String())
 		parentAccessList.MergeReads(scopeAccessList)
 	} else {
 		parentAccessList.Merge(scopeAccessList)
@@ -85,37 +86,18 @@ func (a *BlockAccessListTracer) OnExit(depth int, output []byte, gasUsed uint64,
 }
 
 func (a *BlockAccessListTracer) OnCodeChange(addr common.Address, prevCodeHash common.Hash, prevCode []byte, codeHash common.Hash, code []byte, reason tracing.CodeChangeReason) {
-	if reason == tracing.CodeChangeSelfDestruct {
-		// TODO: not sure whether this will ever run post-Cancun.  Prob should remove it if not
-		panic("FUCK")
-		a.selfdestructedAccount = &addr
-		return
-	} else if reason == tracing.CodeChangeContractCreation {
-		//fmt.Printf("contract creation code change: %x\n", code)
-	} else if len(code) == 0 {
-		fmt.Println("self-destruct happened here")
-		// this is the actual signal for a post-Cancun created-in-same-transaction selfdestruct....
-		a.selfdestructedAccount = &addr
-		return
-	}
 	a.callAccessLists[len(a.callAccessLists)-1].CodeChange(addr, uint16(a.txIdx), code)
 }
 
 func (a *BlockAccessListTracer) OnBalanceChange(addr common.Address, prevBalance, newBalance *big.Int, _ tracing.BalanceChangeReason) {
-	a.callAccessLists[len(a.callAccessLists)-1].BalanceChange(a.txIdx, addr, new(uint256.Int).SetBytes(newBalance.Bytes()))
+	newU256 := new(uint256.Int).SetBytes(newBalance.Bytes())
+	prevU256 := new(uint256.Int).SetBytes(prevBalance.Bytes())
+	a.callAccessLists[len(a.callAccessLists)-1].BalanceChange(addr, prevU256, newU256)
 }
 
 func (a *BlockAccessListTracer) OnNonceChange(addr common.Address, prev uint64, new uint64, reason tracing.NonceChangeReason) {
-	if reason == tracing.NonceChangeContractCreator {
-		// NonceChange hook is called between the Enter/Exit of the contract creation
-		// so it would appear as if it has occurred within the creation initcode.
-		// if the initcode fails, the nonce update is not reverted, so record it directly
-		// on the parent execution scope.
-		a.callAccessLists[len(a.callAccessLists)-2].NonceChange(addr, a.txIdx, new)
-	} else {
 	a.callAccessLists[len(a.callAccessLists)-1].NonceChange(addr, a.txIdx, new)
 }
-}
 
 func (a *BlockAccessListTracer) OnColdStorageRead(addr common.Address, key common.Hash) {
 	a.callAccessLists[len(a.callAccessLists)-1].StorageRead(addr, key)

core/tracing/journal.go

@@ -147,6 +147,18 @@ func (j *journal) OnExit(depth int, output []byte, gasUsed uint64, err error, re
 	}
 }
 
+func (j *journal) OnColdStorageLoad(address common.Address, key common.Hash) {
+	if j.hooks.OnColdStorageRead != nil {
+		j.hooks.OnColdStorageRead(address, key)
+	}
+}
+
+func (j *journal) OnColdAccountLoad(address common.Address) {
+	if j.hooks.OnColdAccountRead != nil {
+		j.hooks.OnColdAccountRead(address)
+	}
+}
+
 func (j *journal) OnBalanceChange(addr common.Address, prev, new *big.Int, reason BalanceChangeReason) {
 	j.entries = append(j.entries, balanceChange{addr: addr, prev: prev, new: new})
 	if j.hooks.OnBalanceChange != nil {

core/types/bal/bal.go

@@ -26,6 +26,27 @@ import (
 	"github.com/holiman/uint256"
 )
 
+/*
+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
+
+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
+*/
+
 // CodeChange contains the runtime bytecode deployed at an address and the
 // transaction index where the deployment took place.
 type CodeChange struct {
@@ -54,10 +75,10 @@ var IgnoredBALAddresses map[common.Address]struct{} = map[common.Address]struct{
 	common.BytesToAddress([]byte{0x11}): {},
 }
 
-// ConstructionAccountAccess contains post-block account state for mutations as well as
+// ConstructionAccountAccesses contains post-block account state for mutations as well as
 // all storage keys that were read during execution.  It is used when building block
 // access list during execution.
-type ConstructionAccountAccess struct {
+type ConstructionAccountAccesses struct {
 	// StorageWrites is the post-state values of an account's storage slots
 	// that were modified in a block, keyed by the slot key and the tx index
 	// where the modification occurred.
@@ -79,29 +100,190 @@ type ConstructionAccountAccess struct {
 	NonceChanges map[uint16]uint64
 
 	CodeChanges map[uint16]CodeChange
+
+	curIdx uint16
+	// changes are first accumulated in here, and "flushed" to the maps above
+	// via invoking Finalise.
+	curIdxChanges constructionAccountAccess
 }
 
-// NewConstructionAccountAccess initializes the account access object.
+func (c *ConstructionAccountAccesses) StorageRead(key common.Hash) {
-func NewConstructionAccountAccess() *ConstructionAccountAccess {
+	c.curIdxChanges.StorageRead(key)
-	return &ConstructionAccountAccess{
+}
+
+func (c *ConstructionAccountAccesses) StorageWrite(key, prevVal, newVal common.Hash) {
+	c.curIdxChanges.StorageWrite(key, prevVal, newVal)
+}
+
+func (c *ConstructionAccountAccesses) BalanceChange(prev, cur *uint256.Int) {
+	c.curIdxChanges.BalanceChange(prev, cur)
+}
+
+func (c *ConstructionAccountAccesses) CodeChange(prev, cur []byte) {
+	c.curIdxChanges.CodeChange(prev, cur)
+}
+
+func (c *ConstructionAccountAccesses) NonceChange(prev, cur uint64) {
+	c.curIdxChanges.NonceChange(prev, cur)
+}
+
+// Called at tx boundaries
+func (c *ConstructionAccountAccesses) Finalise() {
+	c.curIdxChanges.Finalise()
+	if c.curIdxChanges.code != nil {
+		c.CodeChanges[c.curIdx] = CodeChange{c.curIdx, c.curIdxChanges.code}
+	}
+	if c.curIdxChanges.nonce != nil {
+		c.NonceChanges[c.curIdx] = *c.curIdxChanges.nonce
+	}
+	if c.curIdxChanges.balance != nil {
+		c.BalanceChanges[c.curIdx] = c.curIdxChanges.balance
+	}
+	if c.curIdxChanges.storageMutations != nil {
+		for key, val := range c.curIdxChanges.storageMutations {
+			if _, ok := c.StorageWrites[key]; !ok {
+				c.StorageWrites[key] = make(map[uint16]common.Hash)
+			}
+
+			c.StorageWrites[key][c.curIdx] = val
+		}
+	}
+	if c.curIdxChanges.storageReads != nil {
+		for key := range c.StorageReads {
+			c.StorageReads[key] = struct{}{}
+		}
+	}
+	c.curIdxChanges = constructionAccountAccess{}
+}
+
+type constructionAccountAccess 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
+	// represent the prestate values of keys which were written and it would be nice to
+	// somehow make it clear that the field can contain both the mutated values or the
+	// prestate depending on the context (or find a cleaner solution entirely, instead
+	// of reusing the field here)
+	prestate *AccountState
+
+	code    []byte
+	nonce   *uint64
+	balance *uint256.Int
+
+	storageMutations map[common.Hash]common.Hash
+	storageReads     map[common.Hash]struct{}
+}
+
+func (c *constructionAccountAccess) StorageRead(key common.Hash) {
+	if c.storageReads == nil {
+		c.storageReads = make(map[common.Hash]struct{})
+	}
+	c.storageReads[key] = struct{}{}
+}
+
+func (c *constructionAccountAccess) StorageWrite(key, prevVal, newVal common.Hash) {
+	if c.prestate.StorageWrites == nil {
+		c.prestate.StorageWrites = make(map[common.Hash]common.Hash)
+	}
+	if _, ok := c.prestate.StorageWrites[key]; !ok {
+		c.prestate.StorageWrites[key] = prevVal
+	}
+	if c.storageMutations == nil {
+		c.storageMutations = make(map[common.Hash]common.Hash)
+	}
+	c.storageMutations[key] = newVal
+	// a key can be first read and later written, but it must only show up
+	// in either read or write sets, not both.
+	//
+	// the caller should not
+	// call StorageRead on a slot that was already written
+	delete(c.storageReads, key)
+}
+
+func (c *constructionAccountAccess) BalanceChange(prev, cur *uint256.Int) {
+	if c.prestate.Balance == nil {
+		c.prestate.Balance = prev
+	}
+	c.balance = cur
+}
+
+func (c *constructionAccountAccess) CodeChange(prev, cur []byte) {
+	if c.prestate.Code == nil {
+		c.prestate.Code = prev
+	}
+	c.code = cur
+}
+
+func (c *constructionAccountAccess) NonceChange(prev, cur uint64) {
+	if c.prestate.Nonce == nil {
+		c.prestate.Nonce = &prev
+	}
+	c.nonce = &cur
+}
+
+// Finalise clears any fields which had no net mutations compared to their
+// prestate values.
+func (c *constructionAccountAccess) Finalise() {
+	if c.nonce != nil && *c.prestate.Nonce == *c.nonce {
+		c.nonce = nil
+	}
+	if c.balance != nil && c.prestate.Balance.Eq(c.balance) {
+		c.balance = nil
+	}
+	if c.code != nil && bytes.Equal(c.code, c.prestate.Code) {
+		c.code = nil
+	}
+	if c.storageMutations != nil {
+		for key, val := range c.storageMutations {
+			if c.prestate.StorageWrites[key] == val {
+				delete(c.storageMutations, key)
+				c.storageReads[key] = struct{}{}
+			}
+		}
+	}
+}
+
+// NewConstructionAccountAccesses initializes the account access object.
+func NewConstructionAccountAccesses(idx uint16) *ConstructionAccountAccesses {
+	return &ConstructionAccountAccesses{
 		StorageWrites:  make(map[common.Hash]map[uint16]common.Hash),
 		StorageReads:   make(map[common.Hash]struct{}),
 		BalanceChanges: make(map[uint16]*uint256.Int),
 		NonceChanges:   make(map[uint16]uint64),
 		CodeChanges:    make(map[uint16]CodeChange),
+		curIdx:         idx,
 	}
 }
 
 // ConstructionBlockAccessList contains post-block modified state and some state accessed
 // in execution (account addresses and storage keys).
 type ConstructionBlockAccessList struct {
-	Accounts map[common.Address]*ConstructionAccountAccess
+	Accounts map[common.Address]*ConstructionAccountAccesses
+	curIdx   uint16
 }
 
 // NewConstructionBlockAccessList instantiates an empty access list.
 func NewConstructionBlockAccessList() *ConstructionBlockAccessList {
 	return &ConstructionBlockAccessList{
-		Accounts: make(map[common.Address]*ConstructionAccountAccess),
+		make(map[common.Address]*ConstructionAccountAccesses),
+		0,
+	}
+}
+
+// EnterCallScope should be invoked when the EVM execution enters a new call fram
+func (c *ConstructionBlockAccessList) EnterCallScope() {
+}
+
+// ExitCallScope is invoked when the EVM execution exits a call frame.
+// If the call frame reverted, all state changes that occurred within that
+// scope (or any nested calls) are discarded.  Storage reads performed in the
+// nested scope(s) are retained and any storage writes that occurred are denoted
+// as storage reads in the BAL.
+func (c *ConstructionBlockAccessList) ExitCallScope(reverted bool) {
+	if reverted {
+
+	} else {
+
 	}
 }
 
@@ -156,39 +338,34 @@ func (c *ConstructionBlockAccessList) StateAccesses() StateAccesses {
 // AccountRead records the address of an account that has been read during execution.
 func (c *ConstructionBlockAccessList) AccountRead(addr common.Address) {
 	if _, ok := c.Accounts[addr]; !ok {
-		c.Accounts[addr] = NewConstructionAccountAccess()
+		c.Accounts[addr] = NewConstructionAccountAccesses(c.curIdx)
 	}
 }
 
 // StorageRead records a storage key read during execution.
 func (c *ConstructionBlockAccessList) StorageRead(address common.Address, key common.Hash) {
 	if _, ok := c.Accounts[address]; !ok {
-		c.Accounts[address] = NewConstructionAccountAccess()
+		c.Accounts[address] = NewConstructionAccountAccesses(c.curIdx)
 	}
 	if _, ok := c.Accounts[address].StorageWrites[key]; ok {
 		return
 	}
-	c.Accounts[address].StorageReads[key] = struct{}{}
+	c.Accounts[address].StorageRead(key)
 }
 
 // StorageWrite records the post-transaction value of a mutated storage slot.
 // The storage slot is removed from the list of read slots.
-func (c *ConstructionBlockAccessList) StorageWrite(txIdx uint16, address common.Address, key, value common.Hash) {
+func (c *ConstructionBlockAccessList) StorageWrite(address common.Address, key, prev, cur common.Hash) {
 	if _, ok := c.Accounts[address]; !ok {
-		c.Accounts[address] = NewConstructionAccountAccess()
+		c.Accounts[address] = NewConstructionAccountAccesses(c.curIdx)
 	}
-	if _, ok := c.Accounts[address].StorageWrites[key]; !ok {
+	c.Accounts[address].StorageWrite(key, prev, cur)
-		c.Accounts[address].StorageWrites[key] = make(map[uint16]common.Hash)
-	}
-	c.Accounts[address].StorageWrites[key][txIdx] = value
-
-	delete(c.Accounts[address].StorageReads, key)
 }
 
 // CodeChange records the code of a newly-created contract.
 func (c *ConstructionBlockAccessList) CodeChange(address common.Address, txIndex uint16, code []byte) {
 	if _, ok := c.Accounts[address]; !ok {
-		c.Accounts[address] = NewConstructionAccountAccess()
+		c.Accounts[address] = NewConstructionAccountAccesses(c.curIdx)
 	}
 	c.Accounts[address].CodeChanges[txIndex] = CodeChange{
 		TxIdx: txIndex,
@@ -198,24 +375,27 @@ func (c *ConstructionBlockAccessList) CodeChange(address common.Address, txIndex
 
 // NonceChange records tx post-state nonce of any contract-like accounts whose
 // nonce was incremented.
-func (c *ConstructionBlockAccessList) NonceChange(address common.Address, txIdx uint16, postNonce uint64) {
+func (c *ConstructionBlockAccessList) NonceChange(address common.Address, pre uint64, cur uint64) {
 	if _, ok := c.Accounts[address]; !ok {
-		c.Accounts[address] = NewConstructionAccountAccess()
+		c.Accounts[address] = NewConstructionAccountAccesses(c.curIdx)
 	}
-	c.Accounts[address].NonceChanges[txIdx] = postNonce
+	c.Accounts[address].NonceChange(pre, cur)
 }
 
 // BalanceChange records the post-transaction balance of an account whose
 // balance changed.
-func (c *ConstructionBlockAccessList) BalanceChange(txIdx uint16, address common.Address, balance *uint256.Int) {
+func (c *ConstructionBlockAccessList) BalanceChange(address common.Address, old, new *uint256.Int) {
 	if _, ok := c.Accounts[address]; !ok {
-		c.Accounts[address] = NewConstructionAccountAccess()
+		c.Accounts[address] = NewConstructionAccountAccesses(c.curIdx)
 	}
-	c.Accounts[address].BalanceChanges[txIdx] = balance.Clone()
+	c.Accounts[address].BalanceChange(old, new)
 }
 
-func (c *ConstructionBlockAccessList) Delete(address common.Address) {
+func (c *ConstructionBlockAccessList) Finalise() {
-	delete(c.Accounts, address)
+	for _, account := range c.Accounts {
+		account.Finalise()
+	}
+	c.curIdx++
 }
 
 func mergeStorageWrites(cur, next map[common.Hash]map[uint16]common.Hash) map[common.Hash]map[uint16]common.Hash {
@@ -241,7 +421,7 @@ func mergeStorageWrites(cur, next map[common.Hash]map[uint16]common.Hash) map[co
 func (c *ConstructionBlockAccessList) MergeReads(childScope *ConstructionBlockAccessList) {
 	for addr, accountAccess := range childScope.Accounts {
 		if _, ok := c.Accounts[addr]; !ok {
-			c.Accounts[addr] = NewConstructionAccountAccess()
+			c.Accounts[addr] = NewConstructionAccountAccesses(c.curIdx)
 			c.Accounts[addr].StorageReads = accountAccess.StorageReads
 			continue
 		}
@@ -284,7 +464,7 @@ func (c *ConstructionBlockAccessList) Merge(childScope *ConstructionBlockAccessL
 func (c *ConstructionBlockAccessList) Copy() *ConstructionBlockAccessList {
 	res := NewConstructionBlockAccessList()
 	for addr, aa := range c.Accounts {
-		var aaCopy ConstructionAccountAccess
+		var aaCopy ConstructionAccountAccesses
 
 		slotWrites := make(map[common.Hash]map[uint16]common.Hash, len(aa.StorageWrites))
 		for key, m := range aa.StorageWrites {

core/types/bal/bal_encoding.go

@@ -140,7 +140,7 @@ func (e *encodingSlotWrites) validate() error {
 	return errors.New("storage write tx indices not in order")
 }
 
-// AccountAccess is the encoding format of ConstructionAccountAccess.
+// AccountAccess is the encoding format of ConstructionAccountAccesses.
 type AccountAccess struct {
 	Address        common.Address          `json:"address,omitempty"`        // 20-byte Ethereum address
 	StorageChanges []encodingSlotWrites    `json:"storageChanges,omitempty"` // Storage changes (slot -> [tx_index -> new_value])
@@ -227,9 +227,9 @@ func (c *ConstructionBlockAccessList) EncodeRLP(wr io.Writer) error {
 
 var _ rlp.Encoder = &ConstructionBlockAccessList{}
 
-// toEncodingObj creates an instance of the ConstructionAccountAccess of the type that is
+// toEncodingObj creates an instance of the ConstructionAccountAccesses of the type that is
 // used as input for the encoding.
-func (a *ConstructionAccountAccess) toEncodingObj(addr common.Address) AccountAccess {
+func (a *ConstructionAccountAccesses) toEncodingObj(addr common.Address) AccountAccess {
 	res := AccountAccess{
 		Address:        addr,
 		StorageChanges: make([]encodingSlotWrites, 0),

core/types/bal/bal_test.go

@@ -38,7 +38,7 @@ func equalBALs(a *BlockAccessList, b *BlockAccessList) bool {
 
 func makeTestConstructionBAL() *ConstructionBlockAccessList {
 	return &ConstructionBlockAccessList{
-		map[common.Address]*ConstructionAccountAccess{
+		map[common.Address]*ConstructionAccountAccesses{
 			common.BytesToAddress([]byte{0xff, 0xff}): {
 				StorageWrites: map[common.Hash]map[uint16]common.Hash{
 					common.BytesToHash([]byte{0x01}): {