Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions rs/consensus/dkg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1805,7 +1805,8 @@ mod tests {
deps.crypto.as_ref(),
&pool_reader,
&*deps.dkg_pool.read().unwrap(),
parent,
parent.clone(),
&pool_reader.dkg_summary_block(&parent).unwrap(),
block.payload.as_ref(),
deps.state_manager.as_ref(),
&block.context,
Expand Down Expand Up @@ -1954,7 +1955,8 @@ mod tests {
deps.crypto.as_ref(),
&pool_reader,
&*deps.dkg_pool.read().unwrap(),
parent,
parent.clone(),
&pool_reader.dkg_summary_block(&parent).unwrap(),
&payload_without_remote,
deps.state_manager.as_ref(),
&block.context,
Expand Down Expand Up @@ -2061,6 +2063,7 @@ mod tests {
&pool_reader,
&*deps.dkg_pool.read().unwrap(),
parent.clone(),
&pool_reader.dkg_summary_block(&parent).unwrap(),
&payload,
deps.state_manager.as_ref(),
&validation_context,
Expand Down
7 changes: 2 additions & 5 deletions rs/consensus/dkg/src/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,14 @@ pub fn create_payload(
pool_reader: &PoolReader<'_>,
dkg_pool: Arc<RwLock<dyn DkgPool>>,
parent: &Block,
last_summary_block: &Block,
state_reader: &dyn StateReader<State = ReplicatedState>,
validation_context: &ValidationContext,
logger: ReplicaLogger,
max_dealings_per_block: usize,
dkg_payload_metrics: Option<&DkgPayloadMetrics>,
) -> Result<DkgPayload, DkgPayloadCreationError> {
let height = parent.height.increment();
// Get the last summary from the chain.
let last_summary_block = pool_reader
.dkg_summary_block(parent)
.ok_or(DkgPayloadCreationError::MissingDkgStartBlock)?;
let last_dkg_summary = &last_summary_block.payload.as_ref().as_summary().dkg;

if last_dkg_summary.get_next_start_height() == height {
Expand Down Expand Up @@ -96,7 +93,7 @@ pub fn create_payload(
dkg_pool,
parent,
max_dealings_per_block,
&last_summary_block,
last_summary_block,
last_dkg_summary,
crypto,
state_reader,
Expand Down
24 changes: 17 additions & 7 deletions rs/consensus/dkg/src/payload_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub fn validate_payload(
pool_reader: &PoolReader<'_>,
dkg_pool: &dyn DkgPool,
parent: Block,
last_summary_block: &Block,
payload: &BlockPayload,
state_reader: &dyn StateReader<State = ReplicatedState>,
validation_context: &ValidationContext,
Expand All @@ -45,13 +46,7 @@ pub fn validate_payload(
.registry_version(current_height)
.ok_or(DkgPayloadValidationFailure::FailedToGetRegistryVersion)?;

let last_summary_block = pool_reader
.dkg_summary_block(&parent)
// We expect the parent to be valid, so there will be _always_ a DKG start block on the
// chain.
.expect("No DKG start block found for the parent block.");
let last_dkg_summary = &last_summary_block.payload.as_ref().as_summary().dkg;

let is_dkg_start_height = last_dkg_summary.get_next_start_height() == current_height;

match payload {
Expand Down Expand Up @@ -335,6 +330,9 @@ mod tests {
let block = Block::from(pool.make_next_block());
let block_payload = block.payload.as_ref();

let last_summary_block = PoolReader::new(&pool)
.dkg_summary_block(&parent_block)
.unwrap();
assert!(
validate_payload(
subnet_test_id(0),
Expand All @@ -343,6 +341,7 @@ mod tests {
&PoolReader::new(&pool),
dkg_pool.read().unwrap().deref(),
parent_block,
&last_summary_block,
block_payload,
state_manager.as_ref(),
&context,
Expand All @@ -359,6 +358,9 @@ mod tests {
let block = Block::from(pool.make_next_block());
let summary = block.payload.as_ref();

let last_summary_block = PoolReader::new(&pool)
.dkg_summary_block(&parent_block)
.unwrap();
assert!(
validate_payload(
subnet_test_id(0),
Expand All @@ -367,6 +369,7 @@ mod tests {
&PoolReader::new(&pool),
dkg_pool.read().unwrap().deref(),
parent_block,
&last_summary_block,
summary,
state_manager.as_ref(),
&context,
Expand Down Expand Up @@ -579,6 +582,7 @@ mod tests {
idkg: idkg::Payload::default(),
});

let last_summary_block = PoolReader::new(&pool).dkg_summary_block(&parent).unwrap();
assert_eq!(
validate_payload(
SUBNET_1,
Expand All @@ -587,6 +591,7 @@ mod tests {
&PoolReader::new(&pool),
dkg_pool.read().unwrap().deref(),
parent,
&last_summary_block,
&block_payload,
state_manager.as_ref(),
&context,
Expand Down Expand Up @@ -653,13 +658,15 @@ mod tests {
idkg: idkg::Payload::default(),
});

let last_summary_block = PoolReader::new(&pool).dkg_summary_block(&parent).unwrap();
validate_payload(
subnet_id,
registry.as_ref(),
crypto.as_ref(),
&PoolReader::new(&pool),
dkg_pool.read().unwrap().deref(),
parent.clone(),
parent,
&last_summary_block,
&block_payload,
state_manager.as_ref(),
&context,
Expand Down Expand Up @@ -838,13 +845,15 @@ mod tests {
idkg: idkg::Payload::default(),
});

let last_summary_block = PoolReader::new(&pool).dkg_summary_block(&parent).unwrap();
let result = validate_payload(
subnet_id,
registry.as_ref(),
crypto.as_ref(),
&PoolReader::new(&pool),
&dkg_pool,
parent.clone(),
&last_summary_block,
&block_payload,
state_manager.as_ref(),
&context,
Expand All @@ -865,6 +874,7 @@ mod tests {
&PoolReader::new(&pool),
&dkg_pool,
parent,
&last_summary_block,
&block_payload,
state_manager.as_ref(),
&context,
Expand Down
10 changes: 3 additions & 7 deletions rs/consensus/idkg/src/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ pub fn create_summary_payload(
pool_reader: &PoolReader<'_>,
context: &ValidationContext,
parent_block: &Block,
prev_summary_block: &Block,
idkg_payload_metrics: Option<&IDkgPayloadMetrics>,
log: &ReplicaLogger,
) -> Result<idkg::Summary, IDkgPayloadError> {
Expand All @@ -178,9 +179,6 @@ pub fn create_summary_payload(
});

let height = parent_block.height().increment();
let prev_summary_block = pool_reader
.dkg_summary_block(parent_block)
.ok_or_else(|| IDkgPayloadError::ConsensusSummaryBlockNotFound(parent_block.height()))?;

// For this interval: context.registry_version from prev summary block
// which is the same as calling pool_reader.registry_version(height).
Expand Down Expand Up @@ -483,6 +481,7 @@ pub fn create_data_payload(
state_reader: &dyn StateReader<State = ReplicatedState>,
context: &ValidationContext,
parent_block: &Block,
summary_block: &Block,
idkg_payload_metrics: &IDkgPayloadMetrics,
log: &ReplicaLogger,
) -> Result<idkg::Payload, IDkgPayloadError> {
Expand All @@ -495,9 +494,6 @@ pub fn create_data_payload(
if parent_block.payload.as_ref().as_idkg().is_none() {
return Ok(None);
};
let summary_block = pool_reader
.dkg_summary_block(parent_block)
.ok_or_else(|| IDkgPayloadError::ConsensusSummaryBlockNotFound(parent_block.height()))?;

// In case the certified height is below the summary height, add the heights in
// between to the blockchain. This is needed to calculate the total number of pre-
Expand Down Expand Up @@ -529,7 +525,7 @@ pub fn create_data_payload(
subnet_id,
context,
parent_block,
&summary_block,
summary_block,
&block_reader,
&transcript_builder,
state_reader,
Expand Down
3 changes: 1 addition & 2 deletions rs/consensus/idkg/src/payload_builder/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ic_crypto::MegaKeyFromRegistryError;
use ic_types::{
Height, RegistryVersion, SubnetId,
RegistryVersion, SubnetId,
consensus::idkg,
crypto::canister_threshold_sig::{
error::{
Expand All @@ -19,7 +19,6 @@ use super::InvalidChainCacheError;
pub enum IDkgPayloadError {
RegistryClientError(RegistryClientError),
MegaKeyFromRegistryError(MegaKeyFromRegistryError),
ConsensusSummaryBlockNotFound(Height),
StateManagerError(StateManagerError),
SubnetWithNoNodes(SubnetId, RegistryVersion),
PreSignatureError(EcdsaPresignatureQuadrupleCreationError),
Expand Down
16 changes: 7 additions & 9 deletions rs/consensus/idkg/src/payload_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ pub fn validate_payload(
state_reader: &dyn StateReader<State = ReplicatedState>,
context: &ValidationContext,
parent_block: &Block,
last_summary_block: &Block,
payload: &BlockPayload,
metrics: HistogramVec,
) -> ValidationResult<IDkgValidationError> {
Expand All @@ -194,6 +195,7 @@ pub fn validate_payload(
pool_reader,
context,
parent_block,
last_summary_block,
payload.as_summary().idkg.as_ref(),
)
},
Expand All @@ -212,6 +214,7 @@ pub fn validate_payload(
state_reader,
context,
parent_block,
last_summary_block,
payload.as_data().idkg.as_ref(),
&metrics,
)
Expand All @@ -230,6 +233,7 @@ fn validate_summary_payload(
pool_reader: &PoolReader<'_>,
context: &ValidationContext,
parent_block: &Block,
last_summary_block: &Block,
summary_payload: Option<&idkg::IDkgPayload>,
) -> ValidationResult<IDkgValidationError> {
let height = parent_block.height().increment();
Expand All @@ -252,6 +256,7 @@ fn validate_summary_payload(
pool_reader,
context,
parent_block,
last_summary_block,
None,
&ic_logger::replica_logger::no_op_logger(),
) {
Expand Down Expand Up @@ -283,6 +288,7 @@ fn validate_data_payload(
state_reader: &dyn StateReader<State = ReplicatedState>,
context: &ValidationContext,
parent_block: &Block,
summary_block: &Block,
data_payload: Option<&idkg::IDkgPayload>,
metrics: &HistogramVec,
) -> ValidationResult<IDkgValidationError> {
Expand Down Expand Up @@ -329,14 +335,6 @@ fn validate_data_payload(
}
};

let summary_block = pool_reader
.dkg_summary_block(parent_block)
.unwrap_or_else(|| {
panic!(
"Impossible: fail to the summary block that governs height {}",
parent_block.height()
)
});
// In case the certified height is below the summary height, add the heights in
// between to the blockchain. This is needed to calculate the total number of pre-
// signatures in the certified state and every block since then.
Expand Down Expand Up @@ -383,7 +381,7 @@ fn validate_data_payload(
subnet_id,
context,
parent_block,
&summary_block,
summary_block,
&block_reader,
&builder,
state_reader,
Expand Down
37 changes: 22 additions & 15 deletions rs/consensus/src/consensus/batch_delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ pub(crate) fn deliver_batches_with_result_processor(
}
);

// Retrieve the dkg summary block
let Some(summary_block) = pool.dkg_summary_block_for_finalized_height(height) else {
warn!(
every_n_seconds => 30,
log,
"Do not deliver height {} because no summary block was found. \
Finalized height: {}",
height,
finalized_height
);
break;
};
let dkg_summary = &summary_block.payload.as_ref().as_summary().dkg;

if block.payload.is_summary() {
info!(
log,
Expand All @@ -143,7 +157,14 @@ pub(crate) fn deliver_batches_with_result_processor(
}
// When we are not delivering CUP block, we must check if the subnet is halted.
else {
match status::get_status(height, registry_client, subnet_id, pool, log) {
match status::get_status(
height,
&summary_block,
registry_client,
subnet_id,
pool,
log,
) {
Some(Status::Halting | Status::Halted) => {
debug!(
every_n_seconds => 5,
Expand All @@ -166,20 +187,6 @@ pub(crate) fn deliver_batches_with_result_processor(

let randomness = randomness_from_crypto_hashable(&tape);

// Retrieve the dkg summary block
let Some(summary_block) = pool.dkg_summary_block_for_finalized_height(height) else {
warn!(
every_n_seconds => 30,
log,
"Do not deliver height {} because no summary block was found. \
Finalized height: {}",
height,
finalized_height
);
break;
};
let dkg_summary = &summary_block.payload.as_ref().as_summary().dkg;

let mut chain_key_subnet_public_keys = BTreeMap::new();
let (mut idkg_subnet_public_keys, idkg_pre_signatures) =
get_idkg_subnet_public_keys_and_pre_signatures(
Expand Down
Loading
Loading