diff --git a/sei-tendermint/internal/consensus/block_id_match.go b/sei-tendermint/internal/consensus/block_id_match.go new file mode 100644 index 0000000000..816a5043cb --- /dev/null +++ b/sei-tendermint/internal/consensus/block_id_match.go @@ -0,0 +1,62 @@ +package consensus + +import ( + "errors" + "fmt" + + "github.com/sei-protocol/sei-chain/sei-tendermint/types" +) + +// ErrNonCanonicalProposalParts is returned when assembled proposal parts do not +// use the default BlockPartSizeBytes chunking of the block's canonical +// protobuf encoding (i.e. when parts.Header() differs from MakePartSet). +var ErrNonCanonicalProposalParts = errors.New("non-canonical proposal parts") + +// blockIDMatches reports whether block and parts together match the consensus +// BlockID (header hash and PartSetHeader). Consensus identity is the full +// BlockID; comparing only the header hash would treat different part-set +// encodings as the same value. +func blockIDMatches(block *types.Block, parts *types.PartSet, blockID types.BlockID) bool { + if block == nil || parts == nil || !blockID.IsComplete() { + return false + } + return block.HashesTo(blockID.Hash) && parts.HasHeader(blockID.PartSetHeader) +} + +// proposalMatchesLocked reports whether the proposal block and parts equal the +// locked block identity (header hash and PartSetHeader). +func proposalMatchesLocked(proposal, locked *types.Block, proposalParts, lockedParts *types.PartSet) bool { + if locked == nil || lockedParts == nil { + return false + } + return blockIDMatches(proposal, proposalParts, types.BlockID{ + Hash: locked.Hash(), + PartSetHeader: lockedParts.Header(), + }) +} + +// verifyCanonicalProposalParts ensures ProposalBlockParts match +// block.MakePartSet(BlockPartSizeBytes). Parts that carry the same logical +// block bytes under a different chunk size produce a different PartSetHeader; +// those must be rejected so commit/blocksync (which rebuild with +// BlockPartSizeBytes) stay consistent. Proposal.BlockID.Hash is not checked +// here: a mismatched proposal hash must not block later maj23/commit catch-up +// that retargets the same PartSetHeader, and votes already commit to +// ProposalBlock.Hash() + parts.Header(). +func (cs *State) verifyCanonicalProposalParts(block *types.Block) error { + parts := cs.roundState.ProposalBlockParts() + if parts == nil { + return errors.New("nil proposal block parts") + } + canonicalParts, err := block.MakePartSet(types.BlockPartSizeBytes) + if err != nil { + return fmt.Errorf("MakePartSet: %w", err) + } + if !parts.HasHeader(canonicalParts.Header()) { + return fmt.Errorf( + "%w: PartSetHeader got %v, want canonical %v", + ErrNonCanonicalProposalParts, parts.Header(), canonicalParts.Header(), + ) + } + return nil +} diff --git a/sei-tendermint/internal/consensus/block_id_match_state_test.go b/sei-tendermint/internal/consensus/block_id_match_state_test.go new file mode 100644 index 0000000000..964ce79c4b --- /dev/null +++ b/sei-tendermint/internal/consensus/block_id_match_state_test.go @@ -0,0 +1,271 @@ +package consensus + +import ( + "bytes" + "testing" + + "github.com/gogo/protobuf/proto" + + tmconfig "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/sei-protocol/sei-chain/sei-tendermint/crypto" + tmtime "github.com/sei-protocol/sei-chain/sei-tendermint/libs/time" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils/require" + tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" + "github.com/sei-protocol/sei-chain/sei-tendermint/types" +) + +func nonCanonicalPartSet(t *testing.T, block *types.Block) *types.PartSet { + t.Helper() + pbb, err := block.ToProto() + require.NoError(t, err) + bz, err := proto.Marshal(pbb) + require.NoError(t, err) + // Unknown field 999, length-delimited "junk". + nonCanonical := append(append([]byte{}, bz...), 0xba, 0x3e, 0x04, 'j', 'u', 'n', 'k') + parts := types.NewPartSetFromData(nonCanonical, types.BlockPartSizeBytes) + canonical, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + require.False(t, parts.Header().Equals(canonical.Header())) + return parts +} + +func TestRejectNonCanonicalProposalBlockParts(t *testing.T) { + config := configSetup(t) + chainID := tmconfig.TestLoadGenesis(config).ChainID + ctx := t.Context() + + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, validators: 2}) + height, round := cs1.roundState.Height(), cs1.roundState.Round() + round++ + incrementRound(vss[1:]...) + + propBlock, err := cs1.createProposalBlock(ctx) + require.NoError(t, err) + + nonCanonicalParts := nonCanonicalPartSet(t, propBlock) + blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: nonCanonicalParts.Header()} + pubKey, err := vss[1].PrivValidator.GetPubKey(ctx) + require.NoError(t, err) + proposal := types.NewProposal( + height, round, -1, blockID, propBlock.Time, + propBlock.GetTxHashes(), propBlock.Header, propBlock.LastCommit, propBlock.Evidence, pubKey.Address(), + ) + p := proposal.ToProto() + require.NoError(t, vss[1].SignProposal(ctx, chainID, p)) + proposal.Signature = utils.OrPanic1(crypto.SigFromBytes(p.Signature)) + + cs1.startTestRound(ctx, height, round) + peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") + require.NoError(t, err) + + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{proposal}, peerID, tmtime.Now()}, false) + require.Greater(t, nonCanonicalParts.Total(), uint32(0)) + + // Deliver all but the last part without completing. + for i := 0; i < int(nonCanonicalParts.Total())-1; i++ { + cs1.handleMsg(ctx, msgInfo{ + &BlockPartMessage{Height: height, Round: round, Part: nonCanonicalParts.GetPart(i)}, + peerID, + tmtime.Now(), + }, false) + } + + cs1.mtx.Lock() + added, err := cs1.addProposalBlockPart(&BlockPartMessage{ + Height: height, + Round: round, + Part: nonCanonicalParts.GetPart(int(nonCanonicalParts.Total()) - 1), + }, peerID) + cs1.mtx.Unlock() + + require.False(t, added) + require.ErrorIs(t, err, ErrNonCanonicalProposalParts) + require.Nil(t, cs1.GetRoundState().ProposalBlock, "proposal block must not be accepted from non-canonical parts") +} + +// Commit/maj23 catch-up can retarget ProposalBlockParts to a certificate +// PartSetHeader while leaving the original Proposal in place. Assembling +// canonical parts for that certificate must succeed even when the proposal's +// BlockID.PartSetHeader differs. +func TestAcceptCanonicalPartsWhenProposalPartSetHeaderDiffers(t *testing.T) { + config := configSetup(t) + chainID := tmconfig.TestLoadGenesis(config).ChainID + ctx := t.Context() + + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, validators: 2}) + height, round := cs1.roundState.Height(), cs1.roundState.Round() + round++ + incrementRound(vss[1:]...) + + propBlock, err := cs1.createProposalBlock(ctx) + require.NoError(t, err) + canonicalParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + staleHeader := nonCanonicalPartSet(t, propBlock).Header() + require.False(t, staleHeader.Equals(canonicalParts.Header())) + + // Proposal still claims the stale PartSetHeader (as after a mismatched earlier propose). + blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: staleHeader} + pubKey, err := vss[1].PrivValidator.GetPubKey(ctx) + require.NoError(t, err) + proposal := types.NewProposal( + height, round, -1, blockID, propBlock.Time, + propBlock.GetTxHashes(), propBlock.Header, propBlock.LastCommit, propBlock.Evidence, pubKey.Address(), + ) + p := proposal.ToProto() + require.NoError(t, vss[1].SignProposal(ctx, chainID, p)) + proposal.Signature = utils.OrPanic1(crypto.SigFromBytes(p.Signature)) + + cs1.startTestRound(ctx, height, round) + peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") + require.NoError(t, err) + + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{proposal}, peerID, tmtime.Now()}, false) + + // Retarget parts to the certificate/canonical header, as enterCommit does + // when commit BlockID.PartSetHeader differs from the proposal. + cs1.mtx.Lock() + cs1.roundState.SetProposalBlock(nil) + cs1.roundState.SetProposalBlockParts(types.NewPartSetFromHeader(canonicalParts.Header())) + cs1.mtx.Unlock() + + for i := 0; i < int(canonicalParts.Total()); i++ { + cs1.handleMsg(ctx, msgInfo{ + &BlockPartMessage{Height: height, Round: round, Part: canonicalParts.GetPart(i)}, + peerID, + tmtime.Now(), + }, false) + } + + rs := cs1.GetRoundState() + require.NotNil(t, rs.Proposal, "original proposal should remain") + require.False(t, rs.Proposal.BlockID.PartSetHeader.Equals(canonicalParts.Header())) + require.NotNil(t, rs.ProposalBlock, "canonical certificate parts must be accepted") + require.True(t, rs.ProposalBlock.HashesTo(propBlock.Hash())) + require.True(t, rs.ProposalBlockParts.HasHeader(canonicalParts.Header())) +} + +func TestEnterPrecommitDoesNotRelockOnPartSetMismatch(t *testing.T) { + config := configSetup(t) + chainID := tmconfig.TestLoadGenesis(config).ChainID + ctx := t.Context() + + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, validators: 4}) + height, round := cs1.roundState.Height(), cs1.roundState.Round() + voteCh := cs1.subscribeToVoterBuffered(ctx, t, cs1.address(ctx)) + + propBlock, err := cs1.createProposalBlock(ctx) + require.NoError(t, err) + propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + blockID := types.BlockID{Hash: propBlock.Hash(), PartSetHeader: propBlockParts.Header()} + + pubKey, err := vss[0].PrivValidator.GetPubKey(ctx) + require.NoError(t, err) + proposal := types.NewProposal( + height, round, -1, blockID, propBlock.Time, + propBlock.GetTxHashes(), propBlock.Header, propBlock.LastCommit, propBlock.Evidence, pubKey.Address(), + ) + p := proposal.ToProto() + require.NoError(t, vss[0].SignProposal(ctx, chainID, p)) + proposal.Signature = utils.OrPanic1(crypto.SigFromBytes(p.Signature)) + + cs1.startTestRound(ctx, height, round) + peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") + require.NoError(t, err) + + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{proposal}, peerID, tmtime.Now()}, false) + for i := 0; i < int(propBlockParts.Total()); i++ { + cs1.handleMsg(ctx, msgInfo{ + &BlockPartMessage{Height: height, Round: round, Part: propBlockParts.GetPart(i)}, + peerID, + tmtime.Now(), + }, false) + } + ensurePrevote(t, voteCh, height, round) + + cs1.mtx.Lock() + cs1.roundState.SetLockedRound(round) + cs1.roundState.SetLockedBlock(propBlock) + cs1.roundState.SetLockedBlockParts(propBlockParts) + + mismatchedID := types.BlockID{ + Hash: propBlock.Hash(), + PartSetHeader: types.PartSetHeader{ + Total: propBlockParts.Total(), + Hash: crypto.CRandBytes(32), + }, + } + require.False(t, blockIDMatches(propBlock, propBlockParts, mismatchedID)) + + // Inject maj23 directly so tryAddVote does not clear ProposalBlock before + // enterPrecommit; we want the lock/proposal BlockID match path. + for _, vs := range vss[1:] { + vote := signVote(ctx, t, vs, tmproto.PrevoteType, chainID, mismatchedID) + added, err := cs1.roundState.Votes().AddVote(vote, peerID) + require.NoError(t, err) + require.True(t, added) + } + require.NotNil(t, cs1.roundState.ProposalBlock()) + cs1.enterPrecommit(ctx, height, round, "test-partset-mismatch") + cs1.mtx.Unlock() + + // Hash matches lock but PartSetHeader does not → precommit nil, remain locked. + ensurePrecommitMatch(t, voteCh, height, round, nil) + cs1.validatePrecommit(ctx, t, round, round, vss[0], nil, propBlock.Hash()) +} + +// A proposal whose BlockID.Hash lies but whose PartSetHeader matches the +// canonical part set must still assemble. Rejecting here would leave a +// complete PartSet with ProposalBlock==nil and block later maj23/commit +// catch-up that reuses the same header (votes use ProposalBlock.Hash()). +func TestAssembleDespiteProposalHashMismatch(t *testing.T) { + config := configSetup(t) + chainID := tmconfig.TestLoadGenesis(config).ChainID + ctx := t.Context() + + cs1, vss := makeState(ctx, t, makeStateArgs{config: config, validators: 2}) + height, round := cs1.roundState.Height(), cs1.roundState.Round() + round++ + incrementRound(vss[1:]...) + + propBlock, err := cs1.createProposalBlock(ctx) + require.NoError(t, err) + canonicalParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + badHash := crypto.CRandBytes(32) + require.False(t, propBlock.HashesTo(badHash)) + blockID := types.BlockID{Hash: badHash, PartSetHeader: canonicalParts.Header()} + pubKey, err := vss[1].PrivValidator.GetPubKey(ctx) + require.NoError(t, err) + proposal := types.NewProposal( + height, round, -1, blockID, propBlock.Time, + propBlock.GetTxHashes(), propBlock.Header, propBlock.LastCommit, propBlock.Evidence, pubKey.Address(), + ) + p := proposal.ToProto() + require.NoError(t, vss[1].SignProposal(ctx, chainID, p)) + proposal.Signature = utils.OrPanic1(crypto.SigFromBytes(p.Signature)) + + cs1.startTestRound(ctx, height, round) + peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") + require.NoError(t, err) + + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{proposal}, peerID, tmtime.Now()}, false) + for i := 0; i < int(canonicalParts.Total()); i++ { + cs1.handleMsg(ctx, msgInfo{ + &BlockPartMessage{Height: height, Round: round, Part: canonicalParts.GetPart(i)}, + peerID, + tmtime.Now(), + }, false) + } + + rs := cs1.GetRoundState() + require.NotNil(t, rs.Proposal) + require.NotNil(t, rs.ProposalBlock, "lying proposal hash must not block assembly of canonical parts") + require.True(t, rs.ProposalBlock.HashesTo(propBlock.Hash())) + require.False(t, bytes.Equal(rs.Proposal.BlockID.Hash, propBlock.Hash())) + require.True(t, rs.ProposalBlockParts.HasHeader(canonicalParts.Header())) +} diff --git a/sei-tendermint/internal/consensus/block_id_match_test.go b/sei-tendermint/internal/consensus/block_id_match_test.go new file mode 100644 index 0000000000..8dc9cb47e4 --- /dev/null +++ b/sei-tendermint/internal/consensus/block_id_match_test.go @@ -0,0 +1,214 @@ +package consensus + +import ( + "io" + "testing" + + "github.com/gogo/protobuf/proto" + + "github.com/sei-protocol/sei-chain/sei-tendermint/crypto" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils/require" + 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" +) + +func testBlock(t *testing.T) *types.Block { + t.Helper() + return testBlockWith(t, nil, nil) +} + +func testBlockWith(t *testing.T, txs types.Txs, lastCommit *types.Commit) *types.Block { + t.Helper() + valHash := crypto.CRandBytes(32) + if lastCommit == nil { + lastCommit = &types.Commit{} + } + block := &types.Block{ + Header: types.Header{ + Version: version.Consensus{Block: version.BlockProtocol, App: 1}, + ChainID: "test-chain", + Height: 1, + ValidatorsHash: valHash, + NextValidatorsHash: valHash, + ConsensusHash: crypto.CRandBytes(32), + AppHash: crypto.CRandBytes(32), + LastResultsHash: crypto.CRandBytes(32), + ProposerAddress: crypto.CRandBytes(crypto.AddressSize), + }, + Data: types.Data{Txs: txs}, + LastCommit: lastCommit, + } + block.LastCommitHash = block.LastCommit.Hash() + block.DataHash = block.Data.Hash(false) + block.EvidenceHash = block.Evidence.Hash() + require.NotNil(t, block.Hash()) + return block +} + +func TestBlockIDMatches(t *testing.T) { + block := testBlock(t) + hash := block.Hash() + parts, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + matching := types.BlockID{Hash: hash, PartSetHeader: parts.Header()} + require.True(t, blockIDMatches(block, parts, matching)) + + wrongParts := types.BlockID{ + Hash: hash, + PartSetHeader: types.PartSetHeader{ + Total: parts.Total(), + Hash: crypto.CRandBytes(32), + }, + } + require.False(t, blockIDMatches(block, parts, wrongParts)) + + wrongHash := types.BlockID{ + Hash: crypto.CRandBytes(32), + PartSetHeader: parts.Header(), + } + require.False(t, blockIDMatches(block, parts, wrongHash)) + require.False(t, blockIDMatches(nil, parts, matching)) + require.False(t, blockIDMatches(block, nil, matching)) + require.False(t, blockIDMatches(block, parts, types.BlockID{})) + require.False(t, blockIDMatches(block, parts, types.BlockID{ + Hash: hash, // missing PartSetHeader → incomplete + })) +} + +func TestProposalMatchesLocked(t *testing.T) { + block := testBlock(t) + parts, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + require.True(t, proposalMatchesLocked(block, block, parts, parts)) + + otherPartsHeader := types.PartSetHeader{Total: 1, Hash: crypto.CRandBytes(32)} + otherParts := types.NewPartSetFromHeader(otherPartsHeader) + require.False(t, proposalMatchesLocked(block, block, parts, otherParts)) +} + +func TestNonCanonicalPartSetSameHeaderHash(t *testing.T) { + block := testBlock(t) + canonical, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + pbb, err := block.ToProto() + require.NoError(t, err) + bz, err := proto.Marshal(pbb) + require.NoError(t, err) + + // Append an unknown length-delimited protobuf field (field 999). + nonCanonical := append(append([]byte{}, bz...), 0xba, 0x3e, 0x04, 'j', 'u', 'n', 'k') + nonCanonicalParts := types.NewPartSetFromData(nonCanonical, types.BlockPartSizeBytes) + require.False(t, nonCanonicalParts.Header().Equals(canonical.Header())) + + var pbb2 tmproto.Block + require.NoError(t, proto.Unmarshal(nonCanonical, &pbb2)) + decoded, err := types.BlockFromProto(&pbb2) + require.NoError(t, err) + require.True(t, decoded.HashesTo(block.Hash()), "logical header hash unchanged") + require.False(t, blockIDMatches(decoded, nonCanonicalParts, types.BlockID{ + Hash: block.Hash(), + PartSetHeader: canonical.Header(), + })) + require.True(t, blockIDMatches(decoded, nonCanonicalParts, types.BlockID{ + Hash: block.Hash(), + PartSetHeader: nonCanonicalParts.Header(), + })) +} + +func TestCanonicalPartBytesRoundTripShapes(t *testing.T) { + ctx := t.Context() + config := configSetup(t) + cs, _ := makeState(ctx, t, makeStateArgs{config: config, validators: 2}) + + valAddr := crypto.CRandBytes(crypto.AddressSize) + sig := utils.OrPanic1(crypto.SigFromBytes(crypto.CRandBytes(64))) + mixedCommit := &types.Commit{ + Height: 1, + Round: 0, + BlockID: types.BlockID{ + Hash: crypto.CRandBytes(32), + PartSetHeader: types.PartSetHeader{ + Total: 1, + Hash: crypto.CRandBytes(32), + }, + }, + Signatures: []types.CommitSig{ + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: valAddr, + Timestamp: cs.state.LastBlockTime, + Signature: utils.Some(sig), + }, + types.NewCommitSigAbsent(), + }, + } + + cases := []struct { + name string + block *types.Block + }{ + {name: "empty", block: testBlock(t)}, + {name: "with_txs", block: testBlockWith(t, types.Txs{[]byte("tx-a"), []byte("tx-b")}, nil)}, + {name: "mixed_last_commit", block: testBlockWith(t, types.Txs{[]byte("tx")}, mixedCommit)}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + parts, err := tc.block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + // Production path: assemble part bytes → decode → remake PartSetHeader. + // Comparing MakePartSet(tc.block) to itself would be a tautology; the + // invariant is that MakePartSet(decoded) reproduces the proposer's header. + bz, err := io.ReadAll(parts.GetReader()) + require.NoError(t, err) + var pbb tmproto.Block + require.NoError(t, proto.Unmarshal(bz, &pbb)) + decoded, err := types.BlockFromProto(&pbb) + require.NoError(t, err) + + cs.roundState.SetProposal(nil) + cs.roundState.SetProposalBlockParts(parts) + require.NoError(t, cs.verifyCanonicalProposalParts(decoded)) + + // Trailing unknown field → different PartSetHeader under the same chunk size. + junk := append(append([]byte{}, bz...), 0xba, 0x3e, 0x04, 'j', 'u', 'n', 'k') + cs.roundState.SetProposalBlockParts(types.NewPartSetFromData(junk, types.BlockPartSizeBytes)) + err = cs.verifyCanonicalProposalParts(decoded) + require.ErrorIs(t, err, ErrNonCanonicalProposalParts) + }) + } +} + +// Same canonical bytes with non-default part size yield a different PartSetHeader +// and must be rejected (blocksync rebuilds with BlockPartSizeBytes). +func TestRejectNonDefaultPartChunking(t *testing.T) { + ctx := t.Context() + config := configSetup(t) + cs, _ := makeState(ctx, t, makeStateArgs{config: config, validators: 2}) + + // Large enough that a smaller part size splits into multiple parts. + block := testBlockWith(t, types.Txs{make([]byte, 8*1024)}, nil) + pbb, err := block.ToProto() + require.NoError(t, err) + bz, err := proto.Marshal(pbb) + require.NoError(t, err) + + altPartSize := uint32(512) + require.Greater(t, len(bz), int(altPartSize)) + altParts := types.NewPartSetFromData(bz, altPartSize) + canonical, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + require.False(t, altParts.Header().Equals(canonical.Header())) + require.Greater(t, altParts.Total(), uint32(1)) + + cs.roundState.SetProposal(nil) + cs.roundState.SetProposalBlockParts(altParts) + err = cs.verifyCanonicalProposalParts(block) + require.ErrorIs(t, err, ErrNonCanonicalProposalParts) +} diff --git a/sei-tendermint/internal/consensus/metrics.gen.go b/sei-tendermint/internal/consensus/metrics.gen.go index 69f41c6d5b..f2c4de97fa 100644 --- a/sei-tendermint/internal/consensus/metrics.gen.go +++ b/sei-tendermint/internal/consensus/metrics.gen.go @@ -34,6 +34,7 @@ func init() { Global.StepDuration, Global.BlockGossipReceiveLatency, Global.BlockGossipPartsReceived, + Global.NonCanonicalProposalParts, Global.ProposalBlockCreatedOnPropose, Global.ProposalTxs, Global.ProposalMissingTxs, @@ -201,6 +202,12 @@ func NewMetrics() *Metrics { Name: "block_gossip_parts_received", Help: "Number of block parts received by the node, separated by whether the part was relevant to the block the node is trying to gather or not.", }, []string{"matches_current"}), + NonCanonicalProposalParts: tmprometheus.NewCounterIntVec(prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystem, + Name: "non_canonical_proposal_parts", + Help: "Number of non-canonical complete proposal part sets rejected, labeled by consensus step.", + }, []string{"step"}), ProposalBlockCreatedOnPropose: tmprometheus.NewCounterIntVec(prometheus.CounterOpts{ Namespace: MetricsNamespace, Subsystem: MetricsSubsystem, @@ -417,6 +424,10 @@ func (m *Metrics) BlockGossipPartsReceivedAt(matches_current string) *tmpromethe return m.BlockGossipPartsReceived.WithLabelValues(matches_current) } +func (m *Metrics) NonCanonicalProposalPartsAt(step string) *tmprometheus.CounterInt { + return m.NonCanonicalProposalParts.WithLabelValues(step) +} + func (m *Metrics) ProposalBlockCreatedOnProposeAt(success string) *tmprometheus.CounterInt { return m.ProposalBlockCreatedOnPropose.WithLabelValues(success) } diff --git a/sei-tendermint/internal/consensus/metrics.go b/sei-tendermint/internal/consensus/metrics.go index bfce5cc9c8..f4158ded8e 100644 --- a/sei-tendermint/internal/consensus/metrics.go +++ b/sei-tendermint/internal/consensus/metrics.go @@ -89,6 +89,14 @@ type Metrics struct { // was relevant to the block the node is trying to gather or not. BlockGossipPartsReceived tmprometheus.CounterIntVec `metrics_labels:"matches_current"` + // NonCanonicalProposalParts counts complete proposal assemblies rejected + // because the assembled PartSetHeader did not equal + // MakePartSet(block, BlockPartSizeBytes) — non-canonical encoding or + // non-default chunking. Labeled by the consensus step at rejection time so + // post-commit stalls are alertable. + //metrics:Number of non-canonical complete proposal part sets rejected, labeled by consensus step. + NonCanonicalProposalParts tmprometheus.CounterIntVec `metrics_labels:"step"` + // Number of proposal blocks created on propose received. ProposalBlockCreatedOnPropose tmprometheus.CounterIntVec `metrics_labels:"success"` diff --git a/sei-tendermint/internal/consensus/state.go b/sei-tendermint/internal/consensus/state.go index f385b6ceb2..26fd2a1541 100644 --- a/sei-tendermint/internal/consensus/state.go +++ b/sei-tendermint/internal/consensus/state.go @@ -1465,7 +1465,10 @@ func (cs *State) defaultDoPrevote(ctx context.Context, height int64, round int32 cs.signAddVote(ctx, tmproto.PrevoteType, cs.roundState.ProposalBlock().Hash(), cs.roundState.ProposalBlockParts().Header()) return } - if cs.roundState.ProposalBlock().HashesTo(cs.roundState.LockedBlock().Hash()) { + if proposalMatchesLocked( + cs.roundState.ProposalBlock(), cs.roundState.LockedBlock(), + cs.roundState.ProposalBlockParts(), cs.roundState.LockedBlockParts(), + ) { logger.Info("prevote step: ProposalBlock is valid and matches our locked block; prevoting the proposal") cs.signAddVote(ctx, tmproto.PrevoteType, cs.roundState.ProposalBlock().Hash(), cs.roundState.ProposalBlockParts().Header()) return @@ -1490,14 +1493,18 @@ func (cs *State) defaultDoPrevote(ctx context.Context, height int64, round int32 missed the proposal in round 'v_r'. */ blockID, ok := cs.roundState.Votes().Prevotes(cs.roundState.Proposal().POLRound).TwoThirdsMajority() - if ok && cs.roundState.ProposalBlock().HashesTo(blockID.Hash) && cs.roundState.Proposal().POLRound >= 0 && cs.roundState.Proposal().POLRound < cs.roundState.Round() { + if ok && blockIDMatches(cs.roundState.ProposalBlock(), cs.roundState.ProposalBlockParts(), blockID) && + cs.roundState.Proposal().POLRound >= 0 && cs.roundState.Proposal().POLRound < cs.roundState.Round() { if cs.roundState.LockedRound() <= cs.roundState.Proposal().POLRound { logger.Info("prevote step: ProposalBlock is valid and received a 2/3" + "majority in a round later than the locked round; prevoting the proposal") cs.signAddVote(ctx, tmproto.PrevoteType, cs.roundState.ProposalBlock().Hash(), cs.roundState.ProposalBlockParts().Header()) return } - if cs.roundState.ProposalBlock().HashesTo(cs.roundState.LockedBlock().Hash()) { + if proposalMatchesLocked( + cs.roundState.ProposalBlock(), cs.roundState.LockedBlock(), + cs.roundState.ProposalBlockParts(), cs.roundState.LockedBlockParts(), + ) { logger.Info("prevote step: ProposalBlock is valid and matches our locked block; prevoting the proposal") cs.signAddVote(ctx, tmproto.PrevoteType, cs.roundState.ProposalBlock().Hash(), cs.roundState.ProposalBlockParts().Header()) return @@ -1621,8 +1628,9 @@ func (cs *State) enterPrecommit(ctx context.Context, height int64, round int32, return } - // If we're already locked on that block, precommit it, and update the LockedRound - if cs.roundState.LockedBlock().HashesTo(blockID.Hash) { + // If we're already locked on that block, precommit it, and update the LockedRound. + // Match full BlockID (hash + PartSetHeader), not header hash alone. + if blockIDMatches(cs.roundState.LockedBlock(), cs.roundState.LockedBlockParts(), blockID) { logger.Info("precommit step: +2/3 prevoted locked block; relocking") cs.roundState.SetLockedRound(round) @@ -1637,7 +1645,7 @@ func (cs *State) enterPrecommit(ctx context.Context, height int64, round int32, // If greater than 2/3 of the voting power on the network prevoted for // the proposed block, update our locked block to this block and issue a // precommit vote for it. - if cs.roundState.ProposalBlock().HashesTo(blockID.Hash) { + if blockIDMatches(cs.roundState.ProposalBlock(), cs.roundState.ProposalBlockParts(), blockID) { logger.Info("precommit step: +2/3 prevoted proposal block; locking", "hash", blockID.Hash) // Validate the block. @@ -1743,9 +1751,9 @@ func (cs *State) enterCommit(ctx context.Context, height int64, commitRound int3 } // The Locked* fields no longer matter. - // Move them over to ProposalBlock if they match the commit hash, - // otherwise they'll be cleared in updateToState. - if cs.roundState.LockedBlock().HashesTo(blockID.Hash) { + // Move them over to ProposalBlock if they match the commit BlockID + // (hash + PartSetHeader), otherwise they'll be cleared in updateToState. + if blockIDMatches(cs.roundState.LockedBlock(), cs.roundState.LockedBlockParts(), blockID) { logger.Info("commit is for a locked block; set ProposalBlock=LockedBlock", "block_hash", blockID.Hash) cs.roundState.SetProposalBlockParts(cs.roundState.LockedBlockParts()) cs.roundState.SetProposalBlock(cs.roundState.LockedBlock()) @@ -1789,7 +1797,7 @@ func (cs *State) tryFinalizeCommit(ctx context.Context, height int64) { return } - if !cs.roundState.ProposalBlock().HashesTo(blockID.Hash) { + if !blockIDMatches(cs.roundState.ProposalBlock(), cs.roundState.ProposalBlockParts(), blockID) { // TODO: this happens every time if we're not a validator (ugly logs) // TODO: ^^ wait, why does it matter that we're a validator? logger.Info( @@ -1827,11 +1835,11 @@ func (cs *State) finalizeCommit(ctx context.Context, height int64) { if !ok { panic("cannot finalize commit; commit does not have 2/3 majority") } - if !blockParts.HasHeader(blockID.PartSetHeader) { - panic("expected ProposalBlockParts header to be commit header") - } - if !block.HashesTo(blockID.Hash) { - panic("cannot finalize commit; proposal block does not hash to commit hash") + if !blockIDMatches(block, blockParts, blockID) { + panic(fmt.Sprintf( + "cannot finalize commit; proposal block/parts do not match commit BlockID: block=%X parts=%v commit=%v", + block.Hash(), blockParts.Header(), blockID, + )) } if err := cs.blockExec.ValidateBlock(ctx, cs.state, block); err != nil { @@ -2170,6 +2178,17 @@ func (cs *State) addProposalBlockPart( logger.Error("Encountered error building block from parts", "block parts", cs.roundState.ProposalBlockParts()) return false, err } + if err := cs.verifyCanonicalProposalParts(block); err != nil { + Global.NonCanonicalProposalPartsAt(cs.roundState.Step().String()).Add(1) + logger.Error( + "rejecting non-canonical proposal block parts", + "err", err, + "height", height, + "round", round, + "step", cs.roundState.Step().String(), + ) + return false, err + } cs.roundState.SetProposalBlock(block) // NOTE: it's possible to receive complete proposal blocks for future rounds without having the proposal @@ -2232,6 +2251,17 @@ func (cs *State) tryCreateProposalBlock(ctx context.Context) bool { logger.Error("Encountered error building block from parts", "block parts", cs.roundState.ProposalBlockParts()) return false } + if err := cs.verifyCanonicalProposalParts(block); err != nil { + Global.NonCanonicalProposalPartsAt(cs.roundState.Step().String()).Add(1) + logger.Error( + "rejecting non-canonical proposal block parts", + "err", err, + "height", cs.roundState.Height(), + "round", cs.roundState.Round(), + "step", cs.roundState.Step().String(), + ) + return false + } cs.roundState.SetProposalBlock(block) return true } @@ -2275,7 +2305,9 @@ func (cs *State) tryCreateProposalBlock(ctx context.Context) bool { return false } - // Now check if parts were actually expected. + // Now check if parts were actually expected. newParts comes from MakePartSet, + // so matching headers already implies the canonical PartSetHeader; a further + // verifyCanonicalProposalParts call would be a tautology. if !parts.Header().Equals(newParts.Header()) { return false } @@ -2308,7 +2340,7 @@ func (cs *State) handleCompleteProposal(ctx context.Context, height int64, handl prevotes := cs.roundState.Votes().Prevotes(cs.roundState.Round()) blockID, hasTwoThirds := prevotes.TwoThirdsMajority() if hasTwoThirds && !blockID.IsNil() && (cs.roundState.ValidRound() < cs.roundState.Round()) { - if cs.roundState.ProposalBlock().HashesTo(blockID.Hash) { + if blockIDMatches(cs.roundState.ProposalBlock(), cs.roundState.ProposalBlockParts(), blockID) { logger.Debug( "updating valid block to new proposal block", "valid_round", cs.roundState.Round(), @@ -2479,7 +2511,7 @@ func (cs *State) addVote( // Update Valid* if we can. if cs.roundState.ValidRound() < vote.Round && vote.Round == cs.roundState.Round() { - if cs.roundState.ProposalBlock().HashesTo(blockID.Hash) { + if blockIDMatches(cs.roundState.ProposalBlock(), cs.roundState.ProposalBlockParts(), blockID) { logger.Debug("updating valid block because of POL", "valid_round", cs.roundState.ValidRound(), "pol_round", vote.Round) cs.roundState.SetValidRound(vote.Round) cs.roundState.SetValidBlock(cs.roundState.ProposalBlock())