Skip to content

Commit

Permalink
graft Fixed block response limit check paritytech#9692
Browse files Browse the repository at this point in the history
  • Loading branch information
arkpar authored and jordy25519 committed Nov 4, 2021
1 parent ef0dfe4 commit 890016c
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 13 deletions.
6 changes: 4 additions & 2 deletions client/network/src/block_request_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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),
inbound_queue: None,
}
}
Expand Down Expand Up @@ -322,7 +322,9 @@ impl<B: BlockT> BlockRequestHandler<B> {
is_empty_justification,
};

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>();

blocks.push(block_data);

if blocks.len() >= max_blocks as usize || total_size > MAX_BODY_BYTES {
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ impl<B: BlockT> Protocol<B> {
} else {
None
},
receipt: if !block_data.message_queue.is_empty() {
receipt: if !block_data.receipt.is_empty() {
Some(block_data.receipt)
} else {
None
Expand Down
16 changes: 9 additions & 7 deletions client/network/src/protocol/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ mod blocks;
mod extra_requests;

/// Maximum blocks to request in a single packet.
const MAX_BLOCKS_TO_REQUEST: usize = 128;
const MAX_BLOCKS_TO_REQUEST: usize = 64;

/// Maximum blocks to store in the import queue.
const MAX_IMPORTING_BLOCKS: usize = 2048;
Expand Down Expand Up @@ -823,12 +823,14 @@ impl<B: BlockT> ChainSync<B> {
self.pending_requests.add(who);
if let Some(request) = request {
match &mut peer.state {
PeerSyncState::DownloadingNew(start_block) => {
PeerSyncState::DownloadingNew(_) => {
self.blocks.clear_peer_download(who);
let start_block = *start_block;
peer.state = PeerSyncState::Available;
validate_blocks::<B>(&blocks, who, Some(request))?;
self.blocks.insert(start_block, blocks, who.clone());
if let Some(start_block) =
validate_blocks::<B>(&blocks, who, Some(request))?
{
self.blocks.insert(start_block, blocks, who.clone());
}
self.blocks
.drain(self.best_queued_number + One::one())
.into_iter()
Expand Down Expand Up @@ -1859,7 +1861,7 @@ fn validate_blocks<Block: BlockT>(
blocks: &Vec<message::BlockData<Block>>,
who: &PeerId,
request: Option<BlockRequest<Block>>,
) -> Result<(), BadPeer> {
) -> Result<Option<NumberFor<Block>>, BadPeer> {
if let Some(request) = request {
if Some(blocks.len() as _) > request.max {
debug!(
Expand Down Expand Up @@ -1952,7 +1954,7 @@ fn validate_blocks<Block: BlockT>(
}
}

Ok(())
Ok(blocks.first().and_then(|b| b.header.as_ref()).map(|h| *h.number()))
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/protocol/sync/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<B: BlockT> BlockCollection<B> {
for r in ranges {
self.blocks.remove(&r);
}
trace!(target: "sync", "Drained {} blocks", drained.len());
trace!(target: "sync", "Drained {} blocks from {:?}", drained.len(), from);
drained
}

Expand Down
29 changes: 29 additions & 0 deletions client/network/test/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,3 +1087,32 @@ fn syncs_after_missing_announcement() {
net.block_until_sync();
assert!(net.peer(1).client().header(&BlockId::Hash(final_block)).unwrap().is_some());
}

#[test]
fn syncs_huge_blocks() {
use sp_core::storage::well_known_keys::HEAP_PAGES;
use sp_runtime::codec::Encode;
use substrate_test_runtime_client::BlockBuilderExt;

sp_tracing::try_init_simple();
let mut net = TestNet::new(2);

// Increase heap space for bigger blocks.
net.peer(0).generate_blocks(1, BlockOrigin::Own, |mut builder| {
builder.push_storage_change(HEAP_PAGES.to_vec(), Some(256u64.encode())).unwrap();
builder.build().unwrap().block
});

net.peer(0).generate_blocks(32, BlockOrigin::Own, |mut builder| {
// Add 32 extrinsics 32k each = 1MiB total
for _ in 0..32 {
let ex = Extrinsic::IncludeData([42u8; 32 * 1024].to_vec());
builder.push(ex).unwrap();
}
builder.build().unwrap().block
});

net.block_until_sync();
assert_eq!(net.peer(0).client.info().best_number, 33);
assert_eq!(net.peer(1).client.info().best_number, 33);
}
2 changes: 1 addition & 1 deletion primitives/consensus/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl<Block: BlockT> CanAuthorWith<Block> for NeverCanAuthor {
/// A type from which a slot duration can be obtained.
pub trait SlotData {
/// Gets the slot duration.
fn slot_duration(&self) -> sp_std::time::Duration;
fn slot_duration(&self) -> core::time::Duration;

/// The static slot key
const SLOT_KEY: &'static [u8];
Expand Down
2 changes: 1 addition & 1 deletion primitives/timestamp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

use codec::{Encode, Decode};
use sp_inherents::{InherentIdentifier, IsFatalError, InherentData};
use sp_std::time::Duration;
use core::time::Duration;

/// The identifier for the `timestamp` inherent.
pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"timstap0";
Expand Down

0 comments on commit 890016c

Please sign in to comment.