From 15fa1631340b8bf054702f6c37a0c24f3e18ca6b Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Thu, 2 Oct 2025 08:31:05 +0100 Subject: [PATCH] Expand proposal gossip and handling validation rules (#292) Compared to the original Tendermint, Sei-Tendermint expands the proposal structure to include a number of additional fields. Update the basic validation logic to take into account additional fields when validating proposals. Add validation to both direct message handling and ambient gossip handling to avoid propagation of an invalid proposal. Expand tests to cover the additional execution paths at unit and reactor integration level. --- .../internal/consensus/memory_limit_test.go | 1 + .../internal/consensus/peer_state_test.go | 15 +++++ .../internal/consensus/reactor_test.go | 1 + .../internal/consensus/state_test.go | 16 ++++- sei-tendermint/types/block.go | 19 ++++-- sei-tendermint/types/block_meta.go | 6 ++ sei-tendermint/types/evidence.go | 14 +++- sei-tendermint/types/mempool.go | 6 ++ sei-tendermint/types/node_info.go | 2 +- sei-tendermint/types/proposal.go | 64 ++++++++++++++++++- sei-tendermint/types/proposal_test.go | 10 ++- sei-tendermint/types/validator.go | 2 +- sei-tendermint/types/vote.go | 5 +- sei-tendermint/types/vote_test.go | 2 +- 14 files changed, 147 insertions(+), 16 deletions(-) diff --git a/sei-tendermint/internal/consensus/memory_limit_test.go b/sei-tendermint/internal/consensus/memory_limit_test.go index 61e193d8dd..935ee4ec6b 100644 --- a/sei-tendermint/internal/consensus/memory_limit_test.go +++ b/sei-tendermint/internal/consensus/memory_limit_test.go @@ -48,6 +48,7 @@ func TestPeerStateMemoryLimits(t *testing.T) { BlockID: blockID, Timestamp: time.Now(), Signature: []byte("test-signature"), + Header: plausibleTestHeader, } ps.SetHasProposal(proposal) if tc.expectError { diff --git a/sei-tendermint/internal/consensus/peer_state_test.go b/sei-tendermint/internal/consensus/peer_state_test.go index 1eeeee6833..7433a72b9b 100644 --- a/sei-tendermint/internal/consensus/peer_state_test.go +++ b/sei-tendermint/internal/consensus/peer_state_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/version" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/libs/log" @@ -12,6 +13,14 @@ import ( "github.com/tendermint/tendermint/types" ) +var plausibleTestHeader = types.Header{ + Version: version.Consensus{ + Block: version.BlockProtocol, + }, + Height: 1, + ProposerAddress: make(types.Address, 20), +} + func peerStateSetup(h, r, v int) *PeerState { ps := NewPeerState(log.NewNopLogger(), "testPeerState") ps.PRS.Height = int64(h) @@ -122,6 +131,7 @@ func TestSetHasProposal(t *testing.T) { Hash: make([]byte, crypto.HashSize), }, }, + Header: plausibleTestHeader, // Missing signature } ps.SetHasProposal(invalidProposal) @@ -142,6 +152,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps3.SetHasProposal(tooLargeTotalProposal) @@ -160,6 +171,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps.SetHasProposal(validProposal) @@ -179,6 +191,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps2.SetHasProposal(differentProposal) @@ -235,6 +248,7 @@ func TestSetHasProposalMemoryLimit(t *testing.T) { // Set up proposal with test case total proposal.BlockID.PartSetHeader.Total = tc.total + proposal.Header = plausibleTestHeader // Use a plausible header // Try to set the proposal ps.SetHasProposal(proposal) @@ -424,6 +438,7 @@ func TestSetHasProposalEdgeCases(t *testing.T) { }, Timestamp: time.Now(), Signature: []byte("test-signature"), + Header: plausibleTestHeader, }, expectProposal: true, // Should be set expectPanic: false, diff --git a/sei-tendermint/internal/consensus/reactor_test.go b/sei-tendermint/internal/consensus/reactor_test.go index 83ab03aea3..4a4faa4f08 100644 --- a/sei-tendermint/internal/consensus/reactor_test.go +++ b/sei-tendermint/internal/consensus/reactor_test.go @@ -842,6 +842,7 @@ func TestReactorMemoryLimitCoverage(t *testing.T) { }, }, Timestamp: time.Now(), + Header: plausibleTestHeader, Signature: []byte("test-signature"), } diff --git a/sei-tendermint/internal/consensus/state_test.go b/sei-tendermint/internal/consensus/state_test.go index 54b4135019..c4b118fe9b 100644 --- a/sei-tendermint/internal/consensus/state_test.go +++ b/sei-tendermint/internal/consensus/state_test.go @@ -2495,8 +2495,22 @@ func TestGossipTransactionKeyOnlyConfig(t *testing.T) { proposalMsg := ProposalMessage{&proposal} peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") startTestRound(ctx, cs1, height, round) - cs1.handleMsg(ctx, msgInfo{&proposalMsg, peerID, time.Now()}, false) + + // Assert invalid proposal is ignored. + invalidProposal := proposal + invalidProposal.Height = -3 + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{&invalidProposal}, peerID, time.Now()}, false) rs := cs1.GetRoundState() + require.Nil(t, rs.Proposal) + require.Nil(t, rs.ProposalBlock) + require.Zero(t, rs.ProposalBlockParts.Total()) + + // Now assert that a valid proposal is processed correctly. + cs1.handleMsg(ctx, msgInfo{&proposalMsg, peerID, time.Now()}, false) + + // GetRoundStates returns a snapshot of the current state. Therefore, we need to + // get it again after processing the proposal message. + rs = cs1.GetRoundState() // Proposal, ProposalBlock and ProposalBlockParts sohuld be set since gossip-tx-key is true if rs.Proposal == nil { t.Error("rs.Proposal should be set") diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index 4938de5374..ab32045250 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -37,6 +37,11 @@ const ( // Uvarint length of Data.Txs: 4 bytes // Data.Txs field: 1 byte MaxOverheadForBlock int64 = 11 + + // MaxCommitSignatures is the maximum number of signatures in a commit. The max + // value is picked relative to the current number of validators and may need to + // be increased in the future as the network grows. + MaxCommitSignatures = 128 ) // Block defines the atomic unit of a Tendermint blockchain. @@ -89,12 +94,7 @@ func (b *Block) ValidateBasic() error { return fmt.Errorf("wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Data.Txs)) } - // NOTE: b.Evidence may be nil, but we're just looping. - for i, ev := range b.Evidence { - if err := ev.ValidateBasic(); err != nil { - return fmt.Errorf("invalid evidence (#%d): %v", i, err) - } - } + // Evidence is validated as part of proto decoding. See EvidenceList.FromProto. if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { return fmt.Errorf("wrong Header.EvidenceHash. Expected %X, got %X", w, g) @@ -821,6 +821,9 @@ func (commit *Commit) Size() int { // ValidateBasic performs basic validation that doesn't involve state data. // Does not actually check the cryptographic signatures. func (commit *Commit) ValidateBasic() error { + if commit == nil { + return nil + } if commit.Height < 0 { return errors.New("negative Height") } @@ -926,6 +929,10 @@ func CommitFromProto(cp *tmproto.Commit) (*Commit, error) { return nil, err } + if len(cp.Signatures) > MaxCommitSignatures { + return nil, fmt.Errorf("too many signatures in commit: %d (max: %d)", len(cp.Signatures), MaxCommitSignatures) + } + sigs := make([]CommitSig, len(cp.Signatures)) for i := range cp.Signatures { if err := sigs[i].FromProto(cp.Signatures[i]); err != nil { diff --git a/sei-tendermint/types/block_meta.go b/sei-tendermint/types/block_meta.go index b35afd3457..01687767a2 100644 --- a/sei-tendermint/types/block_meta.go +++ b/sei-tendermint/types/block_meta.go @@ -67,9 +67,15 @@ func BlockMetaFromProto(pb *tmproto.BlockMeta) (*BlockMeta, error) { // ValidateBasic performs basic validation. func (bm *BlockMeta) ValidateBasic() error { + if bm == nil { + return errors.New("nil BlockMeta") + } if err := bm.BlockID.ValidateBasic(); err != nil { return err } + if err := bm.Header.ValidateBasic(); err != nil { + return err + } if !bytes.Equal(bm.BlockID.Hash, bm.Header.Hash()) { return fmt.Errorf("expected BlockID#Hash and Header#Hash to be the same, got %X != %X", bm.BlockID.Hash, bm.Header.Hash()) diff --git a/sei-tendermint/types/evidence.go b/sei-tendermint/types/evidence.go index 83cb4b391e..83c41d16a5 100644 --- a/sei-tendermint/types/evidence.go +++ b/sei-tendermint/types/evidence.go @@ -656,7 +656,7 @@ func (evl *EvidenceList) FromProto(eviList *tmproto.EvidenceList) error { eviBzs[i] = evi } *evl = eviBzs - return nil + return evl.ValidateBasic() } // ToProto converts EvidenceList to protobuf @@ -745,6 +745,18 @@ func (evl EvidenceList) ToABCI() []abci.Misbehavior { return el } +func (evl EvidenceList) ValidateBasic() error { + for at, evidence := range evl { + if evidence == nil { + return fmt.Errorf("nil evidence in evidence list at index %d", at) + } + if err := evidence.ValidateBasic(); err != nil { + return fmt.Errorf("invalid evidence at index %d: %w", at, err) + } + } + return nil +} + //------------------------------------------ PROTO -------------------------------------- // EvidenceToProto is a generalized function for encoding evidence that conforms to the diff --git a/sei-tendermint/types/mempool.go b/sei-tendermint/types/mempool.go index a3a0c831eb..b1dedcbec7 100644 --- a/sei-tendermint/types/mempool.go +++ b/sei-tendermint/types/mempool.go @@ -35,6 +35,9 @@ func TxKeyFromProto(dp *tmproto.TxKey) (TxKey, error) { if dp == nil { return TxKey{}, errors.New("nil data") } + if len(dp.TxKey) != sha256.Size { + return TxKey{}, fmt.Errorf("invalid tx key length: %d, expected: %d", len(dp.TxKey), sha256.Size) + } var txBzs [sha256.Size]byte for i := range dp.TxKey { txBzs[i] = dp.TxKey[i] @@ -44,6 +47,9 @@ func TxKeyFromProto(dp *tmproto.TxKey) (TxKey, error) { } func TxKeysListFromProto(dps []*tmproto.TxKey) ([]TxKey, error) { + if len(dps) > maxTxKeysPerProposal { + return nil, fmt.Errorf("too many tx keys in proposal: %d, max: %d", len(dps), maxTxKeysPerProposal) + } var txKeys []TxKey for _, txKey := range dps { txKey, err := TxKeyFromProto(txKey) diff --git a/sei-tendermint/types/node_info.go b/sei-tendermint/types/node_info.go index 3c7758b672..af64bccb8f 100644 --- a/sei-tendermint/types/node_info.go +++ b/sei-tendermint/types/node_info.go @@ -229,7 +229,7 @@ func NodeInfoFromProto(pb *tmp2p.NodeInfo) (NodeInfo, error) { }, } - return dni, nil + return dni, dni.Validate() } // ParseAddressString reads an address string, and returns the IP diff --git a/sei-tendermint/types/proposal.go b/sei-tendermint/types/proposal.go index b8d4d27a05..bbed73f8d6 100644 --- a/sei-tendermint/types/proposal.go +++ b/sei-tendermint/types/proposal.go @@ -3,15 +3,50 @@ package types import ( "errors" "fmt" + "math" "math/bits" + "os" + "strconv" "time" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/internal/libs/protoio" tmbytes "github.com/tendermint/tendermint/libs/bytes" tmtime "github.com/tendermint/tendermint/libs/time" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) +// maxTxKeysPerProposal is the maximum number of transaction keys that can be +// included in a proposal. The limit is determined such that the proposal should +// hit the gas limit before ever reaching the max transaction keys in order to +// cap the maximum. +// +// By default, we set this to 1,000 which should be reasonable for most use +// cases. However, this can be overridden by setting the +// SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL environment variable to a positive +// integer value for load testing purposes. +var maxTxKeysPerProposal int + +func init() { + const defaultMaxTxKeysPerProposal = 1_000 + if value, found := os.LookupEnv("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL"); found { + maxTxKeys, err := strconv.ParseInt(value, 10, 64) + if err != nil { + panic(fmt.Sprintf("Failed to parse SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL %s: %v", value, err)) + } + if maxTxKeys <= 0 { + panic(fmt.Sprintf("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL must be a positive integer, got %d", maxTxKeys)) + } + if maxTxKeys > math.MaxInt { + panic(fmt.Sprintf("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL must be less than or equal to %d, got %d", math.MaxInt, maxTxKeys)) + } + maxTxKeysPerProposal = int(maxTxKeys) + fmt.Printf("Using custom maxTxKeysPerProposal: %d\n", maxTxKeysPerProposal) + } else { + maxTxKeysPerProposal = defaultMaxTxKeysPerProposal + } +} + var ( ErrInvalidBlockPartSignature = errors.New("error invalid block part signature") ErrInvalidBlockPartHash = errors.New("error invalid block part hash") @@ -87,6 +122,28 @@ func (p *Proposal) ValidateBasic() error { if len(p.Signature) > MaxSignatureSize { return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize) } + + if err := p.LastCommit.ValidateBasic(); err != nil { + return fmt.Errorf("invalid LastCommit: %w", err) + } + + // Evidence is validated as part of proto decoding. See EvidenceList.FromProto. + + if err := p.Header.ValidateBasic(); err != nil { + return fmt.Errorf("invalid Header: %w", err) + } + + if len(p.TxKeys) > maxTxKeysPerProposal { + return fmt.Errorf("invalid number of TxKeys: must be at most %d, got %d", maxTxKeysPerProposal, len(p.TxKeys)) + } + + if len(p.ProposerAddress) != crypto.AddressSize { + return fmt.Errorf( + "invalid ProposerAddress length; got: %d, expected: %d", + len(p.ProposerAddress), crypto.AddressSize, + ) + } + return nil } @@ -228,9 +285,14 @@ func ProposalFromProto(pp *tmproto.Proposal) (*Proposal, error) { } p.Header = header lastCommit, err := CommitFromProto(pp.LastCommit) + if err != nil { + return nil, fmt.Errorf("failed to instantiate last commit: %w", err) + } p.LastCommit = lastCommit eviD := new(EvidenceList) - eviD.FromProto(pp.Evidence) + if err := eviD.FromProto(pp.Evidence); err != nil { + return nil, fmt.Errorf("faield to instantiate evidence list: %w", err) + } p.Evidence = *eviD p.ProposerAddress = pp.ProposerAddress diff --git a/sei-tendermint/types/proposal_test.go b/sei-tendermint/types/proposal_test.go index 16f1ec9b8a..24ee3e24de 100644 --- a/sei-tendermint/types/proposal_test.go +++ b/sei-tendermint/types/proposal_test.go @@ -18,6 +18,8 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) +var plausibleTestAddress = crypto.Address(make([]byte, crypto.AddressSize)) + func generateHeader() Header { return Header{ Version: version.Consensus{Block: version.BlockProtocol}, @@ -182,6 +184,10 @@ func TestProposalValidateBasic(t *testing.T) { {"Too big Signature", func(p *Proposal) { p.Signature = make([]byte, MaxSignatureSize+1) }, true}, + {"Invalid LastCommit", func(p *Proposal) { p.LastCommit = &Commit{Height: -1} }, true}, + {"Invalid EvidenceList", func(p *Proposal) { p.Evidence = []Evidence{nil} }, true}, + {"Invalid Header", func(p *Proposal) { p.Header = Header{ChainID: string(make([]byte, 100))} }, true}, + {"Too many TxKeys", func(p *Proposal) { p.TxKeys = make([]TxKey, maxTxKeysPerProposal+1) }, true}, } blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) @@ -208,9 +214,9 @@ 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, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, plausibleTestAddress) proposal.Signature = []byte("sig") - proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) + proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, plausibleTestAddress) testCases := []struct { msg string diff --git a/sei-tendermint/types/validator.go b/sei-tendermint/types/validator.go index b542037f20..5ffe1b0891 100644 --- a/sei-tendermint/types/validator.go +++ b/sei-tendermint/types/validator.go @@ -209,7 +209,7 @@ func ValidatorFromProto(vp *tmproto.Validator) (*Validator, error) { v.VotingPower = vp.GetVotingPower() v.ProposerPriority = vp.GetProposerPriority() - return v, nil + return v, v.ValidateBasic() } //---------------------------------------- diff --git a/sei-tendermint/types/vote.go b/sei-tendermint/types/vote.go index 40f829182d..e759e4c029 100644 --- a/sei-tendermint/types/vote.go +++ b/sei-tendermint/types/vote.go @@ -68,7 +68,7 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { return nil, err } - return &Vote{ + v := &Vote{ Type: pv.Type, Height: pv.Height, Round: pv.Round, @@ -77,7 +77,8 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { ValidatorAddress: pv.ValidatorAddress, ValidatorIndex: pv.ValidatorIndex, Signature: pv.Signature, - }, nil + } + return v, v.ValidateBasic() } // CommitSig converts the Vote to a CommitSig. diff --git a/sei-tendermint/types/vote_test.go b/sei-tendermint/types/vote_test.go index 3ac651ce35..4d46b8b598 100644 --- a/sei-tendermint/types/vote_test.go +++ b/sei-tendermint/types/vote_test.go @@ -301,7 +301,7 @@ func TestVoteProtobuf(t *testing.T) { passesValidateBasic bool }{ {"success", vote, true, true}, - {"fail vote validate basic", &Vote{}, true, false}, + {"fail vote validate basic", &Vote{}, false, false}, } for _, tc := range testCases { protoProposal := tc.vote.ToProto()