fix: set max packet msg payload default to 1MB #2725
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd299f3ec9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
sei-tendermint/config/config.go
Outdated
| // Ethernet is 1500 - 20 -20 = 1460 | ||
| // Source: https://stackoverflow.com/a/3074427/820520 | ||
| MaxPacketMsgPayloadSize: 1400, | ||
| MaxPacketMsgPayloadSize: 1048576, |
There was a problem hiding this comment.
With the default MaxPacketMsgPayloadSize raised to 1MB, upgraded nodes will emit PacketMsg payloads up to 1MB, while older nodes still cap protoio.NewDelimitedReader using their 1400-byte max (via MConnection.maxPacketMsgSize()), so any message >1400 triggers a read error and drops the peer. This makes rolling upgrades or mixed-version networks unable to communicate unless there is negotiation or the default stays at 1400.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2725 +/- ##
==========================================
- Coverage 44.02% 44.00% -0.03%
==========================================
Files 1986 1986
Lines 162970 162970
==========================================
- Hits 71753 71720 -33
- Misses 84692 84725 +33
Partials 6525 6525
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
sei-tendermint/config/config.go
Outdated
| // Ethernet is 1500 - 20 -20 = 1460 | ||
| // Source: https://stackoverflow.com/a/3074427/820520 | ||
| MaxPacketMsgPayloadSize: 1400, | ||
| MaxPacketMsgPayloadSize: 1048576, |
There was a problem hiding this comment.
how about we hardcode it instead? Changing this value to literally anything else breaks the protocol.
There was a problem hiding this comment.
also remove the comment, it is not valid any more with this pr.
There was a problem hiding this comment.
this is hardcoded in DefaultP2PConfig(), and existing nodes won’t pick up changes unless they update config manually. unless we want to remove this param from config.
There was a problem hiding this comment.
good point, removed.
## Describe your changes and provide context ## Testing performed to validate your change (cherry picked from commit 3889e9a)
|
Successfully created backport PR for |
* main: update: set MaxPacketMsgPayloadSize use MB unit (#2729) fix: set max packet msg payload default to 1MB (#2725) Add CI workflow to build libwasmvm dynamic libraries (#2724) Made tcp connection context-aware (#2718) tcp multiplexer for sei giga (#2679) [STO-237] remove unused cosmos invariants (#2719)
Describe your changes and provide context
Testing performed to validate your change