Return target_hash for finality_target instead of an Option (#9867)

* .

cargo +nightly fmt --all

* Fix test

* Simplify test

* They are already imported

* Needless clone()
This commit is contained in:
Liu-Cheng Xu
2021-09-29 21:03:09 +08:00
committed by GitHub
parent 2deed49706
commit 802afa9f22
5 changed files with 96 additions and 378 deletions
@@ -91,11 +91,12 @@ where
&self,
target_hash: Block::Hash,
maybe_max_number: Option<NumberFor<Block>>,
) -> Result<Option<Block::Hash>, ConsensusError> {
) -> Result<Block::Hash, ConsensusError> {
let import_lock = self.backend.get_import_lock();
self.backend
.blockchain()
.best_containing(target_hash, maybe_max_number, import_lock)
.map(|maybe_hash| maybe_hash.unwrap_or(target_hash))
.map_err(|e| ConsensusError::ChainLookup(e.to_string()).into())
}
}
@@ -1165,7 +1165,7 @@ where
debug!(target: "afg", "Finding best chain containing block {:?} with number limit {:?}", block, limit);
let result = match select_chain.finality_target(block, None).await {
Ok(Some(best_hash)) => {
Ok(best_hash) => {
let best_header = client
.header(BlockId::Hash(best_hash))?
.expect("Header known to exist after `finality_target` call; qed");
@@ -1223,10 +1223,6 @@ where
})
.or_else(|| Some((target_header.hash(), *target_header.number())))
},
Ok(None) => {
debug!(target: "afg", "Encountered error finding best chain containing {:?}: couldn't find target block", block);
None
},
Err(e) => {
debug!(target: "afg", "Encountered error finding best chain containing {:?}: {:?}", block, e);
None
@@ -118,10 +118,10 @@ where
)
.await
} else {
Ok(Some(pending_change.canon_hash))
Ok(pending_change.canon_hash)
};
if let Ok(Some(hash)) = effective_block_hash {
if let Ok(hash) = effective_block_hash {
if let Ok(Some(header)) = self.inner.header(BlockId::Hash(hash)) {
if *header.number() == pending_change.effective_number() {
out.push((header.hash(), *header.number()));
+89 -368
View File
@@ -481,25 +481,7 @@ fn best_containing_with_genesis_block() {
assert_eq!(
genesis_hash.clone(),
block_on(longest_chain_select.finality_target(genesis_hash.clone(), None))
.unwrap()
.unwrap(),
);
}
#[test]
fn best_containing_with_hash_not_found() {
// block tree:
// G
let (client, longest_chain_select) = TestClientBuilder::new().build_with_longest_chain();
let uninserted_block = client.new_block(Default::default()).unwrap().build().unwrap().block;
assert_eq!(
None,
block_on(longest_chain_select.finality_target(uninserted_block.hash().clone(), None))
.unwrap(),
block_on(longest_chain_select.finality_target(genesis_hash.clone(), None)).unwrap(),
);
}
@@ -675,22 +657,10 @@ fn best_containing_on_longest_chain_with_single_chain_3_blocks() {
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, None))
.unwrap()
.unwrap()
);
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), None))
.unwrap()
.unwrap()
block_on(longest_chain_select.finality_target(genesis_hash, None)).unwrap()
);
assert_eq!(a2.hash(), block_on(longest_chain_select.finality_target(a1.hash(), None)).unwrap());
assert_eq!(a2.hash(), block_on(longest_chain_select.finality_target(a2.hash(), None)).unwrap());
}
#[test]
@@ -819,343 +789,101 @@ fn best_containing_on_longest_chain_with_multiple_forks() {
assert!(leaves.contains(&d2.hash()));
assert_eq!(leaves.len(), 4);
let finality_target = |target_hash, number| {
block_on(longest_chain_select.finality_target(target_hash, number)).unwrap()
};
// search without restriction
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, None))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a3.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a4.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a5.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b2.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b3.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b4.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
c3.hash(),
block_on(longest_chain_select.finality_target(c3.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(
d2.hash(),
block_on(longest_chain_select.finality_target(d2.hash(), None))
.unwrap()
.unwrap()
);
assert_eq!(a5.hash(), finality_target(genesis_hash, None));
assert_eq!(a5.hash(), finality_target(a1.hash(), None));
assert_eq!(a5.hash(), finality_target(a2.hash(), None));
assert_eq!(a5.hash(), finality_target(a3.hash(), None));
assert_eq!(a5.hash(), finality_target(a4.hash(), None));
assert_eq!(a5.hash(), finality_target(a5.hash(), None));
assert_eq!(b4.hash(), finality_target(b2.hash(), None));
assert_eq!(b4.hash(), finality_target(b3.hash(), None));
assert_eq!(b4.hash(), finality_target(b4.hash(), None));
assert_eq!(c3.hash(), finality_target(c3.hash(), None));
assert_eq!(d2.hash(), finality_target(d2.hash(), None));
// search only blocks with number <= 5. equivalent to without restriction for this scenario
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a3.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a4.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
a5.hash(),
block_on(longest_chain_select.finality_target(a5.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b2.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b3.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b4.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
c3.hash(),
block_on(longest_chain_select.finality_target(c3.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(
d2.hash(),
block_on(longest_chain_select.finality_target(d2.hash(), Some(5)))
.unwrap()
.unwrap()
);
assert_eq!(a5.hash(), finality_target(genesis_hash, Some(5)));
assert_eq!(a5.hash(), finality_target(a1.hash(), Some(5)));
assert_eq!(a5.hash(), finality_target(a2.hash(), Some(5)));
assert_eq!(a5.hash(), finality_target(a3.hash(), Some(5)));
assert_eq!(a5.hash(), finality_target(a4.hash(), Some(5)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(5)));
assert_eq!(b4.hash(), finality_target(b2.hash(), Some(5)));
assert_eq!(b4.hash(), finality_target(b3.hash(), Some(5)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(5)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(5)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(5)));
// search only blocks with number <= 4
assert_eq!(
a4.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
a4.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
a4.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
a4.hash(),
block_on(longest_chain_select.finality_target(a3.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
a4.hash(),
block_on(longest_chain_select.finality_target(a4.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(a5.hash(), Some(4))).unwrap());
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b2.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b3.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
b4.hash(),
block_on(longest_chain_select.finality_target(b4.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
c3.hash(),
block_on(longest_chain_select.finality_target(c3.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(
d2.hash(),
block_on(longest_chain_select.finality_target(d2.hash(), Some(4)))
.unwrap()
.unwrap()
);
assert_eq!(a4.hash(), finality_target(genesis_hash, Some(4)));
assert_eq!(a4.hash(), finality_target(a1.hash(), Some(4)));
assert_eq!(a4.hash(), finality_target(a2.hash(), Some(4)));
assert_eq!(a4.hash(), finality_target(a3.hash(), Some(4)));
assert_eq!(a4.hash(), finality_target(a4.hash(), Some(4)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(4)));
assert_eq!(b4.hash(), finality_target(b2.hash(), Some(4)));
assert_eq!(b4.hash(), finality_target(b3.hash(), Some(4)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(4)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(4)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(4)));
// search only blocks with number <= 3
assert_eq!(
a3.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(
a3.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(
a3.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(
a3.hash(),
block_on(longest_chain_select.finality_target(a3.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(a4.hash(), Some(3))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a5.hash(), Some(3))).unwrap());
assert_eq!(
b3.hash(),
block_on(longest_chain_select.finality_target(b2.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(
b3.hash(),
block_on(longest_chain_select.finality_target(b3.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(b4.hash(), Some(3))).unwrap());
assert_eq!(
c3.hash(),
block_on(longest_chain_select.finality_target(c3.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(
d2.hash(),
block_on(longest_chain_select.finality_target(d2.hash(), Some(3)))
.unwrap()
.unwrap()
);
assert_eq!(a3.hash(), finality_target(genesis_hash, Some(3)));
assert_eq!(a3.hash(), finality_target(a1.hash(), Some(3)));
assert_eq!(a3.hash(), finality_target(a2.hash(), Some(3)));
assert_eq!(a3.hash(), finality_target(a3.hash(), Some(3)));
assert_eq!(a4.hash(), finality_target(a4.hash(), Some(3)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(3)));
assert_eq!(b3.hash(), finality_target(b2.hash(), Some(3)));
assert_eq!(b3.hash(), finality_target(b3.hash(), Some(3)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(3)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(3)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(3)));
// search only blocks with number <= 2
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(2)))
.unwrap()
.unwrap()
);
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), Some(2)))
.unwrap()
.unwrap()
);
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(a2.hash(), Some(2)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(a3.hash(), Some(2))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a4.hash(), Some(2))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a5.hash(), Some(2))).unwrap());
assert_eq!(
b2.hash(),
block_on(longest_chain_select.finality_target(b2.hash(), Some(2)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(b3.hash(), Some(2))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b4.hash(), Some(2))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(c3.hash(), Some(2))).unwrap());
assert_eq!(
d2.hash(),
block_on(longest_chain_select.finality_target(d2.hash(), Some(2)))
.unwrap()
.unwrap()
);
assert_eq!(a2.hash(), finality_target(genesis_hash, Some(2)));
assert_eq!(a2.hash(), finality_target(a1.hash(), Some(2)));
assert_eq!(a2.hash(), finality_target(a2.hash(), Some(2)));
assert_eq!(a3.hash(), finality_target(a3.hash(), Some(2)));
assert_eq!(a4.hash(), finality_target(a4.hash(), Some(2)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(2)));
assert_eq!(b2.hash(), finality_target(b2.hash(), Some(2)));
assert_eq!(b3.hash(), finality_target(b3.hash(), Some(2)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(2)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(2)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(2)));
// search only blocks with number <= 1
assert_eq!(
a1.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(1)))
.unwrap()
.unwrap()
);
assert_eq!(
a1.hash(),
block_on(longest_chain_select.finality_target(a1.hash(), Some(1)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(a2.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a3.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a4.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a5.hash(), Some(1))).unwrap());
assert_eq!(a1.hash(), finality_target(genesis_hash, Some(1)));
assert_eq!(a1.hash(), finality_target(a1.hash(), Some(1)));
assert_eq!(a2.hash(), finality_target(a2.hash(), Some(1)));
assert_eq!(a3.hash(), finality_target(a3.hash(), Some(1)));
assert_eq!(a4.hash(), finality_target(a4.hash(), Some(1)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(1)));
assert_eq!(None, block_on(longest_chain_select.finality_target(b2.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b3.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b4.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(c3.hash(), Some(1))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(d2.hash(), Some(1))).unwrap());
assert_eq!(b2.hash(), finality_target(b2.hash(), Some(1)));
assert_eq!(b3.hash(), finality_target(b3.hash(), Some(1)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(1)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(1)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(1)));
// search only blocks with number <= 0
assert_eq!(
genesis_hash,
block_on(longest_chain_select.finality_target(genesis_hash, Some(0)))
.unwrap()
.unwrap()
);
assert_eq!(None, block_on(longest_chain_select.finality_target(a1.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a2.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a3.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a4.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(a5.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b2.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b3.hash(), Some(0))).unwrap());
assert_eq!(None, block_on(longest_chain_select.finality_target(b4.hash(), Some(0))).unwrap());
assert_eq!(
None,
block_on(longest_chain_select.finality_target(c3.hash().clone(), Some(0))).unwrap(),
);
assert_eq!(
None,
block_on(longest_chain_select.finality_target(d2.hash().clone(), Some(0))).unwrap(),
);
assert_eq!(genesis_hash, finality_target(genesis_hash, Some(0)));
assert_eq!(a1.hash(), finality_target(a1.hash(), Some(0)));
assert_eq!(a2.hash(), finality_target(a2.hash(), Some(0)));
assert_eq!(a3.hash(), finality_target(a3.hash(), Some(0)));
assert_eq!(a4.hash(), finality_target(a4.hash(), Some(0)));
assert_eq!(a5.hash(), finality_target(a5.hash(), Some(0)));
assert_eq!(b2.hash(), finality_target(b2.hash(), Some(0)));
assert_eq!(b3.hash(), finality_target(b3.hash(), Some(0)));
assert_eq!(b4.hash(), finality_target(b4.hash(), Some(0)));
assert_eq!(c3.hash(), finality_target(c3.hash(), Some(0)));
assert_eq!(d2.hash(), finality_target(d2.hash(), Some(0)));
}
#[test]
@@ -1177,9 +905,7 @@ fn best_containing_on_longest_chain_with_max_depth_higher_than_best() {
assert_eq!(
a2.hash(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(10)))
.unwrap()
.unwrap(),
block_on(longest_chain_select.finality_target(genesis_hash, Some(10))).unwrap(),
);
}
@@ -2085,12 +1811,7 @@ fn cleans_up_closed_notification_sinks_on_block_import() {
// NOTE: we need to build the client here instead of using the client
// provided by test_runtime_client otherwise we can't access the private
// `import_notification_sinks` and `finality_notification_sinks` fields.
let mut client = new_in_mem::<
_,
substrate_test_runtime_client::runtime::Block,
_,
substrate_test_runtime_client::runtime::RuntimeApi,
>(
let mut client = new_in_mem::<_, Block, _, RuntimeApi>(
substrate_test_runtime_client::new_native_executor(),
&substrate_test_runtime_client::GenesisParameters::default().genesis_storage(),
None,
@@ -2108,8 +1829,8 @@ fn cleans_up_closed_notification_sinks_on_block_import() {
in_mem::Backend<Block>,
sc_executor::NativeElseWasmExecutor<LocalExecutorDispatch>,
>,
substrate_test_runtime_client::runtime::Block,
substrate_test_runtime_client::runtime::RuntimeApi,
Block,
RuntimeApi,
>;
let import_notif1 = client.import_notification_stream();
@@ -50,7 +50,7 @@ pub trait SelectChain<Block: BlockT>: Sync + Send + Clone {
&self,
target_hash: <Block as BlockT>::Hash,
_maybe_max_number: Option<NumberFor<Block>>,
) -> Result<Option<<Block as BlockT>::Hash>, Error> {
Ok(Some(target_hash))
) -> Result<<Block as BlockT>::Hash, Error> {
Ok(target_hash)
}
}