revamp BAL validation error logging

core/parallel_state_processor.go

@@ -122,9 +122,9 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, allStateR
 	allStateReads.Merge(stateReads)
 
 	balIdx := len(block.Transactions()) + 1
-	if err := postTxState.BlockAccessList().ValidateStateDiff(balIdx, diff); err != nil {
+	if !postTxState.BlockAccessList().ValidateStateDiff(balIdx, diff) {
 		return &ProcessResultWithMetrics{
-			ProcessResult: &ProcessResult{Error: err},
+			ProcessResult: &ProcessResult{Error: fmt.Errorf("BAL validation failure")},
 		}
 	}
 
@@ -272,8 +272,8 @@ func (p *ParallelStateProcessor) execTx(block *types.Block, tx *types.Transactio
 	}
 
 	diff, accesses := balTracer.builder.FinalizedIdxChanges()
-	if err := db.BlockAccessList().ValidateStateDiff(txIdx+1, diff); err != nil {
+	if !db.BlockAccessList().ValidateStateDiff(txIdx+1, diff) {
-		return &txExecResult{err: err}
+		return &txExecResult{err: fmt.Errorf("bal validation failure")}
 	}
 
 	return &txExecResult{
@@ -317,8 +317,8 @@ func (p *ParallelStateProcessor) Process(block *types.Block, stateTransition *st
 	balTracer.OnPreTxExecutionDone()
 
 	diff, stateReads := balTracer.builder.FinalizedIdxChanges()
-	if err := statedb.BlockAccessList().ValidateStateDiff(0, diff); err != nil {
+	if !statedb.BlockAccessList().ValidateStateDiff(0, diff) {
-		return nil, err
+		return nil, fmt.Errorf("BAL validation failure")
 	}
 
 	// compute the post-tx state prestate (before applying final block system calls and eip-4895 withdrawals)

core/state/bal_reader.go

@@ -7,6 +7,7 @@ import (
 	"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"
 )
@@ -298,24 +299,70 @@ func (r *BALReader) readAccountDiff(addr common.Address, idx int) *bal.AccountMu
 	return &res
 }
 
+func mutationsLogfmt(prefix string, mutations *bal.AccountMutations) (logs []interface{}) {
+	if mutations.Code != nil {
+		logs = append(logs, fmt.Sprintf("%s-code", prefix), fmt.Sprintf("%x", mutations.Code))
+	}
+	if mutations.Balance != nil {
+		logs = append(logs, fmt.Sprintf("%s-balance", prefix), mutations.Balance.String())
+	}
+	if mutations.Nonce != nil {
+		logs = append(logs, fmt.Sprintf("%s-nonce", prefix), mutations.Nonce)
+	}
+	if mutations.StorageWrites != nil {
+		for key, val := range mutations.StorageWrites {
+			logs = append(logs, fmt.Sprintf("%s-storage-write-key"), key, fmt.Sprintf("%s-storage-write-value"), val)
+		}
+	}
+	return logs
+}
+
+func logfmtMutationsDiff(local, remote map[common.Address]*bal.AccountMutations) (logs []interface{}) {
+	keys := make(map[common.Address]struct{})
+
+	for addr, _ := range local {
+		keys[addr] = struct{}{}
+	}
+	for addr, _ := range remote {
+		keys[addr] = struct{}{}
+	}
+
+	for addr := range keys {
+		_, hasLocal := local[addr]
+		_, hasRemote := remote[addr]
+
+		if hasLocal && !hasRemote {
+			logs = append(logs, mutationsLogfmt(fmt.Sprintf("local-%x", addr), local[addr])...)
+		}
+		if !hasLocal && hasRemote {
+			logs = append(logs, mutationsLogfmt(fmt.Sprintf("remote-%x", addr), remote[addr])...)
+		}
+	}
+	return logs
+}
+
 // ValidateStateDiff returns an error if the computed state diff is not equal to
 // diff reported from the access list at the given index.
-func (r *BALReader) ValidateStateDiff(idx int, computedDiff *bal.StateDiff) error {
+func (r *BALReader) ValidateStateDiff(idx int, computedDiff *bal.StateDiff) bool {
 	balChanges := r.changesAt(idx)
 	for addr, state := range balChanges.Mutations {
 		computedAccountDiff, ok := computedDiff.Mutations[addr]
 		if !ok {
-			return fmt.Errorf("BAL contained account %x which wasn't present in computed state diff", addr)
+			// TODO: print out the full fields here
+			log.Error("BAL contained account which wasn't present in computed state diff", "address", addr)
+			return false
 		}
 
 		if !state.Eq(computedAccountDiff) {
-			return fmt.Errorf("difference between computed state diff and BAL entry for account %x", addr)
+			state.LogDiff(addr, computedAccountDiff)
+			return false
 		}
 	}
 
 	if len(balChanges.Mutations) != len(computedDiff.Mutations) {
-		return fmt.Errorf("computed state diff contained mutated accounts which weren't reported in BAL")
+		log.Error("computed state diff contained accounts that weren't reported in BAL", logfmtMutationsDiff(computedDiff.Mutations, balChanges.Mutations))
+		return false
 	}
 
-	return nil
+	return true
 }

core/types/bal/bal.go

@@ -599,7 +599,7 @@ func (a *AccountMutations) String() string {
 	return res.String()
 }
 
-func (a *AccountMutations) LogDiff(other *AccountMutations) {
+func (a *AccountMutations) LogDiff(addr common.Address, other *AccountMutations) {
 	var diff []interface{}
 
 	if a.Balance != nil || other.Balance != nil {
@@ -635,15 +635,15 @@ func (a *AccountMutations) LogDiff(other *AccountMutations) {
 				otherVal, inOther := other.StorageWrites[key]
 
 				if (inA && !inOther) || (!inA && inOther) || !bytes.Equal(aVal[:], otherVal[:]) {
-					diff = append(diff, fmt.Sprintf("storageLocal%x", key), aVal)
+					diff = append(diff, fmt.Sprintf("storage-local-%x", key), aVal)
-					diff = append(diff, fmt.Sprintf("storageLocal%x", key), otherVal)
+					diff = append(diff, fmt.Sprintf("storage-remote-%x", key), otherVal)
 				}
 			}
 		}
 	}
 
 	if len(diff) > 0 {
-		log.Error("diff between remote/local BAL", diff...)
+		log.Error(fmt.Sprintf("diff between remote/local BAL for address %x", addr), diff...)
 	}
 }