From d12a5350840ecd8bc8b6c3335f3630c6c51333e8 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Fri, 8 Aug 2025 10:56:20 +0100 Subject: [PATCH 01/14] Expand proposal gossip and handling validation rules 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.go | 5 +++ internal/consensus/peer_state_test.go | 49 ++++++++++++++++++++++++- internal/consensus/reactor_test.go | 1 + internal/consensus/state.go | 4 ++ types/block.go | 3 ++ types/evidence.go | 12 ++++++ types/proposal.go | 22 +++++++++++ types/proposal_test.go | 7 +++- 9 files changed, 102 insertions(+), 2 deletions(-) diff --git a/internal/consensus/memory_limit_test.go b/internal/consensus/memory_limit_test.go index 61e193d8d..935ee4ec6 100644 --- a/internal/consensus/memory_limit_test.go +++ b/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/internal/consensus/peer_state.go b/internal/consensus/peer_state.go index e646182d6..aedb48e20 100644 --- a/internal/consensus/peer_state.go +++ b/internal/consensus/peer_state.go @@ -114,6 +114,11 @@ func (ps *PeerState) SetHasProposal(proposal *types.Proposal) { return } + if err := proposal.ValidateBasic(); err != nil { + // Never accept proposals that do not pass basic validation. + return + } + // Check memory limits before acquiring lock or setting any state if proposal.BlockID.PartSetHeader.Total > types.MaxBlockPartsCount { ps.logger.Debug("PartSetHeader.Total exceeds maximum", "total", proposal.BlockID.PartSetHeader.Total, "max", types.MaxBlockPartsCount) diff --git a/internal/consensus/peer_state_test.go b/internal/consensus/peer_state_test.go index 1eeeee683..ed4b2ec94 100644 --- a/internal/consensus/peer_state_test.go +++ b/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,10 +131,11 @@ func TestSetHasProposal(t *testing.T) { Hash: make([]byte, crypto.HashSize), }, }, + Header: plausibleTestHeader, // Missing signature } ps.SetHasProposal(invalidProposal) - require.True(t, ps.PRS.Proposal, "Valid structure proposal should be accepted regardless of signature") + require.False(t, ps.PRS.Proposal, "Proposal with missing signature should not be accepted") // Test PartSetHeader.Total too large - should be silently ignored // Create a new peer state for this test @@ -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) @@ -404,6 +418,38 @@ func TestSetHasProposalEdgeCases(t *testing.T) { expectProposal: true, // Should remain true expectPanic: false, }, + { + name: "invalid proposal - should be ignored", + 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: 1, // Valid + Hash: make([]byte, 32), + }, + }, + Timestamp: time.Now(), + Signature: []byte("test-signature"), + Header: types.Header{ + Version: version.Consensus{ + Block: version.BlockProtocol, + }, + Height: 1, + ProposerAddress: make(types.Address, 20), + }, + TxKeys: make([]types.TxKey, 2<<10), // Exceeds limit + }, + expectProposal: false, // Should be set + expectPanic: false, + }, { name: "valid proposal - should be accepted", setupPeerState: func(ps *PeerState) { @@ -424,6 +470,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/internal/consensus/reactor_test.go b/internal/consensus/reactor_test.go index 354d0f241..d610fc0f8 100644 --- a/internal/consensus/reactor_test.go +++ b/internal/consensus/reactor_test.go @@ -1059,6 +1059,7 @@ func TestReactorMemoryLimitCoverage(t *testing.T) { }, }, Timestamp: time.Now(), + Header: plausibleTestHeader, Signature: []byte("test-signature"), } diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 1b0ad2c29..9afadb9a3 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1055,6 +1055,10 @@ func (cs *State) handleMsg(ctx context.Context, mi msgInfo, fsyncUponCompletion span.SetAttributes(attribute.Int("round", int(msg.Proposal.Round))) defer span.End() + if err = msg.Proposal.ValidateBasic(); err != nil { + err = fmt.Errorf("invalid proposal msg: %w", err) + break + } // will not cause transition. // once proposal is set, we can receive block parts if err = cs.setProposal(msg.Proposal, mi.ReceiveTime); err == nil { diff --git a/types/block.go b/types/block.go index b5ef3aa44..955e4869d 100644 --- a/types/block.go +++ b/types/block.go @@ -910,6 +910,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") } diff --git a/types/evidence.go b/types/evidence.go index c5b5b6223..d2f6f4fda 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -705,6 +705,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: %v: %w", at, evidence, err) + } + } + return nil +} + //------------------------------------------ PROTO -------------------------------------- // EvidenceToProto is a generalized function for encoding evidence that conforms to the diff --git a/types/proposal.go b/types/proposal.go index b8d4d27a0..c3b4e6cf7 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -12,6 +12,12 @@ import ( 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. +const maxTxKeysPerProposal = 1_000 + var ( ErrInvalidBlockPartSignature = errors.New("error invalid block part signature") ErrInvalidBlockPartHash = errors.New("error invalid block part hash") @@ -87,6 +93,22 @@ 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) + } + + if err := p.Evidence.ValidateBasic(); err != nil { + return fmt.Errorf("invalid Evidence: %w", err) + } + + 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)) + } return nil } diff --git a/types/proposal_test.go b/types/proposal_test.go index b0ce5c8f2..29f730f3f 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -2,11 +2,12 @@ package types import ( "context" - "github.com/tendermint/tendermint/version" "math" "testing" "time" + "github.com/tendermint/tendermint/version" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -185,6 +186,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"))) From 0879eef5f15d1891304ca2747a0fdb867ef1de6e Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Fri, 8 Aug 2025 11:48:12 +0100 Subject: [PATCH 02/14] Improve state test coverage for handling of invalid proposals --- internal/consensus/state_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index a21d4e25d..606282397 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -2952,8 +2952,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") From 539cbb69db943b15a49441f55f0a9cf3f40e8758 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Fri, 8 Aug 2025 15:09:44 +0100 Subject: [PATCH 03/14] Remove redundant call to validate basic Message handler calls validation basic on all types that implement it. Thanks to @sei-will for pointing this out! See: https://github.com/sei-protocol/sei-tendermint/blob/c514864f18f013eb4615ac0006cb6448c15c603e/internal/consensus/msgs.go#L587 --- internal/consensus/peer_state.go | 5 ----- internal/consensus/state.go | 4 ---- 2 files changed, 9 deletions(-) diff --git a/internal/consensus/peer_state.go b/internal/consensus/peer_state.go index aedb48e20..e646182d6 100644 --- a/internal/consensus/peer_state.go +++ b/internal/consensus/peer_state.go @@ -114,11 +114,6 @@ func (ps *PeerState) SetHasProposal(proposal *types.Proposal) { return } - if err := proposal.ValidateBasic(); err != nil { - // Never accept proposals that do not pass basic validation. - return - } - // Check memory limits before acquiring lock or setting any state if proposal.BlockID.PartSetHeader.Total > types.MaxBlockPartsCount { ps.logger.Debug("PartSetHeader.Total exceeds maximum", "total", proposal.BlockID.PartSetHeader.Total, "max", types.MaxBlockPartsCount) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 9afadb9a3..1b0ad2c29 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -1055,10 +1055,6 @@ func (cs *State) handleMsg(ctx context.Context, mi msgInfo, fsyncUponCompletion span.SetAttributes(attribute.Int("round", int(msg.Proposal.Round))) defer span.End() - if err = msg.Proposal.ValidateBasic(); err != nil { - err = fmt.Errorf("invalid proposal msg: %w", err) - break - } // will not cause transition. // once proposal is set, we can receive block parts if err = cs.setProposal(msg.Proposal, mi.ReceiveTime); err == nil { From 51ad8e29c974ecd440b982b968de743d4c9fce5f Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Fri, 8 Aug 2025 15:27:12 +0100 Subject: [PATCH 04/14] Revert test assertions in low lever proposal validity assertion The contextual assumption across the codebase is remote messages get validated once at transport. Then the rest of the internal implementation implicitly assumes/or selectively does not check for validity. To reduce code churn revert tests that asserted close-to-logic validity check. --- internal/consensus/peer_state_test.go | 34 +-------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/internal/consensus/peer_state_test.go b/internal/consensus/peer_state_test.go index ed4b2ec94..7433a72b9 100644 --- a/internal/consensus/peer_state_test.go +++ b/internal/consensus/peer_state_test.go @@ -135,7 +135,7 @@ func TestSetHasProposal(t *testing.T) { // Missing signature } ps.SetHasProposal(invalidProposal) - require.False(t, ps.PRS.Proposal, "Proposal with missing signature should not be accepted") + 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 @@ -418,38 +418,6 @@ func TestSetHasProposalEdgeCases(t *testing.T) { expectProposal: true, // Should remain true expectPanic: false, }, - { - name: "invalid proposal - should be ignored", - 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: 1, // Valid - Hash: make([]byte, 32), - }, - }, - Timestamp: time.Now(), - Signature: []byte("test-signature"), - Header: types.Header{ - Version: version.Consensus{ - Block: version.BlockProtocol, - }, - Height: 1, - ProposerAddress: make(types.Address, 20), - }, - TxKeys: make([]types.TxKey, 2<<10), // Exceeds limit - }, - expectProposal: false, // Should be set - expectPanic: false, - }, { name: "valid proposal - should be accepted", setupPeerState: func(ps *PeerState) { From ca1cbd5dcce998c551c123953a044f2ebead61a6 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 10:37:26 +0100 Subject: [PATCH 05/14] Validate proposer address in proposal CryptoAddress must have a fixed size. Assert the size in Proposal basic validation. --- types/proposal.go | 9 +++++++++ types/proposal_test.go | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/types/proposal.go b/types/proposal.go index c3b4e6cf7..cb4af4f1a 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -6,6 +6,7 @@ import ( "math/bits" "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" @@ -109,6 +110,14 @@ func (p *Proposal) ValidateBasic() error { 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 } diff --git a/types/proposal_test.go b/types/proposal_test.go index 29f730f3f..24a907e76 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -19,6 +19,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}, @@ -218,9 +220,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 From 4aeb6c6bcf40f95a6e2ae69492087694092f7371 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 11:04:01 +0100 Subject: [PATCH 06/14] Handle errors in proposal instantiation from proto A couple of error handling seem to be missing in proposal instantiation from proto. --- types/proposal.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/types/proposal.go b/types/proposal.go index cb4af4f1a..502b30b28 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -259,9 +259,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 From c12b932e9426a7032d7fdf904b50e8ae79574fc5 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 12:49:33 +0100 Subject: [PATCH 07/14] Check max transaction keys while decoding proto By the time we call validation basic it may be too late, in that the decoding process may have allocated many keys unnecessarily. Check earlier on that the list of keys does not reach max. Also assert that each transaction key length meets the expected value. --- types/mempool.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/types/mempool.go b/types/mempool.go index a3a0c831e..b1dedcbec 100644 --- a/types/mempool.go +++ b/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) From 247fc63a2e4b32d179a7348d15ad4d28639541ea Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 13:02:22 +0100 Subject: [PATCH 08/14] Check header in block meta and use existing validate basic for evidence --- types/block.go | 8 +++----- types/block_meta.go | 6 ++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/types/block.go b/types/block.go index 955e4869d..de09f9837 100644 --- a/types/block.go +++ b/types/block.go @@ -89,11 +89,9 @@ 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) - } + // NOTE: b.Evidence may be nil, which is handled by Evidence.ValidateBasic. + if err := b.Evidence.ValidateBasic(); err != nil { + return fmt.Errorf("invalid evidence: %w", err) } if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { diff --git a/types/block_meta.go b/types/block_meta.go index b35afd345..01687767a 100644 --- a/types/block_meta.go +++ b/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()) From 358e1ba0a11af1b88f5f2962839a47e11d6f892b Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 15:00:15 +0100 Subject: [PATCH 09/14] Consistently validate wrappers constructed from proto Some of the *FromProto functions were not validating the constructed wrapper. Consistently validate all. --- types/evidence.go | 2 +- types/node_info.go | 2 +- types/validator.go | 3 +-- types/vote.go | 5 +++-- types/vote_test.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/types/evidence.go b/types/evidence.go index d2f6f4fda..abd529fcb 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -616,7 +616,7 @@ func (evl *EvidenceList) FromProto(eviList *tmproto.EvidenceList) error { eviBzs[i] = evi } *evl = eviBzs - return nil + return evl.ValidateBasic() } // ToProto converts EvidenceList to protobuf diff --git a/types/node_info.go b/types/node_info.go index fd47816e2..9bd22e6c9 100644 --- a/types/node_info.go +++ b/types/node_info.go @@ -230,7 +230,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/types/validator.go b/types/validator.go index 2a3f507b9..5ffe1b089 100644 --- a/types/validator.go +++ b/types/validator.go @@ -209,10 +209,9 @@ func ValidatorFromProto(vp *tmproto.Validator) (*Validator, error) { v.VotingPower = vp.GetVotingPower() v.ProposerPriority = vp.GetProposerPriority() - return v, nil + return v, v.ValidateBasic() } - //---------------------------------------- // RandValidator diff --git a/types/vote.go b/types/vote.go index 6b7d4f2ac..08f1cb9c5 100644 --- a/types/vote.go +++ b/types/vote.go @@ -74,7 +74,7 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { return nil, err } - return &Vote{ + v := &Vote{ Type: pv.Type, Height: pv.Height, Round: pv.Round, @@ -85,7 +85,8 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { Signature: pv.Signature, Extension: pv.Extension, ExtensionSignature: pv.ExtensionSignature, - }, nil + } + return v, v.ValidateBasic() } // CommitSig converts the Vote to a CommitSig. diff --git a/types/vote_test.go b/types/vote_test.go index 37e19a717..3546048f3 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -490,7 +490,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() From dc8470e975ccad025228bc06cf10a0bebefcdf43 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 11 Aug 2025 17:00:25 +0100 Subject: [PATCH 10/14] Cap the maximum number of commit signatures --- types/block.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/types/block.go b/types/block.go index de09f9837..c4a48dd97 100644 --- a/types/block.go +++ b/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. @@ -1036,6 +1041,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: 1000)", len(cp.Signatures)) + } + sigs := make([]CommitSig, len(cp.Signatures)) for i := range cp.Signatures { if err := sigs[i].FromProto(cp.Signatures[i]); err != nil { From 978bc04e3fd1663c618d1722a7f097c6d3750bfb Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 18 Aug 2025 15:52:46 +0100 Subject: [PATCH 11/14] Fix incorrect constant in error message --- types/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/block.go b/types/block.go index c4a48dd97..9c498147e 100644 --- a/types/block.go +++ b/types/block.go @@ -1042,7 +1042,7 @@ func CommitFromProto(cp *tmproto.Commit) (*Commit, error) { } if len(cp.Signatures) > MaxCommitSignatures { - return nil, fmt.Errorf("too many signatures in commit: %d (max: 1000)", len(cp.Signatures)) + return nil, fmt.Errorf("too many signatures in commit: %d (max: %d)", len(cp.Signatures), MaxCommitSignatures) } sigs := make([]CommitSig, len(cp.Signatures)) From abd035d84434348c384a7c8bff5545c4497c8c39 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Mon, 18 Aug 2025 16:12:35 +0100 Subject: [PATCH 12/14] Remove duplicate validation for evidence list --- types/block.go | 5 +---- types/proposal.go | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/types/block.go b/types/block.go index 9c498147e..ac6b8a781 100644 --- a/types/block.go +++ b/types/block.go @@ -94,10 +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, which is handled by Evidence.ValidateBasic. - if err := b.Evidence.ValidateBasic(); err != nil { - return fmt.Errorf("invalid evidence: %w", 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) diff --git a/types/proposal.go b/types/proposal.go index 502b30b28..26d73329d 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -99,9 +99,7 @@ func (p *Proposal) ValidateBasic() error { return fmt.Errorf("invalid LastCommit: %w", err) } - if err := p.Evidence.ValidateBasic(); err != nil { - return fmt.Errorf("invalid Evidence: %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) From 7d469d3995e68571756b44f9267f7ed34a1f3eaf Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Wed, 1 Oct 2025 14:41:24 +0100 Subject: [PATCH 13/14] Make max tx keys per proposals overridable Make max tx keys per proposals overridable via env var `SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL`. --- types/proposal.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/types/proposal.go b/types/proposal.go index 26d73329d..bbed73f8d 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -3,7 +3,10 @@ package types import ( "errors" "fmt" + "math" "math/bits" + "os" + "strconv" "time" "github.com/tendermint/tendermint/crypto" @@ -17,7 +20,32 @@ import ( // 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. -const maxTxKeysPerProposal = 1_000 +// +// 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") From 481d67fa92457bec803b7b31bf4ffdb968d05db1 Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Wed, 1 Oct 2025 14:52:34 +0100 Subject: [PATCH 14/14] Avoid concatenating invalid evidence in error --- types/evidence.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/evidence.go b/types/evidence.go index a5a98b841..83c41d16a 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -751,7 +751,7 @@ func (evl EvidenceList) ValidateBasic() error { 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: %v: %w", at, evidence, err) + return fmt.Errorf("invalid evidence at index %d: %w", at, err) } } return nil