Skip to content

Commit

Permalink
FML: fix tests and cache runtime query
Browse files Browse the repository at this point in the history
  • Loading branch information
@ordian
ordian committed Dec 11, 2023
1 parent 6f1c071 commit dd13f8f
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 13 deletions.
1 change: 0 additions & 1 deletion polkadot/node/core/dispute-coordinator/src/participation/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ fn cannot_participate_if_cannot_recover_validation_code() {
let mut participation = Participation::new(sender, Metrics::default());
activate_leaf(&mut ctx, &mut participation, 10).await.unwrap();
participate(&mut ctx, &mut participation).await.unwrap();

recover_available_data(&mut ctx_handle).await;

assert_matches!(
Expand Down
84 changes: 76 additions & 8 deletions polkadot/node/core/dispute-coordinator/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,19 @@ impl TestState {
)) => {
tx.send(Ok(Vec::new())).unwrap();
},
AllMessages::RuntimeApi(RuntimeApiMessage::Request(
_new_leaf,
RuntimeApiRequest::Version(tx),
)) => {
tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT))
.unwrap();
},
AllMessages::RuntimeApi(RuntimeApiMessage::Request(
_new_leaf,
RuntimeApiRequest::DisabledValidators(tx),
)) => {
tx.send(Ok(Vec::new())).unwrap();
},
AllMessages::ChainApi(ChainApiMessage::Ancestors { hash, k, response_channel }) => {
let target_header = self
.headers
Expand Down Expand Up @@ -730,6 +743,7 @@ fn too_many_unconfirmed_statements_are_considered_spam() {
.await;
gum::trace!("After sending `ImportStatements`");

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash1, HashMap::new())
.await;

Expand Down Expand Up @@ -862,6 +876,7 @@ fn approval_vote_import_works() {
.into_iter()
.collect();

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash1, approval_votes)
.await;

Expand Down Expand Up @@ -969,6 +984,7 @@ fn dispute_gets_confirmed_via_participation() {
})
.await;
gum::debug!("After First import!");
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash1, HashMap::new())
.await;

Expand Down Expand Up @@ -1118,6 +1134,7 @@ fn dispute_gets_confirmed_at_byzantine_threshold() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash1, HashMap::new())
.await;

Expand Down Expand Up @@ -1242,6 +1259,7 @@ fn backing_statements_import_works_and_no_spam() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
assert_matches!(confirmation_rx.await, Ok(ImportStatementsResult::ValidImport));

{
Expand Down Expand Up @@ -1374,6 +1392,7 @@ fn conflicting_votes_lead_to_dispute_participation() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -1493,6 +1512,7 @@ fn positive_votes_dont_trigger_participation() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;

{
let (tx, rx) = oneshot::channel();
Expand Down Expand Up @@ -1603,6 +1623,7 @@ fn wrong_validator_index_is_ignored() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;

{
let (tx, rx) = oneshot::channel();
Expand Down Expand Up @@ -1680,6 +1701,7 @@ fn finality_votes_ignore_disputed_candidates() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -1756,14 +1778,10 @@ fn supermajority_valid_dispute_may_be_finalized() {

let candidate_receipt = make_valid_candidate_receipt();
let candidate_hash = candidate_receipt.hash();
let candidate_events = vec![make_candidate_backed_event(candidate_receipt.clone())];

test_state
.activate_leaf_at_session(
&mut virtual_overseer,
session,
1,
vec![make_candidate_backed_event(candidate_receipt.clone())],
)
.activate_leaf_at_session(&mut virtual_overseer, session, 1, candidate_events)
.await;

let supermajority_threshold =
Expand Down Expand Up @@ -1792,6 +1810,7 @@ fn supermajority_valid_dispute_may_be_finalized() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -1929,6 +1948,7 @@ fn concluded_supermajority_for_non_active_after_time() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -2045,6 +2065,7 @@ fn concluded_supermajority_against_non_active_after_time() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;
assert_matches!(confirmation_rx.await.unwrap(),
Expand Down Expand Up @@ -2160,6 +2181,7 @@ fn resume_dispute_without_local_statement() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -2204,13 +2226,23 @@ fn resume_dispute_without_local_statement() {
let candidate_receipt = make_valid_candidate_receipt();
let candidate_hash = candidate_receipt.hash();

participation_with_distribution(
participation_full_happy_path(
&mut virtual_overseer,
&candidate_hash,
candidate_receipt.commitments_hash,
)
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;

assert_matches!(
virtual_overseer.recv().await,
AllMessages::DisputeDistribution(
DisputeDistributionMessage::SendDispute(msg)
) => {
assert_eq!(msg.candidate_receipt().hash(), candidate_hash);
}
);

let mut statements = Vec::new();
// Getting votes for supermajority. Should already have two valid votes.
for i in vec![3, 4, 5, 6, 7] {
Expand Down Expand Up @@ -2315,6 +2347,7 @@ fn resume_dispute_with_local_statement() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -2412,6 +2445,7 @@ fn resume_dispute_without_local_statement_or_local_key() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(
&mut virtual_overseer,
&candidate_hash,
Expand Down Expand Up @@ -2503,6 +2537,7 @@ fn issue_local_statement_does_cause_distribution_but_not_duplicate_participation
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
assert_eq!(confirmation_rx.await, Ok(ImportStatementsResult::ValidImport));

// Initiate dispute locally:
Expand Down Expand Up @@ -2600,6 +2635,7 @@ fn own_approval_vote_gets_distributed_on_dispute() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -2654,6 +2690,7 @@ fn negative_issue_local_statement_only_triggers_import() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
// Assert that subsystem is not participating.
assert!(virtual_overseer.recv().timeout(TEST_TIMEOUT).await.is_none());

Expand Down Expand Up @@ -2719,6 +2756,7 @@ fn redundant_votes_ignored() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
rx.await.unwrap();

let (tx, rx) = oneshot::channel();
Expand Down Expand Up @@ -2793,6 +2831,7 @@ fn no_onesided_disputes() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
assert_matches!(confirmation_rx.await, Ok(ImportStatementsResult::ValidImport));

// We should not have any active disputes now.
Expand Down Expand Up @@ -2856,6 +2895,7 @@ fn refrain_from_participation() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -2948,6 +2988,7 @@ fn participation_for_included_candidates() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -3036,6 +3077,7 @@ fn local_participation_in_dispute_for_backed_candidate() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

Expand Down Expand Up @@ -3177,6 +3219,7 @@ fn participation_requests_reprioritized_for_newly_included() {
})
.await;

handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
// Handle corresponding messages to unblock import
// we need to handle `ApprovalVotingMessage::GetApprovalSignaturesForCandidate` for
// import
Expand Down Expand Up @@ -3330,6 +3373,7 @@ fn informs_chain_selection_when_dispute_concluded_against() {
},
})
.await;
handle_disabled_validators_queries(&mut virtual_overseer, Vec::new()).await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;
assert_matches!(confirmation_rx.await.unwrap(),
Expand Down Expand Up @@ -3617,3 +3661,27 @@ fn session_info_small_jump_works() {
})
});
}

async fn handle_disabled_validators_queries(
virtual_overseer: &mut VirtualOverseer,
disabled_validators: Vec<ValidatorIndex>,
) {
assert_matches!(
virtual_overseer.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(
_new_leaf,
RuntimeApiRequest::Version(tx),
)) => {
tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT)).unwrap();
}
);
assert_matches!(
virtual_overseer.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(
_new_leaf,
RuntimeApiRequest::DisabledValidators(tx),
)) => {
tx.send(Ok(disabled_validators)).unwrap();
}
);
}
21 changes: 17 additions & 4 deletions polkadot/node/subsystem-util/src/runtime/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ pub struct RuntimeInfo {
/// overseer seems sensible.
session_index_cache: LruMap<Hash, SessionIndex>,

/// In the happy case, we do not query disabled validators at all. In the worst case, we can
/// query it order of `n_cores` times `n_validators` per block, so caching it here seems
/// sensible.
disabled_validators_cache: LruMap<Hash, Vec<ValidatorIndex>>,

/// Look up cached sessions by `SessionIndex`.
session_info_cache: LruMap<SessionIndex, ExtendedSessionInfo>,

Expand Down Expand Up @@ -125,6 +130,7 @@ impl RuntimeInfo {
Self {
session_index_cache: LruMap::new(ByLength::new(cfg.session_cache_lru_size.max(10))),
session_info_cache: LruMap::new(ByLength::new(cfg.session_cache_lru_size)),
disabled_validators_cache: LruMap::new(ByLength::new(100)),
pinned_blocks: LruMap::new(ByLength::new(cfg.session_cache_lru_size)),
keystore: cfg.keystore,
}
Expand Down Expand Up @@ -185,10 +191,17 @@ impl RuntimeInfo {
where
Sender: SubsystemSender<RuntimeApiMessage>,
{
let disabled_validators = get_disabled_validators_with_fallback(sender, relay_parent)
.await
.map_err(|_| JfyiError::NoSuchSession(0))?; // TODO (@ordian): make this method return runtime error
Ok(disabled_validators)
match self.disabled_validators_cache.get(&relay_parent).cloned() {
Some(result) => Ok(result),
None => {
let disabled_validators =
get_disabled_validators_with_fallback(sender, relay_parent)
.await
.map_err(|_| JfyiError::NoSuchSession(0))?; // TODO (@ordian): make this method return runtime error
self.disabled_validators_cache.insert(relay_parent, disabled_validators.clone());
Ok(disabled_validators)
},
}
}

/// Get `ExtendedSessionInfo` by session index.
Expand Down

0 comments on commit dd13f8f

Please sign in to comment.