refactor: rationalise usage of sae.SUT.runConsensusLoop() (#218)

1. Renames `SUT.runConsensusLoop()` to `runConsensusLoopOnPreference()`
to allow (2).
2. Renames `runConsensusLoopFromLastAccepted()` to `runConsensusLoop()`
as it's used so frequently that it should be the shorter, generic
method.
3. Simplifies all usage of `runConsensusLoopOnPreference(...,
sut.lastAcceptedBlock(t) / sut.genesis)` to just `runConsensusLoop()`.
4. Where appropriate, removes unnecessary transactions passed to
`runConsensusLoop()`.

Author: ARR4N

sae/recovery_test.go

@@ -62,7 +62,7 @@ func TestRecoverFromDatabase(t *testing.T) {
 		}
 
 		vmTime.advance(850 * time.Millisecond)
-		b := src.runConsensusLoop(t, src.lastAcceptedBlock(t))
+		b := src.runConsensusLoop(t)
 		if !quick {
 			require.Len(t, b.Transactions(), 1, "transactions in block")
 		}
@@ -113,7 +113,7 @@ func TestRecoverFromDatabase(t *testing.T) {
 					} {
 						t.Run(sys.name, func(t *testing.T) {
 							sys.mustSendTx(t, tx)
-							b := sys.runConsensusLoop(t, sys.lastAcceptedBlock(t))
+							b := sys.runConsensusLoop(t)
 							require.Len(t, b.Transactions(), 1)
 							require.NoError(t, b.WaitUntilExecuted(sys.ctx))
 						})

sae/rpc_stateful_test.go

@@ -43,7 +43,7 @@ func TestEthCall(t *testing.T) {
 	}
 
 	sign := sut.wallet.SetNonceAndSign
-	b := sut.runConsensusLoopFromLastAccepted(t, sign(t, 0, deploy), sign(t, 0, deposit))
+	b := sut.runConsensusLoop(t, sign(t, 0, deploy), sign(t, 0, deposit))
 	require.Len(t, b.Transactions(), 2, "tx count")
 	require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 	for _, r := range b.Receipts() {
@@ -52,7 +52,7 @@ func TestEthCall(t *testing.T) {
 
 	vmTime.advanceToSettle(ctx, t, b)
 	for range 2 {
-		bb := sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+		bb := sut.runConsensusLoop(t)
 		vmTime.advanceToSettle(ctx, t, bb)
 	}
 	_, ok := sut.rawVM.blocks.Load(b.Hash())

sae/rpc_test.go

@@ -166,7 +166,7 @@ func TestSubscriptions(t *testing.T) {
 	runConsensusLoop := func(wantLogs ...types.Log) {
 		t.Helper()
 
-		b := sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+		b := sut.runConsensusLoop(t)
 		require.Equal(t, b.Hash(), (<-newHeads).Hash(), "header hash from newHeads subscription")
 
 		for _, want := range wantLogs {
@@ -423,28 +423,23 @@ func TestEthGetters(t *testing.T) {
 	blockingPrecompile := common.Address{'b', 'l', 'o', 'c', 'k'}
 	registerBlockingPrecompile(t, blockingPrecompile)
 
-	createTx := func(t *testing.T, to common.Address) *types.Transaction {
-		t.Helper()
-		return sut.wallet.SetNonceAndSign(t, 0, &types.DynamicFeeTx{
-			To:        &to,
-			Gas:       params.TxGas,
-			GasFeeCap: big.NewInt(1),
-		})
-	}
-
 	genesis := sut.lastAcceptedBlock(t)
 
 	// Once a block is settled, its ancestors are only accessible from the
 	// database.
-	onDisk := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
+	onDisk := sut.runConsensusLoop(t)
 
-	settled := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
+	settled := sut.runConsensusLoop(t)
 	vmTime.advanceToSettle(ctx, t, settled)
 
-	executed := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
+	executed := sut.runConsensusLoop(t)
 	require.NoErrorf(t, executed.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", executed)
 
-	pending := sut.runConsensusLoopFromLastAccepted(t, createTx(t, blockingPrecompile))
+	pending := sut.runConsensusLoop(t, sut.wallet.SetNonceAndSign(t, 0, &types.DynamicFeeTx{
+		To:        &blockingPrecompile,
+		Gas:       params.TxGas,
+		GasFeeCap: big.NewInt(1),
+	}))
 
 	for _, b := range []*blocks.Block{genesis, onDisk, settled, executed, pending} {
 		t.Run(fmt.Sprintf("block_num_%d", b.Height()), func(t *testing.T) {
@@ -532,15 +527,15 @@ func TestGetLogs(t *testing.T) {
 	// and therefore moved to disk.
 	indexed := make([]*blocks.Block, bloomSectionSize)
 	for i := range indexed {
-		indexed[i] = sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
+		indexed[i] = sut.runConsensusLoop(t, txWithLog(t))
 	}
 
-	settled := sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
+	settled := sut.runConsensusLoop(t, txWithLog(t))
 	vmTime.advanceToSettle(ctx, t, settled)
 
-	noLogs := sut.runConsensusLoopFromLastAccepted(t, txWithoutLog(t))
+	noLogs := sut.runConsensusLoop(t, txWithoutLog(t))
 
-	executed := sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
+	executed := sut.runConsensusLoop(t, txWithLog(t))
 	require.NoErrorf(t, executed.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", executed)
 
 	// Although the FiltersAPI will work without any blocks indexed, such a
@@ -698,7 +693,7 @@ func TestGetReceipts(t *testing.T) {
 
 	slice := func(t *testing.T, from, to int) (*blocks.Block, []*types.Receipt) {
 		t.Helper()
-		b := sut.runConsensusLoopFromLastAccepted(t, txs[from:to]...)
+		b := sut.runConsensusLoop(t, txs[from:to]...)
 		rs := want[from:to]
 
 		var totalGas uint64
@@ -725,7 +720,7 @@ func TestGetReceipts(t *testing.T) {
 		Gas:      params.TxGas,
 		GasPrice: big.NewInt(1),
 	})
-	pending := sut.runConsensusLoopFromLastAccepted(t, pendingTx)
+	pending := sut.runConsensusLoop(t, pendingTx)
 
 	var tests []rpcTest
 	for _, tc := range []struct {
@@ -913,7 +908,7 @@ func TestDebugGetRawTransaction(t *testing.T) {
 		Gas:       params.TxGas,
 		GasFeeCap: big.NewInt(1),
 	})
-	b := sut.runConsensusLoopFromLastAccepted(t, tx)
+	b := sut.runConsensusLoop(t, tx)
 	require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 
 	marshaled, err := tx.MarshalBinary()
@@ -1179,17 +1174,17 @@ func TestResolveBlockNumberOrHash(t *testing.T) {
 	opt, vmTime := withVMTime(t, time.Unix(saeparams.TauSeconds, 0))
 	ctx, sut := newSUT(t, 0, opt)
 
-	settled := sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+	settled := sut.runConsensusLoop(t)
 	vmTime.advanceToSettle(ctx, t, settled)
 
 	for range 2 {
-		b := sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+		b := sut.runConsensusLoop(t)
 		vmTime.advanceToSettle(ctx, t, b)
 	}
 	_, ok := sut.rawVM.blocks.Load(settled.Hash())
 	require.False(t, ok, "settled block still in VM memory")
 
-	accepted := sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+	accepted := sut.runConsensusLoop(t)
 	require.NoError(t, sut.SetPreference(ctx, accepted.ID()), "SetPreference()")
 
 	b, err := sut.BuildBlock(ctx)

sae/tx_test.go

@@ -44,7 +44,7 @@ func TestTxTypeSupport(t *testing.T) {
 			t.FailNow()
 		}
 	}
-	b := sut.runConsensusLoop(t, sut.genesis)
+	b := sut.runConsensusLoop(t)
 	require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 
 	sdb := sut.stateAt(t, b.PostExecutionStateRoot())

sae/vm_test.go

@@ -376,24 +376,26 @@ func (s *SUT) createAndVerifyBlock(tb testing.TB, preference *blocks.Block, txs
 	return b
 }
 
-// createAndAcceptBlock is equivalent to [SUT.createAndVerifyBlock] except that
-// it also accepts the block with [VM.AcceptBlock]. It does NOT wait for it to
-// be executed; to do this automatically, set the [VM] to [snow.Bootstrapping].
+// runConsensusLoopOnPreference is equivalent to [SUT.createAndVerifyBlock]
+// except that it also accepts the block with [VM.AcceptBlock]. It does NOT wait
+// for it to be executed; to do this automatically, set the [VM] to
+// [snow.Bootstrapping].
 //
 // There is no longer any need to wrap the block as an [adaptor.Block] so it is
 // returned in its raw form, unlike earlier steps in the consenus loop.
-func (s *SUT) runConsensusLoop(tb testing.TB, preference *blocks.Block, txs ...*types.Transaction) *blocks.Block {
+func (s *SUT) runConsensusLoopOnPreference(tb testing.TB, preference *blocks.Block, txs ...*types.Transaction) *blocks.Block {
 	tb.Helper()
 	b := s.createAndVerifyBlock(tb, preference, txs...)
 	require.NoErrorf(tb, b.Accept(s.context(tb)), "%T.Accept()", b)
 	return unwrap(tb, b)
 }
 
-// runConsensusLoopFromLastAccepted is a convenience wrapper for
-// [SUT.runConsensusLoop], using [SUT.lastAcceptedBlock] as the preference.
-func (s *SUT) runConsensusLoopFromLastAccepted(tb testing.TB, txs ...*types.Transaction) *blocks.Block {
+// runConsensusLoop is a convenience wrapper for
+// [SUT.runConsensusLoopOnPreference], using [SUT.lastAcceptedBlock] as the
+// preference.
+func (s *SUT) runConsensusLoop(tb testing.TB, txs ...*types.Transaction) *blocks.Block {
 	tb.Helper()
-	return s.runConsensusLoop(tb, s.lastAcceptedBlock(tb), txs...)
+	return s.runConsensusLoopOnPreference(tb, s.lastAcceptedBlock(tb), txs...)
 }
 
 // waitUntilExecuted blocks until an external indicator shows that `b` has been
@@ -545,7 +547,7 @@ func TestIntegration(t *testing.T) {
 	sut.syncMempool(t) // technically we've only proven 1 tx added, as unlikely as a race is
 	require.Equal(t, numTxs, sut.rawVM.numPendingTxs(), "number of pending txs")
 
-	b := sut.runConsensusLoop(t, sut.genesis)
+	b := sut.runConsensusLoopOnPreference(t, sut.genesis)
 	assert.Equal(t, sut.genesis.ID(), b.Parent(), "BuildBlock() builds on preference")
 	require.Lenf(t, b.Transactions(), numTxs, "%T.Transactions()", b)
 
@@ -559,7 +561,7 @@ func TestIntegration(t *testing.T) {
 		// If the tx-inclusion logic were broken then this would include the
 		// transactions again, resulting in a FATAL in the execution loop due to
 		// non-increasing nonce.
-		b := sut.runConsensusLoop(t, b)
+		b := sut.runConsensusLoopOnPreference(t, b)
 		assert.Emptyf(t, b.Transactions(), "%T.Transactions()", b)
 		require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 	})
@@ -580,9 +582,8 @@ func TestEmptyChainConfig(t *testing.T) {
 		}
 	}))
 	for range 5 {
-		sut.runConsensusLoop(t, sut.lastAcceptedBlock(t))
+		sut.runConsensusLoop(t)
 	}
-	sut.waitUntilExecuted(t, sut.lastAcceptedBlock(t))
 }
 
 func TestSyntacticBlockChecks(t *testing.T) {
@@ -639,17 +640,14 @@ func TestAcceptBlock(t *testing.T) {
 	require.NoError(t, sut.SetState(ctx, snow.Bootstrapping), "SetState(Bootstrapping)")
 
 	unsettled := []*blocks.Block{sut.genesis}
-	last := func() *blocks.Block {
-		return unsettled[len(unsettled)-1]
-	}
 	sut.genesis = nil // allow it to be GCd when appropriate
 
 	rng := rand.New(rand.NewPCG(0, 0)) //nolint:gosec // Reproducibility is useful for tests
 	for range 100 {
 		ffMillis := 100 + rng.IntN(1000*(1+saeparams.TauSeconds))
 		vmTime.advance(time.Millisecond * time.Duration(ffMillis))
 
-		b := sut.runConsensusLoop(t, last())
+		b := sut.runConsensusLoop(t)
 		unsettled = append(unsettled, b)
 		sut.assertBlockHashInvariants(ctx, t)
 
@@ -808,10 +806,10 @@ func TestGetBlock(t *testing.T) {
 	genesis := sut.lastAcceptedBlock(t)
 	// Once a block is settled, its ancestors are only accessible from the
 	// database.
-	onDisk := sut.runConsensusLoopFromLastAccepted(t)
-	settled := sut.runConsensusLoopFromLastAccepted(t)
+	onDisk := sut.runConsensusLoop(t)
+	settled := sut.runConsensusLoop(t)
 	vmTime.advanceToSettle(ctx, t, settled)
-	unsettled := sut.runConsensusLoopFromLastAccepted(t)
+	unsettled := sut.runConsensusLoop(t)
 
 	verified := sut.createAndVerifyBlock(t, unsettled)
 	unverified := sut.buildAndParseBlock(

sae/worstcase_test.go

@@ -175,7 +175,7 @@ func TestWorstCase(t *testing.T) {
 			}))
 		}
 
-		b := sut.runConsensusLoop(t, sut.genesis)
+		b := sut.runConsensusLoop(t)
 		require.NoError(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 		require.Lenf(t, b.Receipts(), len(precompileTests), "%T.Receipts()", b)
 		for i, r := range b.Receipts() {