Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 0 additions & 120 deletions consensus/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,126 +316,6 @@ func TestReactorReceivePanicsIfInitPeerHasntBeenCalledYet(t *testing.T) {
})
}

// TestSwitchToConsensusVoteExtensions tests that the SwitchToConsensus correctly
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this test because it's only for vote extensions and we're not using them and they provoke panics and expect them to be recovered, but now we re-panic to stop the node.

// checks for vote extension data when required.
func TestSwitchToConsensusVoteExtensions(t *testing.T) {
for _, testCase := range []struct {
name string
storedHeight int64
initialRequiredHeight int64
includeExtensions bool
shouldPanic bool
}{
{
name: "no vote extensions but not required",
initialRequiredHeight: 0,
storedHeight: 2,
includeExtensions: false,
shouldPanic: false,
},
{
name: "no vote extensions but required this height",
initialRequiredHeight: 2,
storedHeight: 2,
includeExtensions: false,
shouldPanic: true,
},
{
name: "no vote extensions and required in future",
initialRequiredHeight: 3,
storedHeight: 2,
includeExtensions: false,
shouldPanic: false,
},
{
name: "no vote extensions and required previous height",
initialRequiredHeight: 1,
storedHeight: 2,
includeExtensions: false,
shouldPanic: true,
},
{
name: "vote extensions and required previous height",
initialRequiredHeight: 1,
storedHeight: 2,
includeExtensions: true,
shouldPanic: false,
},
} {
t.Run(testCase.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

cs, vs := randState(1)
validator := vs[0]
validator.Height = testCase.storedHeight

cs.state.LastBlockHeight = testCase.storedHeight
cs.state.LastValidators = cs.state.Validators.Copy()
cs.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.initialRequiredHeight

propBlock, blockParts, err := cs.createProposalBlock(ctx)
require.NoError(t, err)

// Consensus is preparing to do the next height after the stored height.
cs.rs.Height = testCase.storedHeight + 1
propBlock.Height = testCase.storedHeight

var voteSet *types.VoteSet
if testCase.includeExtensions {
voteSet = types.NewExtendedVoteSet(cs.state.ChainID, testCase.storedHeight, 0, cmtproto.PrecommitType, cs.state.Validators)
} else {
voteSet = types.NewVoteSet(cs.state.ChainID, testCase.storedHeight, 0, cmtproto.PrecommitType, cs.state.Validators)
}
signedVote := signVote(validator, cmtproto.PrecommitType, propBlock.Hash(), blockParts.Header(), testCase.includeExtensions)

var veHeight int64
if testCase.includeExtensions {
require.NotNil(t, signedVote.ExtensionSignature)
veHeight = testCase.storedHeight
} else {
require.Nil(t, signedVote.Extension)
require.Nil(t, signedVote.ExtensionSignature)
}

added, err := voteSet.AddVote(signedVote)
require.NoError(t, err)
require.True(t, added)

veHeightParam := types.ABCIParams{VoteExtensionsEnableHeight: veHeight}
if testCase.includeExtensions {
cs.blockStore.SaveBlockWithExtendedCommit(propBlock, blockParts, voteSet.MakeExtendedCommit(veHeightParam))
} else {
cs.blockStore.SaveBlock(propBlock, blockParts, voteSet.MakeExtendedCommit(veHeightParam).ToCommit())
}
blockDB := dbm.NewMemDB()
blockStore := store.NewBlockStore(blockDB)
key, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
require.NoError(t, err)
propagator := propagation.NewReactor(key.ID(), propagation.Config{
Store: blockStore,
Mempool: &emptyMempool{},
Privval: cs.privValidator,
ChainID: cs.state.ChainID,
BlockMaxBytes: cs.state.ConsensusParams.Block.MaxBytes,
})
reactor := NewReactor(
cs,
propagator,
true,
)

if testCase.shouldPanic {
assert.Panics(t, func() {
reactor.SwitchToConsensus(cs.state, false)
})
} else {
reactor.SwitchToConsensus(cs.state, false)
}
})
}
}

// Test we record stats about votes and block parts from other peers.
func TestReactorRecordsVotesAndBlockParts(t *testing.T) {
N := 4
Expand Down
1 change: 1 addition & 0 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@ func (cs *State) receiveRoutine(maxSteps int) {
// some console or secure RPC system, but for now, halting the chain upon
// unexpected consensus bugs sounds like the better option.
onExit(cs)
panic(r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol panicking in a recover function

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to re-propagate after we execute onExit() 😆

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are just going to panic, then does it even make sense to recover? Will this cleanly shut down everything. Isn't it possible with the multiplexer that there would be some dangling threads

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I'll check it more closely

}
}()

Expand Down
Loading