Add chunked submission strategy for multi-block solutions (#987)#1016
Merged
Add chunked submission strategy for multi-block solutions (#987)#1016
Conversation
The current implementation submits all pages concurrently, which can be fastest if all submissions succeed quickly and the network/node can handle the load efficiently. However, if an early transaction fails (e.g., nonce N+1), all subsequent transactions will fail due to the nonce gap, resulting in wasted submission attempts and potentially high load on the network/node due to retransmissions. This commit adds support for a `chunked submission strategy` where the size of the chunk can be specified in the `experimental monitor command`. If the chunk size is not specified or zero, we use the fully concurrent approach. Otherwise, we submit a chunk of N pages, wait for all of them to be successfully included in a block, and then move to the next chunk. This offers a good balance between speed and robustness/load. Note that if the chunk size equals 1, we end up with a fully sequential solution (submit 1 page - wait - submit next page, etc.). Testing against a simple zombienet setup with an 8-page solution: | Submission Strategy | Time (seconds) | |---------------------|----------------| | Fully concurrent | ~77s | | Chunked (size=4) | ~105s |
sigurpol
force-pushed
the
chunked_submission_strategy
branch
2 times, most recently
from
9238140 to
93daa2c
Compare
sigurpol
changed the title
This was referenced Apr 14, 2025
sigurpol
force-pushed
the
chunked_submission_strategy
branch
4 times, most recently
from
79031de to
0f603af
Compare
Extract common functionality from inner_submit_pages_concurrent and inner_submit_pages_chunked into a shared submit_pages_batch helper function. This reduces code duplication while preserving the distinct behavior of each submission strategy: - Concurrent: Submit all pages in one batch - Chunked: Submit pages in multiple batches, waiting for each batch to complete before starting the next The refactoring improves maintainability and makes the code easier to understand without changing the underlying functionality or performance characteristics.
sigurpol
force-pushed
the
chunked_submission_strategy
branch
from
0f603af to
0c51ea6
Compare
sigurpol
changed the title
Contributor
Author
|
@niklasad1 , @kianenigma PTAL when/if you have time 🙏 🙇 |
niklasad1
reviewed
Apr 15, 2025
src/dynamic/multi_block.rs
Outdated
| let total_pages = paged_raw_solution.len(); | ||
|
|
||
| // Process pages in chunks | ||
| for chunk_start in (0..total_pages).step_by(chunk_size) { |
Contributor
There was a problem hiding this comment.
you could perhaps use https://doc.rust-lang.org/std/primitive.slice.html#method.chunks here instead of doing it yourself.
niklasad1
reviewed
Apr 15, 2025
src/dynamic/multi_block.rs
Outdated
| pages_to_submit: Vec<(u32, T::Solution)>, | ||
| listen: Listen, | ||
| ) -> Result<Vec<u32>, Error> { | ||
| ) -> Result<(Vec<u32>, HashSet<u32>), Error> |
Contributor
There was a problem hiding this comment.
Both submitted pages and failed pages are returned mostly for logging purposes?
It would be use with a separate type for this like SubmissionResult/Status or something like that wraps that
niklasad1
reviewed
Apr 15, 2025
| T: MinerConfig<AccountId = AccountId> + Send + Sync + 'static, | ||
| T::Solution: Send, | ||
| T::Pages: Send, | ||
| T::Solution: Send + Sync + 'static, |
Contributor
There was a problem hiding this comment.
do you know why these extra trait bounds are needed?
Contributor
Author
There was a problem hiding this comment.
I think it was a leftover from some intermediate version. Should be better now with this commit
sigurpol
force-pushed
the
chunked_submission_strategy
branch
from
6ca18ea to
c8a41fc
Compare
sigurpol
force-pushed
the
chunked_submission_strategy
branch
from
c8a41fc to
9a11d16
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The current implementation submits all pages concurrently, which can be the fastest option if all submissions succeed quickly and the network or node can handle the load efficiently. However, if an early transaction fails (e.g., nonce N+1), all subsequent transactions will also fail due to the nonce gap. This leads to wasted submission attempts and potentially high load on the network or node from retransmissions.
This commit introduces support for a chunked submission strategy, allowing the chunk size to be specified in the experimental monitor command e.g.
RUST_LOG="polkadot-staking-miner=trace,info" ./target/release/polkadot-staking-miner --uri ws://127.0.0.1:9966 experimental-monitor-multi-block --seed-or-path //Bob --chunk-size 4If the chunk size is not specified or set to zero, we will use the fully concurrent approach. Otherwise, we will submit a chunk of N pages, wait for all of them to be successfully included in a block, and then proceed to the next chunk.
This approach aims to balance speed and robustness, but it requires careful validation in real use cases to prove that it is actually worthwhile. Note that if the chunk size is set to 1, we will have a fully sequential solution: submit one page, wait, then submit the next page, and so on, which may significantly slow down the process.
Testing against a simple
zombienetsetup with 8-page or 64-page solution:Example of logs for a 8-page solution with
chunk size = 4:The recommended approach is to merge this PR and test the miner with both fully concurrent and chunked strategy in the field against
AHleaving the fully concurrent submission strategy as default.NOTE: in a followup experimental PR built on top of this one, I've also tried to change the confirmation strategy from waiting for events to
on-chainpolling, with no benefit in comparison with the current implementation so the suggestion is to park the on-chain polling variant.Associated issue here