core, ethclient/gethclient: improve flaky tests

consensus/clique/snapshot_test.go

@@ -19,6 +19,7 @@ package clique
 import (
 	"bytes"
 	"crypto/ecdsa"
+	"fmt"
 	"math/big"
 	"sort"
 	"testing"
@@ -95,17 +96,19 @@ type testerVote struct {
 	newbatch   bool
 }
 
+type cliqueTest struct {
+	epoch   uint64
+	signers []string
+	votes   []testerVote
+	results []string
+	failure error
+}
+
 // Tests that Clique signer voting is evaluated correctly for various simple and
 // complex scenarios, as well as that a few special corner cases fail correctly.
 func TestClique(t *testing.T) {
 	// Define the various voting scenarios to test
-	tests := []struct {
-		epoch   uint64
-		signers []string
-		votes   []testerVote
-		results []string
-		failure error
-	}{
+	tests := []cliqueTest{
 		{
 			// Single signer, no votes cast
 			signers: []string{"A"},
@@ -377,129 +380,129 @@ func TestClique(t *testing.T) {
 			failure: errRecentlySigned,
 		},
 	}
+
 	// Run through the scenarios and test them
 	for i, tt := range tests {
-		// Create the account pool and generate the initial set of signers
-		accounts := newTesterAccountPool()
+		t.Run(fmt.Sprint(i), tt.run)
+	}
+}
 
-		signers := make([]common.Address, len(tt.signers))
-		for j, signer := range tt.signers {
-			signers[j] = accounts.address(signer)
-		}
-		for j := 0; j < len(signers); j++ {
-			for k := j + 1; k < len(signers); k++ {
-				if bytes.Compare(signers[j][:], signers[k][:]) > 0 {
-					signers[j], signers[k] = signers[k], signers[j]
-				}
-			}
-		}
-		// Create the genesis block with the initial set of signers
-		genesis := &core.Genesis{
-			ExtraData: make([]byte, extraVanity+common.AddressLength*len(signers)+extraSeal),
-			BaseFee:   big.NewInt(params.InitialBaseFee),
-		}
-		for j, signer := range signers {
-			copy(genesis.ExtraData[extraVanity+j*common.AddressLength:], signer[:])
-		}
+func (tt *cliqueTest) run(t *testing.T) {
+	// Create the account pool and generate the initial set of signers
+	accounts := newTesterAccountPool()
 
-		// Assemble a chain of headers from the cast votes
-		config := *params.TestChainConfig
-		config.Clique = &params.CliqueConfig{
-			Period: 1,
-			Epoch:  tt.epoch,
+	signers := make([]common.Address, len(tt.signers))
+	for j, signer := range tt.signers {
+		signers[j] = accounts.address(signer)
+	}
+	for j := 0; j < len(signers); j++ {
+		for k := j + 1; k < len(signers); k++ {
+			if bytes.Compare(signers[j][:], signers[k][:]) > 0 {
+				signers[j], signers[k] = signers[k], signers[j]
+			}
 		}
-		genesis.Config = &config
+	}
+	// Create the genesis block with the initial set of signers
+	genesis := &core.Genesis{
+		ExtraData: make([]byte, extraVanity+common.AddressLength*len(signers)+extraSeal),
+		BaseFee:   big.NewInt(params.InitialBaseFee),
+	}
+	for j, signer := range signers {
+		copy(genesis.ExtraData[extraVanity+j*common.AddressLength:], signer[:])
+	}
 
-		engine := New(config.Clique, rawdb.NewMemoryDatabase())
-		engine.fakeDiff = true
+	// Assemble a chain of headers from the cast votes
+	config := *params.TestChainConfig
+	config.Clique = &params.CliqueConfig{
+		Period: 1,
+		Epoch:  tt.epoch,
+	}
+	genesis.Config = &config
 
-		_, blocks, _ := core.GenerateChainWithGenesis(genesis, engine, len(tt.votes), func(j int, gen *core.BlockGen) {
-			// Cast the vote contained in this block
-			gen.SetCoinbase(accounts.address(tt.votes[j].voted))
-			if tt.votes[j].auth {
-				var nonce types.BlockNonce
-				copy(nonce[:], nonceAuthVote)
-				gen.SetNonce(nonce)
-			}
-		})
-		// Iterate through the blocks and seal them individually
-		for j, block := range blocks {
-			// Get the header and prepare it for signing
-			header := block.Header()
-			if j > 0 {
-				header.ParentHash = blocks[j-1].Hash()
-			}
-			header.Extra = make([]byte, extraVanity+extraSeal)
-			if auths := tt.votes[j].checkpoint; auths != nil {
-				header.Extra = make([]byte, extraVanity+len(auths)*common.AddressLength+extraSeal)
-				accounts.checkpoint(header, auths)
-			}
-			header.Difficulty = diffInTurn // Ignored, we just need a valid number
+	engine := New(config.Clique, rawdb.NewMemoryDatabase())
+	engine.fakeDiff = true
 
-			// Generate the signature, embed it into the header and the block
-			accounts.sign(header, tt.votes[j].signer)
-			blocks[j] = block.WithSeal(header)
-		}
-		// Split the blocks up into individual import batches (cornercase testing)
-		batches := [][]*types.Block{nil}
-		for j, block := range blocks {
-			if tt.votes[j].newbatch {
-				batches = append(batches, nil)
-			}
-			batches[len(batches)-1] = append(batches[len(batches)-1], block)
-		}
-		// Pass all the headers through clique and ensure tallying succeeds
-		chain, err := core.NewBlockChain(rawdb.NewMemoryDatabase(), nil, genesis, nil, engine, vm.Config{}, nil, nil)
-		if err != nil {
-			t.Errorf("test %d: failed to create test chain: %v", i, err)
-			continue
-		}
-		failed := false
-		for j := 0; j < len(batches)-1; j++ {
-			if k, err := chain.InsertChain(batches[j]); err != nil {
-				t.Errorf("test %d: failed to import batch %d, block %d: %v", i, j, k, err)
-				failed = true
-				break
-			}
-		}
-		if failed {
-			continue
+	_, blocks, _ := core.GenerateChainWithGenesis(genesis, engine, len(tt.votes), func(j int, gen *core.BlockGen) {
+		// Cast the vote contained in this block
+		gen.SetCoinbase(accounts.address(tt.votes[j].voted))
+		if tt.votes[j].auth {
+			var nonce types.BlockNonce
+			copy(nonce[:], nonceAuthVote)
+			gen.SetNonce(nonce)
 		}
-		if _, err = chain.InsertChain(batches[len(batches)-1]); err != tt.failure {
-			t.Errorf("test %d: failure mismatch: have %v, want %v", i, err, tt.failure)
+	})
+	// Iterate through the blocks and seal them individually
+	for j, block := range blocks {
+		// Get the header and prepare it for signing
+		header := block.Header()
+		if j > 0 {
+			header.ParentHash = blocks[j-1].Hash()
 		}
-		if tt.failure != nil {
-			continue
+		header.Extra = make([]byte, extraVanity+extraSeal)
+		if auths := tt.votes[j].checkpoint; auths != nil {
+			header.Extra = make([]byte, extraVanity+len(auths)*common.AddressLength+extraSeal)
+			accounts.checkpoint(header, auths)
 		}
-		// No failure was produced or requested, generate the final voting snapshot
-		head := blocks[len(blocks)-1]
+		header.Difficulty = diffInTurn // Ignored, we just need a valid number
 
-		snap, err := engine.snapshot(chain, head.NumberU64(), head.Hash(), nil)
-		if err != nil {
-			t.Errorf("test %d: failed to retrieve voting snapshot: %v", i, err)
-			continue
+		// Generate the signature, embed it into the header and the block
+		accounts.sign(header, tt.votes[j].signer)
+		blocks[j] = block.WithSeal(header)
+	}
+	// Split the blocks up into individual import batches (cornercase testing)
+	batches := [][]*types.Block{nil}
+	for j, block := range blocks {
+		if tt.votes[j].newbatch {
+			batches = append(batches, nil)
 		}
-		// Verify the final list of signers against the expected ones
-		signers = make([]common.Address, len(tt.results))
-		for j, signer := range tt.results {
-			signers[j] = accounts.address(signer)
+		batches[len(batches)-1] = append(batches[len(batches)-1], block)
+	}
+	// Pass all the headers through clique and ensure tallying succeeds
+	chain, err := core.NewBlockChain(rawdb.NewMemoryDatabase(), nil, genesis, nil, engine, vm.Config{}, nil, nil)
+	if err != nil {
+		t.Fatalf("failed to create test chain: %v", err)
+	}
+	defer chain.Stop()
+
+	for j := 0; j < len(batches)-1; j++ {
+		if k, err := chain.InsertChain(batches[j]); err != nil {
+			t.Fatalf("failed to import batch %d, block %d: %v", j, k, err)
+			break
 		}
-		for j := 0; j < len(signers); j++ {
-			for k := j + 1; k < len(signers); k++ {
-				if bytes.Compare(signers[j][:], signers[k][:]) > 0 {
-					signers[j], signers[k] = signers[k], signers[j]
-				}
+	}
+	if _, err = chain.InsertChain(batches[len(batches)-1]); err != tt.failure {
+		t.Errorf("failure mismatch: have %v, want %v", err, tt.failure)
+	}
+	if tt.failure != nil {
+		return
+	}
+
+	// No failure was produced or requested, generate the final voting snapshot
+	head := blocks[len(blocks)-1]
+
+	snap, err := engine.snapshot(chain, head.NumberU64(), head.Hash(), nil)
+	if err != nil {
+		t.Fatalf("failed to retrieve voting snapshot: %v", err)
+	}
+	// Verify the final list of signers against the expected ones
+	signers = make([]common.Address, len(tt.results))
+	for j, signer := range tt.results {
+		signers[j] = accounts.address(signer)
+	}
+	for j := 0; j < len(signers); j++ {
+		for k := j + 1; k < len(signers); k++ {
+			if bytes.Compare(signers[j][:], signers[k][:]) > 0 {
+				signers[j], signers[k] = signers[k], signers[j]
 			}
 		}
-		result := snap.signers()
-		if len(result) != len(signers) {
-			t.Errorf("test %d: signers mismatch: have %x, want %x", i, result, signers)
-			continue
-		}
-		for j := 0; j < len(result); j++ {
-			if !bytes.Equal(result[j][:], signers[j][:]) {
-				t.Errorf("test %d, signer %d: signer mismatch: have %x, want %x", i, j, result[j], signers[j])
-			}
+	}
+	result := snap.signers()
+	if len(result) != len(signers) {
+		t.Fatalf("signers mismatch: have %x, want %x", result, signers)
+	}
+	for j := 0; j < len(result); j++ {
+		if !bytes.Equal(result[j][:], signers[j][:]) {
+			t.Fatalf("signer %d: signer mismatch: have %x, want %x", j, result[j], signers[j])
 		}
 	}
 }

core/blockchain.go

@@ -856,9 +856,13 @@ func (bc *BlockChain) writeHeadBlock(block *types.Block) {
 	headBlockGauge.Update(int64(block.NumberU64()))
 }
 
-// Stop stops the blockchain service. If any imports are currently in progress
-// it will abort them using the procInterrupt.
-func (bc *BlockChain) Stop() {
+// stop stops the blockchain service. If any imports are currently in progress
+// it will abort them using the procInterrupt. This method stops all running
+// goroutines, but does not do all the post-stop work of persisting data.
+// OBS! It is generally recommended to use the Stop method!
+// This method has been exposed to allow tests to stop the blockchain while simulating
+// a crash.
+func (bc *BlockChain) stopWithoutSaving() {
 	if !atomic.CompareAndSwapInt32(&bc.running, 0, 1) {
 		return
 	}
@@ -878,6 +882,12 @@ func (bc *BlockChain) Stop() {
 	// returned.
 	bc.chainmu.Close()
 	bc.wg.Wait()
+}
+
+// Stop stops the blockchain service. If any imports are currently in progress
+// it will abort them using the procInterrupt.
+func (bc *BlockChain) Stop() {
+	bc.stopWithoutSaving()
 
 	// Ensure that the entirety of the state snapshot is journalled to disk.
 	var snapBase common.Hash

core/blockchain_repair_test.go

@@ -1826,6 +1826,7 @@ func testRepair(t *testing.T, tt *rewindTest, snapshots bool) {
 	}
 	// Pull the plug on the database, simulating a hard crash
 	db.Close()
+	chain.stopWithoutSaving()
 
 	// Start a new blockchain back up and see where the repair leads us
 	db, err = rawdb.NewLevelDBDatabaseWithFreezer(datadir, 0, 0, datadir, "", false)
@@ -1940,6 +1941,7 @@ func TestIssue23496(t *testing.T) {
 
 	// Pull the plug on the database, simulating a hard crash
 	db.Close()
+	chain.stopWithoutSaving()
 
 	// Start a new blockchain back up and see where the repair leads us
 	db, err = rawdb.NewLevelDBDatabaseWithFreezer(datadir, 0, 0, datadir, "", false)

core/blockchain_sethead_test.go

@@ -1984,6 +1984,8 @@ func testSetHead(t *testing.T, tt *rewindTest, snapshots bool) {
 	if err != nil {
 		t.Fatalf("Failed to create chain: %v", err)
 	}
+	defer chain.Stop()
+
 	// If sidechain blocks are needed, make a light chain and import it
 	var sideblocks types.Blocks
 	if tt.sidechainBlocks > 0 {

core/blockchain_snapshot_test.go

@@ -247,6 +247,7 @@ func (snaptest *crashSnapshotTest) test(t *testing.T) {
 	// Pull the plug on the database, simulating a hard crash
 	db := chain.db
 	db.Close()
+	chain.stopWithoutSaving()
 
 	// Start a new blockchain back up and see where the repair leads us
 	newdb, err := rawdb.NewLevelDBDatabaseWithFreezer(snaptest.datadir, 0, 0, snaptest.datadir, "", false)
@@ -388,15 +389,19 @@ func (snaptest *wipeCrashSnapshotTest) test(t *testing.T) {
 		SnapshotLimit:  256,
 		SnapshotWait:   false, // Don't wait rebuild
 	}
-	_, err = NewBlockChain(snaptest.db, config, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil)
+	tmp, err := NewBlockChain(snaptest.db, config, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil)
 	if err != nil {
 		t.Fatalf("Failed to recreate chain: %v", err)
 	}
+
 	// Simulate the blockchain crash.
+	tmp.stopWithoutSaving()
+
 	newchain, err = NewBlockChain(snaptest.db, nil, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil)
 	if err != nil {
 		t.Fatalf("Failed to recreate chain: %v", err)
 	}
+	defer newchain.Stop()
 	snaptest.verify(t, newchain, blocks)
 }