Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime: allow backing multiple candidates of same parachain on different cores #3231

Merged
merged 63 commits into from
Feb 23, 2024
Merged
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
cab6ab9
Switch statement table from ParaId to CoreIndex
sandreim Feb 1, 2024
d2df658
cargo lock
sandreim Feb 2, 2024
4a8b8d5
add experimental feature
sandreim Feb 6, 2024
9244632
inject core_index from statements
sandreim Feb 6, 2024
22e017b
temporary provisioner fix
sandreim Feb 6, 2024
9dc8927
Support injected `CoreIndex`
sandreim Feb 6, 2024
574b06a
Merge branch 'master' of github.com:paritytech/polkadot-sdk into sand…
sandreim Feb 6, 2024
fbb7351
cargo lock
sandreim Feb 6, 2024
c6b833e
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 6, 2024
6c72918
It was damn hard to fix these tests
sandreim Feb 12, 2024
fc5c109
These tests were easy to fix
sandreim Feb 12, 2024
33351b4
Fix comment
sandreim Feb 12, 2024
0d994bf
clippy was angry
sandreim Feb 12, 2024
534c019
A bit refactor and add a test
sandreim Feb 12, 2024
10d86dd
taplo happy
sandreim Feb 12, 2024
d990a65
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 13, 2024
fbe1ad5
BackedCandidate: make all members private and provide an interface
sandreim Feb 13, 2024
e74f038
refactor based on new BackedCandidate
sandreim Feb 13, 2024
d898740
Fix all parachain runtime tests affected
sandreim Feb 13, 2024
9e40490
fix more broken test on node side
sandreim Feb 13, 2024
42f46a0
wip new test
sandreim Feb 14, 2024
222609c
review feedback
sandreim Feb 14, 2024
532d363
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 14, 2024
ccb2a88
ElasticScalingCoreIndex
sandreim Feb 14, 2024
a5ba157
finish filtering of candidates for elastic scaling
sandreim Feb 14, 2024
a02e896
remove log
sandreim Feb 14, 2024
dd34850
more feedback
sandreim Feb 14, 2024
838a846
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 19, 2024
ad98f18
use next up on available instead of occupied core index
alindima Feb 19, 2024
606d7c4
ElasticScalingCoreIndex -> ElasticScalingMVP
alindima Feb 19, 2024
6fb6b73
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 20, 2024
10f6486
rename ElasticScalingCoreIndex
alindima Feb 20, 2024
f9e178d
address some comments
alindima Feb 20, 2024
ec7b660
+1
alindima Feb 20, 2024
578850b
more comments
alindima Feb 20, 2024
362ff1e
add a backing test
alindima Feb 20, 2024
2385369
add rstest
alindima Feb 20, 2024
c793b89
small nits and typos
alindima Feb 20, 2024
8398bb1
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 20, 2024
27ec25b
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 20, 2024
9f70276
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 21, 2024
19c9a67
review comments
alindima Feb 21, 2024
d7b6ce8
add zombienet test
alindima Feb 21, 2024
afed2a8
fix existing unit tests
alindima Feb 21, 2024
7ea040d
add prdoc
alindima Feb 21, 2024
79f281b
fix clippy
alindima Feb 21, 2024
7521ed9
try fixing prdoc
alindima Feb 21, 2024
9c3dd5c
cache Validator->Group mapping
alindima Feb 21, 2024
0b0b6d1
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 21, 2024
7976e2f
lockfile
alindima Feb 21, 2024
4d6e797
add tests for backedcandidate functions
alindima Feb 21, 2024
4c36440
newlines
alindima Feb 22, 2024
af1cd82
use Arc to avoid cloning
alindima Feb 22, 2024
5cc5b8b
Merge branch 'sandreim/backing_multiple_cores_per_para' into sandreim…
alindima Feb 22, 2024
dc57adb
add check for parachain stall to zombienet test
alindima Feb 22, 2024
bb5968c
add more unit tests
alindima Feb 22, 2024
1ca7a70
Merge remote-tracking branch 'origin/master' into sandreim/runtime_co…
alindima Feb 22, 2024
eb345b0
fix clippy
alindima Feb 22, 2024
058c0c2
refactor
alindima Feb 22, 2024
d4c58bd
fix some bugs and add more unit tests
alindima Feb 23, 2024
cb41758
update some comments
alindima Feb 23, 2024
bffa4e9
review comments
alindima Feb 23, 2024
5d3a85d
fix unit test
alindima Feb 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Switch statement table from ParaId to CoreIndex
Signed-off-by: Andrei Sandu <[email protected]>
  • Loading branch information
sandreim committed Feb 1, 2024

Verified

This commit was signed with the committer’s verified signature.
commit cab6ab9c06995ab90da3b77bd276143052be5574
141 changes: 96 additions & 45 deletions polkadot/node/core/backing/src/lib.rs
Original file line number Diff line number Diff line change
@@ -105,9 +105,9 @@ use polkadot_node_subsystem_util::{
};
use polkadot_primitives::{
BackedCandidate, CandidateCommitments, CandidateHash, CandidateReceipt,
CommittedCandidateReceipt, CoreIndex, CoreState, ExecutorParams, Hash, Id as ParaId,
PersistedValidationData, PvfExecKind, SigningContext, ValidationCode, ValidatorId,
ValidatorIndex, ValidatorSignature, ValidityAttestation,
CommittedCandidateReceipt, CoreIndex, CoreState, ExecutorParams, GroupIndex, Hash,
Id as ParaId, PersistedValidationData, PvfExecKind, SigningContext, ValidationCode,
ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation,
};
use sp_keystore::KeystorePtr;
use statement_table::{
@@ -208,8 +208,10 @@ struct PerRelayParentState {
prospective_parachains_mode: ProspectiveParachainsMode,
/// The hash of the relay parent on top of which this job is doing it's work.
parent: Hash,
/// The `ParaId` assigned to the local validator at this relay parent.
assignment: Option<ParaId>,
/// The `CoreIndex` assigned to the local validator at this relay parent.
assigned_para: Option<ParaId>,
/// The `CoreIndex` assigned to the local validator at this relay parent.
assigned_core: Option<CoreIndex>,
/// The candidates that are backed by enough validators in their group, by hash.
backed: HashSet<CandidateHash>,
/// The table of candidates and statements under this relay-parent.
@@ -382,7 +384,7 @@ struct AttestingData {
#[derive(Default)]
struct TableContext {
validator: Option<Validator>,
groups: HashMap<ParaId, Vec<ValidatorIndex>>,
groups: HashMap<CoreIndex, Vec<ValidatorIndex>>,
validators: Vec<ValidatorId>,
disabled_validators: Vec<ValidatorIndex>,
}
@@ -404,23 +406,19 @@ impl TableContext {
impl TableContextTrait for TableContext {
type AuthorityId = ValidatorIndex;
type Digest = CandidateHash;
type GroupId = ParaId;
type GroupId = CoreIndex;
type Signature = ValidatorSignature;
type Candidate = CommittedCandidateReceipt;

fn candidate_digest(candidate: &CommittedCandidateReceipt) -> CandidateHash {
candidate.hash()
}

fn candidate_group(candidate: &CommittedCandidateReceipt) -> ParaId {
candidate.descriptor().para_id
fn is_member_of(&self, authority: &ValidatorIndex, core: &CoreIndex) -> bool {
self.groups.get(core).map_or(false, |g| g.iter().any(|a| a == authority))
}

fn is_member_of(&self, authority: &ValidatorIndex, group: &ParaId) -> bool {
self.groups.get(group).map_or(false, |g| g.iter().any(|a| a == authority))
}

fn get_group_size(&self, group: &ParaId) -> Option<usize> {
fn get_group_size(&self, group: &CoreIndex) -> Option<usize> {
self.groups.get(group).map(|g| g.len())
}
}
@@ -442,20 +440,22 @@ fn primitive_statement_to_table(s: &SignedFullStatementWithPVD) -> TableSignedSt

fn table_attested_to_backed(
attested: TableAttestedCandidate<
ParaId,
CoreIndex,
CommittedCandidateReceipt,
ValidatorIndex,
ValidatorSignature,
>,
table_context: &TableContext,
) -> Option<BackedCandidate> {
let TableAttestedCandidate { candidate, validity_votes, group_id: para_id } = attested;
let TableAttestedCandidate { candidate, validity_votes, group_id: core_index } = attested;

let (ids, validity_votes): (Vec<_>, Vec<ValidityAttestation>) =
validity_votes.into_iter().map(|(id, vote)| (id, vote.into())).unzip();

let group = table_context.groups.get(&para_id)?;
let group = table_context.groups.get(&core_index)?;

// TODO: This si a temporary fix and will not work if a para is assigned to
// different sized backing groups. We need core index in the candidate descriptor
let mut validator_indices = BitVec::with_capacity(group.len());

validator_indices.resize(group.len(), false);
@@ -981,6 +981,56 @@ async fn handle_active_leaves_update<Context>(
Ok(())
}

macro_rules! try_runtime_api {
($x: expr) => {
match $x {
Ok(x) => x,
Err(err) => {
// Only bubble up fatal errors.
error::log_error(Err(Into::<runtime::Error>::into(err).into()))?;

// We can't do candidate validation work if we don't have the
// requisite runtime API data. But these errors should not take
// down the node.
return Ok(None)
},
}
};
}

#[overseer::contextbounds(CandidateBacking, prefix = self::overseer)]
async fn core_index_from_statement<Context>(
ctx: &mut Context,
relay_parent: Hash,
statement: &SignedFullStatementWithPVD,
) -> Result<Option<CoreIndex>, Error> {
let parent = relay_parent;

let (groups, cores) = futures::try_join!(
request_validator_groups(parent, ctx.sender()).await,
request_from_runtime(parent, ctx.sender(), |tx| {
RuntimeApiRequest::AvailabilityCores(tx)
},)
.await,
)
.map_err(Error::JoinMultiple)?;
let (validator_groups, group_rotation_info) = try_runtime_api!(groups);
let cores = try_runtime_api!(cores);

let statement_validator_index = statement.validator_index();
for (group_index, group) in validator_groups.iter().enumerate() {
for validator_index in group {
if *validator_index == statement_validator_index {
return Ok(Some(
group_rotation_info.core_for_group(GroupIndex(group_index as u32), cores.len()),
))
}
}
}

Ok(None)
}

/// Load the data necessary to do backing work on top of a relay-parent.
#[overseer::contextbounds(CandidateBacking, prefix = self::overseer)]
async fn construct_per_relay_parent_state<Context>(
@@ -989,23 +1039,6 @@ async fn construct_per_relay_parent_state<Context>(
keystore: &KeystorePtr,
mode: ProspectiveParachainsMode,
) -> Result<Option<PerRelayParentState>, Error> {
macro_rules! try_runtime_api {
($x: expr) => {
match $x {
Ok(x) => x,
Err(err) => {
// Only bubble up fatal errors.
error::log_error(Err(Into::<runtime::Error>::into(err).into()))?;

// We can't do candidate validation work if we don't have the
// requisite runtime API data. But these errors should not take
// down the node.
return Ok(None)
},
}
};
}

let parent = relay_parent;

let (session_index, validators, groups, cores) = futures::try_join!(
@@ -1055,9 +1088,11 @@ async fn construct_per_relay_parent_state<Context>(
},
};

let mut groups = HashMap::new();
let n_cores = cores.len();
let mut assignment = None;

let mut groups = HashMap::<CoreIndex, Vec<ValidatorIndex>>::new();
let mut assigned_core = None;
let mut assigned_para = None;

for (idx, core) in cores.into_iter().enumerate() {
let core_para_id = match core {
@@ -1077,11 +1112,13 @@ async fn construct_per_relay_parent_state<Context>(
let group_index = group_rotation_info.group_for_core(core_index, n_cores);
if let Some(g) = validator_groups.get(group_index.0 as usize) {
if validator.as_ref().map_or(false, |v| g.contains(&v.index())) {
assignment = Some(core_para_id);
assigned_para = Some(core_para_id);
assigned_core = Some(core_index);
}
groups.insert(core_para_id, g.clone());
groups.insert(core_index, g.clone());
}
}
gum::debug!(target: LOG_TARGET, ?groups, "TableContext" );

let table_context = TableContext { validator, groups, validators, disabled_validators };
let table_config = TableConfig {
@@ -1094,7 +1131,8 @@ async fn construct_per_relay_parent_state<Context>(
Ok(Some(PerRelayParentState {
prospective_parachains_mode: mode,
parent,
assignment,
assigned_core,
assigned_para,
backed: HashSet::new(),
table: Table::new(table_config),
table_context,
@@ -1519,15 +1557,16 @@ async fn import_statement<Context>(
per_candidate: &mut HashMap<CandidateHash, PerCandidateState>,
statement: &SignedFullStatementWithPVD,
) -> Result<Option<TableSummary>, Error> {
let candidate_hash = statement.payload().candidate_hash();

gum::debug!(
target: LOG_TARGET,
statement = ?statement.payload().to_compact(),
validator_index = statement.validator_index().0,
?candidate_hash,
"Importing statement",
);

let candidate_hash = statement.payload().candidate_hash();

// If this is a new candidate (statement is 'seconded' and candidate is unknown),
// we need to create an entry in the `PerCandidateState` map.
//
@@ -1593,7 +1632,11 @@ async fn import_statement<Context>(

let stmt = primitive_statement_to_table(statement);

Ok(rp_state.table.import_statement(&rp_state.table_context, stmt))
let core = core_index_from_statement(ctx, rp_state.parent, statement)
.await
.unwrap()
.unwrap();
Ok(rp_state.table.import_statement(&rp_state.table_context, core, stmt))
}

/// Handles a summary received from [`import_statement`] and dispatches `Backed` notifications and
@@ -1654,8 +1697,14 @@ async fn post_import_statement_actions<Context>(
);
ctx.send_unbounded_message(message);
}
} else {
gum::debug!(target: LOG_TARGET, ?candidate_hash, "Cannot get BackedCandidate");
}
} else {
gum::debug!(target: LOG_TARGET, ?candidate_hash, "Candidate already known");
}
} else {
gum::debug!(target: LOG_TARGET, "No attested candidate");
}

issue_new_misbehaviors(ctx, rp_state.parent, &mut rp_state.table);
@@ -1859,9 +1908,10 @@ async fn maybe_validate_and_import<Context>(

let candidate_hash = summary.candidate;

if Some(summary.group_id) != rp_state.assignment {
if Some(summary.group_id) != rp_state.assigned_core {
return Ok(())
}

let attesting = match statement.payload() {
StatementWithPVD::Seconded(receipt, _) => {
let attesting = AttestingData {
@@ -2004,10 +2054,11 @@ async fn handle_second_message<Context>(
}

// Sanity check that candidate is from our assignment.
if Some(candidate.descriptor().para_id) != rp_state.assignment {
if Some(candidate.descriptor().para_id) != rp_state.assigned_para {
gum::debug!(
target: LOG_TARGET,
our_assignment = ?rp_state.assignment,
our_assignment_core = ?rp_state.assigned_core,
our_assignment_para = ?rp_state.assigned_para,
collation = ?candidate.descriptor().para_id,
"Subsystem asked to second for para outside of our assignment",
);
2 changes: 2 additions & 0 deletions polkadot/primitives/Cargo.toml
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@ hex-literal = "0.4.1"
parity-scale-codec = { version = "3.6.1", default-features = false, features = ["bit-vec", "derive"] }
scale-info = { version = "2.10.0", default-features = false, features = ["bit-vec", "derive", "serde"] }
serde = { version = "1.0.195", default-features = false, features = ["alloc", "derive"] }
log = { version = "0.4.17", default-features = false }

application-crypto = { package = "sp-application-crypto", path = "../../substrate/primitives/application-crypto", default-features = false, features = ["serde"] }
inherents = { package = "sp-inherents", path = "../../substrate/primitives/inherents", default-features = false }
@@ -44,6 +45,7 @@ std = [
"primitives/std",
"runtime_primitives/std",
"scale-info/std",
"log/std",
"serde/std",
"sp-api/std",
"sp-arithmetic/std",
33 changes: 32 additions & 1 deletion polkadot/primitives/src/v6/mod.rs
Original file line number Diff line number Diff line change
@@ -72,6 +72,7 @@ pub use metrics::{

/// The key type ID for a collator key.
pub const COLLATOR_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"coll");
const LOG_TARGET: &str = "runtime::primitives";

mod collator_app {
use application_crypto::{app_crypto, sr25519};
@@ -743,17 +744,35 @@ impl<H> BackedCandidate<H> {
///
/// Returns either an error, indicating that one of the signatures was invalid or that the index
/// was out-of-bounds, or the number of signatures checked.
pub fn check_candidate_backing<H: AsRef<[u8]> + Clone + Encode>(
pub fn check_candidate_backing<H: AsRef<[u8]> + Clone + Encode + core::fmt::Debug>(
backed: &BackedCandidate<H>,
signing_context: &SigningContext<H>,
group_len: usize,
validator_lookup: impl Fn(usize) -> Option<ValidatorId>,
) -> Result<usize, ()> {
log::debug!(
target: LOG_TARGET,
"checking candidate {:?}",
backed
);

if backed.validator_indices.len() != group_len {
log::debug!(
target: LOG_TARGET,
"indices mismatch: group_len = {} , indices_len = {}",
group_len,
backed.validator_indices.len(),
);
return Err(())
}

if backed.validity_votes.len() > group_len {
log::debug!(
target: LOG_TARGET,
"Too many votes, expected: {}, found: {}",
group_len,
backed.validity_votes.len(),
);
return Err(())
}

@@ -775,11 +794,23 @@ pub fn check_candidate_backing<H: AsRef<[u8]> + Clone + Encode>(
if sig.verify(&payload[..], &validator_id) {
signed += 1;
} else {
log::debug!(
target: LOG_TARGET,
"Invalid signature. validator_id = {:?}, validator_index = {} ",
validator_id,
val_in_group_idx,
);
return Err(())
}
}

if signed != backed.validity_votes.len() {
log::error!(
target: LOG_TARGET,
"Too many signatures, expected = {}, found = {}",
backed.validity_votes.len() ,
signed,
);
return Err(())
}

Loading