Skip to content

Commit

Permalink
Fix PVF precompilation for Kusama (paritytech#5606)
Browse files Browse the repository at this point in the history
![image](https://github.com/user-attachments/assets/2deaee85-67c3-4119-b0c0-d2e7f818b4ea)

Because on Kusama validators.len() < discovery_keys.len() we can tweak
the PVF precompilation to allow prepare PVFs when the node is an
authority but not a validator.
  • Loading branch information
AndreiEres committed Sep 6, 2024
1 parent 986e7ae commit 5040b3c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
11 changes: 6 additions & 5 deletions polkadot/node/core/candidate-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use polkadot_primitives::{
},
AuthorityDiscoveryId, CandidateCommitments, CandidateDescriptor, CandidateEvent,
CandidateReceipt, ExecutorParams, Hash, OccupiedCoreAssumption, PersistedValidationData,
PvfExecKind, PvfPrepKind, SessionIndex, ValidationCode, ValidationCodeHash,
PvfExecKind, PvfPrepKind, SessionIndex, ValidationCode, ValidationCodeHash, ValidatorId,
};
use sp_application_crypto::{AppCrypto, ByteArray};
use sp_keystore::KeystorePtr;
Expand Down Expand Up @@ -427,14 +427,15 @@ where
.iter()
.any(|v| keystore.has_keys(&[(v.to_raw_vec(), AuthorityDiscoveryId::ID)]));

let is_present_authority = session_info
.discovery_keys
// We could've checked discovery_keys but on Kusama validators.len() < discovery_keys.len().
let is_present_validator = session_info
.validators
.iter()
.any(|v| keystore.has_keys(&[(v.to_raw_vec(), AuthorityDiscoveryId::ID)]));
.any(|v| keystore.has_keys(&[(v.to_raw_vec(), ValidatorId::ID)]));

// There is still a chance to be a previous session authority, but this extra work does not
// affect the finalization.
is_past_present_or_future_authority && !is_present_authority
is_past_present_or_future_authority && !is_present_validator
}

// Sends PVF with unknown code hashes to the validation host returning the list of code hashes sent.
Expand Down
21 changes: 10 additions & 11 deletions polkadot/node/core/candidate-validation/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ use polkadot_node_subsystem::messages::AllMessages;
use polkadot_node_subsystem_util::reexports::SubsystemContext;
use polkadot_overseer::ActivatedLeaf;
use polkadot_primitives::{
CoreIndex, GroupIndex, HeadData, Id as ParaId, IndexedVec, SessionInfo, UpwardMessage,
ValidatorId, ValidatorIndex,
CoreIndex, GroupIndex, HeadData, Id as ParaId, SessionInfo, UpwardMessage, ValidatorId,
};
use polkadot_primitives_test_helpers::{
dummy_collator, dummy_collator_signature, dummy_hash, make_valid_candidate_descriptor,
};
use sp_core::testing::TaskExecutor;
use sp_core::{sr25519::Public, testing::TaskExecutor};
use sp_keyring::Sr25519Keyring;
use sp_keystore::{testing::MemoryKeystore, Keystore};

Expand Down Expand Up @@ -1194,10 +1193,10 @@ fn dummy_candidate_backed(
)
}

fn dummy_session_info(discovery_keys: Vec<AuthorityDiscoveryId>) -> SessionInfo {
fn dummy_session_info(keys: Vec<Public>) -> SessionInfo {
SessionInfo {
validators: IndexedVec::<ValidatorIndex, ValidatorId>::from(vec![]),
discovery_keys,
validators: keys.iter().cloned().map(Into::into).collect(),
discovery_keys: keys.iter().cloned().map(Into::into).collect(),
assignment_keys: vec![],
validator_groups: Default::default(),
n_cores: 4u32,
Expand Down Expand Up @@ -1246,7 +1245,7 @@ fn maybe_prepare_validation_golden_path() {
ctx_handle.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionInfo(index, tx))) => {
assert_eq!(index, 1);
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public().into()]))));
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public()]))));
}
);

Expand Down Expand Up @@ -1364,7 +1363,7 @@ fn maybe_prepare_validation_resets_state_on_a_new_session() {
ctx_handle.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionInfo(index, tx))) => {
assert_eq!(index, 2);
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public().into()]))));
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public()]))));
}
);
};
Expand Down Expand Up @@ -1510,7 +1509,7 @@ fn maybe_prepare_validation_does_not_prepare_pvfs_if_not_a_validator_in_the_next
ctx_handle.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionInfo(index, tx))) => {
assert_eq!(index, 1);
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public().into()]))));
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public()]))));
}
);
};
Expand Down Expand Up @@ -1557,7 +1556,7 @@ fn maybe_prepare_validation_does_not_prepare_pvfs_if_a_validator_in_the_current_
ctx_handle.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionInfo(index, tx))) => {
assert_eq!(index, 1);
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Alice.public().into()]))));
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Alice.public()]))));
}
);
};
Expand Down Expand Up @@ -1604,7 +1603,7 @@ fn maybe_prepare_validation_prepares_a_limited_number_of_pvfs() {
ctx_handle.recv().await,
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionInfo(index, tx))) => {
assert_eq!(index, 1);
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public().into()]))));
let _ = tx.send(Ok(Some(dummy_session_info(vec![Sr25519Keyring::Bob.public()]))));
}
);

Expand Down
13 changes: 13 additions & 0 deletions prdoc/pr_5606.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix PVF precompilation for Kusama

doc:
- audience: Node Operator
description: |
Tweaks the PVF precompilation on Kusama to allow prepare PVFs when the node is an authority but not a validator.

crates:
- name: polkadot-node-core-candidate-validation
bump: patch

0 comments on commit 5040b3c

Please sign in to comment.