feat(wireguard): per-instance MaxCount + extend to Autobahn fields#3606
Conversation
PR SummaryMedium Risk Overview Autobahn protos now annotate repeated fields with Enforcement path: Semantics change: Plugin fix: proto3 Reviewed by Cursor Bugbot for commit 5ac7d77. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3606 +/- ##
==========================================
- Coverage 59.02% 58.14% -0.88%
==========================================
Files 2215 2150 -65
Lines 182513 174057 -8456
==========================================
- Hits 107720 101212 -6508
+ Misses 65101 63857 -1244
+ Partials 9692 8988 -704
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…c (CON-298 follow-up) Annotates all repeated fields in autobahn.proto with (wireguard.max_count), then generalises the wiring so no per-channel or per-call plumbing is needed: - Plugin now emits WireguardScan([]byte) error on each schema-bearing type instead of init()/registry calls. - protoutils.Unmarshal[T] asserts wireScanner and scans before proto.Unmarshal automatically. - transport.go asserts wireScanner on the channel MessageType, replacing the explicit PreDecode field on ChannelDescriptorT. - Removes PreDecode from ChannelDescriptorT and all four reactor call-sites (blocksync, consensus, evidence, statesync); protection is now derived from the proto type, not the channel config. - Fixes plugin bug: proto3 optional fields were incorrectly treated as oneof variants, generating non-existent wrapper type names. - Wiring tests rewritten to call WireguardScan directly; no-op tests added for channels whose message variants don't reach a capped field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
api.wireguard.go was generated alongside autobahn.wireguard.go but missed from the initial commit — the giga LaneReq/LaneResp/etc types transitively reach the annotated Autobahn fields and get WireguardScan automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The (wireguard.max_count) options added to autobahn.proto change the embedded raw descriptor in autobahn.pb.go. Regenerated via sei-tendermint/internal/buf.gen.yaml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sigs, votes, lane_ranges, lane_qcs, and block headers in QC messages are bounded by the validator set size, which is far below 10000. Use 100 as a tighter, more accurate ceiling. Payload.txs stays at 2000. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wen-coding
force-pushed
the
wen/autobahn_wireguard_caps
branch
from
2d80c06 to
17b85f2
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 17b85f2. Configure here.
Add TestPlugin_Proto3OptionalDoesNotEmitWrapperType: a message with a proto3 optional field (synthetic oneof in the descriptor, no Go wrapper struct) alongside a capped repeated field. Asserts the plugin does not emit a nonexistent Foo_Bar wrapper type and correctly caps items. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| @@ -0,0 +1,11 @@ | |||
| version: v2 | |||
There was a problem hiding this comment.
this can be just added to the internal/buf.gen.yaml
| // wireScanner is implemented by proto types whose generated *.wireguard.go | ||
| // adds a WireguardScan method. Unmarshal calls it automatically before | ||
| // proto.Unmarshal so no per-call wiring is needed. | ||
| type wireScanner interface { |
There was a problem hiding this comment.
this should be exposed by wireguard
| require.NoError(t, ev.WireguardScan(wgtest.Marshal(t, &ev))) | ||
| } | ||
|
|
||
| func init() { |
There was a problem hiding this comment.
init wrapper is useless here
- Fold autobahn.wireguard.buf.gen.yaml into sei-tendermint/internal/buf.gen.yaml; the wireguard plugin now lives alongside the hashable plugin in one template - Export wireguard.Scanner interface from the wireguard package; remove the unexported wireScanner duplicate in protoutils/msg.go - Remove useless init() compile-time assertion from evidence/wiring_test.go Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the counts map was shared across the entire Scan call, so a cap on an inner field (e.g. LaneQC.sigs = 100) accumulated across all occurrences of the outer message (e.g. every LaneQC in lane_qcs). A FullProposal with 10 LaneQCs each carrying 10 sigs would hit the cap even though no single LaneQC was over limit. Fix: pass a fresh counts map when recursing into each nested-field occurrence. The cap is now checked per instance of the containing message, which is the correct local semantics. Update two tests that asserted the old global-accumulation behaviour; add TestScan_CountsArePerNestedInstance to cover the new contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a comment at the recursion site and in Rule.MaxCount explaining
that per-instance counting is both intuitive ("at most N per outer
element") and still bounds total memory by the product of caps along
the nesting path. Update tests to make the memory-bound reasoning
explicit in the comment.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wen-coding
changed the title
Remove SharedBudgetAcrossLastCommitAndEvidence and EvidenceCommitsShareBudget tests from blocksync, consensus, and types — these asserted global accumulation, which no longer holds. Replace with MultipleCommitsEachAtCapPass to document the new per-instance contract. Update RejectsDuplicateNonRepeatedFields → RejectsDuplicateNonRepeatedFieldOverCap: test that a single over-cap occurrence is still rejected, not that two at-cap occurrences sum past the cap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
Extends the pre-decode wire scanner from #3458 to Autobahn messages, and generalises the wiring so adding protection to a new message type requires only a proto annotation — no per-channel or per-call plumbing.
Proto annotations
autobahn.protogains(wireguard.max_count)on all repeated fields:LaneQC.sigs,PrepareQC.sigs,CommitQC.sigs,AppQC.sigsTimeoutQC.votesFullProposal.lane_qcs,Proposal.lane_ranges,FullCommitQC.headersPayload.txsMaxTxsPerBlock)Caps are checked per instance of the containing message, not summed globally. A
FullProposalwith 10LaneQCs each carrying 10 sigs is fine; total memory at any nesting depth is bounded by the product of the caps along the path (outer_cap × inner_cap × …). To raise a cap later, update(wireguard.max_count)inautobahn.protoand re-run:Automatic wiring (main change)
Previously, each channel had to set
PreDecodeexplicitly in the reactor. Now the schema is wired directly to the proto type:WireguardScan([]byte) erroron each schema-bearing type (instead ofinit()/registry calls).protoutils.Unmarshal[T]assertswireguard.Scannerand scans beforeproto.Unmarshal— zero cost for types without a schema.transport.goassertswireguard.Scanneron the channelMessageType, replacing thePreDecodefield onChannelDescriptorT.ChannelDescriptorT.PreDecoderemoved — all four explicit assignments (blocksync, consensus, evidence, statesync) dropped; protection follows from the type.wireguard package changes
wireguard.Scannerinterface exported from the wireguard package (was an unexported duplicate inprotoutils/msg.go).MaxCountsemantics tightened to per-instance: each nested-message occurrence gets a fresh counter map, so the cap is "at most N per outer element" rather than "N total across the payload".sei-tendermint/internal/buf.gen.yamlalongside the hashable plugin (separateautobahn.wireguard.buf.gen.yamldeleted).Plugin bug fix
Proto3
optionalfields produce a synthetic oneof in the descriptor but no wrapper struct in Go. The plugin was incorrectly using theMessage_Fieldwrapper name for them. Fixed with!f.ContainingOneof().IsSynthetic().Test plan
go test ./sei-tendermint/internal/autobahn/...— all autobahn packages passgo test ./sei-tendermint/internal/blocksync/... ./sei-tendermint/internal/statesync/... ./sei-tendermint/internal/consensus/... ./sei-tendermint/internal/evidence/...— wiring tests pass including no-op assertionsgo test ./sei-tendermint/internal/protoutils/...— wireguard scanner + plugin tests pass, including per-instance cap and proto3 optional testsgo build ./sei-tendermint/...— clean build🤖 Generated with Claude Code