Skip to content

Commit 974ec76

Browse files
kpop-dfinityIDX GitHub Automation
andauthored
chore(consensus): move get_block_maker_delay function from consensus_utils crate to consensus crate (#1527)
The functions doesn't need to be shared across crates. Also, removed unneeded dependencies --------- Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
1 parent 29f688a commit 974ec76

File tree

6 files changed

+58
-58
lines changed

6 files changed

+58
-58
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rs/consensus/src/consensus/block_maker.rs

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use crate::{
1010
};
1111
use ic_consensus_utils::{
1212
find_lowest_ranked_non_disqualified_proposals, get_block_hash_string,
13-
get_notarization_delay_settings, get_subnet_record, is_time_to_make_block,
14-
membership::Membership, pool_reader::PoolReader,
13+
get_notarization_delay_settings, get_subnet_record, membership::Membership,
14+
pool_reader::PoolReader,
1515
};
1616
use ic_interfaces::{
1717
consensus::PayloadBuilder, dkg::DkgPool, idkg::IDkgPool, time_source::TimeSource,
@@ -29,7 +29,7 @@ use ic_types::{
2929
},
3030
replica_config::ReplicaConfig,
3131
time::current_time,
32-
CountBytes, Height, NodeId, RegistryVersion,
32+
CountBytes, Height, NodeId, RegistryVersion, SubnetId,
3333
};
3434
use std::{
3535
sync::{Arc, RwLock},
@@ -514,13 +514,57 @@ pub(crate) fn already_proposed(pool: &PoolReader<'_>, h: Height, this_node: Node
514514
.any(|p| p.signature.signer == this_node)
515515
}
516516

517+
/// Calculate the required delay for block making based on the block maker's
518+
/// rank.
519+
pub(super) fn get_block_maker_delay(
520+
log: &ReplicaLogger,
521+
registry_client: &dyn RegistryClient,
522+
subnet_id: SubnetId,
523+
registry_version: RegistryVersion,
524+
rank: Rank,
525+
) -> Option<Duration> {
526+
get_notarization_delay_settings(log, registry_client, subnet_id, registry_version)
527+
.map(|settings| settings.unit_delay * rank.0 as u32)
528+
}
529+
530+
/// Return true if the time since round start is greater than the required block
531+
/// maker delay for the given rank.
532+
pub(super) fn is_time_to_make_block(
533+
log: &ReplicaLogger,
534+
registry_client: &dyn RegistryClient,
535+
subnet_id: SubnetId,
536+
pool: &PoolReader<'_>,
537+
height: Height,
538+
rank: Rank,
539+
time_source: &dyn TimeSource,
540+
) -> bool {
541+
let Some(registry_version) = pool.registry_version(height) else {
542+
return false;
543+
};
544+
let Some(block_maker_delay) =
545+
get_block_maker_delay(log, registry_client, subnet_id, registry_version, rank)
546+
else {
547+
return false;
548+
};
549+
550+
// If the relative time indicates that not enough time has passed, we fall
551+
// back to the the monotonic round start time. We do this to safeguard
552+
// against a stalled relative clock.
553+
pool.get_round_start_time(height)
554+
.is_some_and(|start_time| time_source.get_relative_time() >= start_time + block_maker_delay)
555+
|| pool
556+
.get_round_start_instant(height, time_source.get_origin_instant())
557+
.is_some_and(|start_instant| {
558+
time_source.get_instant() >= start_instant + block_maker_delay
559+
})
560+
}
561+
517562
#[cfg(test)]
518563
mod tests {
519564
use crate::idkg::test_utils::create_idkg_pool;
520565

521566
use super::*;
522567
use ic_consensus_mocks::{dependencies_with_subnet_params, Dependencies, MockPayloadBuilder};
523-
use ic_consensus_utils::get_block_maker_delay;
524568
use ic_interfaces::consensus_pool::ConsensusPool;
525569
use ic_logger::replica_logger::no_op_logger;
526570
use ic_metrics::MetricsRegistry;

rs/consensus/src/consensus/validator.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::{
1313
use ic_consensus_utils::{
1414
active_high_threshold_nidkg_id, active_low_threshold_nidkg_id,
1515
crypto::ConsensusCrypto,
16-
get_oldest_idkg_state_registry_version, is_time_to_make_block,
16+
get_oldest_idkg_state_registry_version,
1717
membership::{Membership, MembershipError},
1818
pool_reader::PoolReader,
1919
RoundRobin,
@@ -53,6 +53,8 @@ use std::{
5353
time::Duration,
5454
};
5555

56+
use super::block_maker::is_time_to_make_block;
57+
5658
/// The number of seconds spent in unvalidated pool, after which we start
5759
/// logging why we cannot validate an artifact.
5860
const SECONDS_TO_LOG_UNVALIDATED: u64 = 300;
@@ -1889,9 +1891,13 @@ impl Validator {
18891891
#[cfg(test)]
18901892
pub mod test {
18911893
use super::*;
1892-
use crate::idkg::test_utils::{
1893-
add_available_quadruple_to_payload, empty_idkg_payload, fake_ecdsa_master_public_key_id,
1894-
fake_signature_request_context_with_pre_sig, fake_state_with_signature_requests,
1894+
use crate::{
1895+
consensus::block_maker::get_block_maker_delay,
1896+
idkg::test_utils::{
1897+
add_available_quadruple_to_payload, empty_idkg_payload,
1898+
fake_ecdsa_master_public_key_id, fake_signature_request_context_with_pre_sig,
1899+
fake_state_with_signature_requests,
1900+
},
18951901
};
18961902
use assert_matches::assert_matches;
18971903
use ic_artifact_pool::dkg_pool::DkgPoolImpl;
@@ -1900,7 +1906,6 @@ pub mod test {
19001906
dependencies_with_subnet_params, dependencies_with_subnet_records_with_raw_state_manager,
19011907
Dependencies, RefMockPayloadBuilder,
19021908
};
1903-
use ic_consensus_utils::get_block_maker_delay;
19041909
use ic_interfaces::{
19051910
messaging::XNetPayloadValidationFailure, p2p::consensus::MutablePool,
19061911
time_source::TimeSource,

rs/consensus/utils/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ DEPENDENCIES = [
2020

2121
DEV_DEPENDENCIES = [
2222
# Keep sorted.
23-
"//rs/consensus",
2423
"//rs/consensus/mocks",
2524
"//rs/test_utilities",
2625
"//rs/test_utilities/registry",

rs/consensus/utils/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ slog = { workspace = true }
2222

2323
[dev-dependencies]
2424
assert_matches = { workspace = true }
25-
ic-consensus = { path = "../" }
2625
ic-consensus-mocks = { path = "../mocks" }
2726
ic-management-canister-types = { path = "../../types/management_canister_types" }
2827
ic-test-utilities = { path = "../../test_utilities" }

rs/consensus/utils/src/lib.rs

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use crate::{crypto::Aggregate, membership::Membership, pool_reader::PoolReader};
33
use ic_interfaces::{
44
consensus::{PayloadValidationError, PayloadValidationFailure},
55
consensus_pool::ConsensusPoolCache,
6-
time_source::TimeSource,
76
validation::ValidationError,
87
};
98
use ic_interfaces_registry::RegistryClient;
@@ -86,51 +85,6 @@ pub fn crypto_hashable_to_seed<T: CryptoHashable>(hashable: &T) -> [u8; 32] {
8685
seed
8786
}
8887

89-
/// Calculate the required delay for block making based on the block maker's
90-
/// rank.
91-
pub fn get_block_maker_delay(
92-
log: &ReplicaLogger,
93-
registry_client: &dyn RegistryClient,
94-
subnet_id: SubnetId,
95-
registry_version: RegistryVersion,
96-
rank: Rank,
97-
) -> Option<Duration> {
98-
get_notarization_delay_settings(log, registry_client, subnet_id, registry_version)
99-
.map(|settings| settings.unit_delay * rank.0 as u32)
100-
}
101-
102-
/// Return true if the time since round start is greater than the required block
103-
/// maker delay for the given rank.
104-
pub fn is_time_to_make_block(
105-
log: &ReplicaLogger,
106-
registry_client: &dyn RegistryClient,
107-
subnet_id: SubnetId,
108-
pool: &PoolReader<'_>,
109-
height: Height,
110-
rank: Rank,
111-
time_source: &dyn TimeSource,
112-
) -> bool {
113-
let Some(registry_version) = pool.registry_version(height) else {
114-
return false;
115-
};
116-
let Some(block_maker_delay) =
117-
get_block_maker_delay(log, registry_client, subnet_id, registry_version, rank)
118-
else {
119-
return false;
120-
};
121-
122-
// If the relative time indicates that not enough time has passed, we fall
123-
// back to the the monotonic round start time. We do this to safeguard
124-
// against a stalled relative clock.
125-
pool.get_round_start_time(height)
126-
.is_some_and(|start_time| time_source.get_relative_time() >= start_time + block_maker_delay)
127-
|| pool
128-
.get_round_start_instant(height, time_source.get_origin_instant())
129-
.is_some_and(|start_instant| {
130-
time_source.get_instant() >= start_instant + block_maker_delay
131-
})
132-
}
133-
13488
/// Calculate the required delay for notary based on the rank of block to notarize,
13589
/// adjusted by a multiplier depending on the gap between finalized and notarized
13690
/// heights, adjusted by how far the certified height lags behind the finalized

0 commit comments

Comments
 (0)