feat: mpool msg limits #5732
Merged
feat: mpool msg limits #5732
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LesnyRumcajs
force-pushed
the
mpoll-msg-limits
branch
15 times, most recently
from
9091ffe to
39218ff
Compare
LesnyRumcajs
force-pushed
the
mpoll-msg-limits
branch
3 times, most recently
from
e4c59ae to
6913987
Compare
LesnyRumcajs
commented
Jul 4, 2025
Comment on lines
+16
to
+25
| # This test checks the limitations of the message pool, which can take a while. | ||
| [[profile.default.overrides]] | ||
| filter = 'test(message_pool::msgpool::selection::test_selection::message_selection_trimming_msgs_two_senders)' | ||
| slow-timeout = { period = "120s", terminate-after = 3 } | ||
|
|
||
| # This test checks the limitations of the message pool, which can take a while. | ||
| [[profile.default.overrides]] | ||
| filter = 'test(message_pool::msgpool::selection::test_selection::message_selection_trimming_msgs_two_senders_complex)' | ||
| slow-timeout = { period = "120s", terminate-after = 3 } | ||
|
|
There was a problem hiding this comment.
This could be mitigated by #5812. I'm not certain that's the culprit, but my spidey senses tell me that it's the only reasonable possibility.
LesnyRumcajs
force-pushed
the
mpoll-msg-limits
branch
from
273cb4c to
8a37ea8
Compare
LesnyRumcajs
commented
Jul 4, 2025
| @@ -218,7 +226,7 @@ impl Chains { | |||
| chain_node.msgs.clear(); | |||
| chain_node.valid = false; | |||
| } else { | |||
| chain_node.msgs.drain(0..i as usize + 1); | |||
| chain_node.msgs.truncate(i as usize + 1); | |||
There was a problem hiding this comment.
outstanding bug from 2021 that made on of the new tests fail
LesnyRumcajs
commented
Jul 4, 2025
| @@ -429,7 +439,7 @@ where | |||
| let network_version = chain_config.network_version(ts.epoch()); | |||
|
|
|||
| let min_gas = price_list_by_network_version(network_version) | |||
| .on_chain_message(to_vec(m)?.len()) | |||
| .on_chain_message(m.chain_length()?) | |||
There was a problem hiding this comment.
outstanding bug that made us overcharge for some messages (thus them or others not being selected)
LesnyRumcajs
commented
Jul 4, 2025
Comment on lines
-231
to
+234
| if to_vec(msg)?.len() > 32 * 1024 { | ||
| if to_vec(msg)?.len() > MAX_MESSAGE_SIZE { |
There was a problem hiding this comment.
Lotus allows messages of up to 64 KiB, so should we.
LesnyRumcajs
force-pushed
the
mpoll-msg-limits
branch
from
7ab78c1 to
3ec807d
Compare
LesnyRumcajs
requested review from
hanabi1224 and
akaladarshi
and removed request for
a team
LesnyRumcajs
added
the
RPC
Graphite App
hanabi1224
reviewed
Jul 7, 2025
hanabi1224
reviewed
Jul 7, 2025
hanabi1224
approved these changes
Jul 7, 2025
akaladarshi
reviewed
Jul 7, 2025
| @@ -29,6 +33,253 @@ type Pending = HashMap<Address, HashMap<u64, SignedMessage>>; | |||
| // A cap on maximum number of message to include in a block | |||
| const MAX_BLOCK_MSGS: usize = 16000; | |||
| const MAX_BLOCKS: usize = 15; | |||
| const CBOR_GEN_LIMIT: usize = 8192; // Same limits as in Go's `cbor-gen`. | |||
There was a problem hiding this comment.
Can you add the link to the go code.
akaladarshi
approved these changes
Jul 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Follow up of #5729.
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #5730
Other information and links
Change checklist