This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Fixed block response limit check #9692
Merged
Conversation
This file contains 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
arkpar
added
A0-please_review
gilescope
reviewed
Sep 4, 2021
| @@ -355,7 +355,8 @@ impl<B: BlockT> BlockRequestHandler<B> { | |||
| indexed_body, | |||
| }; | |||
|
|
|||
| total_size += block_data.body.len(); | |||
| total_size += block_data.body.iter().map(|ex| ex.len()).sum::<usize>(); | |||
| total_size += block_data.indexed_body.iter().map(|ex| ex.len()).sum::<usize>(); | |||
There was a problem hiding this comment.
Might this work well as a size() function on block_data?
There was a problem hiding this comment.
BlockData struct is auto generated from protobuf description. Also, we don't need all of the field to be sized here, just the bodies. The rest do not vary much and should definitely fit in the margin.
bkchr
approved these changes
Sep 4, 2021
| @@ -62,7 +62,7 @@ pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { | |||
| name: generate_protocol_name(protocol_id).into(), | |||
| max_request_size: 1024 * 1024, | |||
| max_response_size: 16 * 1024 * 1024, | |||
| request_timeout: Duration::from_secs(40), | |||
| request_timeout: Duration::from_secs(20), | |||
There was a problem hiding this comment.
It's a bit off-topic for this PR, but this timeout is too high anyway.
There was a problem hiding this comment.
Right it was too high in the first place, and since I've also decreased the number of requested blocks the timeout should be readjusted.
tomaka
approved these changes
Sep 5, 2021
arkpar
added
B5-clientnoteworthy
and removed
B0-silent
|
Added a test and fixed response start block detection. Rather than trusting that it matches the request it should be detected from the response. |
|
@arkpar did you missed to push the test changes? |
|
bot merge |
|
Trying merge. |
ghost
merged commit
chevdor
pushed a commit
that referenced
this pull request
Sep 6, 2021
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
crystalin
pushed a commit
to moonbeam-foundation/substrate
that referenced
this pull request
Sep 6, 2021
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
GopherJ
pushed a commit
to parallel-finance/substrate
that referenced
this pull request
Sep 14, 2021
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
jasl
added a commit
to Phala-Network/khala-parachain
that referenced
this pull request
Sep 20, 2021
15 tasks
frisitano
added a commit
to frisitano/reef-chain
that referenced
this pull request
Oct 8, 2021
jordy25519
pushed a commit
to cennznet/substrate
that referenced
this pull request
Nov 4, 2021
jordy25519
added a commit
to cennznet/substrate
that referenced
this pull request
Nov 4, 2021
Co-authored-by: Arkadiy Paronyan <arkady.paronyan@gmail.com>
Neopallium
pushed a commit
to PolymeshAssociation/substrate
that referenced
this pull request
Jan 10, 2022
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
Neopallium
pushed a commit
to PolymeshAssociation/substrate
that referenced
this pull request
Jan 10, 2022
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
Neopallium
pushed a commit
to PolymeshAssociation/substrate
that referenced
this pull request
Jan 10, 2022
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
13 tasks
adamdossa
pushed a commit
to PolymeshAssociation/substrate
that referenced
this pull request
Jan 14, 2022
* Fixed block response limit check * Fixed start block detection and added a test * Missing test
Githubinspiration
added a commit
to Githubinspiration/reef_chain-rust
that referenced
this pull request
May 10, 2024
pjakekell
added a commit
to pjakekell/chain-node
that referenced
this pull request
Aug 5, 2024
0xchainlover
pushed a commit
to Reef-network/reef-chain
that referenced
this pull request
Nov 8, 2024
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When computing block response size, the code was incorrectly counting extrinsics rather than bytes.
Also decreased
MAX_BLOCKS_TO_REQUESTso that the nodes can sync without waiting for others to upgrade and tweaked timeout value.Fixes paritytech/polkadot#3778