diff --git a/polkadot/node/core/approval-voting/src/import.rs b/polkadot/node/core/approval-voting/src/import.rs index c0fb23c2b2..1d382096f7 100644 --- a/polkadot/node/core/approval-voting/src/import.rs +++ b/polkadot/node/core/approval-voting/src/import.rs @@ -303,7 +303,7 @@ pub(crate) async fn handle_new_head( head: Hash, finalized_number: &Option, ) -> SubsystemResult> { - // 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"); @@ -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!( @@ -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(), diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index 4427b83fbc..688c7185a4 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -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 = None; + let mut last_finalized_height: Option = { + 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); @@ -1143,7 +1153,7 @@ async fn handle_from_overseer( actions }, FromOverseer::Signal(OverseerSignal::BlockFinalized(block_hash, block_number)) => { - tracing::debug!(target: LOG_TARGET, ?block_hash, ?block_number, "Block finalized",); + tracing::debug!(target: LOG_TARGET, ?block_hash, ?block_number, "Block finalized"); *last_finalized_height = Some(block_number); crate::ops::canonicalize(db, block_number, block_hash) diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index 7b6d9964fd..023551b02d 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -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); @@ -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; @@ -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(); @@ -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; @@ -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; @@ -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); @@ -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; @@ -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; @@ -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; @@ -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); @@ -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); @@ -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; @@ -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; @@ -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 { @@ -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 { @@ -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); @@ -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 = { @@ -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), @@ -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(); @@ -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(); @@ -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); @@ -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); diff --git a/polkadot/node/service/src/relay_chain_selection.rs b/polkadot/node/service/src/relay_chain_selection.rs index 317cdb03f3..56789e2aa6 100644 --- a/polkadot/node/service/src/relay_chain_selection.rs +++ b/polkadot/node/service/src/relay_chain_selection.rs @@ -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";