Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
approval-voting: query finalized block on startup and increase look back
Browse files Browse the repository at this point in the history
  • Loading branch information
@ordian
ordian committed Jan 12, 2022
1 parent 1601058 commit 7a195db
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 6 deletions.
10 changes: 6 additions & 4 deletions node/core/approval-voting/src/import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub(crate) async fn handle_new_head(
head: Hash,
finalized_number: &Option<BlockNumber>,
) -> SubsystemResult<Vec<BlockImportedCandidates>> {
// Update session info based on most recent head.
const MAX_HEADS_LOOK_BACK: BlockNumber = 500;

let mut span = jaeger::Span::new(head, "approval-checking-import");

Expand All @@ -329,6 +329,7 @@ pub(crate) async fn handle_new_head(
}
};

// Update session info based on most recent head.
match state.cache_session_info_for_head(ctx, head).await {
Err(e) => {
tracing::debug!(
Expand All @@ -350,9 +351,10 @@ pub(crate) async fn handle_new_head(
Ok(_) => {},
}

// If we've just started the node and haven't yet received any finality notifications,
// we don't do any look-back. Approval voting is only for nodes were already online.
let lower_bound_number = finalized_number.unwrap_or(header.number.saturating_sub(1));
// If we've just started the node and are far behind,
// import at most `MAX_HEADS_LOOK_BACK` blocks.
let lower_bound_number = header.number.saturating_sub(MAX_HEADS_LOOK_BACK);
let lower_bound_number = finalized_number.unwrap_or(lower_bound_number).max(lower_bound_number);

let new_blocks = determine_new_blocks(
ctx.sender(),
Expand Down
12 changes: 11 additions & 1 deletion node/core/approval-voting/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,17 @@ where
let mut currently_checking_set = CurrentlyCheckingSet::default();
let mut approvals_cache = lru::LruCache::new(APPROVAL_CACHE_SIZE);

let mut last_finalized_height: Option<BlockNumber> = None;
let mut last_finalized_height: Option<BlockNumber> = {
let (tx, rx) = oneshot::channel();
ctx.send_message(ChainApiMessage::FinalizedBlockNumber(tx)).await;
match rx.await? {
Ok(number) => Some(number),
Err(err) => {
tracing::warn!(target: LOG_TARGET, ?err, "Failed fetching finalized number");
None
},
}
};

loop {
let mut overlayed_db = OverlayedBackend::new(&backend);
Expand Down
145 changes: 144 additions & 1 deletion node/core/approval-voting/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,13 @@ fn subsystem_rejects_bad_assignment_ok_criteria() {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
let validator = ValidatorIndex(0);
Expand Down Expand Up @@ -1004,6 +1011,12 @@ fn subsystem_rejects_bad_assignment_err_criteria() {
test_harness(config, |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
Expand Down Expand Up @@ -1041,6 +1054,12 @@ fn subsystem_rejects_bad_assignment_err_criteria() {
fn blank_subsystem_act_on_bad_block() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle, .. } = test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let (tx, rx) = oneshot::channel();

Expand Down Expand Up @@ -1086,6 +1105,12 @@ fn subsystem_rejects_approval_if_no_candidate_entry() {
test_harness(config, |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
Expand Down Expand Up @@ -1143,6 +1168,12 @@ fn subsystem_rejects_approval_if_no_block_entry() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
Expand Down Expand Up @@ -1179,6 +1210,12 @@ fn subsystem_rejects_approval_before_assignment() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);

Expand Down Expand Up @@ -1237,6 +1274,12 @@ fn subsystem_rejects_assignment_in_future() {
test_harness(config, |test_harness| async move {
let TestHarness { mut virtual_overseer, clock, sync_oracle_handle: _sync_oracle_handle } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
Expand Down Expand Up @@ -1285,6 +1328,12 @@ fn subsystem_accepts_duplicate_assignment() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 0;
Expand Down Expand Up @@ -1330,6 +1379,12 @@ fn subsystem_rejects_assignment_with_unknown_candidate() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_index = 7;
Expand Down Expand Up @@ -1370,6 +1425,12 @@ fn subsystem_accepts_and_imports_approval_after_assignment() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;
assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);

Expand Down Expand Up @@ -1434,7 +1495,12 @@ fn subsystem_second_approval_import_only_schedules_wakeups() {
sync_oracle_handle: _sync_oracle_handle,
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);
clock.inner.lock().set_tick(APPROVAL_DELAY);

let block_hash = Hash::repeat_byte(0x01);
Expand Down Expand Up @@ -1545,6 +1611,13 @@ fn subsystem_assignment_import_updates_candidate_entry_and_schedules_wakeup() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);

let candidate_index = 0;
Expand Down Expand Up @@ -1587,6 +1660,13 @@ fn subsystem_process_wakeup_schedules_wakeup() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);

let candidate_index = 0;
Expand Down Expand Up @@ -1633,6 +1713,13 @@ fn linear_import_act_on_leaf() {
test_harness(HarnessConfig::default(), |test_harness| async move {
let TestHarness { mut virtual_overseer, .. } = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let mut head: Hash = ChainBuilder::GENESIS_HASH;
let mut builder = ChainBuilder::new();
for i in 1..session {
Expand Down Expand Up @@ -1684,6 +1771,13 @@ fn forkful_import_at_same_height_act_on_leaf() {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let mut head: Hash = ChainBuilder::GENESIS_HASH;
let mut builder = ChainBuilder::new();
for i in 1..session {
Expand Down Expand Up @@ -1751,6 +1845,13 @@ fn import_checked_approval_updates_entries_and_schedules() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let validator_index_a = ValidatorIndex(0);
let validator_index_b = ValidatorIndex(1);
Expand Down Expand Up @@ -1886,6 +1987,13 @@ fn subsystem_import_checked_approval_sets_one_block_bit_at_a_time() {
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);

let candidate_receipt1 = {
Expand Down Expand Up @@ -2016,6 +2124,13 @@ fn approved_ancestor_test(
let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } =
test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hashes = vec![
Hash::repeat_byte(0x01),
Hash::repeat_byte(0x02),
Expand Down Expand Up @@ -2176,6 +2291,13 @@ fn subsystem_validate_approvals_cache() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let fork_block_hash = Hash::repeat_byte(0x02);
let candidate_commitments = CandidateCommitments::default();
Expand Down Expand Up @@ -2383,6 +2505,13 @@ where
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let candidate_receipt = dummy_candidate_receipt(block_hash);
let candidate_hash = candidate_receipt.hash();
Expand Down Expand Up @@ -2696,6 +2825,13 @@ fn pre_covers_dont_stall_approval() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

let block_hash = Hash::repeat_byte(0x01);
let validator_index_a = ValidatorIndex(0);
let validator_index_b = ValidatorIndex(1);
Expand Down Expand Up @@ -2867,6 +3003,13 @@ fn waits_until_approving_assignments_are_old_enough() {
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);

clock.inner.lock().set_tick(APPROVAL_DELAY);

let block_hash = Hash::repeat_byte(0x01);
Expand Down
2 changes: 2 additions & 0 deletions node/service/src/relay_chain_selection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ use std::sync::Arc;
/// or disputes.
///
/// This is a safety net that should be removed at some point in the future.
// Until it's not, make sure to also update `MAX_HEADS_LOOK_BACK` in `approval-voting`
// when changing its value.
const MAX_FINALITY_LAG: polkadot_primitives::v1::BlockNumber = 500;

const LOG_TARGET: &str = "parachain::chain-selection";
Expand Down

0 comments on commit 7a195db

Please sign in to comment.