Skip to content

Commit

Permalink
core, ethclient/gethclient: improve flaky tests (#25918)
Browse files Browse the repository at this point in the history 
* ethclient/gethclient: improve time-sensitive flaky test

* eth/catalyst: fix (?) flaky test

* core: stop blockchains in tests after use

* core: fix dangling blockchain instances

* core: rm whitespace

* eth/gasprice, eth/tracers, consensus/clique: stop dangling blockchains in tests

* all: address review concerns

* core: goimports

* eth/catalyst: fix another time-sensitive test

* consensus/clique: add snapshot test run function

* core: rename stop() to stopWithoutSaving()

Co-authored-by: Felix Lange <fjl@twurst.com>
  • Loading branch information
@holiman @fjl
holiman and fjl authored Oct 6, 2022
1 parent deead99 commit 067bac3
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 149 deletions.
231 changes: 117 additions & 114 deletions consensus/clique/snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package clique
import (
"bytes"
"crypto/ecdsa"
"fmt"
"math/big"
"sort"
"testing"
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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])
}
}
}
16 changes: 13 additions & 3 deletions core/blockchain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions core/blockchain_repair_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions core/blockchain_sethead_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion core/blockchain_snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit 067bac3

Please sign in to comment.