diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs
index 436fa8ff6e7f8a300c9841ddd7643e4c5955e7bd..529523b5b8f9bedcb9e7a14eae533524d7928b86 100644
--- a/polkadot/node/core/approval-voting/src/lib.rs
+++ b/polkadot/node/core/approval-voting/src/lib.rs
@@ -1239,6 +1239,15 @@ async fn handle_approved_ancestor(
// ancestry is moving backwards.
all_approved_max = Some((block_hash, target_number - i as BlockNumber));
}
+ block_descriptions.push(BlockDescription {
+ block_hash,
+ session: entry.session(),
+ candidates: entry
+ .candidates()
+ .iter()
+ .map(|(_idx, candidate_hash)| *candidate_hash)
+ .collect(),
+ });
} else if bits.len() <= ABNORMAL_DEPTH_THRESHOLD {
all_approved_max = None;
block_descriptions.clear();
@@ -1321,15 +1330,6 @@ async fn handle_approved_ancestor(
}
}
}
- block_descriptions.push(BlockDescription {
- block_hash,
- session: entry.session(),
- candidates: entry
- .candidates()
- .iter()
- .map(|(_idx, candidate_hash)| *candidate_hash)
- .collect(),
- });
}
tracing::trace!(
diff --git a/polkadot/node/service/src/tests.rs b/polkadot/node/service/src/tests.rs
index 269a7f2da0553bd06af23d56d47fd54e592cf9c8..2b438aa6f1ccbd7f18aef5b5f81862321d3b5eac 100644
--- a/polkadot/node/service/src/tests.rs
+++ b/polkadot/node/service/src/tests.rs
@@ -157,9 +157,6 @@ impl TestChainStorage {
minimum_block_number: BlockNumber,
leaf: Hash,
) -> Option<HighestApprovedAncestorBlock> {
- let hash = leaf;
- let number = self.blocks_by_hash.get(&leaf)?.number;
-
let mut descriptions = Vec::new();
let mut block_hash = leaf;
let mut highest_approved_ancestor = None;
@@ -168,31 +165,23 @@ impl TestChainStorage {
if minimum_block_number >= block.number {
break
}
- descriptions.push(BlockDescription {
- session: 1 as _, // dummy, not checked
- block_hash,
- candidates: vec![], // not relevant for any test cases
- });
if !self.approved_blocks.contains(&block_hash) {
highest_approved_ancestor = None;
descriptions.clear();
- } else if highest_approved_ancestor.is_none() {
- descriptions.clear();
- highest_approved_ancestor = Some(block_hash);
- }
- let next = block.parent_hash();
- if &leaf != next {
- block_hash = *next;
} else {
- break
+ if highest_approved_ancestor.is_none() {
+ highest_approved_ancestor = Some((block_hash, block.number));
+ }
+ descriptions.push(BlockDescription {
+ session: 1 as _, // dummy, not checked
+ block_hash,
+ candidates: vec![], // not relevant for any test cases
+ });
}
+ block_hash = *block.parent_hash();
}
- if highest_approved_ancestor.is_none() {
- return None
- }
-
- Some(HighestApprovedAncestorBlock {
+ highest_approved_ancestor.map(|(hash, number)| HighestApprovedAncestorBlock {
hash,
number,
descriptions: descriptions.into_iter().rev().collect(),
@@ -212,16 +201,12 @@ impl TestChainStorage {
let mut undisputed_chain = Some(highest_approved_block_hash);
let mut block_hash = highest_approved_block_hash;
while let Some(block) = self.blocks_by_hash.get(&block_hash) {
- block_hash = block.hash();
- if ChainBuilder::GENESIS_HASH == block_hash {
- return None
- }
let next = block.parent_hash();
if self.disputed_blocks.contains(&block_hash) {
undisputed_chain = Some(*next);
}
if block.number() == &base_blocknumber {
- return None
+ break
}
block_hash = *next;
}
@@ -434,36 +419,38 @@ fn run_specialized_test_w_harness<F: FnOnce() -> CaseVars>(case_var_provider: F)
// Verify test integrity: the provided highest approved
// ancestor must match the chain derived one.
- let highest_approved_ancestor_w_desc =
- best_chain_containing_block.and_then(|best_chain_containing_block| {
- let min_blocknumber = chain
- .blocks_by_hash
- .get(&best_chain_containing_block)
- .map(|x| x.number)
- .unwrap_or_default();
- let highest_approved_ancestor_w_desc =
- chain.highest_approved_ancestors(min_blocknumber, best_chain_containing_block);
- if let (
- Some(highest_approved_ancestor_w_desc),
- Some(highest_approved_ancestor_block),
- ) = (&highest_approved_ancestor_w_desc, highest_approved_ancestor_block)
- {
- assert_eq!(
- highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash,
- "TestCaseIntegrity: Provided and expected approved ancestor hash mismatch: {:?} vs {:?}",
- highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash,
- );
- }
- highest_approved_ancestor_w_desc
- });
- if let Some(haacwd) = &highest_approved_ancestor_w_desc {
- let expected = chain.undisputed_chain(haacwd.number, haacwd.hash);
- assert_eq!(
- expected, undisputed_chain,
- "TestCaseIntegrity: Provided and anticipated undisputed chain mismatch: {:?} vs {:?}",
- undisputed_chain, expected,
- )
- }
+ let highest_approved_ancestor_w_desc = best_chain_containing_block
+ .and_then(|best_chain_containing_block| {
+ chain.blocks_by_hash.get(&target_block).map(|target_block_header| {
+ let target_blocknumber = target_block_header.number;
+ let highest_approved_ancestor_w_desc = chain.highest_approved_ancestors(
+ target_blocknumber,
+ best_chain_containing_block,
+ );
+ if let (
+ Some(highest_approved_ancestor_w_desc),
+ Some(highest_approved_ancestor_block),
+ ) = (&highest_approved_ancestor_w_desc, highest_approved_ancestor_block)
+ {
+ assert_eq!(
+ highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash,
+ "TestCaseIntegrity: Provided and expected approved ancestor hash mismatch: {:?} vs {:?}",
+ highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash,
+ );
+
+ let expected = chain
+ .undisputed_chain(target_blocknumber, highest_approved_ancestor_block);
+
+ assert_eq!(
+ expected, undisputed_chain,
+ "TestCaseIntegrity: Provided and anticipated undisputed chain mismatch: {:?} vs {:?}",
+ undisputed_chain, expected,
+ );
+ }
+ highest_approved_ancestor_w_desc
+ })
+ })
+ .flatten();
test_skeleton(
&chain,
@@ -512,10 +499,10 @@ struct CaseVars {
/// ```raw
/// genesis -- 0xA1 --- 0xA2 --- 0xA3 --- 0xA4(!avail) --- 0xA5(!avail)
-/// \
-/// `- 0xB2
+/// \
+/// `- 0xB2
/// ```
-fn chain_0() -> CaseVars {
+fn chain_undisputed() -> CaseVars {
let head: Hash = ChainBuilder::GENESIS_HASH;
let mut builder = ChainBuilder::new();
@@ -531,6 +518,37 @@ fn chain_0() -> CaseVars {
builder.set_heads(vec![a5, b3]);
+ CaseVars {
+ chain: builder.init(),
+ target_block: a1,
+ best_chain_containing_block: Some(a5),
+ highest_approved_ancestor_block: Some(a3),
+ undisputed_chain: Some(a3),
+ expected_finality_target_result: Some(a3),
+ }
+}
+
+/// ```raw
+/// genesis -- 0xA1 --- 0xA2 --- 0xA3(disputed) --- 0xA4(!avail) --- 0xA5(!avail)
+/// \
+/// `- 0xB2
+/// ```
+fn chain_0() -> CaseVars {
+ let head: Hash = ChainBuilder::GENESIS_HASH;
+ let mut builder = ChainBuilder::new();
+
+ let a1 = builder.fast_forward_approved(0xA0, head, 1);
+ let a2 = builder.fast_forward_approved(0xA0, a1, 2);
+ let a3 = builder.fast_forward_disputed(0xA0, a2, 3);
+ let a4 = builder.fast_forward(0xA0, a3, 4);
+ let a5 = builder.fast_forward(0xA0, a4, 5);
+
+ let b1 = builder.fast_forward_approved(0xB0, a1, 2);
+ let b2 = builder.fast_forward_approved(0xB0, b1, 3);
+ let b3 = builder.fast_forward_approved(0xB0, b2, 4);
+
+ builder.set_heads(vec![a5, b3]);
+
CaseVars {
chain: builder.init(),
target_block: a1,
@@ -543,8 +561,8 @@ fn chain_0() -> CaseVars {
/// ```raw
/// genesis -- 0xA1 --- 0xA2(disputed) --- 0xA3
-/// \
-/// `- 0xB2 --- 0xB3(!available)
+/// \
+/// `- 0xB2 --- 0xB3(!available)
/// ```
fn chain_1() -> CaseVars {
let head: Hash = ChainBuilder::GENESIS_HASH;
@@ -555,7 +573,7 @@ fn chain_1() -> CaseVars {
let a3 = builder.fast_forward_approved(0xA0, a2, 3);
let b2 = builder.fast_forward_approved(0xB0, a1, 2);
- let b3 = builder.fast_forward_approved(0xB0, b2, 3);
+ let b3 = builder.fast_forward(0xB0, b2, 3);
builder.set_heads(vec![a3, b3]);
@@ -571,8 +589,8 @@ fn chain_1() -> CaseVars {
/// ```raw
/// genesis -- 0xA1 --- 0xA2(disputed) --- 0xA3
-/// \
-/// `- 0xB2 --- 0xB3
+/// \
+/// `- 0xB2 --- 0xB3
/// ```
fn chain_2() -> CaseVars {
let head: Hash = ChainBuilder::GENESIS_HASH;
@@ -599,8 +617,8 @@ fn chain_2() -> CaseVars {
/// ```raw
/// genesis -- 0xA1 --- 0xA2 --- 0xA3(disputed)
-/// \
-/// `- 0xB2 --- 0xB3
+/// \
+/// `- 0xB2 --- 0xB3
/// ```
fn chain_3() -> CaseVars {
let head: Hash = ChainBuilder::GENESIS_HASH;
@@ -627,10 +645,10 @@ fn chain_3() -> CaseVars {
/// ```raw
/// genesis -- 0xA1 --- 0xA2 --- 0xA3(disputed)
-/// \
-/// `- 0xB2 --- 0xB3
+/// \
+/// `- 0xB2 --- 0xB3
///
-/// ? --- NEX(does_not_exist)
+/// ? --- NEX(does_not_exist)
/// ```
fn chain_4() -> CaseVars {
let head: Hash = ChainBuilder::GENESIS_HASH;
@@ -715,6 +733,11 @@ fn chain_6() -> CaseVars {
}
}
+#[test]
+fn chain_sel_undisputed() {
+ run_specialized_test_w_harness(chain_undisputed);
+}
+
#[test]
fn chain_sel_0() {
run_specialized_test_w_harness(chain_0);