refactor: readability improvements

Author: ARR4N

core/vm/preprocess.libevm_test.go

@@ -29,8 +29,6 @@ import (
 
 	"github.com/ava-labs/libevm/common"
 	"github.com/ava-labs/libevm/core"
-	"github.com/ava-labs/libevm/core/rawdb"
-	"github.com/ava-labs/libevm/core/state"
 	"github.com/ava-labs/libevm/core/types"
 	"github.com/ava-labs/libevm/core/vm"
 	"github.com/ava-labs/libevm/crypto"
@@ -148,12 +146,7 @@ func TestChargePreprocessingGas(t *testing.T) {
 			t.Logf("Extra gas charge: %d", tt.charge)
 
 			t.Run("ApplyTransaction", func(t *testing.T) {
-				sdb, err := state.New(
-					types.EmptyRootHash,
-					state.NewDatabase(rawdb.NewMemoryDatabase()),
-					nil,
-				)
-				require.NoError(t, err, "state.New(types.EmptyRootHash, [memory db], nil)")
+				_, _, sdb := ethtest.NewEmptyStateDB(t)
 				sdb.SetTxContext(tx.Hash(), i)
 				sdb.SetBalance(eoa, new(uint256.Int).SetAllOne())
 				sdb.SetNonce(eoa, tx.Nonce())

libevm/precompiles/parallel/parallel.go

@@ -69,18 +69,6 @@ type result[T any] struct {
 	val *T
 }
 
-// A stateDBSharer allows concurrent workers to make copies of a primary
-// database. When the `nextAvailable` channel is closed, all workers call
-// [state.StateDB.Copy] then signal completion on the [sync.WaitGroup]. The
-// channel is replaced for each round of distribution.
-type stateDBSharer struct {
-	nextAvailable chan struct{}
-	primary       *state.StateDB
-	mu            sync.Mutex
-	workers       int
-	wg            sync.WaitGroup
-}
-
 // New constructs a new [Processor] with the specified number of concurrent
 // workers. [Processor.Close] must be called after the final call to
 // [Processor.FinishBlock] to avoid leaking goroutines.
@@ -107,6 +95,29 @@ func New[R any](h Handler[R], workers int) *Processor[R] {
 	return p
 }
 
+// A stateDBSharer allows concurrent workers to make copies of a primary
+// database. When the `nextAvailable` channel is closed, all workers call
+// [state.StateDB.Copy] then signal completion on the [sync.WaitGroup]. The
+// channel is replaced for each round of distribution.
+type stateDBSharer struct {
+	nextAvailable chan struct{}
+	primary       *state.StateDB
+	mu            sync.Mutex
+	workers       int
+	wg            sync.WaitGroup
+}
+
+func (s *stateDBSharer) distribute(sdb *state.StateDB) {
+	s.primary = sdb // no need to Copy() as each worker does it
+
+	ch := s.nextAvailable                 // already copied by [Processor.worker], which is waiting for it to close
+	s.nextAvailable = make(chan struct{}) // will be copied, ready for the next distribution
+
+	s.wg.Add(s.workers)
+	close(ch)
+	s.wg.Wait()
+}
+
 func (p *Processor[R]) worker() {
 	defer p.workers.Done()
 
@@ -192,25 +203,14 @@ func (p *Processor[R]) StartBlock(b *types.Block, rules params.Rules, sdb *state
 	return nil
 }
 
-func (s *stateDBSharer) distribute(sdb *state.StateDB) {
-	s.primary = sdb // no need to Copy() as each worker does it
-
-	ch := s.nextAvailable
-	s.nextAvailable = make(chan struct{}) // already copied by each worker
-
-	s.wg.Add(s.workers)
-	close(ch)
-	s.wg.Wait()
-}
-
 // FinishBlock returns the [Processor] to a state ready for the next block. A
 // return from FinishBlock guarantees that all dispatched work from the
 // respective call to [Processor.StartBlock] has been completed.
 func (p *Processor[R]) FinishBlock(b *types.Block) {
 	for i := range len(b.Transactions()) {
 		// Every result channel is guaranteed to have some value in its buffer
 		// because [Processor.BeforeBlock] either sends a nil *R or it
-		// dispatches a job that will send a non-nil *R.
+		// dispatches a job, which will send a non-nil *R.
 		tx := (<-p.results[i]).tx
 		delete(p.txGas, tx)
 	}
@@ -243,7 +243,7 @@ func (p *Processor[R]) Result(i int) (R, bool) {
 	return *r.val, true
 }
 
-func (p *Processor[R]) shouldProcess(tx *types.Transaction, rules params.Rules) (process bool, err error) {
+func (p *Processor[R]) shouldProcess(tx *types.Transaction, rules params.Rules) (process bool, retErr error) {
 	// An explicit 0 is necessary to avoid [Processor.PreprocessingGasCharge]
 	// returning [ErrTxUnknown].
 	p.txGas[tx.Hash()] = 0
@@ -253,7 +253,7 @@ func (p *Processor[R]) shouldProcess(tx *types.Transaction, rules params.Rules)
 		return false, nil
 	}
 	defer func() {
-		if process && err == nil {
+		if process && retErr == nil {
 			p.txGas[tx.Hash()] = cost
 		}
 	}()
@@ -262,11 +262,12 @@ func (p *Processor[R]) shouldProcess(tx *types.Transaction, rules params.Rules)
 	if err != nil {
 		return false, fmt.Errorf("calculating intrinsic gas of %v: %v", tx.Hash(), err)
 	}
-
-	// This could only overflow if the gas limit was insufficient to cover
-	// the intrinsic cost, which would have invalidated it for inclusion.
-	left := tx.Gas() - spent
-	return left >= cost, nil
+	if spent > tx.Gas() {
+		// If this happens then consensus has a bug because the tx shouldn't
+		// have been included. We include the check, however, for completeness.
+		return false, core.ErrIntrinsicGas
+	}
+	return tx.Gas()-spent >= cost, nil
 }
 
 func txIntrinsicGas(tx *types.Transaction, rules *params.Rules) (uint64, error) {

libevm/precompiles/parallel/parallel_test.go

@@ -47,41 +47,39 @@ func TestMain(m *testing.M) {
 	goleak.VerifyTestMain(m, goleak.IgnoreCurrent())
 }
 
-type reverser struct {
+type concat struct {
 	headerExtra []byte
 	addr        common.Address
 	stateKey    common.Hash
 	gas         uint64
 }
 
-func (r *reverser) BeforeBlock(h *types.Header) {
-	r.headerExtra = slices.Clone(h.Extra)
+func (c *concat) BeforeBlock(h *types.Header) {
+	c.headerExtra = slices.Clone(h.Extra)
 }
 
-func (r *reverser) Gas(tx *types.Transaction) (uint64, bool) {
-	if to := tx.To(); to == nil || *to != r.addr {
-		return 0, false
+func (c *concat) Gas(tx *types.Transaction) (uint64, bool) {
+	if to := tx.To(); to != nil && *to == c.addr {
+		return c.gas, true
 	}
-	return r.gas, true
+	return 0, false
 }
 
-func reverserOutput(txData []byte, state common.Hash, extra []byte) []byte {
-	out := slices.Concat(txData, state[:], extra)
-	slices.Reverse(out)
-	return out
+func concatOutput(txData []byte, state common.Hash, extra []byte) []byte {
+	return slices.Concat(txData, state[:], extra)
 }
 
-func (r *reverser) Process(sdb libevm.StateReader, i int, tx *types.Transaction) []byte {
-	return reverserOutput(
+func (c *concat) Process(sdb libevm.StateReader, i int, tx *types.Transaction) []byte {
+	return concatOutput(
 		tx.Data(),
-		sdb.GetTransientState(r.addr, r.stateKey),
-		r.headerExtra,
+		sdb.GetTransientState(c.addr, c.stateKey),
+		c.headerExtra,
 	)
 }
 
 func TestProcessor(t *testing.T) {
-	handler := &reverser{
-		addr:     common.Address{'r', 'e', 'v', 'e', 'r', 's', 'e'},
+	handler := &concat{
+		addr:     common.Address{'c', 'o', 'n', 'c', 'a', 't'},
 		stateKey: common.Hash{'k', 'e', 'y'},
 		gas:      1e6,
 	}
@@ -164,7 +162,7 @@ func TestProcessor(t *testing.T) {
 
 				data := binary.BigEndian.AppendUint64(nil, uint64(i))
 				gas, err := intrinsicGas(data, types.AccessList{}, &handler.addr, &rules)
-				require.NoError(t, err, "core.IntrinsicGas(%#x, nil, false, true, true, true)", data)
+				require.NoError(t, err, "core.IntrinsicGas(%#x, nil, false, ...)", data)
 
 				txs[i] = types.NewTx(&types.LegacyTx{
 					To:   &to,
@@ -183,7 +181,7 @@ func TestProcessor(t *testing.T) {
 
 				var want []byte
 				if wantOK {
-					want = reverserOutput(tx.Data(), stateVal, extra)
+					want = concatOutput(tx.Data(), stateVal, extra)
 				}
 
 				got, gotOK := p.Result(i)
@@ -210,8 +208,8 @@ func (h *vmHooks) PreprocessingGasCharge(tx common.Hash) (uint64, error) {
 
 func TestIntegration(t *testing.T) {
 	const handlerGas = 500
-	handler := &reverser{
-		addr: common.Address{'r', 'e', 'v', 'e', 'r', 's', 'e'},
+	handler := &concat{
+		addr: common.Address{'c', 'o', 'n', 'c', 'a', 't'},
 		gas:  handlerGas,
 	}
 	sut := New(handler, 8)
@@ -277,7 +275,7 @@ func TestIntegration(t *testing.T) {
 		data := []byte("hello, world")
 
 		gas, err := intrinsicGas(data, types.AccessList{}, &addr, &rules)
-		require.NoError(t, err, "core.IntrinsicGas(%#x, nil, false, false, false, false)", data)
+		require.NoError(t, err, "core.IntrinsicGas(%#x, nil, false, ...)", data)
 		if addr == handler.addr {
 			gas += handlerGas
 		}
@@ -300,7 +298,7 @@ func TestIntegration(t *testing.T) {
 			wantR.Logs = []*types.Log{{
 				TxHash:  tx.Hash(),
 				TxIndex: ui,
-				Data:    reverserOutput(data, common.Hash{}, nil),
+				Data:    concatOutput(data, common.Hash{}, nil),
 			}}
 		}
 		want = append(want, wantR)