diff --git a/sei-ibc-go/modules/core/02-client/keeper/client_test.go b/sei-ibc-go/modules/core/02-client/keeper/client_test.go index 3baf146eea..ec1be02d10 100644 --- a/sei-ibc-go/modules/core/02-client/keeper/client_test.go +++ b/sei-ibc-go/modules/core/02-client/keeper/client_test.go @@ -463,7 +463,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) // Create signer array and ensure it is in same order as bothValSet - _, suiteVal := suite.valSet.GetByIndex(0) + _, suiteVal, ok := suite.valSet.GetByIndex(0) + suite.Require().True(ok) bothSigners := ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) altSigners := []tmtypes.PrivValidator{altPrivVal} diff --git a/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go b/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go index cd8aa1d596..c8f6f381ad 100644 --- a/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -28,7 +28,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { // Create alternative validator set with only altVal altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) - _, suiteVal := suite.valSet.GetByIndex(0) + _, suiteVal, ok := suite.valSet.GetByIndex(0) + suite.Require().True(ok) // Create signer array and ensure it is in same order as bothValSet bothSigners := ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) diff --git a/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_test.go b/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_test.go index d0bb91fcd3..9ab8b9ac12 100644 --- a/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_test.go +++ b/sei-ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_test.go @@ -44,7 +44,8 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { signers := []tmtypes.PrivValidator{suite.privVal} // Create signer array and ensure it is in same order as bothValSet - _, suiteVal := suite.valSet.GetByIndex(0) + _, suiteVal, ok := suite.valSet.GetByIndex(0) + suite.Require().True(ok) bothSigners := ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) altSigners := []tmtypes.PrivValidator{altPrivVal} diff --git a/sei-ibc-go/modules/light-clients/07-tendermint/types/tendermint_test.go b/sei-ibc-go/modules/light-clients/07-tendermint/types/tendermint_test.go index f1f7b004c2..0566c5996b 100644 --- a/sei-ibc-go/modules/light-clients/07-tendermint/types/tendermint_test.go +++ b/sei-ibc-go/modules/light-clients/07-tendermint/types/tendermint_test.go @@ -98,7 +98,8 @@ func getBothSigners(suite *TendermintTestSuite, altVal *tmtypes.Validator, altPr // Create bothValSet with both suite validator and altVal. Would be valid update bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) // Create signer array and ensure it is in same order as bothValSet - _, suiteVal := suite.valSet.GetByIndex(0) + _, suiteVal, ok := suite.valSet.GetByIndex(0) + suite.Require().True(ok) bothSigners := ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) return bothValSet, bothSigners } diff --git a/sei-ibc-go/modules/light-clients/07-tendermint/types/update_test.go b/sei-ibc-go/modules/light-clients/07-tendermint/types/update_test.go index 7cbe1d43ec..f9ac4f53fd 100644 --- a/sei-ibc-go/modules/light-clients/07-tendermint/types/update_test.go +++ b/sei-ibc-go/modules/light-clients/07-tendermint/types/update_test.go @@ -318,7 +318,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { signers = []tmtypes.PrivValidator{suite.privVal} // Create signer array and ensure it is in same order as bothValSet - _, suiteVal := suite.valSet.GetByIndex(0) + _, suiteVal, ok := suite.valSet.GetByIndex(0) + suite.Require().True(ok) bothSigners = ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) consStateHeight = height // must be explicitly changed diff --git a/sei-ibc-go/testing/chain.go b/sei-ibc-go/testing/chain.go index 097d66a0a2..b5b178b0a4 100644 --- a/sei-ibc-go/testing/chain.go +++ b/sei-ibc-go/testing/chain.go @@ -471,7 +471,7 @@ func (chain *TestChain) CreateTMClientHeader(chainID string, blockHeight int64, ChainID: chainID, Height: blockHeight, Time: timestamp, - LastBlockID: MakeBlockID(make([]byte, tmhash.Size), 10_000, make([]byte, tmhash.Size)), + LastBlockID: MakeBlockID(make([]byte, tmhash.Size), tmtypes.MaxBlockPartsCount, make([]byte, tmhash.Size)), LastCommitHash: chain.App.LastCommitID().Hash, DataHash: tmhash.Sum([]byte("data_hash")), ValidatorsHash: vsetHash, diff --git a/sei-tendermint/internal/consensus/common_test.go b/sei-tendermint/internal/consensus/common_test.go index 75ea68031e..49619f7b4d 100644 --- a/sei-tendermint/internal/consensus/common_test.go +++ b/sei-tendermint/internal/consensus/common_test.go @@ -301,8 +301,8 @@ func validatePrevote( address := pubKey.Address() - vote := prevotes.GetByAddress(address) - require.NotNil(t, vote, "Failed to find prevote from validator") + vote, ok := prevotes.GetByAddress(address) + require.True(t, ok, "Failed to find prevote from validator") if blockHash == nil { require.Nil(t, vote.BlockID.Hash, "Expected prevote to be for nil, got %X", vote.BlockID.Hash) @@ -319,8 +319,8 @@ func validateLastPrecommit(ctx context.Context, t *testing.T, cs *State, privVal require.NoError(t, err) address := pv.Address() - vote := votes.GetByAddress(address) - require.NotNil(t, vote) + vote, ok := votes.GetByAddress(address) + require.True(t, ok) require.True(t, bytes.Equal(vote.BlockID.Hash, blockHash), "Expected precommit to be for %X, got %X", blockHash, vote.BlockID.Hash) @@ -343,8 +343,8 @@ func validatePrecommit( require.NoError(t, err) address := pv.Address() - vote := precommits.GetByAddress(address) - require.NotNil(t, vote, "Failed to find precommit from validator") + vote, ok := precommits.GetByAddress(address) + require.True(t, ok, "Failed to find precommit from validator") if votedBlockHash == nil { require.Nil(t, vote.BlockID.Hash, "Expected precommit to be for nil") diff --git a/sei-tendermint/internal/consensus/invalid_test.go b/sei-tendermint/internal/consensus/invalid_test.go index 2fb4b4605a..f1ab4761d8 100644 --- a/sei-tendermint/internal/consensus/invalid_test.go +++ b/sei-tendermint/internal/consensus/invalid_test.go @@ -11,8 +11,10 @@ import ( "github.com/stretchr/testify/require" "github.com/sei-protocol/sei-chain/sei-tendermint/crypto" + cstypes "github.com/sei-protocol/sei-chain/sei-tendermint/internal/consensus/types" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/eventbus" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/bits" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/bytes" tmrand "github.com/sei-protocol/sei-chain/sei-tendermint/libs/rand" tmtime "github.com/sei-protocol/sei-chain/sei-tendermint/libs/time" @@ -20,8 +22,136 @@ import ( tmcons "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/consensus" tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" "github.com/sei-protocol/sei-chain/sei-tendermint/types" + "github.com/sei-protocol/sei-chain/sei-tendermint/version" ) +// Test checking that if peer sends a ProposalPOLMessage with a bitarray with bad length, +// the node will handle it gracefully. +func TestGossipVotesForHeightPoisonedProposalPOL(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + cfg := configSetup(t) + states, cleanup := makeConsensusState(ctx, t, cfg, 2, "consensus_reactor_test", newMockTickerFunc(true)) + t.Cleanup(cleanup) + + rts := setup(ctx, t, 2, states, 1) + + var nodeIDs []types.NodeID + for _, node := range rts.network.Nodes() { + nodeIDs = append(nodeIDs, node.NodeID) + } + require.Len(t, nodeIDs, 2) + + reactor := rts.reactors[nodeIDs[0]] + peerID := nodeIDs[1] + state := reactor.state.GetState() + reactor.SwitchToConsensus(ctx, state, false) + + require.Eventually(t, func() bool { + _, ok := reactor.GetPeerState(peerID) + return ok + }, time.Hour, 50*time.Millisecond) + + valSet, privVals := types.RandValidatorSet(4, 1) + proposerPubKey, err := privVals[0].GetPubKey(ctx) + require.NoError(t, err) + + proposal := types.NewProposal( + 1, + 1, + 0, + types.BlockID{ + Hash: crypto.CRandBytes(crypto.HashSize), + PartSetHeader: types.PartSetHeader{ + Total: 1, + Hash: crypto.CRandBytes(crypto.HashSize), + }, + }, + time.Now(), + nil, + types.Header{ + Version: version.Consensus{Block: version.BlockProtocol}, + Height: 1, + ProposerAddress: proposerPubKey.Address(), + }, + &types.Commit{}, + nil, + proposerPubKey.Address(), + ) + proposal.Signature = makeSig("invalid-signature") + + require.NoError(t, reactor.handleStateMessage(p2p.RecvMsg[*tmcons.Message]{ + From: peerID, + Message: MsgToProto(&NewRoundStepMessage{ + HRS: cstypes.HRS{ + Height: 1, + Round: 1, + Step: cstypes.RoundStepPrevote, + }, + SecondsSinceStartTime: 1, + LastCommitRound: -1, + }), + })) + + require.NoError(t, reactor.handleDataMessage(ctx, p2p.RecvMsg[*tmcons.Message]{ + From: peerID, + Message: MsgToProto(&ProposalMessage{ + Proposal: proposal, + }), + })) + + require.NoError(t, reactor.handleDataMessage(ctx, p2p.RecvMsg[*tmcons.Message]{ + From: peerID, + Message: MsgToProto(&ProposalPOLMessage{ + Height: 1, + ProposalPOLRound: 0, + ProposalPOL: bits.NewBitArray(1), + }), + })) + + ps, ok := reactor.GetPeerState(peerID) + require.True(t, ok) + prs := ps.GetRoundState() + require.Equal(t, int64(1), prs.Height) + require.Equal(t, int32(1), prs.Round) + require.Equal(t, int32(0), prs.ProposalPOLRound) + require.Equal(t, 1, prs.ProposalPOL.Size()) + + voteSet := cstypes.NewHeightVoteSet("test-chain", 1, valSet) + voteSet.SetRound(1) + + voter := newValidatorStub(privVals[1], 1) + voter.Height = 1 + voter.Round = 0 + + vote := signVote(ctx, t, voter, tmproto.PrevoteType, "test-chain", types.BlockID{ + Hash: crypto.CRandBytes(crypto.HashSize), + PartSetHeader: types.PartSetHeader{ + Total: 1, + Hash: crypto.CRandBytes(crypto.HashSize), + }, + }) + added, err := voteSet.AddVote(vote, "") + require.NoError(t, err) + require.True(t, added) + + rs := &cstypes.RoundState{ + HRS: cstypes.HRS{ + Height: 1, + Round: 1, + Step: cstypes.RoundStepPrevote, + }, + Votes: voteSet, + } + + // Gossip should take into consideration that PeerState might contain + // invalid length bitarrays. + for range 10 { + reactor.gossipVotesForHeight(rs, ps.GetRoundState(), ps) + } +} + func TestReactorInvalidPrecommit(t *testing.T) { t.Skip("test doesn't check anything useful") ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) @@ -128,7 +258,10 @@ func invalidDoPrevoteFunc( require.NoError(t, err) addr := pubKey.Address() - valIndex, _ := cs.roundState.Validators().GetByAddress(addr) + valIndex, _, ok := cs.roundState.Validators().GetByAddress(addr) + if !ok { + panic("missing validator") + } // precommit a random block blockHash := bytes.HexBytes(tmrand.Bytes(32)) diff --git a/sei-tendermint/internal/consensus/memory_limit_test.go b/sei-tendermint/internal/consensus/memory_limit_test.go index 7d65ac616c..908cdd1473 100644 --- a/sei-tendermint/internal/consensus/memory_limit_test.go +++ b/sei-tendermint/internal/consensus/memory_limit_test.go @@ -2,11 +2,9 @@ package consensus import ( "testing" - "time" "github.com/sei-protocol/sei-chain/sei-tendermint/crypto" "github.com/sei-protocol/sei-chain/sei-tendermint/crypto/ed25519" - tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" "github.com/sei-protocol/sei-chain/sei-tendermint/types" "github.com/stretchr/testify/require" ) @@ -32,40 +30,6 @@ func TestPeerStateMemoryLimits(t *testing.T) { {"very_large_total", 4294967295, true}, } - // Test SetHasProposal memory limits - t.Run("SetHasProposal", func(t *testing.T) { - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ps := NewPeerState(peerID) - ps.PRS.Height = 1 - ps.PRS.Round = 0 - blockID := types.BlockID{ - Hash: make([]byte, 32), - PartSetHeader: types.PartSetHeader{ - Total: tc.total, - Hash: make([]byte, 32), - }, - } - // Create a minimal proposal with basic required fields - proposal := &types.Proposal{ - Type: tmproto.ProposalType, - Height: 1, - Round: 0, - POLRound: -1, - BlockID: blockID, - Timestamp: time.Now(), - Signature: makeSig("test-signature"), - } - ps.SetHasProposal(proposal) - if tc.expectError { - require.False(t, ps.PRS.Proposal, "Expected proposal to be silently ignored for excessive Total") - } else { - require.True(t, ps.PRS.Proposal, "Expected proposal to be accepted for valid Total") - } - }) - } - }) - // Test InitProposalBlockParts memory limits t.Run("InitProposalBlockParts", func(t *testing.T) { for _, tc := range testCases { diff --git a/sei-tendermint/internal/consensus/msgs.go b/sei-tendermint/internal/consensus/msgs.go index df9b292e16..819dd26acd 100644 --- a/sei-tendermint/internal/consensus/msgs.go +++ b/sei-tendermint/internal/consensus/msgs.go @@ -131,7 +131,7 @@ func (m *ProposalMessage) String() string { return fmt.Sprintf("[Proposal %v]", m.Proposal) } -// ProposalPOLMessage is sent when a previous proposal is re-proposed. +// ProposalPOLMessage is sent when a node needs POL round votes. type ProposalPOLMessage struct { Height int64 ProposalPOLRound int32 diff --git a/sei-tendermint/internal/consensus/msgs_test.go b/sei-tendermint/internal/consensus/msgs_test.go index e923215c77..40e90d8960 100644 --- a/sei-tendermint/internal/consensus/msgs_test.go +++ b/sei-tendermint/internal/consensus/msgs_test.go @@ -65,8 +65,8 @@ func TestMsgToProto(t *testing.T) { proposal := types.Proposal{ Type: tmproto.ProposalType, - Height: 1, - Round: 1, + Height: 3, + Round: 2, POLRound: 1, BlockID: bi, Timestamp: time.Now(), diff --git a/sei-tendermint/internal/consensus/peer_state.go b/sei-tendermint/internal/consensus/peer_state.go index 5511b8b230..0cef20ce94 100644 --- a/sei-tendermint/internal/consensus/peer_state.go +++ b/sei-tendermint/internal/consensus/peer_state.go @@ -93,12 +93,6 @@ func (ps *PeerState) SetHasProposal(proposal *types.Proposal) { return } - // Check memory limits before acquiring lock or setting any state - if proposal.BlockID.PartSetHeader.Total > types.MaxBlockPartsCount { - logger.Debug("PartSetHeader.Total exceeds maximum", "total", proposal.BlockID.PartSetHeader.Total, "max", types.MaxBlockPartsCount) - return - } - ps.mtx.Lock() defer ps.mtx.Unlock() @@ -187,12 +181,8 @@ func (ps *PeerState) PickVoteToSend(votes types.VoteSetReader) (*types.Vote, boo } if index, ok := votes.BitArray().Sub(psVotes).PickRandom(); ok { - vote := votes.GetByIndex(int32(index)) //nolint:gosec // index is bounded by validator set size which fits in int32 - if vote != nil { - return vote, true - } + return votes.GetByIndex(int32(index)) //nolint:gosec // index is bounded by validator set size which fits in int32 } - return nil, false } diff --git a/sei-tendermint/internal/consensus/peer_state_test.go b/sei-tendermint/internal/consensus/peer_state_test.go index 6de04bad62..d4bb6c0814 100644 --- a/sei-tendermint/internal/consensus/peer_state_test.go +++ b/sei-tendermint/internal/consensus/peer_state_test.go @@ -125,26 +125,6 @@ func TestSetHasProposal(t *testing.T) { ps.SetHasProposal(invalidProposal) require.True(t, ps.PRS.Proposal, "Valid structure proposal should be accepted regardless of signature") - // Test PartSetHeader.Total too large - should be silently ignored - // Create a new peer state for this test - ps3 := peerStateSetup(1, 1, 1) - tooLargeTotalProposal := &types.Proposal{ - Type: tmproto.ProposalType, - Height: 1, - Round: 1, - POLRound: -1, - BlockID: types.BlockID{ - Hash: crypto.CRandBytes(crypto.HashSize), - PartSetHeader: types.PartSetHeader{ - Total: types.MaxBlockPartsCount + 1, // Too large - Hash: crypto.CRandBytes(crypto.HashSize), - }, - }, - Signature: makeSig("signature"), - } - ps3.SetHasProposal(tooLargeTotalProposal) - require.False(t, ps3.PRS.Proposal, "Proposal with too large Total should be silently ignored") - // Test valid proposal validProposal := &types.Proposal{ Type: tmproto.ProposalType, @@ -183,75 +163,6 @@ func TestSetHasProposal(t *testing.T) { require.True(t, ps2.PRS.Proposal, "Proposal with matching height should be accepted") } -func TestSetHasProposalMemoryLimit(t *testing.T) { - - peerID := types.NodeID("aa") - ps := NewPeerState(peerID) - - // Create a valid block hash - hash := crypto.CRandBytes(crypto.HashSize) - - // Create a dummy signature - sig := makeSig("dummy-sig") - - // Create a proposal with a large PartSetHeader.Total - proposal := &types.Proposal{ - Type: tmproto.ProposalType, - Height: 1, - Round: 0, - POLRound: -1, - BlockID: types.BlockID{ - Hash: hash, - PartSetHeader: types.PartSetHeader{ - Hash: hash, // Use same hash for simplicity - }, - }, - Timestamp: time.Now(), - Signature: sig, - } - - // Test with different Total values - testCases := []struct { - name string - total uint32 - expectError bool - errorType string // "max_block_parts" - }{ - {"valid small total", 1, false, ""}, - {"valid max total", types.MaxBlockPartsCount, false, ""}, // 101 - {"over max block parts", types.MaxBlockPartsCount + 1, true, "max_block_parts"}, // 102 - {"way over max block parts", 1000, true, "max_block_parts"}, // Way over max - {"DoS attack scenario - max uint32", 4294967295, true, "max_block_parts"}, // The actual DoS attack value - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - // Reset peer state and set height/round to match proposal - ps = NewPeerState(peerID) - ps.PRS.Height = proposal.Height - ps.PRS.Round = proposal.Round - - // Set up proposal with test case total - proposal.BlockID.PartSetHeader.Total = tc.total - - // Try to set the proposal - ps.SetHasProposal(proposal) - - if tc.expectError { - // Should be silently ignored, so no proposal should be set - require.False(t, ps.PRS.Proposal, "Proposal with excessive Total should be silently ignored") - require.Nil(t, ps.PRS.ProposalBlockParts, "ProposalBlockParts should remain nil") - } else { - // For valid cases, verify the proposal was accepted - require.True(t, ps.PRS.Proposal, "Valid proposal should be accepted") - require.NotNil(t, ps.PRS.ProposalBlockParts, "ProposalBlockParts should be created") - require.Equal(t, int(tc.total), ps.PRS.ProposalBlockParts.Size()) - require.NotNil(t, ps.PRS.ProposalBlockParts.Elems) - } - }) - } -} - func TestInitProposalBlockPartsMemoryLimit(t *testing.T) { peerID := types.NodeID("test-peer") @@ -329,30 +240,6 @@ func TestSetHasProposalEdgeCases(t *testing.T) { expectProposal bool expectPanic bool }{ - { - name: "memory limit exceeded - should silently ignore", - setupPeerState: func(ps *PeerState) { - ps.PRS.Height = 1 - ps.PRS.Round = 0 - }, - proposal: &types.Proposal{ - Type: tmproto.ProposalType, - Height: 1, - Round: 0, - POLRound: -1, - BlockID: types.BlockID{ - Hash: make([]byte, 32), - PartSetHeader: types.PartSetHeader{ - Total: types.MaxBlockPartsCount + 1, // Exceeds limit - Hash: make([]byte, 32), - }, - }, - Timestamp: time.Now(), - Signature: makeSig("test-signature"), - }, - expectProposal: false, // Should silently ignore - expectPanic: false, - }, { name: "wrong height - should ignore", setupPeerState: func(ps *PeerState) { diff --git a/sei-tendermint/internal/consensus/reactor.go b/sei-tendermint/internal/consensus/reactor.go index fed0876de8..7f499c75c1 100644 --- a/sei-tendermint/internal/consensus/reactor.go +++ b/sei-tendermint/internal/consensus/reactor.go @@ -468,12 +468,15 @@ func (r *Reactor) pickSendVote(ps *PeerState, votes types.VoteSetReader) bool { } logger.Debug("sending vote message", "ps", string(psJson), "vote", vote) } - r.channels.vote.Send(MsgToProto(&VoteMessage{Vote: vote}), ps.peerID) + // SetHasVote can fail because ps state (in particular the bitarrays) + // is not verified and it depends what peer has sent us. if err := ps.SetHasVote(vote); err != nil { - panic(fmt.Errorf("ps.SetHasVote(): %w", err)) + logger.Info("ps.SetHasVote()", "peerID", ps.peerID, "err", err) + return false } + r.channels.vote.Send(MsgToProto(&VoteMessage{Vote: vote}), ps.peerID) return true } diff --git a/sei-tendermint/internal/consensus/reactor_test.go b/sei-tendermint/internal/consensus/reactor_test.go index dee02cbea6..0fe5adfe83 100644 --- a/sei-tendermint/internal/consensus/reactor_test.go +++ b/sei-tendermint/internal/consensus/reactor_test.go @@ -30,7 +30,6 @@ import ( "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils/scope" tmcons "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/consensus" - tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" "github.com/sei-protocol/sei-chain/sei-tendermint/types" ) @@ -516,27 +515,6 @@ func TestReactorMemoryLimitCoverage(t *testing.T) { ps.PRS.Height = 1 ps.PRS.Round = 0 - // Create an invalid proposal with excessive PartSetHeader.Total - invalidProposal := &types.Proposal{ - Type: tmproto.ProposalType, - Height: 1, - Round: 0, - POLRound: -1, - BlockID: types.BlockID{ - Hash: make([]byte, 32), - PartSetHeader: types.PartSetHeader{ - Total: types.MaxBlockPartsCount + 1, // Exceeds limit - Hash: make([]byte, 32), - }, - }, - Timestamp: time.Now(), - Signature: makeSig("test-signature"), - } - - // Test direct SetHasProposal call (this is what reactor calls) - ps.SetHasProposal(invalidProposal) - require.False(t, ps.PRS.Proposal, "SetHasProposal should silently ignore proposal with excessive Total") - // Test that reactor would handle this silently by verifying the defensive programming approach // This provides coverage for the silent handling in handleDataMessage t.Log("Coverage test: reactor silently ignores invalid proposals via PeerState validation") diff --git a/sei-tendermint/internal/consensus/state.go b/sei-tendermint/internal/consensus/state.go index 1ae3bcde01..3f7723b178 100644 --- a/sei-tendermint/internal/consensus/state.go +++ b/sei-tendermint/internal/consensus/state.go @@ -34,7 +34,6 @@ import ( // Consensus sentinel errors var ( ErrInvalidProposalSignature = errors.New("error invalid proposal signature") - ErrInvalidProposalPOLRound = errors.New("error invalid proposal POL round") ErrAddingVote = errors.New("error adding vote") ErrSignatureFoundInPastBlocks = errors.New("found signature from the same key") ErrInvalidProposalPartSetHeader = errors.New("error invalid proposal part set header") @@ -2045,7 +2044,7 @@ func (cs *State) RecordMetrics(height int64, block *types.Block) { for _, ev := range block.Evidence { if dve, ok := ev.(*types.DuplicateVoteEvidence); ok { - if _, val := cs.roundState.Validators().GetByAddress(dve.VoteA.ValidatorAddress); val != nil { + if _, val, ok := cs.roundState.Validators().GetByAddress(dve.VoteA.ValidatorAddress); ok { byzantineValidatorsCount++ byzantineValidatorsPower += val.VotingPower } @@ -2103,7 +2102,10 @@ func (cs *State) defaultSetProposal(proposal *types.Proposal, recvTime time.Time if cs.roundState.Proposal() != nil || proposal == nil { return nil } - + // Preemptively re-verify the proposal. + if err := proposal.ValidateBasic(); err != nil { + return err + } // Does not apply if proposal.Height != cs.roundState.Height() || proposal.Round != cs.roundState.Round() { return nil @@ -2125,12 +2127,6 @@ func (cs *State) defaultSetProposal(proposal *types.Proposal, recvTime time.Time } } - // Verify POLRound, which must be -1 or in range [0, proposal.Round). - if proposal.POLRound < -1 || - (proposal.POLRound >= 0 && proposal.POLRound >= proposal.Round) { - return ErrInvalidProposalPOLRound - } - p := proposal.ToProto() // Verify signature if err := cs.roundState.Validators().GetProposer().PubKey.Verify( @@ -2491,7 +2487,10 @@ func (cs *State) addVote( } if vote.Round == cs.roundState.Round() { vals := cs.state.Validators - _, val := vals.GetByIndex(vote.ValidatorIndex) + _, val, ok := vals.GetByIndex(vote.ValidatorIndex) + if !ok { + panic(fmt.Errorf("validator index %v out of range", vote.ValidatorIndex)) + } cs.metrics.MarkVoteReceived(vote.Type, val.VotingPower, vals.TotalVotingPower()) } @@ -2619,8 +2618,10 @@ func (cs *State) signVote( } addr := privValidatorPubKey.Address() - valIdx, _ := cs.roundState.Validators().GetByAddress(addr) - + valIdx, _, ok := cs.roundState.Validators().GetByAddress(addr) + if !ok { + panic(fmt.Errorf("validator %v not in committee", addr)) + } vote := &types.Vote{ ValidatorAddress: addr, ValidatorIndex: valIdx, @@ -2748,7 +2749,10 @@ func (cs *State) calculatePrevoteMessageDelayMetrics() { var votingPowerSeen int64 for _, v := range pl { - _, val := cs.roundState.Validators().GetByAddress(v.ValidatorAddress) + _, val, ok := cs.roundState.Validators().GetByAddress(v.ValidatorAddress) + if !ok { + panic(fmt.Errorf("validator %v not in committee", v.ValidatorAddress)) + } votingPowerSeen += val.VotingPower if votingPowerSeen >= cs.roundState.Validators().TotalVotingPower()*2/3+1 { cs.metrics.QuorumPrevoteDelay.With("proposer_address", cs.roundState.Validators().GetProposer().Address.String()).Set(v.Timestamp.Sub(cs.roundState.Proposal().Timestamp).Seconds()) diff --git a/sei-tendermint/internal/consensus/types/peer_round_state.go b/sei-tendermint/internal/consensus/types/peer_round_state.go index c6dd26d5f4..d4a3ff0ef4 100644 --- a/sei-tendermint/internal/consensus/types/peer_round_state.go +++ b/sei-tendermint/internal/consensus/types/peer_round_state.go @@ -31,6 +31,7 @@ type PeerRoundState struct { StartTime time.Time // Estimated start of round 0 at this height + // WARNING: this only partially validated, so logic accessing it should be conservative. Proposal bool // True if peer has proposal for this round ProposalBlockPartSetHeader types.PartSetHeader ProposalBlockParts *bits.BitArray diff --git a/sei-tendermint/internal/consensus/types/round_state.go b/sei-tendermint/internal/consensus/types/round_state.go index 814f521b8a..0571b7e33f 100644 --- a/sei-tendermint/internal/consensus/types/round_state.go +++ b/sei-tendermint/internal/consensus/types/round_state.go @@ -400,7 +400,7 @@ type RoundStateSimple struct { Proposer types.ValidatorInfo `json:"proposer"` } -// Compress the RoundState to RoundStateSimple +// Compress the RoundState to RoundStateSimple. func (rs *RoundState) RoundStateSimple() RoundStateSimple { votesJSON, err := rs.Votes.MarshalJSON() if err != nil { @@ -408,7 +408,10 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple { } addr := rs.Validators.GetProposer().Address - idx, _ := rs.Validators.GetByAddress(addr) + idx, _, ok := rs.Validators.GetByAddress(addr) + if !ok { + panic(fmt.Errorf("validator %v not in committee", addr)) + } return RoundStateSimple{ HeightRoundStep: fmt.Sprintf("%d/%d/%d", rs.Height, rs.Round, rs.Step), @@ -427,8 +430,10 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple { // NewRoundEvent returns the RoundState with proposer information as an event. func (rs *RoundState) NewRoundEvent() types.EventDataNewRound { addr := rs.Validators.GetProposer().Address - idx, _ := rs.Validators.GetByAddress(addr) - + idx, _, ok := rs.Validators.GetByAddress(addr) + if !ok { + panic(fmt.Errorf("validator %v not in committee", addr)) + } return types.EventDataNewRound{ Height: rs.Height, Round: rs.Round, diff --git a/sei-tendermint/internal/evidence/pool_test.go b/sei-tendermint/internal/evidence/pool_test.go index 0a55e0d2f5..db09a46ef2 100644 --- a/sei-tendermint/internal/evidence/pool_test.go +++ b/sei-tendermint/internal/evidence/pool_test.go @@ -566,7 +566,7 @@ func initializeBlockStore(db dbm.DB, state sm.State, valAddr []byte) (*store.Blo block.Header.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute) block.Header.Version = version.Consensus{Block: version.BlockProtocol, App: 1} - const parts = 1 + const parts = types.BlockPartSizeBytes partSet, err := block.MakePartSet(parts) if err != nil { return nil, err diff --git a/sei-tendermint/internal/evidence/reactor_test.go b/sei-tendermint/internal/evidence/reactor_test.go index 1b761c70f2..f5e905686d 100644 --- a/sei-tendermint/internal/evidence/reactor_test.go +++ b/sei-tendermint/internal/evidence/reactor_test.go @@ -414,7 +414,7 @@ func TestEvidenceListSerialization(t *testing.T) { BlockID: types.BlockID{ Hash: crypto.Checksum([]byte("blockID_hash")), PartSetHeader: types.PartSetHeader{ - Total: 1000000, + Total: types.MaxBlockPartsCount, Hash: crypto.Checksum([]byte("blockID_part_set_header_hash")), }, }, @@ -444,7 +444,7 @@ func TestEvidenceListSerialization(t *testing.T) { }{ "DuplicateVoteEvidence": { []types.Evidence{dupl}, - "0a85020a82020a79080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0b08b1d381d20510809dca6f32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb031279080110031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0b08b1d381d20510809dca6f32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb03180a200a2a060880dbaae105", + "0a81020afe010a7708021003180222480a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc8012240865122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0b08b1d381d20510809dca6f32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb03127708011003180222480a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc8012240865122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0b08b1d381d20510809dca6f32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb03180a200a2a060880dbaae105", }, } diff --git a/sei-tendermint/internal/evidence/verify.go b/sei-tendermint/internal/evidence/verify.go index 2f13a710e2..d09163f3a1 100644 --- a/sei-tendermint/internal/evidence/verify.go +++ b/sei-tendermint/internal/evidence/verify.go @@ -71,8 +71,10 @@ func (evpool *Pool) verify(ctx context.Context, evidence types.Evidence) error { return types.NewErrInvalidEvidence(evidence, err) } - _, val := valSet.GetByAddress(ev.VoteA.ValidatorAddress) - + _, val, ok := valSet.GetByAddress(ev.VoteA.ValidatorAddress) + if !ok { + return fmt.Errorf("validator %v not in committee", ev.VoteA.ValidatorAddress) + } if err := ev.ValidateABCI(val, valSet, evTime); err != nil { ev.GenerateABCI(val, valSet, evTime) if addErr := evpool.addPendingEvidence(ctx, ev); addErr != nil { @@ -202,8 +204,8 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t // - the block ID's must be different // - The signatures must both be valid func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet *types.ValidatorSet) error { - _, val := valSet.GetByAddress(e.VoteA.ValidatorAddress) - if val == nil { + _, val, ok := valSet.GetByAddress(e.VoteA.ValidatorAddress) + if !ok { return fmt.Errorf("address %X was not a validator at height %d", e.VoteA.ValidatorAddress, e.Height()) } pubKey := val.PubKey diff --git a/sei-tendermint/internal/evidence/verify_test.go b/sei-tendermint/internal/evidence/verify_test.go index 00b6ec5a7b..d818449d65 100644 --- a/sei-tendermint/internal/evidence/verify_test.go +++ b/sei-tendermint/internal/evidence/verify_test.go @@ -242,7 +242,7 @@ func TestVerifyLightClientAttack_Equivocation(t *testing.T) { Timestamp: defaultEvidenceTime, } - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) + trustedBlockID := makeBlockID(trustedHeader.Hash(), types.MaxBlockPartsCount, []byte("partshash")) trustedVoteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals) trustedCommit, err := factory.MakeCommit(ctx, trustedBlockID, 10, 1, trustedVoteSet, conflictingPrivVals, defaultEvidenceTime) @@ -321,7 +321,7 @@ func TestVerifyLightClientAttack_Amnesia(t *testing.T) { // we are simulating an amnesia attack where all the validators in the conflictingVals set // except the last validator vote twice. However this time the commits are of different rounds. - blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) + blockID := makeBlockID(conflictingHeader.Hash(), types.MaxBlockPartsCount, []byte("partshash")) voteSet := types.NewVoteSet(evidenceChainID, height, 0, tmproto.SignedMsgType(2), conflictingVals) commit, err := factory.MakeCommit(ctx, blockID, height, 0, voteSet, conflictingPrivVals, defaultEvidenceTime) require.NoError(t, err) @@ -340,7 +340,7 @@ func TestVerifyLightClientAttack_Amnesia(t *testing.T) { Timestamp: defaultEvidenceTime, } - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) + trustedBlockID := makeBlockID(trustedHeader.Hash(), types.MaxBlockPartsCount, []byte("partshash")) trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) trustedCommit, err := factory.MakeCommit(ctx, trustedBlockID, height, 1, trustedVoteSet, conflictingPrivVals, defaultEvidenceTime) @@ -397,10 +397,10 @@ func TestVerifyDuplicateVoteEvidence(t *testing.T) { val2 := types.NewMockPV() valSet := types.NewValidatorSet([]*types.Validator{val.ExtractIntoValidator(ctx, 1)}) - blockID := makeBlockID([]byte("blockhash"), 1000, []byte("partshash")) - blockID2 := makeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) - blockID3 := makeBlockID([]byte("blockhash"), 10000, []byte("partshash")) - blockID4 := makeBlockID([]byte("blockhash"), 10000, []byte("partshash2")) + blockID := makeBlockID([]byte("blockhash"), types.MaxBlockPartsCount, []byte("partshash")) + blockID2 := makeBlockID([]byte("blockhash2"), types.MaxBlockPartsCount, []byte("partshash")) + blockID3 := makeBlockID([]byte("blockhash"), types.MaxBlockPartsCount-1, []byte("partshash")) + blockID4 := makeBlockID([]byte("blockhash"), types.MaxBlockPartsCount, []byte("partshash2")) const chainID = "mychain" diff --git a/sei-tendermint/internal/rpc/core/status.go b/sei-tendermint/internal/rpc/core/status.go index c996aea45c..4e479900af 100644 --- a/sei-tendermint/internal/rpc/core/status.go +++ b/sei-tendermint/internal/rpc/core/status.go @@ -126,8 +126,8 @@ func (env *Environment) validatorAtHeight(h int64) utils.Option[*types.Validator } } - _, val := valsWithH.GetByAddress(privValAddress) - if val != nil { + _, val, ok := valsWithH.GetByAddress(privValAddress) + if ok { return utils.Some(val) } return none diff --git a/sei-tendermint/internal/state/execution_test.go b/sei-tendermint/internal/state/execution_test.go index 99d34f9f1f..aaa924fc35 100644 --- a/sei-tendermint/internal/state/execution_test.go +++ b/sei-tendermint/internal/state/execution_test.go @@ -151,7 +151,7 @@ func TestFinalizeBlockByzantineValidators(t *testing.T) { defaultEvidenceTime := time.Date(2019, 1, 1, 0, 0, 0, 0, time.UTC) privVal := privVals[state.Validators.Validators[0].Address.String()] - blockID := makeBlockID([]byte("headerhash"), 1000, []byte("partshash")) + blockID := makeBlockID([]byte("headerhash"), types.MaxBlockPartsCount, []byte("partshash")) header := &types.Header{ Version: version.Consensus{Block: version.BlockProtocol, App: 1}, ChainID: state.ChainID, @@ -179,7 +179,7 @@ func TestFinalizeBlockByzantineValidators(t *testing.T) { Header: header, Commit: &types.Commit{ Height: 10, - BlockID: makeBlockID(header.Hash(), 100, []byte("partshash")), + BlockID: makeBlockID(header.Hash(), types.MaxBlockPartsCount, []byte("partshash")), Signatures: []types.CommitSig{{ BlockIDFlag: types.BlockIDFlagNil, ValidatorAddress: crypto.AddressHash([]byte("validator_address")), @@ -509,8 +509,7 @@ func TestFinalizeBlockValidatorUpdates(t *testing.T) { require.NoError(t, err) // test new validator was added to NextValidators if assert.Equal(t, state.Validators.Size()+1, state.NextValidators.Size()) { - idx, _ := state.NextValidators.GetByAddress(pubkey.Address()) - if idx < 0 { + if _, _, ok := state.NextValidators.GetByAddress(pubkey.Address()); !ok { t.Fatalf("can't find address %v in the set %v", pubkey.Address(), state.NextValidators) } } diff --git a/sei-tendermint/internal/state/helpers_test.go b/sei-tendermint/internal/state/helpers_test.go index 1b4349761d..302c239bf9 100644 --- a/sei-tendermint/internal/state/helpers_test.go +++ b/sei-tendermint/internal/state/helpers_test.go @@ -84,7 +84,10 @@ func makeValidCommit( sigs := make([]types.CommitSig, vals.Size()) votes := make([]*types.Vote, vals.Size()) for i := 0; i < vals.Size(); i++ { - _, val := vals.GetByIndex(int32(i)) + _, val, ok := vals.GetByIndex(int32(i)) + if !ok { + panic("validator missing") + } vote, err := factory.MakeVote(ctx, privVals[val.Address.String()], chainID, int32(i), height, 0, 2, blockID, time.Now()) require.NoError(t, err) sigs[i] = vote.CommitSig() @@ -153,7 +156,10 @@ func makeHeaderPartsResponsesValPubKeyChange( block := sf.MakeBlock(state, state.LastBlockHeight+1, new(types.Commit)) finalizeBlockResponses := &abci.ResponseFinalizeBlock{} // If the pubkey is new, remove the old and add the new. - _, val := state.NextValidators.GetByIndex(0) + _, val, ok := state.NextValidators.GetByIndex(0) + if !ok { + panic("validator missing") + } if pubkey != val.PubKey { vPbPk := crypto.PubKeyToProto(val.PubKey) pbPk := crypto.PubKeyToProto(pubkey) @@ -178,7 +184,10 @@ func makeHeaderPartsResponsesValPowerChange( finalizeBlockResponses := &abci.ResponseFinalizeBlock{} // If the pubkey is new, remove the old and add the new. - _, val := state.NextValidators.GetByIndex(0) + _, val, ok := state.NextValidators.GetByIndex(0) + if !ok { + panic("validator missing") + } if val.VotingPower != power { vPbPk := crypto.PubKeyToProto(val.PubKey) diff --git a/sei-tendermint/internal/state/rollback_test.go b/sei-tendermint/internal/state/rollback_test.go index a5e3f8abc4..dd94e21fc2 100644 --- a/sei-tendermint/internal/state/rollback_test.go +++ b/sei-tendermint/internal/state/rollback_test.go @@ -286,7 +286,7 @@ func makeBlockIDRandom() types.BlockID { return types.BlockID{ Hash: blockHash, PartSetHeader: types.PartSetHeader{ - Total: 123, + Total: types.MaxBlockPartsCount, Hash: partSetHash, }, } diff --git a/sei-tendermint/internal/state/state_test.go b/sei-tendermint/internal/state/state_test.go index 21080f0bd3..03e2f74b0f 100644 --- a/sei-tendermint/internal/state/state_test.go +++ b/sei-tendermint/internal/state/state_test.go @@ -265,7 +265,8 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) { // with the right validator set for each height. highestHeight := changeHeights[N-1] + 5 changeIndex := 0 - _, val := state.Validators.GetByIndex(0) + _, val, ok := state.Validators.GetByIndex(0) + require.True(t, ok) power := val.VotingPower var err error var validatorUpdates []*types.Validator @@ -305,7 +306,8 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) { v, err := stateStore.LoadValidators(int64(i + 1 + 1)) // +1 because vset changes delayed by 1 block. assert.NoError(t, err, fmt.Sprintf("expected no err at height %d", i)) assert.Equal(t, v.Size(), 1, "validator set size is greater than 1: %d", v.Size()) - _, val := v.GetByIndex(0) + _, val, ok := v.GetByIndex(0) + require.True(t, ok) assert.Equal(t, val.VotingPower, power, fmt.Sprintf(`unexpected powerat height %d`, i)) @@ -413,14 +415,20 @@ func testProposerFreq(t *testing.T, caseNum int, valSet *types.ValidatorSet) { freqs := make([]int, N) for range runs { prop := valSet.GetProposer() - idx, _ := valSet.GetByAddress(prop.Address) + idx, _, ok := valSet.GetByAddress(prop.Address) + if !ok { + panic("validator missing") + } freqs[idx]++ valSet.IncrementProposerPriority(1) } // assert frequencies match expected (max off by 1) for i, freq := range freqs { - _, val := valSet.GetByIndex(int32(i)) + _, val, ok := valSet.GetByIndex(int32(i)) + if !ok { + panic("validator missing") + } expectFreq := int(val.VotingPower) * runMult gotFreq := freq abs := int(math.Abs(float64(expectFreq - gotFreq))) @@ -486,8 +494,10 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) { assert.NoError(t, err) require.Equal(t, len(updatedState2.NextValidators.Validators), 2) - _, updatedVal1 := updatedState2.NextValidators.GetByAddress(val1PubKey.Address()) - _, addedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address()) + _, updatedVal1, ok := updatedState2.NextValidators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, addedVal2, ok := updatedState2.NextValidators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) // adding a validator should not lead to a ProposerPriority equal to zero (unless the combination of averaging and // incrementing would cause so; which is not the case here) @@ -528,10 +538,14 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) { assert.NoError(t, err) require.Equal(t, len(updatedState3.NextValidators.Validators), 2) - _, prevVal1 := updatedState3.Validators.GetByAddress(val1PubKey.Address()) - _, prevVal2 := updatedState3.Validators.GetByAddress(val2PubKey.Address()) - _, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address()) - _, updatedVal2 := updatedState3.NextValidators.GetByAddress(val2PubKey.Address()) + _, prevVal1, ok := updatedState3.Validators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, prevVal2, ok := updatedState3.Validators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) + _, updatedVal1, ok = updatedState3.NextValidators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, updatedVal2, ok := updatedState3.NextValidators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) // 2. Scale // old prios: v1(10):-38, v2(1):39 @@ -621,9 +635,12 @@ func TestProposerPriorityProposerAlternates(t *testing.T) { assert.Equal(t, updatedState2.Validators.Proposer.Address, val1PubKey.Address()) assert.Equal(t, updatedState2.NextValidators.Proposer.Address, val1PubKey.Address()) - _, updatedVal1 := updatedState2.NextValidators.GetByAddress(val1PubKey.Address()) - _, oldVal1 := updatedState2.Validators.GetByAddress(val1PubKey.Address()) - _, updatedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address()) + _, updatedVal1, ok := updatedState2.NextValidators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, oldVal1, ok := updatedState2.Validators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, updatedVal2, ok := updatedState2.NextValidators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) // 1. Add val2VotingPower := val1VotingPower @@ -661,8 +678,10 @@ func TestProposerPriorityProposerAlternates(t *testing.T) { assert.Equal(t, updatedState3.Validators.Proposer.Address, updatedState3.NextValidators.Proposer.Address) assert.Equal(t, updatedState3.Validators, updatedState2.NextValidators) - _, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address()) - _, updatedVal2 = updatedState3.NextValidators.GetByAddress(val2PubKey.Address()) + _, updatedVal1, ok = updatedState3.NextValidators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, updatedVal2, ok = updatedState3.NextValidators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) // val1 will still be proposer: assert.Equal(t, val1PubKey.Address(), updatedState3.NextValidators.Proposer.Address) @@ -730,8 +749,10 @@ func TestProposerPriorityProposerAlternates(t *testing.T) { ) assert.Equal(t, oldState.Validators.Proposer.Address, updatedState.NextValidators.Proposer.Address, "iter: %v", i) - _, updatedVal1 = updatedState.NextValidators.GetByAddress(val1PubKey.Address()) - _, updatedVal2 = updatedState.NextValidators.GetByAddress(val2PubKey.Address()) + _, updatedVal1, ok = updatedState.NextValidators.GetByAddress(val1PubKey.Address()) + require.True(t, ok) + _, updatedVal2, ok = updatedState.NextValidators.GetByAddress(val2PubKey.Address()) + require.True(t, ok) if i%2 == 0 { assert.Equal(t, updatedState.Validators.Proposer.Address, val2PubKey.Address()) @@ -847,10 +868,14 @@ func TestLargeGenesisValidator(t *testing.T) { // set oldState to state before above iteration oldState = updatedState - _, oldGenesisVal := oldState.NextValidators.GetByAddress(genesisVal.Address) - _, newGenesisVal := state.NextValidators.GetByAddress(genesisVal.Address) - _, addedOldVal := oldState.NextValidators.GetByAddress(firstAddedValPubKey.Address()) - _, addedNewVal := state.NextValidators.GetByAddress(firstAddedValPubKey.Address()) + _, oldGenesisVal, ok := oldState.NextValidators.GetByAddress(genesisVal.Address) + require.True(t, ok) + _, newGenesisVal, ok := state.NextValidators.GetByAddress(genesisVal.Address) + require.True(t, ok) + _, addedOldVal, ok := oldState.NextValidators.GetByAddress(firstAddedValPubKey.Address()) + require.True(t, ok) + _, addedNewVal, ok := state.NextValidators.GetByAddress(firstAddedValPubKey.Address()) + require.True(t, ok) // expect large negative proposer priority for both (genesis validator decreased, 2nd validator increased): assert.True(t, oldGenesisVal.ProposerPriority > newGenesisVal.ProposerPriority) assert.True(t, addedOldVal.ProposerPriority < addedNewVal.ProposerPriority) @@ -1003,7 +1028,8 @@ func TestManyValidatorChangesSaveLoad(t *testing.T) { err := stateStore.Save(state) require.NoError(t, err) - _, valOld := state.Validators.GetByIndex(0) + _, valOld, ok := state.Validators.GetByIndex(0) + require.True(t, ok) var pubkeyOld = valOld.PubKey pubkey := ed25519.GenerateSecretKey().Public() @@ -1027,21 +1053,15 @@ func TestManyValidatorChangesSaveLoad(t *testing.T) { v0, err := stateStore.LoadValidators(nextHeight) assert.NoError(t, err) assert.Equal(t, valSetSize, v0.Size()) - index, val := v0.GetByAddress(pubkeyOld.Address()) - assert.NotNil(t, val) - if index < 0 { - t.Fatal("expected to find old validator") - } + _, _, ok = v0.GetByAddress(pubkeyOld.Address()) + require.True(t, ok) // Load nextheight+1, it should be the new pubkey. v1, err := stateStore.LoadValidators(nextHeight + 1) assert.NoError(t, err) assert.Equal(t, valSetSize, v1.Size()) - index, val = v1.GetByAddress(pubkey.Address()) - assert.NotNil(t, val) - if index < 0 { - t.Fatal("expected to find newly added validator") - } + _, _, ok = v1.GetByAddress(pubkey.Address()) + require.True(t, ok) } func TestStateMakeBlock(t *testing.T) { diff --git a/sei-tendermint/internal/store/store_test.go b/sei-tendermint/internal/store/store_test.go index 17132e7bcf..e6acb41b50 100644 --- a/sei-tendermint/internal/store/store_test.go +++ b/sei-tendermint/internal/store/store_test.go @@ -68,6 +68,26 @@ func newInMemoryBlockStore() (*BlockStore, dbm.DB) { return NewBlockStore(db), db } +// makeValidMultiPartSet chooses a small enough part size to keep the block +// split across multiple parts without exceeding types.MaxBlockPartsCount. +func makeValidMultiPartSet(t *testing.T, block *types.Block) *types.PartSet { + t.Helper() + + blockSize := block.Size() + partSize := uint32(1) + if blockSize > 1 { + partSize = uint32((blockSize + int(types.MaxBlockPartsCount) - 1) / int(types.MaxBlockPartsCount)) + if partSize >= uint32(blockSize) { + partSize = uint32(blockSize - 1) + } + } + + partSet, err := block.MakePartSet(partSize) + require.NoError(t, err) + require.LessOrEqual(t, partSet.Total(), uint32(types.MaxBlockPartsCount)) + return partSet +} + // TODO: This test should be simplified ... func TestBlockStoreSaveLoadBlock(t *testing.T) { state, bs, cleanup, err := makeStateAndBlockStore(t.TempDir()) @@ -86,8 +106,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) { // save a block block := factory.MakeBlock(state, bs.Height()+1, new(types.Commit)) - validPartSet, err := block.MakePartSet(2) - require.NoError(t, err) + validPartSet := makeValidMultiPartSet(t, block) part2 := validPartSet.GetPart(1) seenCommit := makeTestCommit(block.Header.Height, tmtime.Now()) @@ -292,8 +311,7 @@ func TestLoadBaseMeta(t *testing.T) { for h := int64(1); h <= 10; h++ { block := factory.MakeBlock(state, h, new(types.Commit)) - partSet, err := block.MakePartSet(2) - require.NoError(t, err) + partSet := makeValidMultiPartSet(t, block) seenCommit := makeTestCommit(h, tmtime.Now()) bs.SaveBlock(block, partSet, seenCommit) } @@ -339,8 +357,7 @@ func TestLoadBlockPart(t *testing.T) { // 3. A good block serialized and saved to the DB should be retrievable block := factory.MakeBlock(state, height, new(types.Commit)) - partSet, err := block.MakePartSet(2) - require.NoError(t, err) + partSet := makeValidMultiPartSet(t, block) part1 := partSet.GetPart(0) require.NoError(t, db.Set(blockPartKey(height, index), mustEncode(part1.ToProto()))) @@ -370,8 +387,7 @@ func TestPruneBlocks(t *testing.T) { // make more than 1000 blocks, to test batch deletions for h := int64(1); h <= 1500; h++ { block := factory.MakeBlock(state, h, new(types.Commit)) - partSet, err := block.MakePartSet(2) - require.NoError(t, err) + partSet := makeValidMultiPartSet(t, block) seenCommit := makeTestCommit(h, tmtime.Now()) bs.SaveBlock(block, partSet, seenCommit) } @@ -478,8 +494,7 @@ func TestBlockFetchAtHeight(t *testing.T) { require.Equal(t, bs.Height(), int64(0), "initially the height should be zero") block := factory.MakeBlock(state, bs.Height()+1, new(types.Commit)) - partSet, err := block.MakePartSet(2) - require.NoError(t, err) + partSet := makeValidMultiPartSet(t, block) seenCommit := makeTestCommit(block.Header.Height, tmtime.Now()) bs.SaveBlock(block, partSet, seenCommit) require.Equal(t, bs.Height(), block.Header.Height, "expecting the new height to be changed") @@ -522,8 +537,7 @@ func TestSeenAndCanonicalCommit(t *testing.T) { for h := int64(3); h <= 5; h++ { blockCommit := makeTestCommit(h-1, tmtime.Now()) block := factory.MakeBlock(state, h, blockCommit) - partSet, err := block.MakePartSet(2) - require.NoError(t, err) + partSet := makeValidMultiPartSet(t, block) seenCommit := makeTestCommit(h, tmtime.Now()) store.SaveBlock(block, partSet, seenCommit) c3 := store.LoadSeenCommit() diff --git a/sei-tendermint/light/helpers_test.go b/sei-tendermint/light/helpers_test.go index 6b778d3308..468d65c318 100644 --- a/sei-tendermint/light/helpers_test.go +++ b/sei-tendermint/light/helpers_test.go @@ -84,7 +84,10 @@ func makeVote(t testing.TB, header *types.Header, valset *types.ValidatorSet, ke t.Helper() addr := key.Public().Address() - idx, _ := valset.GetByAddress(addr) + idx, _, ok := valset.GetByAddress(addr) + if !ok { + panic("validator missing") + } vote := &types.Vote{ ValidatorAddress: addr, ValidatorIndex: idx, diff --git a/sei-tendermint/node/node_test.go b/sei-tendermint/node/node_test.go index 606fbeba33..22401fe5c1 100644 --- a/sei-tendermint/node/node_test.go +++ b/sei-tendermint/node/node_test.go @@ -282,8 +282,8 @@ func TestCreateProposalBlock(t *testing.T) { maxEvidenceBytes := int64(maxBytes / 2) state.ConsensusParams.Block.MaxBytes = int64(maxBytes) state.ConsensusParams.Evidence.MaxBytes = maxEvidenceBytes - proposerAddr, _ := state.Validators.GetByIndex(0) - + proposerAddr, _, ok := state.Validators.GetByIndex(0) + require.True(t, ok) mp := mempool.NewTxMempool( cfg.Mempool, app, @@ -375,7 +375,8 @@ func TestMaxTxsProposalBlockSize(t *testing.T) { const maxBytes int64 = 16384 const partSize uint32 = 256 state.ConsensusParams.Block.MaxBytes = maxBytes - proposerAddr, _ := state.Validators.GetByIndex(0) + proposerAddr, _, ok := state.Validators.GetByIndex(0) + require.True(t, ok) // Make Mempool @@ -439,7 +440,8 @@ func TestMaxProposalBlockSize(t *testing.T) { blockStore := store.NewBlockStore(dbm.NewMemDB()) const maxBytes int64 = 1024 * 1024 * 2 state.ConsensusParams.Block.MaxBytes = maxBytes - proposerAddr, _ := state.Validators.GetByIndex(0) + proposerAddr, _, ok := state.Validators.GetByIndex(0) + require.True(t, ok) // Make Mempool mp := mempool.NewTxMempool( @@ -476,7 +478,7 @@ func TestMaxProposalBlockSize(t *testing.T) { blockID := types.BlockID{ Hash: crypto.Checksum([]byte("blockID_hash")), PartSetHeader: types.PartSetHeader{ - Total: math.MaxInt32, + Total: types.MaxBlockPartsCount, Hash: crypto.Checksum([]byte("blockID_part_set_header_hash")), }, } @@ -508,8 +510,8 @@ func TestMaxProposalBlockSize(t *testing.T) { for i := 0; i < types.MaxVotesCount; i++ { pubKey, err := privVals[i].GetPubKey(ctx) require.NoError(t, err) - valIdx, val := state.Validators.GetByAddress(pubKey.Address()) - require.NotNil(t, val) + valIdx, val, ok := state.Validators.GetByAddress(pubKey.Address()) + require.True(t, ok) vote := &types.Vote{ Type: tmproto.PrecommitType, @@ -546,11 +548,11 @@ func TestMaxProposalBlockSize(t *testing.T) { pb, err := block.ToProto() require.NoError(t, err) - // require that the header and commit be the max possible size - require.Equal(t, int64(pb.Header.Size()), types.MaxHeaderBytes) - require.Equal(t, int64(pb.LastCommit.Size()), types.MaxCommitBytes(types.MaxVotesCount)) + // The encoded header must stay within the documented maximum. + require.LessOrEqual(t, int64(pb.Header.Size()), types.MaxHeaderBytes) + require.LessOrEqual(t, int64(pb.LastCommit.Size()), types.MaxCommitBytes(types.MaxVotesCount)) // make sure that the block is less than the max possible size - assert.Equal(t, int64(pb.Size()), maxBytes) + assert.LessOrEqual(t, int64(pb.Size()), maxBytes) // because of the proto overhead we expect the part set bytes to be equal or // less than the pb block size assert.LessOrEqual(t, partSet.ByteSize(), int64(pb.Size())) @@ -685,7 +687,7 @@ func state(t *testing.T, nVals int, height int64) (sm.State, dbm.DB, []types.Pri t.Helper() privVals := make([]types.PrivValidator, nVals) vals := make([]types.GenesisValidator, nVals) - for i := 0; i < nVals; i++ { + for i := range nVals { privVal := types.NewMockPV() privVals[i] = privVal vals[i] = types.GenesisValidator{ diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index c745ff38ca..8451383957 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -135,11 +135,11 @@ func onlyValidatorIsUs(state sm.State, pubKey utils.Option[crypto.PubKey]) bool if !ok { return false } - if state.Validators.Size() > 1 { + if state.Validators.Size() != 1 { return false } - addr, _ := state.Validators.GetByIndex(0) - return bytes.Equal(k.Address(), addr) + addr, _, ok := state.Validators.GetByIndex(0) + return ok && bytes.Equal(k.Address(), addr) } func createMempoolReactor( diff --git a/sei-tendermint/rpc/client/evidence_test.go b/sei-tendermint/rpc/client/evidence_test.go index 3e949cc915..3edb0b695a 100644 --- a/sei-tendermint/rpc/client/evidence_test.go +++ b/sei-tendermint/rpc/client/evidence_test.go @@ -55,7 +55,7 @@ func makeEvidences( BlockID: types.BlockID{ Hash: crypto.Checksum(tmrand.Bytes(crypto.HashSize)), PartSetHeader: types.PartSetHeader{ - Total: 1000, + Total: types.MaxBlockPartsCount, Hash: crypto.Checksum([]byte("partset")), }, }, diff --git a/sei-tendermint/test/e2e/runner/evidence.go b/sei-tendermint/test/e2e/runner/evidence.go index e7882bba67..4d146a3bf4 100644 --- a/sei-tendermint/test/e2e/runner/evidence.go +++ b/sei-tendermint/test/e2e/runner/evidence.go @@ -227,12 +227,12 @@ func generateDuplicateVoteEvidence( func getRandomValidatorIndex(privVals []types.MockPV, vals *types.ValidatorSet) (types.MockPV, int32, error) { for _, idx := range rand.Perm(len(privVals)) { pv := privVals[idx] - valIdx, _ := vals.GetByAddress(pv.PrivKey.Public().Address()) - if valIdx >= 0 { + valIdx, _, ok := vals.GetByAddress(pv.PrivKey.Public().Address()) + if ok { return pv, valIdx, nil } } - return types.MockPV{}, -1, errors.New("no private validator found in validator set") + return types.MockPV{}, 0, errors.New("no private validator found in validator set") } func readPrivKey(keyFilePath string) (crypto.PrivKey, error) { diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index 212154836b..fd3baec252 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -808,7 +808,10 @@ type Commit struct { // signature will not be present in the returned vote. // Returns nil if the precommit at valIdx is nil. // Panics if valIdx >= commit.Size(). -func (commit *Commit) GetVote(valIdx int32) *Vote { +func (commit *Commit) GetVote(valIdx int32) (*Vote, bool) { + if int(valIdx) >= len(commit.Signatures) { + return nil, false + } commitSig := commit.Signatures[valIdx] return &Vote{ Type: tmproto.PrecommitType, @@ -819,7 +822,7 @@ func (commit *Commit) GetVote(valIdx int32) *Vote { ValidatorAddress: commitSig.ValidatorAddress, ValidatorIndex: valIdx, Signature: commitSig.Signature, - } + }, true } // VoteSignBytes returns the bytes of the Vote corresponding to valIdx for @@ -831,9 +834,12 @@ func (commit *Commit) GetVote(valIdx int32) *Vote { // Panics if valIdx >= commit.Size(). // // See VoteSignBytes -func (commit *Commit) VoteSignBytes(chainID string, valIdx int32) []byte { - v := commit.GetVote(valIdx).ToProto() - return VoteSignBytes(chainID, v) +func (commit *Commit) VoteSignBytes(chainID string, valIdx int32) ([]byte, bool) { + v, ok := commit.GetVote(valIdx) + if !ok { + return nil, false + } + return VoteSignBytes(chainID, v.ToProto()), true } // Size returns the number of signatures in the commit. @@ -991,7 +997,10 @@ func (commit *Commit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet { if cs.BlockIDFlag == BlockIDFlagAbsent { continue // OK, some precommits can be missing. } - vote := commit.GetVote(int32(idx)) //nolint:gosec // idx is bounded by commit.Signatures length which fits in int32 + vote, ok := commit.GetVote(int32(idx)) //nolint:gosec // idx is bounded by commit.Signatures length which fits in int32 + if !ok { + panic(fmt.Errorf("too many signatures")) + } if err := vote.ValidateBasic(); err != nil { panic(fmt.Errorf("failed to validate vote reconstructed from commit: %w", err)) } @@ -1033,7 +1042,7 @@ func (ec *Commit) BitArray() *bits.BitArray { // GetByIndex returns the vote corresponding to a given validator index. // Panics if `index >= extCommit.Size()`. // Implements VoteSetReader. -func (ec *Commit) GetByIndex(valIdx int32) *Vote { +func (ec *Commit) GetByIndex(valIdx int32) (*Vote, bool) { return ec.GetVote(valIdx) } diff --git a/sei-tendermint/types/block_meta_test.go b/sei-tendermint/types/block_meta_test.go index 8ef931bed3..db32dc6bbb 100644 --- a/sei-tendermint/types/block_meta_test.go +++ b/sei-tendermint/types/block_meta_test.go @@ -11,7 +11,7 @@ import ( func TestBlockMeta_ToProto(t *testing.T) { h := MakeRandHeader() - bi := BlockID{Hash: h.Hash(), PartSetHeader: PartSetHeader{Total: 123, Hash: tmrand.Bytes(crypto.HashSize)}} + bi := BlockID{Hash: h.Hash(), PartSetHeader: PartSetHeader{Total: MaxBlockPartsCount, Hash: tmrand.Bytes(crypto.HashSize)}} bm := &BlockMeta{ BlockID: bi, @@ -48,11 +48,11 @@ func TestBlockMeta_ToProto(t *testing.T) { func TestBlockMeta_ValidateBasic(t *testing.T) { h := MakeRandHeader() - bi := BlockID{Hash: h.Hash(), PartSetHeader: PartSetHeader{Total: 123, Hash: tmrand.Bytes(crypto.HashSize)}} + bi := BlockID{Hash: h.Hash(), PartSetHeader: PartSetHeader{Total: MaxBlockPartsCount, Hash: tmrand.Bytes(crypto.HashSize)}} bi2 := BlockID{Hash: tmrand.Bytes(crypto.HashSize), - PartSetHeader: PartSetHeader{Total: 123, Hash: tmrand.Bytes(crypto.HashSize)}} + PartSetHeader: PartSetHeader{Total: MaxBlockPartsCount, Hash: tmrand.Bytes(crypto.HashSize)}} bi3 := BlockID{Hash: []byte("incorrect hash"), - PartSetHeader: PartSetHeader{Total: 123, Hash: []byte("incorrect hash")}} + PartSetHeader: PartSetHeader{Total: MaxBlockPartsCount, Hash: []byte("incorrect hash")}} bm := &BlockMeta{ BlockID: bi, diff --git a/sei-tendermint/types/block_test.go b/sei-tendermint/types/block_test.go index fa04eb697d..c72397ddf1 100644 --- a/sei-tendermint/types/block_test.go +++ b/sei-tendermint/types/block_test.go @@ -216,7 +216,7 @@ func makeBlockIDRandom() BlockID { ) rand.Read(blockHash) //nolint: errcheck // ignore errcheck for read rand.Read(partSetHash) //nolint: errcheck // ignore errcheck for read - return BlockID{blockHash, PartSetHeader{123, partSetHash}} + return BlockID{blockHash, PartSetHeader{MaxBlockPartsCount, partSetHash}} } func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) BlockID { @@ -270,8 +270,11 @@ func TestCommit(t *testing.T) { require.NotNil(t, commit.BitArray()) assert.Equal(t, bits.NewBitArray(10).Size(), commit.BitArray().Size()) - - assert.Equal(t, voteSet.GetByIndex(0), commit.GetByIndex(0)) + vsv, ok := voteSet.GetByIndex(0) + require.True(t, ok) + cv, ok := commit.GetByIndex(0) + require.True(t, ok) + assert.Equal(t, vsv, cv) assert.True(t, commit.IsCommit()) } @@ -665,8 +668,10 @@ func TestVoteSetToCommit(t *testing.T) { ec := voteSet.MakeCommit() for i := int32(0); int(i) < len(vals); i++ { - vote1 := voteSet.GetByIndex(i) - vote2 := ec.GetVote(i) + vote1, ok := voteSet.GetByIndex(i) + require.True(t, ok) + vote2, ok := ec.GetVote(i) + require.True(t, ok) vote1bz, err := vote1.ToProto().Marshal() require.NoError(t, err) @@ -694,9 +699,12 @@ func TestCommitToVoteSet(t *testing.T) { voteSet2 := commit.ToVoteSet(chainID, valSet) for i := int32(0); int(i) < len(vals); i++ { - vote1 := voteSet.GetByIndex(i) - vote2 := voteSet2.GetByIndex(i) - vote3 := commit.GetVote(i) + vote1, ok := voteSet.GetByIndex(i) + require.True(t, ok) + vote2, ok := voteSet2.GetByIndex(i) + require.True(t, ok) + vote3, ok := commit.GetVote(i) + require.True(t, ok) vote1bz, err := vote1.ToProto().Marshal() require.NoError(t, err) @@ -710,7 +718,7 @@ func TestCommitToVoteSet(t *testing.T) { } func TestCommitToVoteSetWithVotesForNilBlock(t *testing.T) { - blockID := makeBlockID([]byte("blockhash"), 1000, []byte("partshash")) + blockID := makeBlockID([]byte("blockhash"), MaxBlockPartsCount, []byte("partshash")) const ( height = int64(3) diff --git a/sei-tendermint/types/evidence.go b/sei-tendermint/types/evidence.go index d78c0b206c..c20e329c53 100644 --- a/sei-tendermint/types/evidence.go +++ b/sei-tendermint/types/evidence.go @@ -106,8 +106,8 @@ func NewDuplicateVoteEvidence(vote1, vote2 *Vote, blockTime time.Time, valSet *V if valSet == nil { return nil, errors.New("missing validator set") } - idx, val := valSet.GetByAddress(vote1.ValidatorAddress) - if idx == -1 { + _, val, ok := valSet.GetByAddress(vote1.ValidatorAddress) + if !ok { return nil, errors.New("validator not in validator set") } @@ -354,8 +354,8 @@ func (l *LightClientAttackEvidence) GetByzantineValidators(commonVals *Validator continue } - _, val := commonVals.GetByAddress(commitSig.ValidatorAddress) - if val == nil { + _, val, ok := commonVals.GetByAddress(commitSig.ValidatorAddress) + if !ok { // validator wasn't in the common validator set continue } @@ -379,7 +379,10 @@ func (l *LightClientAttackEvidence) GetByzantineValidators(commonVals *Validator continue } - _, val := l.ConflictingBlock.ValidatorSet.GetByAddress(sigA.ValidatorAddress) + _, val, ok := l.ConflictingBlock.ValidatorSet.GetByAddress(sigA.ValidatorAddress) + if !ok { + panic(fmt.Errorf("validator %v not in committee", sigA.ValidatorAddress)) + } validators = append(validators, val) } sort.Sort(ValidatorsByVotingPower(validators)) diff --git a/sei-tendermint/types/evidence_test.go b/sei-tendermint/types/evidence_test.go index 5c3671dff4..ea90e667c6 100644 --- a/sei-tendermint/types/evidence_test.go +++ b/sei-tendermint/types/evidence_test.go @@ -72,8 +72,8 @@ func TestEvidenceListProtoBuf(t *testing.T) { func randomDuplicateVoteEvidence(ctx context.Context, t *testing.T) *DuplicateVoteEvidence { t.Helper() val := NewMockPV() - blockID := makeBlockID([]byte("blockhash"), 1000, []byte("partshash")) - blockID2 := makeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) + blockID := makeBlockID([]byte("blockhash"), MaxBlockPartsCount, []byte("partshash")) + blockID2 := makeBlockID([]byte("blockhash2"), MaxBlockPartsCount, []byte("partshash")) const chainID = "mychain" return &DuplicateVoteEvidence{ VoteA: NewEncodedVote(makeVote(ctx, t, val, chainID, 0, 10, 2, 1, blockID, defaultVoteTime)), @@ -97,8 +97,8 @@ func TestDuplicateVoteEvidence(t *testing.T) { func TestDuplicateVoteEvidenceValidation(t *testing.T) { val := NewMockPV() - blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) - blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) + blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) const chainID = "mychain" ctx := t.Context() @@ -147,7 +147,7 @@ func TestLightClientAttackEvidenceBasic(t *testing.T) { header := makeHeaderRandom() header.Height = height - blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) commit, err := MakeCommit(ctx, blockID, height, 1, voteSet, privVals, defaultVoteTime) require.NoError(t, err) @@ -211,7 +211,7 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { header := makeHeaderRandom() header.Height = height header.ValidatorsHash = valSet.Hash() - blockID := makeBlockID(header.Hash(), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(header.Hash(), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) commit, err := MakeCommit(ctx, blockID, height, 1, voteSet, privVals, time.Now()) require.NoError(t, err) @@ -342,8 +342,8 @@ func TestEvidenceProto(t *testing.T) { // -------- Votes -------- val := NewMockPV() - blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) - blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) + blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) const chainID = "mychain" v := NewEncodedVote(makeVote(ctx, t, val, chainID, math.MaxInt32, math.MaxInt64, 1, 0x01, blockID, defaultVoteTime)) v2 := NewEncodedVote(makeVote(ctx, t, val, chainID, math.MaxInt32, math.MaxInt64, 2, 0x01, blockID2, defaultVoteTime)) @@ -386,8 +386,8 @@ func TestEvidenceVectors(t *testing.T) { val := NewMockPV() // WARNING: this key has to be stable, otherwise hashes break. val.PrivKey = ed25519.TestSecretKey([]byte("it's a secret")) - blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) - blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) + blockID2 := makeBlockID(crypto.Checksum([]byte("blockhash2")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) const chainID = "mychain" v := NewEncodedVote(makeVote(ctx, t, val, chainID, math.MaxInt32, math.MaxInt64, 1, 0x01, blockID, defaultVoteTime)) v2 := NewEncodedVote(makeVote(ctx, t, val, chainID, math.MaxInt32, math.MaxInt64, 2, 0x01, blockID2, defaultVoteTime)) @@ -415,7 +415,7 @@ func TestEvidenceVectors(t *testing.T) { EvidenceHash: []byte("f2564c78071e26643ae9b3e2a19fa0dc10d4d9e873aa0be808660123f11a1e78"), ProposerAddress: []byte("2915b7b15f979e48ebc61774bb1d86ba3136b7eb"), } - blockID3 := makeBlockID(header.Hash(), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID3 := makeBlockID(header.Hash(), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) commit, err := MakeCommit(ctx, blockID3, height, 1, voteSet, privVals, defaultVoteTime) require.NoError(t, err) lcae := &LightClientAttackEvidence{ @@ -440,15 +440,15 @@ func TestEvidenceVectors(t *testing.T) { }{ {"duplicateVoteEvidence", EvidenceList{&DuplicateVoteEvidence{VoteA: v2, VoteB: v}}, - "a9ce28d13bb31001fc3e5b7927051baf98f86abdbd64377643a304164c826923", + "92ebbd40f4ab47e1736d7d89efdf88caa54e199eaf755dfe076590095e4f384e", }, {"LightClientAttackEvidence", EvidenceList{lcae}, - "2f8782163c3905b26e65823ababc977fe54e97b94e60c0360b1e4726b668bb8e", + "c99409ac7170ef13763f185e0e288f496f90549212cf91d5d37b1853090af7f2", }, {"LightClientAttackEvidence & DuplicateVoteEvidence", EvidenceList{&DuplicateVoteEvidence{VoteA: v2, VoteB: v}, lcae}, - "eedb4b47d6dbc9d43f53da8aa50bb826e8d9fc7d897da777c8af6a04aa74163e", + "5fa180fc78714a2a744479acb0bcbf72bba68608e41c31cba86a864ea97d149e", }, } diff --git a/sei-tendermint/types/part_set.go b/sei-tendermint/types/part_set.go index bed233c5b6..d77355943a 100644 --- a/sei-tendermint/types/part_set.go +++ b/sei-tendermint/types/part_set.go @@ -115,6 +115,10 @@ func (psh PartSetHeader) ValidateBasic() error { if err := ValidateHash(psh.Hash); err != nil { return fmt.Errorf("wrong Hash: %w", err) } + // Check memory limits before acquiring lock or setting any state + if psh.Total > MaxBlockPartsCount { + return fmt.Errorf("Total = %v, want <=%v", psh.Total, MaxBlockPartsCount) + } return nil } @@ -172,7 +176,7 @@ func NewPartSetFromData(data []byte, partSize uint32) *PartSet { parts := make([]*Part, total) partsBytes := make([][]byte, total) partsBitArray := bits.NewBitArray(int(total)) //nolint:gosec // total fits in int since it's derived from block-bounded data - for i := uint32(0); i < total; i++ { + for i := range total { part := &Part{ Index: i, Bytes: data[i*partSize : tmmath.MinInt(len(data), int((i+1)*partSize))], //nolint:gosec // partSize is small (4KB); product fits in uint32 diff --git a/sei-tendermint/types/proposal.go b/sei-tendermint/types/proposal.go index e6275bd36d..e766ebb564 100644 --- a/sei-tendermint/types/proposal.go +++ b/sei-tendermint/types/proposal.go @@ -68,8 +68,8 @@ func (p *Proposal) ValidateBasic() error { if p.Round < 0 { return errors.New("negative Round") } - if p.POLRound < -1 { - return errors.New("negative POLRound (exception: -1)") + if p.POLRound < -1 || p.POLRound >= p.Round { + return fmt.Errorf("invalid POLRound: got %v, want [-1,%v)", p.POLRound, p.Round) } if err := p.BlockID.ValidateBasic(); err != nil { return fmt.Errorf("wrong BlockID: %w", err) diff --git a/sei-tendermint/types/proposal_test.go b/sei-tendermint/types/proposal_test.go index 8ac0804974..7760ec5371 100644 --- a/sei-tendermint/types/proposal_test.go +++ b/sei-tendermint/types/proposal_test.go @@ -1,7 +1,6 @@ package types import ( - "math" "testing" "time" @@ -51,7 +50,7 @@ func getTestProposal(t testing.TB) *Proposal { Height: 12345, Round: 23456, BlockID: BlockID{Hash: []byte("--June_15_2020_amino_was_removed"), - PartSetHeader: PartSetHeader{Total: 111, Hash: []byte("--June_15_2020_amino_was_removed")}}, + PartSetHeader: PartSetHeader{Total: MaxBlockPartsCount, Hash: []byte("--June_15_2020_amino_was_removed")}}, POLRound: -1, Timestamp: stamp, } @@ -69,7 +68,7 @@ func TestProposalSignable(t *testing.T) { func TestProposalString(t *testing.T) { str := getTestProposal(t).String() - expected := `Proposal{12345/23456 (2D2D4A756E655F31355F323032305F616D696E6F5F7761735F72656D6F766564:111:2D2D4A756E65, -1) 000000000000 @ 2018-02-11T07:09:22.765Z}` + expected := `Proposal{12345/23456 (2D2D4A756E655F31355F323032305F616D696E6F5F7761735F72656D6F766564:101:2D2D4A756E65, -1) 000000000000 @ 2018-02-11T07:09:22.765Z}` if str != expected { t.Errorf("got unexpected string for Proposal. Expected:\n%v\nGot:\n%v", expected, str) } @@ -84,8 +83,8 @@ func TestProposalVerifySignature(t *testing.T) { txKeys := make([]TxKey, 0) prop := NewProposal( - 4, 2, 2, - BlockID{tmrand.Bytes(crypto.HashSize), PartSetHeader{777, tmrand.Bytes(crypto.HashSize)}}, tmtime.Now(), txKeys, generateHeader(), &Commit{}, EvidenceList{}, pubKey.Address()) + 4, 2, 1, + BlockID{tmrand.Bytes(crypto.HashSize), PartSetHeader{MaxBlockPartsCount, tmrand.Bytes(crypto.HashSize)}}, tmtime.Now(), txKeys, generateHeader(), &Commit{}, EvidenceList{}, pubKey.Address()) p := prop.ToProto() signBytes := ProposalSignBytes("test_chain_id", p) @@ -169,7 +168,7 @@ func TestProposalValidateBasic(t *testing.T) { p.BlockID = BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}} }, true}, } - blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) + blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), MaxBlockPartsCount, crypto.Checksum([]byte("partshash"))) for _, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { @@ -179,7 +178,7 @@ func TestProposalValidateBasic(t *testing.T) { pubKey, err := privVal.GetPubKey(ctx) require.NoError(t, err) prop := NewProposal( - 4, 2, 2, + 4, 2, 1, blockID, tmtime.Now(), txKeys, generateHeader(), &Commit{}, EvidenceList{}, pubKey.Address()) p := prop.ToProto() @@ -194,7 +193,7 @@ func TestProposalValidateBasic(t *testing.T) { func TestProposalProtoBuf(t *testing.T) { var txKeys []TxKey - proposal := NewProposal(1, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) + proposal := NewProposal(1, 3, 2, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) proposal.Signature = testKey.Sign([]byte("sig")) proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) diff --git a/sei-tendermint/types/validation.go b/sei-tendermint/types/validation.go index c76627fe9e..f22dc353ce 100644 --- a/sei-tendermint/types/validation.go +++ b/sei-tendermint/types/validation.go @@ -191,11 +191,12 @@ func verifyCommitBatch( return fmt.Errorf("commit.Signatures[%v].ValidatorAddress = %v, want %v", idx, commitSig.ValidatorAddress, val.Address) } } else { - valIdx, val = vals.GetByAddress(commitSig.ValidatorAddress) + var ok bool + valIdx, val, ok = vals.GetByAddress(commitSig.ValidatorAddress) // if the signature doesn't belong to anyone in the validator set // then we just skip over it - if val == nil { + if !ok { continue } @@ -209,7 +210,10 @@ func verifyCommitBatch( } // Validate signature. - voteSignBytes := commit.VoteSignBytes(chainID, int32(idx)) //nolint:gosec // idx is bounded by len(commit.Signatures) which is validated against validator set size + voteSignBytes, ok := commit.VoteSignBytes(chainID, int32(idx)) //nolint:gosec // idx is bounded by len(commit.Signatures) which is validated against validator set size + if !ok { + panic("VoteSignBytes() failed unexpectedly") + } // add the key, sig and message to the verifier sig, ok := commitSig.Signature.Get() @@ -272,7 +276,6 @@ func verifyCommitSingle( val *Validator valIdx int32 talliedVotingPower int64 - voteSignBytes []byte seenVals = make(map[int32]int, len(commit.Signatures)) ) for idx, commitSig := range commit.Signatures { @@ -288,11 +291,12 @@ func verifyCommitSingle( return fmt.Errorf("commit.Signatures[%v].ValidatorAddress = %v, want %v", idx, commitSig.ValidatorAddress, val.Address) } } else { - valIdx, val = vals.GetByAddress(commitSig.ValidatorAddress) + var ok bool + valIdx, val, ok = vals.GetByAddress(commitSig.ValidatorAddress) // if the signature doesn't belong to anyone in the validator set // then we just skip over it - if val == nil { + if !ok { continue } @@ -305,7 +309,10 @@ func verifyCommitSingle( seenVals[valIdx] = idx } - voteSignBytes = commit.VoteSignBytes(chainID, int32(idx)) //nolint:gosec // idx is bounded by len(commit.Signatures) which is validated against validator set size + voteSignBytes, ok := commit.VoteSignBytes(chainID, int32(idx)) //nolint:gosec // idx is bounded by len(commit.Signatures) which is validated against validator set size + if !ok { + panic("VoteSignBytes() failed unexpectedly") + } sig, ok := commitSig.Signature.Get() if !ok { return fmt.Errorf("missing signature at idx %v", idx) diff --git a/sei-tendermint/types/validation_test.go b/sei-tendermint/types/validation_test.go index d45f90b898..5e3c335a67 100644 --- a/sei-tendermint/types/validation_test.go +++ b/sei-tendermint/types/validation_test.go @@ -46,7 +46,7 @@ func TestValidatorSet_VerifyCommit_All(t *testing.T) { round = int32(0) height = int64(100) - blockID = makeBlockID([]byte("blockhash"), 1000, []byte("partshash")) + blockID = makeBlockID([]byte("blockhash"), MaxBlockPartsCount, []byte("partshash")) chainID = "Lalande21185" trustLevel = tmmath.Fraction{Numerator: 2, Denominator: 3} ) @@ -199,7 +199,8 @@ func TestValidatorSet_VerifyCommit_CheckAllSignatures(t *testing.T) { require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit)) // malleate 4th signature - vote := voteSet.GetByIndex(3) + vote, ok := voteSet.GetByIndex(3) + require.True(t, ok) v := vote.ToProto() require.NoError(t, vals[3].SignVote(ctx, "CentaurusA", v)) vote.Signature = utils.Some(utils.OrPanic1(crypto.SigFromBytes(v.Signature))) @@ -227,7 +228,8 @@ func TestValidatorSet_VerifyCommitLight_ReturnsAsSoonAsMajorityOfVotingPowerSign require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit)) // malleate 4th signature (3 signatures are enough for 2/3+) - vote := voteSet.GetByIndex(3) + vote, ok := voteSet.GetByIndex(3) + require.True(t, ok) v := vote.ToProto() require.NoError(t, vals[3].SignVote(ctx, "CentaurusA", v)) vote.Signature = utils.Some(utils.OrPanic1(crypto.SigFromBytes(v.Signature))) @@ -252,7 +254,8 @@ func TestValidatorSet_VerifyCommitLightTrusting_ReturnsAsSoonAsTrustLevelOfVotin require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit)) // malleate 3rd signature (2 signatures are enough for 1/3+ trust level) - vote := voteSet.GetByIndex(2) + vote, ok := voteSet.GetByIndex(2) + require.True(t, ok) v := vote.ToProto() require.NoError(t, vals[2].SignVote(ctx, "CentaurusA", v)) vote.Signature = utils.Some(utils.OrPanic1(crypto.SigFromBytes(v.Signature))) diff --git a/sei-tendermint/types/validator_set.go b/sei-tendermint/types/validator_set.go index 3318bcd14e..8a84e21107 100644 --- a/sei-tendermint/types/validator_set.go +++ b/sei-tendermint/types/validator_set.go @@ -277,25 +277,25 @@ func (vals *ValidatorSet) HasAddress(address []byte) bool { // GetByAddress returns an index of the validator with address and validator // itself (copy) if found. Otherwise, -1 and nil are returned. -func (vals *ValidatorSet) GetByAddress(address []byte) (index int32, val *Validator) { +func (vals *ValidatorSet) GetByAddress(address []byte) (index int32, val *Validator, ok bool) { for idx, val := range vals.Validators { if bytes.Equal(val.Address, address) { - return int32(idx), val.Copy() //nolint:gosec // validator set size is consensus-bounded, fits in int32 + return int32(idx), val.Copy(), true //nolint:gosec // validator set size is consensus-bounded, fits in int32 } } - return -1, nil + return 0, nil, false } // GetByIndex returns the validator's address and validator itself (copy) by // index. // It returns nil values if index is less than 0 or greater or equal to // len(ValidatorSet.Validators). -func (vals *ValidatorSet) GetByIndex(index int32) (address []byte, val *Validator) { +func (vals *ValidatorSet) GetByIndex(index int32) (address []byte, val *Validator, ok bool) { if index < 0 || int(index) >= len(vals.Validators) { - return nil, nil + return nil, nil, false } val = vals.Validators[index] - return val.Address, val.Copy() + return val.Address, val.Copy(), true } // Size returns the length of the validator set. @@ -458,8 +458,8 @@ func verifyUpdates( ) (tvpAfterUpdatesBeforeRemovals int64, err error) { delta := func(update *Validator, vals *ValidatorSet) int64 { - _, val := vals.GetByAddress(update.Address) - if val != nil { + _, val, ok := vals.GetByAddress(update.Address) + if ok { return update.VotingPower - val.VotingPower } return update.VotingPower @@ -505,8 +505,8 @@ func numNewValidators(updates []*Validator, vals *ValidatorSet) int { func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotalVotingPower int64) { for _, valUpdate := range updates { address := valUpdate.Address - _, val := vals.GetByAddress(address) - if val == nil { + _, val, ok := vals.GetByAddress(address) + if !ok { // add val // Set ProposerPriority to -C*totalVotingPower (with C ~= 1.125) to make sure validators can't // un-bond and then re-bond to reset their (potentially previously negative) ProposerPriority to zero. @@ -570,8 +570,8 @@ func verifyRemovals(deletes []*Validator, vals *ValidatorSet) (votingPower int64 removedVotingPower := int64(0) for _, valUpdate := range deletes { address := valUpdate.Address - _, val := vals.GetByAddress(address) - if val == nil { + _, val, ok := vals.GetByAddress(address) + if !ok { return removedVotingPower, fmt.Errorf("failed to find validator %X to remove", address) } removedVotingPower += val.VotingPower @@ -974,7 +974,7 @@ func RandValidatorSet(numValidators int, votingPower int64) (*ValidatorSet, []Pr privValidators = make([]PrivValidator, numValidators) ) - for i := 0; i < numValidators; i++ { + for i := range numValidators { val, privValidator := RandValidator(false, votingPower) valz[i] = val privValidators[i] = privValidator diff --git a/sei-tendermint/types/validator_set_test.go b/sei-tendermint/types/validator_set_test.go index d573f68570..98e271421d 100644 --- a/sei-tendermint/types/validator_set_test.go +++ b/sei-tendermint/types/validator_set_test.go @@ -34,18 +34,14 @@ func TestValidatorSetBasic(t *testing.T) { assert.EqualValues(t, vset, vset.Copy()) assert.False(t, vset.HasAddress([]byte("some val"))) - idx, val := vset.GetByAddress([]byte("some val")) - assert.EqualValues(t, -1, idx) - assert.Nil(t, val) - addr, val := vset.GetByIndex(-100) - assert.Nil(t, addr) - assert.Nil(t, val) - addr, val = vset.GetByIndex(0) - assert.Nil(t, addr) - assert.Nil(t, val) - addr, val = vset.GetByIndex(100) - assert.Nil(t, addr) - assert.Nil(t, val) + idx, val, ok := vset.GetByAddress([]byte("some val")) + require.False(t, ok) + addr, val, ok := vset.GetByIndex(-100) + require.False(t, ok) + addr, val, ok = vset.GetByIndex(0) + require.False(t, ok) + addr, val, ok = vset.GetByIndex(100) + require.False(t, ok) assert.Zero(t, vset.Size()) assert.Equal(t, int64(0), vset.TotalVotingPower()) assert.Nil(t, vset.GetProposer()) @@ -57,9 +53,11 @@ func TestValidatorSetBasic(t *testing.T) { assert.NoError(t, vset.UpdateWithChangeSet([]*Validator{val})) assert.True(t, vset.HasAddress(val.Address)) - idx, _ = vset.GetByAddress(val.Address) + idx, _, ok = vset.GetByAddress(val.Address) + assert.True(t, ok) assert.EqualValues(t, 0, idx) - addr, _ = vset.GetByIndex(0) + addr, _, ok = vset.GetByIndex(0) + assert.True(t, ok) assert.Equal(t, []byte(val.Address), addr) assert.Equal(t, 1, vset.Size()) assert.Equal(t, val.VotingPower, vset.TotalVotingPower()) @@ -70,15 +68,16 @@ func TestValidatorSetBasic(t *testing.T) { // update val = randModuloValidator(vset.TotalVotingPower()) assert.NoError(t, vset.UpdateWithChangeSet([]*Validator{val})) - _, val = vset.GetByAddress(val.Address) + _, val, ok = vset.GetByAddress(val.Address) + assert.True(t, ok) val.VotingPower += 100 proposerPriority := val.ProposerPriority val.ProposerPriority = 0 assert.NoError(t, vset.UpdateWithChangeSet([]*Validator{val})) - _, val = vset.GetByAddress(val.Address) + _, val, ok = vset.GetByAddress(val.Address) + assert.True(t, ok) assert.Equal(t, proposerPriority, val.ProposerPriority) - } func TestValidatorSetValidateBasic(t *testing.T) { @@ -534,7 +533,8 @@ func TestAveragingInIncrementProposerPriority(t *testing.T) { // work on copy to have the old ProposerPriorities: newVset := tc.vs.CopyIncrementProposerPriority(tc.times) for _, val := range tc.vs.Validators { - _, updatedVal := newVset.GetByAddress(val.Address) + _, updatedVal, ok := newVset.GetByAddress(val.Address) + assert.True(t, ok) assert.Equal(t, updatedVal.ProposerPriority, val.ProposerPriority-tc.avg, "test case: %v", i) } } diff --git a/sei-tendermint/types/vote_set.go b/sei-tendermint/types/vote_set.go index 6afa940d8b..9c8bf297bc 100644 --- a/sei-tendermint/types/vote_set.go +++ b/sei-tendermint/types/vote_set.go @@ -166,8 +166,8 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) { } // Ensure that signer is a validator. - lookupAddr, val := voteSet.valSet.GetByIndex(valIndex) - if val == nil { + lookupAddr, val, ok := voteSet.valSet.GetByIndex(valIndex) + if !ok { return false, fmt.Errorf( "cannot find validator %d in valSet of size %d: %w", valIndex, voteSet.valSet.Size(), ErrVoteInvalidValidatorIndex) @@ -356,16 +356,19 @@ func (voteSet *VoteSet) BitArrayByBlockID(blockID BlockID) *bits.BitArray { // NOTE: if validator has conflicting votes, returns "canonical" vote // Implements VoteSetReader. -func (voteSet *VoteSet) GetByIndex(valIndex int32) *Vote { +func (voteSet *VoteSet) GetByIndex(valIndex int32) (*Vote, bool) { if voteSet == nil { - return nil + return nil, false } voteSet.mtx.Lock() defer voteSet.mtx.Unlock() if int(valIndex) >= len(voteSet.votes) { - return nil + return nil, false } - return voteSet.votes[valIndex] + if vote := voteSet.votes[valIndex]; vote != nil { + return vote, true + } + return nil, false } // List returns a copy of the list of votes stored by the VoteSet. @@ -382,17 +385,20 @@ func (voteSet *VoteSet) List() []Vote { return votes } -func (voteSet *VoteSet) GetByAddress(address []byte) *Vote { +func (voteSet *VoteSet) GetByAddress(address []byte) (*Vote, bool) { if voteSet == nil { - return nil + return nil, false } voteSet.mtx.Lock() defer voteSet.mtx.Unlock() - valIndex, val := voteSet.valSet.GetByAddress(address) - if val == nil { - panic("GetByAddress(address) returned nil") + valIndex, _, ok := voteSet.valSet.GetByAddress(address) + if !ok { + return nil, false + } + if vote := voteSet.votes[valIndex]; vote != nil { + return vote, true } - return voteSet.votes[valIndex] + return nil, false } func (voteSet *VoteSet) HasTwoThirdsMajority() bool { @@ -681,6 +687,6 @@ type VoteSetReader interface { Type() byte Size() int BitArray() *bits.BitArray - GetByIndex(int32) *Vote + GetByIndex(int32) (*Vote, bool) IsCommit() bool } diff --git a/sei-tendermint/types/vote_set_test.go b/sei-tendermint/types/vote_set_test.go index a0a199f2fb..987924b446 100644 --- a/sei-tendermint/types/vote_set_test.go +++ b/sei-tendermint/types/vote_set_test.go @@ -27,7 +27,8 @@ func TestVoteSet_AddVote_Good(t *testing.T) { require.NoError(t, err) val0Addr := val0p.Address() - assert.Nil(t, voteSet.GetByAddress(val0Addr)) + _, ok := voteSet.GetByAddress(val0Addr) + assert.False(t, ok) assert.False(t, voteSet.BitArray().GetIndex(0)) blockID, ok := voteSet.TwoThirdsMajority() assert.False(t, ok || !blockID.IsNil(), "there should be no 2/3 majority") @@ -44,7 +45,8 @@ func TestVoteSet_AddVote_Good(t *testing.T) { _, err = signAddVote(ctx, val0, vote, voteSet) require.NoError(t, err) - assert.NotNil(t, voteSet.GetByAddress(val0Addr)) + _, ok = voteSet.GetByAddress(val0Addr) + require.True(t, ok) assert.True(t, voteSet.BitArray().GetIndex(0)) blockID, ok = voteSet.TwoThirdsMajority() assert.False(t, ok || !blockID.IsNil(), "there should be no 2/3 majority") @@ -187,7 +189,7 @@ func TestVoteSet_2_3MajorityRedux(t *testing.T) { voteSet, _, privValidators := randVoteSet(ctx, t, height, round, tmproto.PrevoteType, 100, 1) blockHash := crypto.CRandBytes(32) - blockPartsTotal := uint32(123) + blockPartsTotal := MaxBlockPartsCount - 1 blockPartSetHeader := PartSetHeader{blockPartsTotal, crypto.CRandBytes(32)} voteProto := &Vote{ @@ -419,7 +421,7 @@ func TestVoteSet_MakeCommit(t *testing.T) { height, round := int64(1), int32(0) voteSet, _, privValidators := randVoteSet(ctx, t, height, round, tmproto.PrecommitType, 10, 1) - blockHash, blockPartSetHeader := crypto.CRandBytes(32), PartSetHeader{123, crypto.CRandBytes(32)} + blockHash, blockPartSetHeader := crypto.CRandBytes(32), PartSetHeader{MaxBlockPartsCount, crypto.CRandBytes(32)} voteProto := &Vote{ ValidatorAddress: nil, @@ -453,7 +455,7 @@ func TestVoteSet_MakeCommit(t *testing.T) { addr := pv.Address() vote := withValidator(voteProto, addr, 6) vote = withBlockHash(vote, tmrand.Bytes(32)) - vote = withBlockPartSetHeader(vote, PartSetHeader{123, tmrand.Bytes(32)}) + vote = withBlockPartSetHeader(vote, PartSetHeader{MaxBlockPartsCount, tmrand.Bytes(32)}) _, err = signAddVote(ctx, privValidators[6], vote, voteSet) require.NoError(t, err) diff --git a/sei-tendermint/types/vote_test.go b/sei-tendermint/types/vote_test.go index 4e01801401..e3792e9e6d 100644 --- a/sei-tendermint/types/vote_test.go +++ b/sei-tendermint/types/vote_test.go @@ -39,7 +39,7 @@ func exampleVote(tb testing.TB, t byte) *Vote { BlockID: BlockID{ Hash: crypto.Checksum([]byte("blockID_hash")), PartSetHeader: PartSetHeader{ - Total: 1000000, + Total: MaxBlockPartsCount, Hash: crypto.Checksum([]byte("blockID_part_set_header_hash")), }, }, @@ -315,7 +315,7 @@ func TestVoteProtobuf(t *testing.T) { } } -var sink interface{} +var sink any func getSampleCommit(ctx context.Context, t testing.TB) *Commit { t.Helper() @@ -342,7 +342,7 @@ func BenchmarkVoteSignBytes(b *testing.B) { } // Reset the sink. - sink = (interface{})(nil) + sink = nil } func BenchmarkCommitVoteSignBytes(b *testing.B) { @@ -355,7 +355,7 @@ func BenchmarkCommitVoteSignBytes(b *testing.B) { for i := 0; i < b.N; i++ { for index := range sampleCommit.Signatures { - sink = sampleCommit.VoteSignBytes("test_chain_id", int32(index)) + sink, _ = sampleCommit.VoteSignBytes("test_chain_id", int32(index)) } } @@ -364,5 +364,5 @@ func BenchmarkCommitVoteSignBytes(b *testing.B) { } // Reset the sink. - sink = (interface{})(nil) + sink = nil } diff --git a/sei-wasmd/x/wasm/ibctesting/chain.go b/sei-wasmd/x/wasm/ibctesting/chain.go index 02b9834152..5004f69ba8 100644 --- a/sei-wasmd/x/wasm/ibctesting/chain.go +++ b/sei-wasmd/x/wasm/ibctesting/chain.go @@ -470,7 +470,7 @@ func (chain *TestChain) CreateTMClientHeader(chainID string, blockHeight int64, ChainID: chainID, Height: blockHeight, Time: timestamp, - LastBlockID: MakeBlockID(make([]byte, tmhash.Size), 10_000, make([]byte, tmhash.Size)), + LastBlockID: MakeBlockID(make([]byte, tmhash.Size), tmtypes.MaxBlockPartsCount, make([]byte, tmhash.Size)), LastCommitHash: chain.App.LastCommitID().Hash, DataHash: tmhash.Sum([]byte("data_hash")), ValidatorsHash: vsetHash,