Skip to content

feat(rpc): wire UnmarshalWithLimit into Stream.Recv#3630

Open
wen-coding wants to merge 4 commits into
mainfrom
wen/unmarshal_with_limit_callsites
Open

feat(rpc): wire UnmarshalWithLimit into Stream.Recv#3630
wen-coding wants to merge 4 commits into
mainfrom
wen/unmarshal_with_limit_callsites

Conversation

@wen-coding

Copy link
Copy Markdown
Contributor

Summary

Stream.Recv is the single decode point for all inbound P2P v2 proto messages.
This PR replaces protoutils.Unmarshal with protoutils.UnmarshalWithLimit
there, bounding heap allocation per message to MsgSize * allocMultiplier
before Unmarshal runs.

  • allocMultiplier defaults to 2. The largest inbound message is
    LaneProposal at ~2.07 MB wire; load testing showed its alloc estimate fits
    comfortably within 4 MB (2 × MsgSize).
  • P2PConfig.ProtoAllocLimitMultiplier (TOML key proto-alloc-limit-multiplier,
    default 2) lets operators override the multiplier without recompiling.
  • rpc.SetAllocMultiplier is called once in createRouter and applies to all
    subsequently opened streams.
  • The Autobahn / GigaRouter hot-path (gogo-proto messages) is not touched.

Files changed

File Change
sei-tendermint/internal/p2p/rpc/rpc.go Add allocLimit to Stream; compute from MsgSize × allocMultiplier in Call/Serve; call UnmarshalWithLimit in Recv
sei-tendermint/config/config.go Add ProtoAllocLimitMultiplier int to P2PConfig, default 2
sei-tendermint/node/setup.go Call rpc.SetAllocMultiplier from createRouter when config value is non-zero

Test plan

  • go test ./sei-tendermint/internal/p2p/rpc/... — existing tests pass with new Recv signature
  • go test ./sei-tendermint/internal/protoutils/...UnmarshalWithLimit unit tests still green
  • go build ./sei-tendermint/... — compiles cleanly
  • Manual: set proto-alloc-limit-multiplier = 1 in config, confirm a large block message is rejected when wire bytes exceed MsgSize

🤖 Generated with Claude Code

…verride

Stream.Recv now calls protoutils.UnmarshalWithLimit instead of Unmarshal,
bounding per-message heap allocation to MsgSize * allocMultiplier.

The multiplier defaults to 2 (a value load-tested against the largest
message, LaneProposal at ~2MB wire / ~4MB alloc estimate). It is
configurable via P2PConfig.ProtoAllocLimitMultiplier and applied at node
startup through rpc.SetAllocMultiplier.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cursor

cursor Bot commented Jun 23, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes the single decode path for all inbound P2P v2 RPC messages; an overly tight multiplier could reject valid large payloads, while the cap is meant to reduce allocation-amplification abuse.

Overview
Stream.Recv now decodes inbound P2P v2 protobuf through protoutils.UnmarshalWithLimit instead of unbounded Unmarshal, so heap use is capped before decode runs.

Each stream gets an allocLimit derived from that RPC’s configured MsgSize × recvAllocMultiplier (default 2): Call uses the response spec (client-side receives), Serve uses the request spec (server-side receives). The multiplier is documented against load testing on large messages such as LaneProposal (~2 MB wire).

Reviewed by Cursor Bugbot for commit 2fdc4ac. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 23, 2026, 11:15 PM

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.16%. Comparing base (51eb1fd) to head (2fdc4ac).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3630      +/-   ##
==========================================
- Coverage   59.03%   58.16%   -0.87%     
==========================================
  Files        2243     2169      -74     
  Lines      184689   176141    -8548     
==========================================
- Hits       109028   102452    -6576     
+ Misses      65933    64661    -1272     
+ Partials     9728     9028     -700     
Flag Coverage Δ
sei-chain-pr 86.56% <100.00%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-tendermint/internal/p2p/rpc/rpc.go 86.56% <100.00%> (ø)

... and 75 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

wen-coding and others added 3 commits June 23, 2026 15:43
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant