From 73b002bc9be78e311ac099fc2d9e677bffac5d3a Mon Sep 17 00:00:00 2001 From: Wen Date: Thu, 30 Apr 2026 14:36:52 -0700 Subject: [PATCH 1/5] rpc: populate LastCommit on Autobahn-routed Block / BlockByHash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit translateGlobalBlock used to leave Block.LastCommit nil. Trace replay in evmrpc (StateAtTransaction → initializeBlock → ToReqBeginBlock) unconditionally dereferences `b.LastCommit.Signatures[i]` to build the BeginBlock VoteInfo slice, so the nil deref crashed the whole RPC handler with a generic "method handler crashed" — debug_traceCall was the most visible symptom but any path through ToReqBeginBlock panicked the same way. Fix: rebuild a CometBFT-shaped LastCommit from the CommitQC that finalized the prior block. We read it best-effort from data.State (committed alongside the block, pruned alongside it on the same RetainHeight schedule) and translate the QC's Signatures into a []CommitSig sized to and ordered by genDoc.Validators — that's the order /validators returns under Autobahn, which ToReqBeginBlock pairs with Signatures[i] by index. Validators present in the QC get BlockIDFlagCommit + their derived address + the raw ed25519 signature; the rest get BlockIDFlagAbsent. When the prior QC isn't available (genesis block or pruned past the retain window) we emit an all-absent slice; downstream consumers iterate without panicking, distribution.BeginBlocker sees no signers and distributes nothing during replay (correct since the original commit already paid out). Mechanics: - types/commit_qc.go: add CommitQC.Sigs() iter.Seq[*Signature] (zero-copy, single-pass — matches SortedSet.All() pattern). - types/msg.go: expose Signature.Key()/Sig() and PublicKey.Address() so the translator can pull what it needs without poking package-internal fields. - p2p/giga_router.go: translateGlobalBlock takes ctx, fetches the prior CommitQC via fetchCommitQC, and feeds it through commitSigsFromQC. fetchCommitQC maps data.ErrPruned → None and surfaces every other error. The fetch+translate logic is shared between BlockByNumber and BlockByHash. Tests: - TestCommitSigsFromQC: full-committee, strict-subset, None, committee-drift (extra signer not in genVals), and empty-genVals cases. Verifies sig bytes round-trip and Absent entries pass CommitSig.ValidateBasic (zero address, zero timestamp, no sig). - TestGigaRouter_FinalizeBlocks asserts LastCommit is populated end-to-end, sized to genDoc.Validators. Verified live with `make autobahn-integration-test` (5/5 subtests green) and curl probes against a 4-node cluster: debug_traceCall and debug_traceTransaction (callTracer + prestateTracer/diffMode) all return proper traces, no more "method handler crashed". Stacks on #3310 (wen/fix_evm_rpc_for_autobahn) — that PR adds translateGlobalBlock and the routed Block / BlockByHash entry points this change extends. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/autobahn/types/commit_qc.go | 19 +++ sei-tendermint/internal/autobahn/types/msg.go | 13 ++ sei-tendermint/internal/p2p/giga_router.go | 88 +++++++++- .../internal/p2p/giga_router_test.go | 151 ++++++++++++++++++ 4 files changed, 267 insertions(+), 4 deletions(-) diff --git a/sei-tendermint/internal/autobahn/types/commit_qc.go b/sei-tendermint/internal/autobahn/types/commit_qc.go index 3f2562cc78..96316523f1 100644 --- a/sei-tendermint/internal/autobahn/types/commit_qc.go +++ b/sei-tendermint/internal/autobahn/types/commit_qc.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "iter" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/autobahn/pb" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/protoutils" @@ -45,6 +46,24 @@ func (m *CommitQC) GlobalRange(c *Committee) GlobalRange { return m.Proposal().GlobalRange(c) } +// Sigs iterates the validator signatures recorded in this QC, in the +// order they were added. Zero-copy: the iterator yields the QC's own +// *Signature pointers. The CommitQC type carries utils.ReadOnly, and +// Signature exposes only Key/Sig accessors, so callers can't mutate +// QC-internal state via the yielded pointers. Used by +// p2p.GigaRouter.translateGlobalBlock to reconstruct CometBFT-shaped +// LastCommit signatures during trace replay; the QC always represents +// at least CommitQuorum signers. +func (m *CommitQC) Sigs() iter.Seq[*Signature] { + return func(yield func(*Signature) bool) { + for _, s := range m.sigs { + if !yield(s) { + return + } + } + } +} + // Verify verifies the CommitQC against the committee. // Currently it doesn't require the previous CommitQC. func (m *CommitQC) Verify(c *Committee) error { diff --git a/sei-tendermint/internal/autobahn/types/msg.go b/sei-tendermint/internal/autobahn/types/msg.go index 10a5fda531..c86e27aa0b 100644 --- a/sei-tendermint/internal/autobahn/types/msg.go +++ b/sei-tendermint/internal/autobahn/types/msg.go @@ -10,6 +10,7 @@ import ( "github.com/sei-protocol/sei-chain/sei-tendermint/internal/autobahn/pb" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/hashable" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/protoutils" + tmbytes "github.com/sei-protocol/sei-chain/sei-tendermint/libs/bytes" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" ) @@ -96,6 +97,12 @@ func (k PublicKey) Compare(other PublicKey) int { return k.key.Compare(other.key // Bytes converts the public key to bytes. func (k PublicKey) Bytes() []byte { return k.key.Bytes() } +// Address returns the SHA-256-truncated tendermint validator address derived +// from the underlying ed25519 public key, matching CommitSig.ValidatorAddress +// produced on the CometBFT path. Used by GigaRouter.translateGlobalBlock to +// fill LastCommit entries during trace replay. +func (k PublicKey) Address() tmbytes.HexBytes { return k.key.Address() } + // PublicKeyFromBytes constructs a public key from bytes. func PublicKeyFromBytes(b []byte) (PublicKey, error) { k, err := ed25519.PublicKeyFromBytes(b) @@ -169,6 +176,12 @@ type Signature struct { sig ed25519.Signature } +// Key returns the public key of the validator that produced this signature. +func (s *Signature) Key() PublicKey { return s.key } + +// Sig returns the raw ed25519 signature bytes. +func (s *Signature) Sig() ed25519.Signature { return s.sig } + // Signed is a hashed message with its signature. type Signed[T Msg] struct { utils.ReadOnly diff --git a/sei-tendermint/internal/p2p/giga_router.go b/sei-tendermint/internal/p2p/giga_router.go index 8c81500cc5..267f7c704a 100644 --- a/sei-tendermint/internal/p2p/giga_router.go +++ b/sei-tendermint/internal/p2p/giga_router.go @@ -167,7 +167,7 @@ func (r *GigaRouter) BlockByNumber(ctx context.Context, n atypes.GlobalBlockNumb } return nil, fmt.Errorf("data.GlobalBlock(%v): %w", n, err) } - return r.translateGlobalBlock(gb), nil + return r.translateGlobalBlock(ctx, gb) } // BlockByHash returns the finalized global block keyed by Autobahn block- @@ -184,7 +184,7 @@ func (r *GigaRouter) BlockByNumber(ctx context.Context, n atypes.GlobalBlockNumb // TODO(autobahn): replace this with a direct read from // sei-db/ledger_db/block.BlockDB.GetBlockByHash once a writer is wired into // block execution. The data.State-side index can also go away at that point. -func (r *GigaRouter) BlockByHash(_ context.Context, hash atypes.BlockHeaderHash) (*coretypes.ResultBlock, error) { +func (r *GigaRouter) BlockByHash(ctx context.Context, hash atypes.BlockHeaderHash) (*coretypes.ResultBlock, error) { opt, err := r.data.GlobalBlockByHash(hash) if err != nil { return nil, fmt.Errorf("data.GlobalBlockByHash: %w", err) @@ -198,7 +198,7 @@ func (r *GigaRouter) BlockByHash(_ context.Context, hash atypes.BlockHeaderHash) if !ok { return &coretypes.ResultBlock{}, nil } - return r.translateGlobalBlock(gb), nil + return r.translateGlobalBlock(ctx, gb) } // translateGlobalBlock converts an Autobahn GlobalBlock to the CometBFT @@ -209,7 +209,27 @@ func (r *GigaRouter) BlockByHash(_ context.Context, hash atypes.BlockHeaderHash) // dereferences b.Header without a nil-check on the same type. The // "no such block" case is rejected at the BlockByHash call site // before delegating here. -func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock) *coretypes.ResultBlock { +// +// LastCommit is built from the CommitQC that finalized +// gb.GlobalNumber-1 — the CometBFT-shape semantic for block N's +// LastCommit (proof of N-1's commit, included in N's data). When the +// prior QC is unavailable (genesis block or pruned out of data.State's +// retain window) the resulting LastCommit carries an all-absent +// signature slice sized to genDoc.Validators, which is enough for +// downstream consumers including trace replay's BeginBlock to run +// without panicking. Genuine QC fetch errors (ctx cancel, etc.) +// propagate. +func (r *GigaRouter) translateGlobalBlock(ctx context.Context, gb *atypes.GlobalBlock) (*coretypes.ResultBlock, error) { + // LastCommit of block n carries the proof that n-1 was committed. + // gb.GlobalNumber-1 doesn't underflow: NewGigaRouter rejects + // InitialHeight<1, Committee.FirstBlock()=InitialHeight>=1, and + // data.State only hands out blocks at >=FirstBlock. fetchCommitQC + // maps the sub-genesis read at n-1 (i.e. when n==FirstBlock) to + // ErrPruned → None, which commitSigsFromQC turns into all-absent. + lastQC, err := r.fetchCommitQC(ctx, gb.GlobalNumber-1) + if err != nil { + return nil, err + } srcTxs := gb.Payload.Txs() tmTxs := make(types.Txs, len(srcTxs)) for i, tx := range srcTxs { @@ -228,8 +248,68 @@ func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock) *coretypes.Res Time: gb.Timestamp, }, Data: types.Data{Txs: tmTxs}, + LastCommit: &types.Commit{ + // genDoc.Validators is the order /validators returns + // under Autobahn; ToReqBeginBlock pairs Signatures[i] + // with vals[i] by index, so we order to match. + Signatures: commitSigsFromQC(r.cfg.GenDoc.Validators, lastQC, gb.Timestamp), + }, }, + }, nil +} + +// fetchCommitQC returns the CommitQC that finalized block n. Returns +// None when the QC has been pruned out of data.State's retain window — +// that's expected for blocks beyond Sei's MinRetainBlocks horizon and +// for sub-genesis lookups; downstream consumers turn None into an +// all-absent LastCommit. Genuine errors (ctx cancellation, etc.) +// propagate so callers don't silently mask them. +func (r *GigaRouter) fetchCommitQC(ctx context.Context, n atypes.GlobalBlockNumber) (utils.Option[*atypes.CommitQC], error) { + full, err := r.data.QC(ctx, n) + if errors.Is(err, data.ErrPruned) { + return utils.None[*atypes.CommitQC](), nil + } + if err != nil { + return utils.None[*atypes.CommitQC](), fmt.Errorf("data.QC(%v): %w", n, err) + } + return utils.Some(full.QC()), nil +} + +// commitSigsFromQC translates an Autobahn CommitQC into the CometBFT +// []CommitSig shape, sized to and ordered by genVals — Block.ToReqBeginBlock +// pairs Signatures[i] with vals[i] by index, and `vals` here comes from +// /validators (genDoc order under Autobahn), so we order to match. +// Validators whose key appears in the QC's signatures get +// BlockIDFlagCommit with their derived address and the raw ed25519 sig; +// the rest get BlockIDFlagAbsent. None qc → all-absent (genesis block +// or pruned-prior cases). ts is applied uniformly since Autobahn QCs +// don't carry per-vote timestamps; downstream consumers that use +// Timestamp (none on the trace path) see the block timestamp. +func commitSigsFromQC(genVals []types.GenesisValidator, qc utils.Option[*atypes.CommitQC], ts time.Time) []types.CommitSig { + sigs := make([]types.CommitSig, len(genVals)) + // Key the map on raw pubkey bytes so we don't have to convert + // between crypto.PubKey (genVals) and atypes.PublicKey (QC sigs); + // both wrap the same ed25519 value and Bytes() returns the same + // canonical 32 bytes on both sides. + sigByKeyBytes := map[string]*atypes.Signature{} + if commitQC, ok := qc.Get(); ok { + for s := range commitQC.Sigs() { + sigByKeyBytes[string(s.Key().Bytes())] = s + } + } + for i, gv := range genVals { + if s, ok := sigByKeyBytes[string(gv.PubKey.Bytes())]; ok { + sigs[i] = types.CommitSig{ + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: gv.PubKey.Address(), + Timestamp: ts, + Signature: utils.Some(s.Sig()), + } + } else { + sigs[i] = types.CommitSig{BlockIDFlag: types.BlockIDFlagAbsent} + } } + return sigs } func (r *GigaRouter) executeBlock(ctx context.Context, b *atypes.GlobalBlock) (*abci.ResponseCommit, error) { diff --git a/sei-tendermint/internal/p2p/giga_router_test.go b/sei-tendermint/internal/p2p/giga_router_test.go index 075a976f61..63be38b6de 100644 --- a/sei-tendermint/internal/p2p/giga_router_test.go +++ b/sei-tendermint/internal/p2p/giga_router_test.go @@ -372,6 +372,13 @@ func TestGigaRouter_FinalizeBlocks(t *testing.T) { require.Equal(t, committed, rb.Block.Height, "router[%v].BlockByNumber(%v) height", i, committed) require.NotEmpty(t, rb.BlockID.Hash, "router[%v].BlockByNumber(%v) block hash", i, committed) require.Equal(t, genDoc.ChainID, rb.Block.Header.ChainID, "router[%v].BlockByNumber(%v) chain id", i, committed) + // LastCommit must be populated so trace replay's BeginBlock + // (which iterates Signatures by index against genDoc.Validators) + // doesn't nil-deref. We don't assert per-signer flags here — + // those are unit-tested in TestCommitSigsFromQC — just that + // the slice is sized to the validator set. + require.NotNil(t, rb.Block.LastCommit, "router[%v].BlockByNumber(%v) LastCommit", i, committed) + require.Len(t, rb.Block.LastCommit.Signatures, len(genDoc.Validators), "router[%v].BlockByNumber(%v) Signatures len", i, committed) // Round-trip the just-fetched block hash back through // BlockByHash and assert we get the same ResultBlock back. var hashKey atypes.BlockHeaderHash @@ -407,3 +414,147 @@ func TestGigaRouter_FinalizeBlocks(t *testing.T) { }) require.NoError(t, err) } + +// TestCommitSigsFromQC covers the LastCommit-shape translation that +// GigaRouter.translateGlobalBlock applies on every Block / BlockByHash +// response. Each subtest builds a fresh test environment so failures +// describe a single case in isolation. +// +// Beyond happy-path full/subset signing, two boundary cases get +// dedicated coverage: +// - None qc: simulates genesis or pruned-prior; every position must +// come back Absent with zero Timestamp/empty Signature so that +// types.CommitSig.ValidateBasic accepts each entry. +// - QC carries a signer not in genVals: simulates committee drift +// after a validator-set change. The extra signer is silently +// dropped, all genVals still resolve to their correct slots, no +// panic. Catches a subtle alignment-by-iteration bug where an +// extraneous signer would shift indexes. +func TestCommitSigsFromQC(t *testing.T) { + rng := utils.TestRng() + // Build 4 keys; genVals stays in insertion order (NOT sorted by + // pubkey) so the test catches any code path that incorrectly sorts. + keys := make([]atypes.SecretKey, 4) + genVals := make([]types.GenesisValidator, 4) + for i := range keys { + keys[i] = atypes.GenSecretKey(rng) + pub, err := ed25519.PublicKeyFromBytes(keys[i].Public().Bytes()) + require.NoError(t, err) + genVals[i] = types.GenesisValidator{ + Address: pub.Address(), + PubKey: pub, + Power: 10, + Name: fmt.Sprintf("v%d", i), + } + } + ts := time.Unix(1_700_000_000, 0) + + // mkQC returns a CommitQC where each signerSecrets entry signs the + // same CommitVote, plus the matching []*atypes.Signed slice so the + // test can pull out exact ed25519 bytes for sig-byte assertions. + // NewCommitQC requires len > 0, so empty-signer cases are tested + // through the None path. + mkQC := func(signerSecrets []atypes.SecretKey) (utils.Option[*atypes.CommitQC], []*atypes.Signed[*atypes.CommitVote]) { + vote := atypes.GenCommitVote(rng) + signed := make([]*atypes.Signed[*atypes.CommitVote], len(signerSecrets)) + for i, sk := range signerSecrets { + signed[i] = atypes.Sign(sk, vote) + } + return utils.Some(atypes.NewCommitQC(signed)), signed + } + + // expectedSigBytes maps the secret key's pubkey-bytes to the raw + // ed25519 sig bytes the corresponding Signed produced. Lets us + // assert Signature carries the actual signing material, not just + // "something non-None". + sigBytesByKey := func(signed []*atypes.Signed[*atypes.CommitVote]) map[string][]byte { + m := map[string][]byte{} + for _, sm := range signed { + m[string(sm.Sig().Key().Bytes())] = sm.Sig().Sig().Bytes() + } + return m + } + + assertAbsent := func(t *testing.T, sig types.CommitSig, msg string) { + t.Helper() + require.Equal(t, types.BlockIDFlagAbsent, sig.BlockIDFlag, "%s: BlockIDFlag", msg) + require.Empty(t, sig.ValidatorAddress, "%s: address must be empty per ValidateBasic", msg) + require.True(t, sig.Timestamp.IsZero(), "%s: timestamp must be zero per ValidateBasic", msg) + require.False(t, sig.Signature.IsPresent(), "%s: signature must be absent", msg) + } + + t.Run("full committee signs", func(t *testing.T) { + qc, signed := mkQC(keys) + want := sigBytesByKey(signed) + sigs := commitSigsFromQC(genVals, qc, ts) + require.Len(t, sigs, len(genVals)) + for i, sig := range sigs { + require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) + // sigs[i] aligns with genVals[i] (insertion order). + require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) + require.Equal(t, ts, sig.Timestamp, "sigs[%d] timestamp", i) + gotSig, ok := sig.Signature.Get() + require.True(t, ok, "sigs[%d] should carry the ed25519 signature", i) + require.Equal(t, want[string(genVals[i].PubKey.Bytes())], gotSig.Bytes(), "sigs[%d] signature bytes", i) + } + }) + + t.Run("strict subset of committee signs", func(t *testing.T) { + // genVals[0] and genVals[2] sign. + subset := []atypes.SecretKey{keys[0], keys[2]} + qc, signed := mkQC(subset) + want := sigBytesByKey(signed) + didSign := map[int]bool{0: true, 2: true} + sigs := commitSigsFromQC(genVals, qc, ts) + require.Len(t, sigs, len(genVals)) + for i, sig := range sigs { + if didSign[i] { + require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) + require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) + require.Equal(t, ts, sig.Timestamp, "sigs[%d] timestamp", i) + gotSig, ok := sig.Signature.Get() + require.True(t, ok, "sigs[%d] should carry the ed25519 signature", i) + require.Equal(t, want[string(genVals[i].PubKey.Bytes())], gotSig.Bytes(), "sigs[%d] signature bytes", i) + } else { + assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) + } + } + }) + + t.Run("None qc (genesis or pruned)", func(t *testing.T) { + sigs := commitSigsFromQC(genVals, utils.None[*atypes.CommitQC](), ts) + require.Len(t, sigs, len(genVals), "all-absent slice still sized to validator set") + for i, sig := range sigs { + assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) + } + }) + + t.Run("QC signer not in genVals (committee drift)", func(t *testing.T) { + // QC contains genVals[0], genVals[2], and one extra signer that + // isn't in genVals at all. The extra is dropped silently; the + // other two still land in their correct slots. + extra := atypes.GenSecretKey(rng) + signers := []atypes.SecretKey{keys[0], keys[2], extra} + qc, _ := mkQC(signers) + didSign := map[int]bool{0: true, 2: true} + sigs := commitSigsFromQC(genVals, qc, ts) + require.Len(t, sigs, len(genVals)) + for i, sig := range sigs { + if didSign[i] { + require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) + require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) + } else { + assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) + } + } + }) + + t.Run("empty genVals", func(t *testing.T) { + // Edge: a chain with no validators (impossible in practice, but + // the function must not panic — used to share early-exit code + // with downstream consumers that may iterate len(sigs)). + qc, _ := mkQC(keys) + require.Empty(t, commitSigsFromQC(nil, qc, ts)) + require.Empty(t, commitSigsFromQC(nil, utils.None[*atypes.CommitQC](), ts)) + }) +} From 2bab2ca60347506d29ca02f86efcf08f61a5f6b5 Mon Sep 17 00:00:00 2001 From: Wen Date: Fri, 1 May 2026 14:53:28 -0700 Subject: [PATCH 2/5] rpc: address #3349 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes per review: 1. translateGlobalBlock is now a pure transform — takes the already- loaded prior CommitQC as an argument instead of accessing storage internally. Callers (BlockByNumber, BlockByHash) explicitly fetch the QC via loadPriorCommitQC and pass it in. Shifts the load-vs-translate concerns into separate functions. 2. loadPriorCommitQC has an explicit `if n == 0 { return None }` guard to avoid uint64 underflow on n-1, even though data.State only hands out blocks at >=FirstBlock>=1 per NewGigaRouter. Belt-and-suspenders defensive check matches reviewer preference. Bonus: rename fetchCommitQC → loadPriorCommitQC (it's specifically the "prior block's QC for this block's LastCommit") and shift the n-1 arithmetic into the helper, so call sites just pass `gb.GlobalNumber`. Co-Authored-By: Claude Opus 4.7 (1M context) --- sei-tendermint/internal/p2p/giga_router.go | 81 +++++++++++----------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/sei-tendermint/internal/p2p/giga_router.go b/sei-tendermint/internal/p2p/giga_router.go index 267f7c704a..71bc91abd9 100644 --- a/sei-tendermint/internal/p2p/giga_router.go +++ b/sei-tendermint/internal/p2p/giga_router.go @@ -167,7 +167,11 @@ func (r *GigaRouter) BlockByNumber(ctx context.Context, n atypes.GlobalBlockNumb } return nil, fmt.Errorf("data.GlobalBlock(%v): %w", n, err) } - return r.translateGlobalBlock(ctx, gb) + lastQC, err := r.loadPriorCommitQC(ctx, gb.GlobalNumber) + if err != nil { + return nil, err + } + return r.translateGlobalBlock(gb, lastQC), nil } // BlockByHash returns the finalized global block keyed by Autobahn block- @@ -198,38 +202,30 @@ func (r *GigaRouter) BlockByHash(ctx context.Context, hash atypes.BlockHeaderHas if !ok { return &coretypes.ResultBlock{}, nil } - return r.translateGlobalBlock(ctx, gb) -} - -// translateGlobalBlock converts an Autobahn GlobalBlock to the CometBFT -// coretypes.ResultBlock shape used by env.Block / env.BlockByHash and -// downstream evmrpc consumers. Caller must pass a non-nil *GlobalBlock -// with non-nil Header and Payload — that's the contract data.State -// guarantees on a successful lookup, and matches how executeBlock -// dereferences b.Header without a nil-check on the same type. The -// "no such block" case is rejected at the BlockByHash call site -// before delegating here. -// -// LastCommit is built from the CommitQC that finalized -// gb.GlobalNumber-1 — the CometBFT-shape semantic for block N's -// LastCommit (proof of N-1's commit, included in N's data). When the -// prior QC is unavailable (genesis block or pruned out of data.State's -// retain window) the resulting LastCommit carries an all-absent -// signature slice sized to genDoc.Validators, which is enough for -// downstream consumers including trace replay's BeginBlock to run -// without panicking. Genuine QC fetch errors (ctx cancel, etc.) -// propagate. -func (r *GigaRouter) translateGlobalBlock(ctx context.Context, gb *atypes.GlobalBlock) (*coretypes.ResultBlock, error) { - // LastCommit of block n carries the proof that n-1 was committed. - // gb.GlobalNumber-1 doesn't underflow: NewGigaRouter rejects - // InitialHeight<1, Committee.FirstBlock()=InitialHeight>=1, and - // data.State only hands out blocks at >=FirstBlock. fetchCommitQC - // maps the sub-genesis read at n-1 (i.e. when n==FirstBlock) to - // ErrPruned → None, which commitSigsFromQC turns into all-absent. - lastQC, err := r.fetchCommitQC(ctx, gb.GlobalNumber-1) + lastQC, err := r.loadPriorCommitQC(ctx, gb.GlobalNumber) if err != nil { return nil, err } + return r.translateGlobalBlock(gb, lastQC), nil +} + +// translateGlobalBlock is a pure transform: Autobahn GlobalBlock + already- +// loaded prior-block CommitQC → CometBFT coretypes.ResultBlock. No storage +// access; callers fetch the QC via loadPriorCommitQC and pass it in. +// +// Caller must pass a non-nil *GlobalBlock with non-nil Header and Payload — +// that's the contract data.State guarantees on a successful lookup, and +// matches how executeBlock dereferences b.Header without a nil-check on the +// same type. The "no such block" case is rejected at the BlockByHash call +// site before delegating here. +// +// lastQC is the CommitQC that finalized gb.GlobalNumber-1 — the CometBFT- +// shape semantic for block N's LastCommit (proof of N-1's commit, included +// in N's data). lastQC=None produces an all-absent signature slice sized +// to genDoc.Validators, which is enough for downstream consumers including +// trace replay's BeginBlock to run without panicking; genesis blocks and +// pruned-prior cases use that path. +func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock, lastQC utils.Option[*atypes.CommitQC]) *coretypes.ResultBlock { srcTxs := gb.Payload.Txs() tmTxs := make(types.Txs, len(srcTxs)) for i, tx := range srcTxs { @@ -255,22 +251,27 @@ func (r *GigaRouter) translateGlobalBlock(ctx context.Context, gb *atypes.Global Signatures: commitSigsFromQC(r.cfg.GenDoc.Validators, lastQC, gb.Timestamp), }, }, - }, nil + } } -// fetchCommitQC returns the CommitQC that finalized block n. Returns -// None when the QC has been pruned out of data.State's retain window — -// that's expected for blocks beyond Sei's MinRetainBlocks horizon and -// for sub-genesis lookups; downstream consumers turn None into an -// all-absent LastCommit. Genuine errors (ctx cancellation, etc.) -// propagate so callers don't silently mask them. -func (r *GigaRouter) fetchCommitQC(ctx context.Context, n atypes.GlobalBlockNumber) (utils.Option[*atypes.CommitQC], error) { - full, err := r.data.QC(ctx, n) +// loadPriorCommitQC returns the CommitQC that finalized the block before n, +// for the LastCommit field of block n. Genesis case (n==0, which can't +// actually happen since data.State only hands out blocks at >=FirstBlock>=1 +// per NewGigaRouter, but guarded explicitly to avoid uint64 underflow on +// n-1) returns None directly without a storage hit. Pruned-prior case maps +// data.ErrPruned to None — expected for blocks beyond Sei's MinRetainBlocks +// horizon. Both are surfaced as all-absent LastCommit by commitSigsFromQC. +// Genuine errors (ctx cancellation, etc.) propagate. +func (r *GigaRouter) loadPriorCommitQC(ctx context.Context, n atypes.GlobalBlockNumber) (utils.Option[*atypes.CommitQC], error) { + if n == 0 { + return utils.None[*atypes.CommitQC](), nil + } + full, err := r.data.QC(ctx, n-1) if errors.Is(err, data.ErrPruned) { return utils.None[*atypes.CommitQC](), nil } if err != nil { - return utils.None[*atypes.CommitQC](), fmt.Errorf("data.QC(%v): %w", n, err) + return utils.None[*atypes.CommitQC](), fmt.Errorf("data.QC(%v): %w", n-1, err) } return utils.Some(full.QC()), nil } From aa930d97e05961a4a9895df34a5b988f94519dfc Mon Sep 17 00:00:00 2001 From: Wen Date: Fri, 1 May 2026 15:11:58 -0700 Subject: [PATCH 3/5] =?UTF-8?q?rpc:=20simplify=20LastCommit=20to=20all-Abs?= =?UTF-8?q?ent=20=E2=80=94=20drop=20CommitQC=20integration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per pompon0 (#3349 review): "the commitqc of previous block is with high probability the same as commitqc of the current block. Do we care? Can we just return empty LastCommit instead? It is not like it provides tendermint-compatible values anyway." He's right. The values we synthesized — raw ed25519 sigs, single uniform timestamp, addresses derived from genDoc keys — aren't tendermint-compatible (CometBFT VoteInfo carries per-vote timestamps and round info that Autobahn's QC doesn't track). And the BeginBlock handlers that read LastCommitInfo (distribution, slashing) are no-ops during trace replay anyway: distribution wouldn't pay rewards a second time on an already-committed block; slashing's missed-block counter update lands in a snapshot/copy state that's discarded. ToReqBeginBlock's only requirement is `LastCommit != nil` and `len(Signatures) >= len(vals)` — beyond that the *values* don't matter on the trace path. So sized-with-all-Absent is sufficient. Net delete: - loadPriorCommitQC + commitSigsFromQC + the n==0 guard (~50 lines) - TestCommitSigsFromQC's 5 subtests (~150 lines, validating logic that no longer exists) - CommitQC.Sigs() iter, Signature.Key()/Sig(), PublicKey.Address() — all added solely for the QC translation, now unused TestGigaRouter_FinalizeBlocks asserts the property that actually matters on the trace path: LastCommit non-nil, Signatures sized to genDoc.Validators, all entries Absent. Verified: debug_traceTransaction (callTracer + prestateTracer/diffMode) and debug_traceCall both return proper traces on a fresh 4-node Autobahn cluster — no "method handler crashed". 3/3 EVMCompatabilityTest trace tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/autobahn/types/commit_qc.go | 19 --- sei-tendermint/internal/autobahn/types/msg.go | 13 -- sei-tendermint/internal/p2p/giga_router.go | 117 +++---------- .../internal/p2p/giga_router_test.go | 158 +----------------- 4 files changed, 34 insertions(+), 273 deletions(-) diff --git a/sei-tendermint/internal/autobahn/types/commit_qc.go b/sei-tendermint/internal/autobahn/types/commit_qc.go index 96316523f1..3f2562cc78 100644 --- a/sei-tendermint/internal/autobahn/types/commit_qc.go +++ b/sei-tendermint/internal/autobahn/types/commit_qc.go @@ -2,7 +2,6 @@ package types import ( "fmt" - "iter" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/autobahn/pb" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/protoutils" @@ -46,24 +45,6 @@ func (m *CommitQC) GlobalRange(c *Committee) GlobalRange { return m.Proposal().GlobalRange(c) } -// Sigs iterates the validator signatures recorded in this QC, in the -// order they were added. Zero-copy: the iterator yields the QC's own -// *Signature pointers. The CommitQC type carries utils.ReadOnly, and -// Signature exposes only Key/Sig accessors, so callers can't mutate -// QC-internal state via the yielded pointers. Used by -// p2p.GigaRouter.translateGlobalBlock to reconstruct CometBFT-shaped -// LastCommit signatures during trace replay; the QC always represents -// at least CommitQuorum signers. -func (m *CommitQC) Sigs() iter.Seq[*Signature] { - return func(yield func(*Signature) bool) { - for _, s := range m.sigs { - if !yield(s) { - return - } - } - } -} - // Verify verifies the CommitQC against the committee. // Currently it doesn't require the previous CommitQC. func (m *CommitQC) Verify(c *Committee) error { diff --git a/sei-tendermint/internal/autobahn/types/msg.go b/sei-tendermint/internal/autobahn/types/msg.go index c86e27aa0b..10a5fda531 100644 --- a/sei-tendermint/internal/autobahn/types/msg.go +++ b/sei-tendermint/internal/autobahn/types/msg.go @@ -10,7 +10,6 @@ import ( "github.com/sei-protocol/sei-chain/sei-tendermint/internal/autobahn/pb" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/hashable" "github.com/sei-protocol/sei-chain/sei-tendermint/internal/protoutils" - tmbytes "github.com/sei-protocol/sei-chain/sei-tendermint/libs/bytes" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" ) @@ -97,12 +96,6 @@ func (k PublicKey) Compare(other PublicKey) int { return k.key.Compare(other.key // Bytes converts the public key to bytes. func (k PublicKey) Bytes() []byte { return k.key.Bytes() } -// Address returns the SHA-256-truncated tendermint validator address derived -// from the underlying ed25519 public key, matching CommitSig.ValidatorAddress -// produced on the CometBFT path. Used by GigaRouter.translateGlobalBlock to -// fill LastCommit entries during trace replay. -func (k PublicKey) Address() tmbytes.HexBytes { return k.key.Address() } - // PublicKeyFromBytes constructs a public key from bytes. func PublicKeyFromBytes(b []byte) (PublicKey, error) { k, err := ed25519.PublicKeyFromBytes(b) @@ -176,12 +169,6 @@ type Signature struct { sig ed25519.Signature } -// Key returns the public key of the validator that produced this signature. -func (s *Signature) Key() PublicKey { return s.key } - -// Sig returns the raw ed25519 signature bytes. -func (s *Signature) Sig() ed25519.Signature { return s.sig } - // Signed is a hashed message with its signature. type Signed[T Msg] struct { utils.ReadOnly diff --git a/sei-tendermint/internal/p2p/giga_router.go b/sei-tendermint/internal/p2p/giga_router.go index 71bc91abd9..537fe3089a 100644 --- a/sei-tendermint/internal/p2p/giga_router.go +++ b/sei-tendermint/internal/p2p/giga_router.go @@ -167,11 +167,7 @@ func (r *GigaRouter) BlockByNumber(ctx context.Context, n atypes.GlobalBlockNumb } return nil, fmt.Errorf("data.GlobalBlock(%v): %w", n, err) } - lastQC, err := r.loadPriorCommitQC(ctx, gb.GlobalNumber) - if err != nil { - return nil, err - } - return r.translateGlobalBlock(gb, lastQC), nil + return r.translateGlobalBlock(gb), nil } // BlockByHash returns the finalized global block keyed by Autobahn block- @@ -202,35 +198,36 @@ func (r *GigaRouter) BlockByHash(ctx context.Context, hash atypes.BlockHeaderHas if !ok { return &coretypes.ResultBlock{}, nil } - lastQC, err := r.loadPriorCommitQC(ctx, gb.GlobalNumber) - if err != nil { - return nil, err - } - return r.translateGlobalBlock(gb, lastQC), nil + return r.translateGlobalBlock(gb), nil } -// translateGlobalBlock is a pure transform: Autobahn GlobalBlock + already- -// loaded prior-block CommitQC → CometBFT coretypes.ResultBlock. No storage -// access; callers fetch the QC via loadPriorCommitQC and pass it in. -// -// Caller must pass a non-nil *GlobalBlock with non-nil Header and Payload — -// that's the contract data.State guarantees on a successful lookup, and -// matches how executeBlock dereferences b.Header without a nil-check on the -// same type. The "no such block" case is rejected at the BlockByHash call -// site before delegating here. +// translateGlobalBlock converts an Autobahn GlobalBlock to the CometBFT +// coretypes.ResultBlock shape used by env.Block / env.BlockByHash and +// downstream evmrpc consumers. Caller must pass a non-nil *GlobalBlock with +// non-nil Header and Payload — that's the contract data.State guarantees on +// a successful lookup, and matches how executeBlock dereferences b.Header +// without a nil-check on the same type. The "no such block" case is +// rejected at the BlockByHash call site before delegating here. // -// lastQC is the CommitQC that finalized gb.GlobalNumber-1 — the CometBFT- -// shape semantic for block N's LastCommit (proof of N-1's commit, included -// in N's data). lastQC=None produces an all-absent signature slice sized -// to genDoc.Validators, which is enough for downstream consumers including -// trace replay's BeginBlock to run without panicking; genesis blocks and -// pruned-prior cases use that path. -func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock, lastQC utils.Option[*atypes.CommitQC]) *coretypes.ResultBlock { +// LastCommit is sized to len(genDoc.Validators) with all entries +// BlockIDFlagAbsent. ToReqBeginBlock pairs Signatures[i] with vals[i] by +// index, so a correctly-sized slice is enough to avoid the nil/OOB deref +// on the trace replay path; the actual signer values aren't tendermint- +// compatible under Autobahn (no per-vote timestamps, sig-aggregation +// instead of per-validator sigs) and the BeginBlock handlers that read +// LastCommitInfo (distribution, slashing) are no-ops during replay anyway, +// so populating from the prior CommitQC adds storage IO without buying +// anything observable. +func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock) *coretypes.ResultBlock { srcTxs := gb.Payload.Txs() tmTxs := make(types.Txs, len(srcTxs)) for i, tx := range srcTxs { tmTxs[i] = tx } + absentSigs := make([]types.CommitSig, len(r.cfg.GenDoc.Validators)) + for i := range absentSigs { + absentSigs[i] = types.CommitSig{BlockIDFlag: types.BlockIDFlagAbsent} + } h := gb.Header.Hash() return &coretypes.ResultBlock{ BlockID: types.BlockID{Hash: tmbytes.HexBytes(h.Bytes())}, @@ -243,76 +240,12 @@ func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock, lastQC utils.O Height: utils.Clamp[int64](gb.GlobalNumber), Time: gb.Timestamp, }, - Data: types.Data{Txs: tmTxs}, - LastCommit: &types.Commit{ - // genDoc.Validators is the order /validators returns - // under Autobahn; ToReqBeginBlock pairs Signatures[i] - // with vals[i] by index, so we order to match. - Signatures: commitSigsFromQC(r.cfg.GenDoc.Validators, lastQC, gb.Timestamp), - }, + Data: types.Data{Txs: tmTxs}, + LastCommit: &types.Commit{Signatures: absentSigs}, }, } } -// loadPriorCommitQC returns the CommitQC that finalized the block before n, -// for the LastCommit field of block n. Genesis case (n==0, which can't -// actually happen since data.State only hands out blocks at >=FirstBlock>=1 -// per NewGigaRouter, but guarded explicitly to avoid uint64 underflow on -// n-1) returns None directly without a storage hit. Pruned-prior case maps -// data.ErrPruned to None — expected for blocks beyond Sei's MinRetainBlocks -// horizon. Both are surfaced as all-absent LastCommit by commitSigsFromQC. -// Genuine errors (ctx cancellation, etc.) propagate. -func (r *GigaRouter) loadPriorCommitQC(ctx context.Context, n atypes.GlobalBlockNumber) (utils.Option[*atypes.CommitQC], error) { - if n == 0 { - return utils.None[*atypes.CommitQC](), nil - } - full, err := r.data.QC(ctx, n-1) - if errors.Is(err, data.ErrPruned) { - return utils.None[*atypes.CommitQC](), nil - } - if err != nil { - return utils.None[*atypes.CommitQC](), fmt.Errorf("data.QC(%v): %w", n-1, err) - } - return utils.Some(full.QC()), nil -} - -// commitSigsFromQC translates an Autobahn CommitQC into the CometBFT -// []CommitSig shape, sized to and ordered by genVals — Block.ToReqBeginBlock -// pairs Signatures[i] with vals[i] by index, and `vals` here comes from -// /validators (genDoc order under Autobahn), so we order to match. -// Validators whose key appears in the QC's signatures get -// BlockIDFlagCommit with their derived address and the raw ed25519 sig; -// the rest get BlockIDFlagAbsent. None qc → all-absent (genesis block -// or pruned-prior cases). ts is applied uniformly since Autobahn QCs -// don't carry per-vote timestamps; downstream consumers that use -// Timestamp (none on the trace path) see the block timestamp. -func commitSigsFromQC(genVals []types.GenesisValidator, qc utils.Option[*atypes.CommitQC], ts time.Time) []types.CommitSig { - sigs := make([]types.CommitSig, len(genVals)) - // Key the map on raw pubkey bytes so we don't have to convert - // between crypto.PubKey (genVals) and atypes.PublicKey (QC sigs); - // both wrap the same ed25519 value and Bytes() returns the same - // canonical 32 bytes on both sides. - sigByKeyBytes := map[string]*atypes.Signature{} - if commitQC, ok := qc.Get(); ok { - for s := range commitQC.Sigs() { - sigByKeyBytes[string(s.Key().Bytes())] = s - } - } - for i, gv := range genVals { - if s, ok := sigByKeyBytes[string(gv.PubKey.Bytes())]; ok { - sigs[i] = types.CommitSig{ - BlockIDFlag: types.BlockIDFlagCommit, - ValidatorAddress: gv.PubKey.Address(), - Timestamp: ts, - Signature: utils.Some(s.Sig()), - } - } else { - sigs[i] = types.CommitSig{BlockIDFlag: types.BlockIDFlagAbsent} - } - } - return sigs -} - func (r *GigaRouter) executeBlock(ctx context.Context, b *atypes.GlobalBlock) (*abci.ResponseCommit, error) { app := r.cfg.TxMempool.App() hash := b.Header.Hash() diff --git a/sei-tendermint/internal/p2p/giga_router_test.go b/sei-tendermint/internal/p2p/giga_router_test.go index 63be38b6de..61bfd7f0f0 100644 --- a/sei-tendermint/internal/p2p/giga_router_test.go +++ b/sei-tendermint/internal/p2p/giga_router_test.go @@ -372,13 +372,17 @@ func TestGigaRouter_FinalizeBlocks(t *testing.T) { require.Equal(t, committed, rb.Block.Height, "router[%v].BlockByNumber(%v) height", i, committed) require.NotEmpty(t, rb.BlockID.Hash, "router[%v].BlockByNumber(%v) block hash", i, committed) require.Equal(t, genDoc.ChainID, rb.Block.Header.ChainID, "router[%v].BlockByNumber(%v) chain id", i, committed) - // LastCommit must be populated so trace replay's BeginBlock - // (which iterates Signatures by index against genDoc.Validators) - // doesn't nil-deref. We don't assert per-signer flags here — - // those are unit-tested in TestCommitSigsFromQC — just that - // the slice is sized to the validator set. + // LastCommit must be non-nil and Signatures sized to the + // validator set so trace replay's BeginBlock (which iterates + // Signatures by index against genDoc.Validators) doesn't + // nil-deref or OOB. All entries are BlockIDFlagAbsent — see + // translateGlobalBlock for why we don't bother populating + // from the prior CommitQC. require.NotNil(t, rb.Block.LastCommit, "router[%v].BlockByNumber(%v) LastCommit", i, committed) require.Len(t, rb.Block.LastCommit.Signatures, len(genDoc.Validators), "router[%v].BlockByNumber(%v) Signatures len", i, committed) + for j, s := range rb.Block.LastCommit.Signatures { + require.Equal(t, types.BlockIDFlagAbsent, s.BlockIDFlag, "router[%v].BlockByNumber(%v) Signatures[%d] flag", i, committed, j) + } // Round-trip the just-fetched block hash back through // BlockByHash and assert we get the same ResultBlock back. var hashKey atypes.BlockHeaderHash @@ -414,147 +418,3 @@ func TestGigaRouter_FinalizeBlocks(t *testing.T) { }) require.NoError(t, err) } - -// TestCommitSigsFromQC covers the LastCommit-shape translation that -// GigaRouter.translateGlobalBlock applies on every Block / BlockByHash -// response. Each subtest builds a fresh test environment so failures -// describe a single case in isolation. -// -// Beyond happy-path full/subset signing, two boundary cases get -// dedicated coverage: -// - None qc: simulates genesis or pruned-prior; every position must -// come back Absent with zero Timestamp/empty Signature so that -// types.CommitSig.ValidateBasic accepts each entry. -// - QC carries a signer not in genVals: simulates committee drift -// after a validator-set change. The extra signer is silently -// dropped, all genVals still resolve to their correct slots, no -// panic. Catches a subtle alignment-by-iteration bug where an -// extraneous signer would shift indexes. -func TestCommitSigsFromQC(t *testing.T) { - rng := utils.TestRng() - // Build 4 keys; genVals stays in insertion order (NOT sorted by - // pubkey) so the test catches any code path that incorrectly sorts. - keys := make([]atypes.SecretKey, 4) - genVals := make([]types.GenesisValidator, 4) - for i := range keys { - keys[i] = atypes.GenSecretKey(rng) - pub, err := ed25519.PublicKeyFromBytes(keys[i].Public().Bytes()) - require.NoError(t, err) - genVals[i] = types.GenesisValidator{ - Address: pub.Address(), - PubKey: pub, - Power: 10, - Name: fmt.Sprintf("v%d", i), - } - } - ts := time.Unix(1_700_000_000, 0) - - // mkQC returns a CommitQC where each signerSecrets entry signs the - // same CommitVote, plus the matching []*atypes.Signed slice so the - // test can pull out exact ed25519 bytes for sig-byte assertions. - // NewCommitQC requires len > 0, so empty-signer cases are tested - // through the None path. - mkQC := func(signerSecrets []atypes.SecretKey) (utils.Option[*atypes.CommitQC], []*atypes.Signed[*atypes.CommitVote]) { - vote := atypes.GenCommitVote(rng) - signed := make([]*atypes.Signed[*atypes.CommitVote], len(signerSecrets)) - for i, sk := range signerSecrets { - signed[i] = atypes.Sign(sk, vote) - } - return utils.Some(atypes.NewCommitQC(signed)), signed - } - - // expectedSigBytes maps the secret key's pubkey-bytes to the raw - // ed25519 sig bytes the corresponding Signed produced. Lets us - // assert Signature carries the actual signing material, not just - // "something non-None". - sigBytesByKey := func(signed []*atypes.Signed[*atypes.CommitVote]) map[string][]byte { - m := map[string][]byte{} - for _, sm := range signed { - m[string(sm.Sig().Key().Bytes())] = sm.Sig().Sig().Bytes() - } - return m - } - - assertAbsent := func(t *testing.T, sig types.CommitSig, msg string) { - t.Helper() - require.Equal(t, types.BlockIDFlagAbsent, sig.BlockIDFlag, "%s: BlockIDFlag", msg) - require.Empty(t, sig.ValidatorAddress, "%s: address must be empty per ValidateBasic", msg) - require.True(t, sig.Timestamp.IsZero(), "%s: timestamp must be zero per ValidateBasic", msg) - require.False(t, sig.Signature.IsPresent(), "%s: signature must be absent", msg) - } - - t.Run("full committee signs", func(t *testing.T) { - qc, signed := mkQC(keys) - want := sigBytesByKey(signed) - sigs := commitSigsFromQC(genVals, qc, ts) - require.Len(t, sigs, len(genVals)) - for i, sig := range sigs { - require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) - // sigs[i] aligns with genVals[i] (insertion order). - require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) - require.Equal(t, ts, sig.Timestamp, "sigs[%d] timestamp", i) - gotSig, ok := sig.Signature.Get() - require.True(t, ok, "sigs[%d] should carry the ed25519 signature", i) - require.Equal(t, want[string(genVals[i].PubKey.Bytes())], gotSig.Bytes(), "sigs[%d] signature bytes", i) - } - }) - - t.Run("strict subset of committee signs", func(t *testing.T) { - // genVals[0] and genVals[2] sign. - subset := []atypes.SecretKey{keys[0], keys[2]} - qc, signed := mkQC(subset) - want := sigBytesByKey(signed) - didSign := map[int]bool{0: true, 2: true} - sigs := commitSigsFromQC(genVals, qc, ts) - require.Len(t, sigs, len(genVals)) - for i, sig := range sigs { - if didSign[i] { - require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) - require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) - require.Equal(t, ts, sig.Timestamp, "sigs[%d] timestamp", i) - gotSig, ok := sig.Signature.Get() - require.True(t, ok, "sigs[%d] should carry the ed25519 signature", i) - require.Equal(t, want[string(genVals[i].PubKey.Bytes())], gotSig.Bytes(), "sigs[%d] signature bytes", i) - } else { - assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) - } - } - }) - - t.Run("None qc (genesis or pruned)", func(t *testing.T) { - sigs := commitSigsFromQC(genVals, utils.None[*atypes.CommitQC](), ts) - require.Len(t, sigs, len(genVals), "all-absent slice still sized to validator set") - for i, sig := range sigs { - assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) - } - }) - - t.Run("QC signer not in genVals (committee drift)", func(t *testing.T) { - // QC contains genVals[0], genVals[2], and one extra signer that - // isn't in genVals at all. The extra is dropped silently; the - // other two still land in their correct slots. - extra := atypes.GenSecretKey(rng) - signers := []atypes.SecretKey{keys[0], keys[2], extra} - qc, _ := mkQC(signers) - didSign := map[int]bool{0: true, 2: true} - sigs := commitSigsFromQC(genVals, qc, ts) - require.Len(t, sigs, len(genVals)) - for i, sig := range sigs { - if didSign[i] { - require.Equal(t, types.BlockIDFlagCommit, sig.BlockIDFlag, "sigs[%d]", i) - require.Equal(t, genVals[i].PubKey.Address().Bytes(), sig.ValidatorAddress.Bytes(), "sigs[%d] address", i) - } else { - assertAbsent(t, sig, fmt.Sprintf("sigs[%d]", i)) - } - } - }) - - t.Run("empty genVals", func(t *testing.T) { - // Edge: a chain with no validators (impossible in practice, but - // the function must not panic — used to share early-exit code - // with downstream consumers that may iterate len(sigs)). - qc, _ := mkQC(keys) - require.Empty(t, commitSigsFromQC(nil, qc, ts)) - require.Empty(t, commitSigsFromQC(nil, utils.None[*atypes.CommitQC](), ts)) - }) -} From 1a07abfdfcbc300fff53a8405c5f41c1139a3579 Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 4 May 2026 10:34:04 -0700 Subject: [PATCH 4/5] rpc: drop absent-sig padding from translateGlobalBlock for FinalizeBlock parity Mirror executeBlock's empty abci.CommitInfo on the read side: under Autobahn the committee is fixed by genesis and the application is not in control of jailing, so surfacing N "absent sig" entries to trace replay would diverge from production by bumping missed-block counters. Tighten ToReqBeginBlock to skip the per-validator loop when Signatures is empty so empty Votes flow into distribution/slashing on both paths (also retains the original OOB-guard the PR was after). Co-Authored-By: Claude Opus 4.7 (1M context) --- sei-tendermint/internal/p2p/giga_router.go | 23 +++++++---------- .../internal/p2p/giga_router_test.go | 17 ++++++------- sei-tendermint/types/block.go | 25 +++++++++++++------ 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/sei-tendermint/internal/p2p/giga_router.go b/sei-tendermint/internal/p2p/giga_router.go index 537fe3089a..8554f9040c 100644 --- a/sei-tendermint/internal/p2p/giga_router.go +++ b/sei-tendermint/internal/p2p/giga_router.go @@ -209,25 +209,20 @@ func (r *GigaRouter) BlockByHash(ctx context.Context, hash atypes.BlockHeaderHas // without a nil-check on the same type. The "no such block" case is // rejected at the BlockByHash call site before delegating here. // -// LastCommit is sized to len(genDoc.Validators) with all entries -// BlockIDFlagAbsent. ToReqBeginBlock pairs Signatures[i] with vals[i] by -// index, so a correctly-sized slice is enough to avoid the nil/OOB deref -// on the trace replay path; the actual signer values aren't tendermint- -// compatible under Autobahn (no per-vote timestamps, sig-aggregation -// instead of per-validator sigs) and the BeginBlock handlers that read -// LastCommitInfo (distribution, slashing) are no-ops during replay anyway, -// so populating from the prior CommitQC adds storage IO without buying -// anything observable. +// LastCommit is non-nil with empty Signatures, mirroring executeBlock's +// FinalizeBlock call which passes an empty abci.CommitInfo. Under Autobahn +// the committee is fixed by genesis (no validator-set updates), so the +// application is not in control of jailing — surfacing N "absent sig" +// entries here would make trace replay's BeginBlock bump missed-block +// counters and diverge from production. ToReqBeginBlock skips the per- +// validator loop when Signatures is empty, so empty Votes flow into +// distribution/slashing on both paths. func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock) *coretypes.ResultBlock { srcTxs := gb.Payload.Txs() tmTxs := make(types.Txs, len(srcTxs)) for i, tx := range srcTxs { tmTxs[i] = tx } - absentSigs := make([]types.CommitSig, len(r.cfg.GenDoc.Validators)) - for i := range absentSigs { - absentSigs[i] = types.CommitSig{BlockIDFlag: types.BlockIDFlagAbsent} - } h := gb.Header.Hash() return &coretypes.ResultBlock{ BlockID: types.BlockID{Hash: tmbytes.HexBytes(h.Bytes())}, @@ -241,7 +236,7 @@ func (r *GigaRouter) translateGlobalBlock(gb *atypes.GlobalBlock) *coretypes.Res Time: gb.Timestamp, }, Data: types.Data{Txs: tmTxs}, - LastCommit: &types.Commit{Signatures: absentSigs}, + LastCommit: &types.Commit{}, }, } } diff --git a/sei-tendermint/internal/p2p/giga_router_test.go b/sei-tendermint/internal/p2p/giga_router_test.go index 61bfd7f0f0..eb15d2593d 100644 --- a/sei-tendermint/internal/p2p/giga_router_test.go +++ b/sei-tendermint/internal/p2p/giga_router_test.go @@ -372,17 +372,14 @@ func TestGigaRouter_FinalizeBlocks(t *testing.T) { require.Equal(t, committed, rb.Block.Height, "router[%v].BlockByNumber(%v) height", i, committed) require.NotEmpty(t, rb.BlockID.Hash, "router[%v].BlockByNumber(%v) block hash", i, committed) require.Equal(t, genDoc.ChainID, rb.Block.Header.ChainID, "router[%v].BlockByNumber(%v) chain id", i, committed) - // LastCommit must be non-nil and Signatures sized to the - // validator set so trace replay's BeginBlock (which iterates - // Signatures by index against genDoc.Validators) doesn't - // nil-deref or OOB. All entries are BlockIDFlagAbsent — see - // translateGlobalBlock for why we don't bother populating - // from the prior CommitQC. + // LastCommit is non-nil with empty Signatures — mirrors + // executeBlock's FinalizeBlock(DecidedLastCommit: empty) + // so trace replay and production both see "no votes" on + // the prior block. ToReqBeginBlock skips the per-val loop + // when Signatures is empty, so this is also enough to + // avoid the OOB deref the original PR was guarding against. require.NotNil(t, rb.Block.LastCommit, "router[%v].BlockByNumber(%v) LastCommit", i, committed) - require.Len(t, rb.Block.LastCommit.Signatures, len(genDoc.Validators), "router[%v].BlockByNumber(%v) Signatures len", i, committed) - for j, s := range rb.Block.LastCommit.Signatures { - require.Equal(t, types.BlockIDFlagAbsent, s.BlockIDFlag, "router[%v].BlockByNumber(%v) Signatures[%d] flag", i, committed, j) - } + require.Empty(t, rb.Block.LastCommit.Signatures, "router[%v].BlockByNumber(%v) Signatures", i, committed) // Round-trip the just-fetched block hash back through // BlockByHash and assert we get the same ResultBlock back. var hashKey atypes.BlockHeaderHash diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index 07d85242d0..cc55b23529 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -251,13 +251,24 @@ func (b *Block) ToProto() (*tmproto.Block, error) { func (b *Block) ToReqBeginBlock(vals []*Validator) abci.RequestBeginBlock { tmHeader := b.Header.ToProto() - votes := make([]abci.VoteInfo, 0, b.LastCommit.Size()) - for i, val := range vals { - commitSig := b.LastCommit.Signatures[i] - votes = append(votes, abci.VoteInfo{ - Validator: TM2PB.Validator(val), - SignedLastBlock: commitSig.BlockIDFlag != BlockIDFlagAbsent, - }) + // When LastCommit carries no per-validator signatures, the application + // sees no votes for the prior block — no jailing, no precommit-power + // share. Autobahn-routed blocks land here on the trace replay path: + // giga_router.executeBlock calls FinalizeBlock with an empty CommitInfo + // (the committee is hardcoded by Autobahn, so per-vote info wouldn't + // be tendermint-compatible anyway), and translateGlobalBlock mirrors + // that empty shape on the read side. Skipping the val loop keeps the + // two paths consistent and avoids OOB-indexing Signatures[i]. + var votes []abci.VoteInfo + if len(b.LastCommit.Signatures) > 0 { + votes = make([]abci.VoteInfo, 0, b.LastCommit.Size()) + for i, val := range vals { + commitSig := b.LastCommit.Signatures[i] + votes = append(votes, abci.VoteInfo{ + Validator: TM2PB.Validator(val), + SignedLastBlock: commitSig.BlockIDFlag != BlockIDFlagAbsent, + }) + } } abciEvidence := b.Evidence.ToABCI() byzantineValidators := make([]abci.Evidence, 0, len(abciEvidence)) From afd93f8f546a2e0899e1c8ab74551f0548238e5f Mon Sep 17 00:00:00 2001 From: Wen Date: Mon, 4 May 2026 11:46:59 -0700 Subject: [PATCH 5/5] rpc: shorten ToReqBeginBlock empty-Signatures comment Production callers always pass a fully-sized Signatures slice (CometBFT enforces commitSize == valSetLen upstream), so the empty case is only reachable from the Autobahn trace path. Drop the long rationale and just say so. Co-Authored-By: Claude Opus 4.7 (1M context) --- sei-tendermint/types/block.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index cc55b23529..8382fcb430 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -251,14 +251,7 @@ func (b *Block) ToProto() (*tmproto.Block, error) { func (b *Block) ToReqBeginBlock(vals []*Validator) abci.RequestBeginBlock { tmHeader := b.Header.ToProto() - // When LastCommit carries no per-validator signatures, the application - // sees no votes for the prior block — no jailing, no precommit-power - // share. Autobahn-routed blocks land here on the trace replay path: - // giga_router.executeBlock calls FinalizeBlock with an empty CommitInfo - // (the committee is hardcoded by Autobahn, so per-vote info wouldn't - // be tendermint-compatible anyway), and translateGlobalBlock mirrors - // that empty shape on the read side. Skipping the val loop keeps the - // two paths consistent and avoids OOB-indexing Signatures[i]. + // b.LastCommit.Signatures is only empty on the trace path. var votes []abci.VoteInfo if len(b.LastCommit.Signatures) > 0 { votes = make([]abci.VoteInfo, 0, b.LastCommit.Size())