From 801632d2c23a7ef88bf4adbe1268beb717aea4d0 Mon Sep 17 00:00:00 2001 From: Jordan Date: Fri, 5 Nov 2021 10:52:59 +1300 Subject: [PATCH] graft Fixed block response limit check #9692 (#15) Co-authored-by: Arkadiy Paronyan --- client/network/src/block_request_handler.rs | 6 +++-- client/network/src/protocol.rs | 2 +- client/network/src/protocol/sync.rs | 16 +++++++----- client/network/src/protocol/sync/blocks.rs | 2 +- client/network/test/src/sync.rs | 29 +++++++++++++++++++++ primitives/consensus/common/src/lib.rs | 2 +- primitives/timestamp/src/lib.rs | 2 +- 7 files changed, 46 insertions(+), 13 deletions(-) diff --git a/client/network/src/block_request_handler.rs b/client/network/src/block_request_handler.rs index 332635dbe7901..15dca4176da11 100644 --- a/client/network/src/block_request_handler.rs +++ b/client/network/src/block_request_handler.rs @@ -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, } } @@ -322,7 +322,9 @@ impl BlockRequestHandler { is_empty_justification, }; - total_size += block_data.body.len(); + total_size += block_data.body.iter().map(|ex| ex.len()).sum::(); + // total_size += block_data.indexed_body.iter().map(|ex| ex.len()).sum::(); + blocks.push(block_data); if blocks.len() >= max_blocks as usize || total_size > MAX_BODY_BYTES { diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 9f939899ec91c..58fe0d3b9daf5 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -563,7 +563,7 @@ impl Protocol { } else { None }, - receipt: if !block_data.message_queue.is_empty() { + receipt: if !block_data.receipt.is_empty() { Some(block_data.receipt) } else { None diff --git a/client/network/src/protocol/sync.rs b/client/network/src/protocol/sync.rs index 6e07bd4c96971..6d16fd776679a 100644 --- a/client/network/src/protocol/sync.rs +++ b/client/network/src/protocol/sync.rs @@ -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; @@ -823,12 +823,14 @@ impl ChainSync { 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::(&blocks, who, Some(request))?; - self.blocks.insert(start_block, blocks, who.clone()); + if let Some(start_block) = + validate_blocks::(&blocks, who, Some(request))? + { + self.blocks.insert(start_block, blocks, who.clone()); + } self.blocks .drain(self.best_queued_number + One::one()) .into_iter() @@ -1859,7 +1861,7 @@ fn validate_blocks( blocks: &Vec>, who: &PeerId, request: Option>, -) -> Result<(), BadPeer> { +) -> Result>, BadPeer> { if let Some(request) = request { if Some(blocks.len() as _) > request.max { debug!( @@ -1952,7 +1954,7 @@ fn validate_blocks( } } - Ok(()) + Ok(blocks.first().and_then(|b| b.header.as_ref()).map(|h| *h.number())) } #[cfg(test)] diff --git a/client/network/src/protocol/sync/blocks.rs b/client/network/src/protocol/sync/blocks.rs index 60492f24ed8c3..e173fe83dac1c 100644 --- a/client/network/src/protocol/sync/blocks.rs +++ b/client/network/src/protocol/sync/blocks.rs @@ -181,7 +181,7 @@ impl BlockCollection { 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 } diff --git a/client/network/test/src/sync.rs b/client/network/test/src/sync.rs index 553a769ec14a4..d16a5669273c0 100644 --- a/client/network/test/src/sync.rs +++ b/client/network/test/src/sync.rs @@ -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); +} diff --git a/primitives/consensus/common/src/lib.rs b/primitives/consensus/common/src/lib.rs index 642b6b12e7d6f..219f5bbb5e429 100644 --- a/primitives/consensus/common/src/lib.rs +++ b/primitives/consensus/common/src/lib.rs @@ -311,7 +311,7 @@ impl CanAuthorWith 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]; diff --git a/primitives/timestamp/src/lib.rs b/primitives/timestamp/src/lib.rs index 542522c9b8500..9071b35ae9730 100644 --- a/primitives/timestamp/src/lib.rs +++ b/primitives/timestamp/src/lib.rs @@ -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";