Skip to content

Commit

Permalink
Enable download of future forks (paritytech#10739)
Browse files Browse the repository at this point in the history 
* Enable download of future forks

* Fixed external tests
  • Loading branch information
@arkpar @agryaznov
arkpar authored and agryaznov committed Feb 4, 2022
1 parent 78c7ecb commit 9d0d27e
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 32 deletions.
8 changes: 7 additions & 1 deletion client/network/src/protocol/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2418,7 +2418,11 @@ fn fork_sync_request<B: BlockT>(
if !r.peers.contains(id) {
continue
}
if r.number <= best_num {
// Download the fork only if it is behind or not too far ahead our tip of the chain
// Otherwise it should be downloaded in full sync mode.
if r.number <= best_num ||
(r.number - best_num).saturated_into::<u32>() < MAX_BLOCKS_TO_REQUEST as u32
{
let parent_status = r.parent_hash.as_ref().map_or(BlockStatus::Unknown, check_block);
let count = if parent_status == BlockStatus::Unknown {
(r.number - finalized).saturated_into::<u32>() // up to the last finalized block
Expand All @@ -2438,6 +2442,8 @@ fn fork_sync_request<B: BlockT>(
max: Some(count),
},
))
} else {
trace!(target: "sync", "Fork too far in the future: {:?} (#{})", hash, r.number);
}
}
None
Expand Down
58 changes: 37 additions & 21 deletions client/network/test/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,14 @@ type AuthorityId = sp_consensus_babe::AuthorityId;
#[derive(Clone)]
pub struct PassThroughVerifier {
finalized: bool,
fork_choice: ForkChoiceStrategy,
}

impl PassThroughVerifier {
/// Create a new instance.
///
/// Every verified block will use `finalized` for the `BlockImportParams`.
pub fn new(finalized: bool) -> Self {
Self { finalized, fork_choice: ForkChoiceStrategy::LongestChain }
}

/// Create a new instance.
///
/// Every verified block will use `finalized` for the `BlockImportParams` and
/// the given [`ForkChoiceStrategy`].
pub fn new_with_fork_choice(finalized: bool, fork_choice: ForkChoiceStrategy) -> Self {
Self { finalized, fork_choice }
Self { finalized }
}
}

Expand All @@ -121,8 +112,10 @@ impl<B: BlockT> Verifier<B> for PassThroughVerifier {
.or_else(|| l.try_as_raw(OpaqueDigestItemId::Consensus(b"babe")))
})
.map(|blob| vec![(well_known_cache_keys::AUTHORITIES, blob.to_vec())]);
if block.fork_choice.is_none() {
block.fork_choice = Some(ForkChoiceStrategy::LongestChain);
};
block.finalized = self.finalized;
block.fork_choice = Some(self.fork_choice.clone());
Ok((block, maybe_keys))
}
}
Expand Down Expand Up @@ -309,6 +302,33 @@ where
false,
true,
true,
ForkChoiceStrategy::LongestChain,
)
}

/// Add blocks to the peer -- edit the block before adding and use custom fork choice rule.
pub fn generate_blocks_with_fork_choice<F>(
&mut self,
count: usize,
origin: BlockOrigin,
edit_block: F,
fork_choice: ForkChoiceStrategy,
) -> H256
where
F: FnMut(
BlockBuilder<Block, PeersFullClient, substrate_test_runtime_client::Backend>,
) -> Block,
{
let best_hash = self.client.info().best_hash;
self.generate_blocks_at(
BlockId::Hash(best_hash),
count,
origin,
edit_block,
false,
true,
true,
fork_choice,
)
}

Expand All @@ -323,6 +343,7 @@ where
headers_only: bool,
inform_sync_about_new_best_block: bool,
announce_block: bool,
fork_choice: ForkChoiceStrategy,
) -> H256
where
F: FnMut(
Expand All @@ -346,6 +367,7 @@ where
let header = block.header.clone();
let mut import_block = BlockImportParams::new(origin, header.clone());
import_block.body = if headers_only { None } else { Some(block.extrinsics) };
import_block.fork_choice = Some(fork_choice);
let (import_block, cache) =
futures::executor::block_on(self.verifier.verify(import_block)).unwrap();
let cache = if let Some(cache) = cache {
Expand Down Expand Up @@ -442,6 +464,7 @@ where
headers_only,
inform_sync_about_new_best_block,
announce_block,
ForkChoiceStrategy::LongestChain,
)
} else {
self.generate_blocks_at(
Expand All @@ -452,6 +475,7 @@ where
headers_only,
inform_sync_about_new_best_block,
announce_block,
ForkChoiceStrategy::LongestChain,
)
}
}
Expand Down Expand Up @@ -989,14 +1013,6 @@ where

pub struct TestNet {
peers: Vec<Peer<(), PeersClient>>,
fork_choice: ForkChoiceStrategy,
}

impl TestNet {
/// Create a `TestNet` that used the given fork choice rule.
pub fn with_fork_choice(fork_choice: ForkChoiceStrategy) -> Self {
Self { peers: Vec::new(), fork_choice }
}
}

impl TestNetFactory for TestNet {
Expand All @@ -1006,7 +1022,7 @@ impl TestNetFactory for TestNet {

/// Create new test network with peers and given config.
fn from_config(_config: &ProtocolConfig) -> Self {
TestNet { peers: Vec::new(), fork_choice: ForkChoiceStrategy::LongestChain }
TestNet { peers: Vec::new() }
}

fn make_verifier(
Expand All @@ -1015,7 +1031,7 @@ impl TestNetFactory for TestNet {
_config: &ProtocolConfig,
_peer_data: &(),
) -> Self::Verifier {
PassThroughVerifier::new_with_fork_choice(false, self.fork_choice.clone())
PassThroughVerifier::new(false)
}

fn make_block_import(
Expand Down
63 changes: 53 additions & 10 deletions client/network/test/src/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,38 @@ fn can_sync_small_non_best_forks() {
}));
}

#[test]
fn can_sync_forks_ahead_of_the_best_chain() {
sp_tracing::try_init_simple();
let mut net = TestNet::new(2);
net.peer(0).push_blocks(1, false);
net.peer(1).push_blocks(1, false);

net.block_until_connected();
// Peer 0 is on 2-block fork which is announced with is_best=false
let fork_hash = net.peer(0).generate_blocks_with_fork_choice(
2,
BlockOrigin::Own,
|builder| builder.build().unwrap().block,
ForkChoiceStrategy::Custom(false),
);
// Peer 1 is on 1-block fork
net.peer(1).push_blocks(1, false);
assert!(net.peer(0).client().header(&BlockId::Hash(fork_hash)).unwrap().is_some());
assert_eq!(net.peer(0).client().info().best_number, 1);
assert_eq!(net.peer(1).client().info().best_number, 2);

// after announcing, peer 1 downloads the block.
block_on(futures::future::poll_fn::<(), _>(|cx| {
net.poll(cx);

if net.peer(1).client().header(&BlockId::Hash(fork_hash)).unwrap().is_none() {
return Poll::Pending
}
Poll::Ready(())
}));
}

#[test]
fn can_sync_explicit_forks() {
sp_tracing::try_init_simple();
Expand Down Expand Up @@ -678,7 +710,7 @@ impl BlockAnnounceValidator<Block> for FailingBlockAnnounceValidator {
#[test]
fn sync_blocks_when_block_announce_validator_says_it_is_new_best() {
sp_tracing::try_init_simple();
let mut net = TestNet::with_fork_choice(ForkChoiceStrategy::Custom(false));
let mut net = TestNet::new(0);
net.add_full_peer_with_config(Default::default());
net.add_full_peer_with_config(Default::default());
net.add_full_peer_with_config(FullPeerConfig {
Expand All @@ -688,16 +720,17 @@ fn sync_blocks_when_block_announce_validator_says_it_is_new_best() {

net.block_until_connected();

let block_hash = net.peer(0).push_blocks(1, false);
// Add blocks but don't set them as best
let block_hash = net.peer(0).generate_blocks_with_fork_choice(
1,
BlockOrigin::Own,
|builder| builder.build().unwrap().block,
ForkChoiceStrategy::Custom(false),
);

while !net.peer(2).has_block(&block_hash) {
net.block_until_idle();
}

// Peer1 should not have the block, because peer 0 did not reported the block
// as new best. However, peer2 has a special block announcement validator
// that flags all blocks as `is_new_best` and thus, it should have synced the blocks.
assert!(!net.peer(1).has_block(&block_hash));
}

/// Waits for some time until the validation is successfull.
Expand All @@ -721,7 +754,7 @@ impl BlockAnnounceValidator<Block> for DeferredBlockAnnounceValidator {
#[test]
fn wait_until_deferred_block_announce_validation_is_ready() {
sp_tracing::try_init_simple();
let mut net = TestNet::with_fork_choice(ForkChoiceStrategy::Custom(false));
let mut net = TestNet::new(0);
net.add_full_peer_with_config(Default::default());
net.add_full_peer_with_config(FullPeerConfig {
block_announce_validator: Some(Box::new(NewBestBlockAnnounceValidator)),
Expand All @@ -730,7 +763,13 @@ fn wait_until_deferred_block_announce_validation_is_ready() {

net.block_until_connected();

let block_hash = net.peer(0).push_blocks(1, true);
// Add blocks but don't set them as best
let block_hash = net.peer(0).generate_blocks_with_fork_choice(
1,
BlockOrigin::Own,
|builder| builder.build().unwrap().block,
ForkChoiceStrategy::Custom(false),
);

while !net.peer(1).has_block(&block_hash) {
net.block_until_idle();
Expand Down Expand Up @@ -847,7 +886,10 @@ fn block_announce_data_is_propagated() {
// Wait until peer 1 is connected to both nodes.
block_on(futures::future::poll_fn::<(), _>(|cx| {
net.poll(cx);
if net.peer(1).num_peers() == 2 {
if net.peer(1).num_peers() == 2 &&
net.peer(0).num_peers() == 1 &&
net.peer(2).num_peers() == 1
{
Poll::Ready(())
} else {
Poll::Pending
Expand Down Expand Up @@ -1109,6 +1151,7 @@ fn syncs_indexed_blocks() {
false,
true,
true,
ForkChoiceStrategy::LongestChain,
);
let indexed_key = sp_runtime::traits::BlakeTwo256::hash(&42u64.to_le_bytes());
assert!(net
Expand Down

0 comments on commit 9d0d27e

Please sign in to comment.