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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ message StandardEngineReplicaVersionRecord {
// 1. Compute SHA-256 of the concatenation of
// a. len("upgrade priority"): a single u8
// b. "upgrade priority": ASCII encoded
// c. new_replica_version_id: hex decoded
// d. the engine's subnet_id: raw bytes
// c. new_replica_version_id: (not hex decoded)
// d. the engine's subnet_id: text/display representation (not raw bytes)
//
// 2. Take the first 8 bytes of that hash, and interpret them as a
// little-endian 64 bit unsigned integer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub struct StandardEngineReplicaVersionRecord {
/// 1. Compute SHA-256 of the concatenation of
/// a. len("upgrade priority"): a single u8
/// b. "upgrade priority": ASCII encoded
/// c. new_replica_version_id: hex decoded
/// d. the engine's subnet_id: raw bytes
/// c. new_replica_version_id: (not hex decoded)
/// d. the engine's subnet_id: text/display representation (not raw bytes)
///
/// 1. Take the first 8 bytes of that hash, and interpret them as a
/// little-endian 64 bit unsigned integer.
Expand Down
3 changes: 3 additions & 0 deletions rs/registry/helpers/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ rust_library(
version = "0.9.0",
deps = [
# Keep sorted.
"//rs/crypto/sha2",
"//rs/interfaces/registry",
"//rs/limits",
"//rs/protobuf",
Expand All @@ -30,6 +31,7 @@ rust_test(
crate = ":helpers",
deps = [
# Keep sorted.
"//rs/crypto/sha2",
"//rs/interfaces/registry",
"//rs/limits",
"//rs/protobuf",
Expand All @@ -56,6 +58,7 @@ rust_test_suite(
deps = [
# Keep sorted.
":helpers",
"//rs/crypto/sha2",
"//rs/interfaces/registry",
"//rs/limits",
"//rs/protobuf",
Expand Down
1 change: 1 addition & 0 deletions rs/registry/helpers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ documentation.workspace = true

[dependencies]
ic-base-types = { path = "../../types/base_types" }
ic-crypto-sha2 = { path = "../../crypto/sha2" }
ic-interfaces-registry = { path = "../../interfaces/registry" }
ic-limits = { path = "../../limits" }
ic-management-canister-types-private = { path = "../../types/management_canister_types" }
Expand Down
280 changes: 274 additions & 6 deletions rs/registry/helpers/src/subnet.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::deserialize_registry_value;
use ic_crypto_sha2::{DomainSeparationContext, Sha256};
use ic_interfaces_registry::{
RegistryClient, RegistryClientResult, RegistryClientVersionedResult, RegistryVersionedRecord,
};
Expand All @@ -7,13 +8,15 @@ use ic_protobuf::{
registry::{
node::v1::NodeRecord,
replica_version::v1::ReplicaVersionRecord,
standard_engine_replica_version::v1::StandardEngineReplicaVersionRecord,
subnet::v1::{CatchUpPackageContents, SubnetListRecord, SubnetRecord, SubnetType},
},
types::v1::SubnetId as SubnetIdProto,
};
use ic_registry_keys::{
DEFAULT_INITIAL_DKG_SUBNET_ID_KEY, ROOT_SUBNET_ID_KEY, make_catch_up_package_contents_key,
make_node_record_key, make_replica_version_key, make_subnet_list_record_key,
make_node_record_key, make_replica_version_key,
make_standard_engine_replica_version_record_key, make_subnet_list_record_key,
make_subnet_record_key,
};
use ic_registry_subnet_features::{ChainKeyConfig, SubnetFeatures};
Expand Down Expand Up @@ -155,8 +158,27 @@ pub trait SubnetRegistry {
version: RegistryVersion,
) -> RegistryClientResult<bool>;

/// Return the [ReplicaVersion] as recorded in the subnet record
/// at the given height.
/// Except for CloudEngine subnets, this just returns the value in the
/// [SubnetRecord]'s `replica_version_id` field, which is generally the git
/// commit ID from which Replica was built.
///
/// But when the subnet is of type CloudEngine, `replica_version_id` can be
/// blank, which means that the engine is following
/// [StandardEngineReplicaVersionRecord]. Non-blank `replica_version_id`
/// means the same thing as in the non-CloudEngine case.
///
/// Err is returned in various cases, but call out a couple in particular,
/// because the error type is unintuitive: DecodeError is returned in the
/// following cases:
///
/// 1. CloudEngine with blank replica_version_id, but no
/// StandardEngineReplicaVersionRecord.
///
/// 2. Non-CloudEngine with blank replica_version_id.
///
/// In practice, such data problems are prevented from happening elsewhere
/// (specifically, Registry's invariants checks), but we mention them here
/// for completeness.
fn get_replica_version(
&self,
subnet_id: SubnetId,
Expand Down Expand Up @@ -411,8 +433,54 @@ impl<T: RegistryClient + ?Sized> SubnetRegistry for T {
version: RegistryVersion,
) -> RegistryClientResult<ReplicaVersion> {
let bytes = self.get_value(&make_subnet_record_key(subnet_id), version);
Ok(deserialize_registry_value::<SubnetRecord>(bytes)?
.and_then(|record| ReplicaVersion::try_from(record.replica_version_id.as_ref()).ok()))
let Some(subnet_record) = deserialize_registry_value::<SubnetRecord>(bytes)? else {
return Ok(None);
};

// Engines may opt out of the standard deployment by setting their own
// replica_version_id; if they have, that is authoritative.
if !subnet_record.replica_version_id.is_empty() {
return Ok(ReplicaVersion::try_from(subnet_record.replica_version_id.as_ref()).ok());
}

// Only engines are allowed to have a blank replica_version_id (i.e.
// follow the standard engine deployment). Any other subnet type
// with a blank replica_version_id indicates a real inconsistency.
if subnet_record.subnet_type() != SubnetType::CloudEngine {
return Err(DecodeError {
error: format!(
"get_replica_version(): subnet {subnet_id} has a blank replica_version_id, \
but its subnet_type is {:?}, not CloudEngine",
subnet_record.subnet_type()
),
});
}

let Some(standard_engine_record) =
get_standard_engine_replica_version_record(self, version)?
else {
return Err(DecodeError {
error: format!(
"get_replica_version(): subnet {subnet_id} has a blank replica_version_id, \
but no StandardEngineReplicaVersionRecord exists to resolve it"
),
});
};

// Decide whether to take new or old version, based on our upgrade
// priority vs. deployment_progress.
let priority =
engine_upgrade_priority(subnet_id, &standard_engine_record.new_replica_version_id);
let resolved_replica_version_id = if priority <= standard_engine_record.deployment_progress
{
standard_engine_record.new_replica_version_id
} else {
standard_engine_record.old_replica_version_id
};

// At this point, resolved_replica_version_id should be a git commit ID
// in the ic repo. This just converts it from a raw string.
Ok(ReplicaVersion::try_from(resolved_replica_version_id.as_ref()).ok())
}

fn get_replica_version_record(
Expand Down Expand Up @@ -531,6 +599,36 @@ impl<T: RegistryClient + ?Sized> SubnetRegistry for T {
}
}

fn get_standard_engine_replica_version_record<T: RegistryClient + ?Sized>(
client: &T,
version: RegistryVersion,
) -> RegistryClientResult<StandardEngineReplicaVersionRecord> {
let bytes = client.get_value(&make_standard_engine_replica_version_record_key(), version);
deserialize_registry_value::<StandardEngineReplicaVersionRecord>(bytes)
}

/// Computes an engine's upgrade priority, a pseudo-random real/floating point
/// number in the closed interval [0.0, 1.0].
///
/// When an engine's upgrade priority <= deployment_progress, the engine takes
/// the standard new replica version (otherwise, it takes the old one).
///
/// Based on 2 things (their text/display representations, for consistency):
///
/// 1. the engine's ID
/// 2. the new replica version
fn engine_upgrade_priority(subnet_id: SubnetId, new_replica_version_id: &str) -> f64 {
let mut hasher = Sha256::new_with_context(&DomainSeparationContext::new("upgrade priority"));
hasher.write(new_replica_version_id.as_bytes());
hasher.write(subnet_id.to_string().as_bytes());
let digest = hasher.finish();

let first_8_bytes = <[u8; 8]>::try_from(&digest[0..8]).unwrap();
let priority_int = u64::from_le_bytes(first_8_bytes);

priority_int as f64 / u64::MAX as f64
}
Comment thread
daniel-wong-dfinity-org-twin marked this conversation as resolved.

pub fn get_node_ids_from_subnet_record(
subnet: &SubnetRecord,
) -> Result<Vec<NodeId>, PrincipalIdBlobParseError> {
Expand Down Expand Up @@ -632,10 +730,11 @@ impl<T: RegistryClient + ?Sized> SubnetTransportRegistry for T {
#[cfg(test)]
mod tests {
use super::*;
use assert_matches::assert_matches;
use ic_registry_client_fake::FakeRegistryClient;
use ic_registry_proto_data_provider::ProtoRegistryDataProvider;
use ic_types::PrincipalId;
use std::sync::Arc;
use std::{str::FromStr, sync::Arc};

fn node_id(id: u64) -> NodeId {
NodeId::from(PrincipalId::new_node_test_id(id))
Expand Down Expand Up @@ -726,6 +825,175 @@ mod tests {
assert_eq!(result, Some(replica_version_record))
}

#[test]
fn engine_priority_matches_hand_computed_value() {
// Step 1: Prepare the world. Constructed directly from its known
// text representation, so this is correct by construction.
let subnet_id =
SubnetId::from(PrincipalId::from_str("y6zu2-uqdaa-aaaaa-aaaap-yai").unwrap());

// Step 2: Run the code under test.
let priority =
engine_upgrade_priority(subnet_id, "eb3ab997954f2a91db8a42f84132cf37078d481c");

// Step 3: Verify result(s). Value independently computed (via
// Python's hashlib.sha256, not this crate's own code) over
// len(domain) || domain || "eb3ab997954f2a91db8a42f84132cf37078d481c" || subnet_id.to_string(),
// where domain = "upgrade priority". This confirms engine_upgrade_priority
// implements the specified recipe exactly, not just some other
// deterministic-but-wrong one.
assert!((priority - 0.211_377).abs() < 1e-6, "got {priority}");
}

#[test]
fn engine_replica_version_selection() {
// Step 1: Prepare the world.

// The first two subnets in Registry use the standard engine replica
// version by having a blank replica_version_id.
let id_dead = subnet_id(0xDEAD);
let id_beef = subnet_id(0xBEEF);
let priority_dead = engine_upgrade_priority(id_dead, "new");
let priority_beef = engine_upgrade_priority(id_beef, "new");
let (low_subnet_id, high_subnet_id) = if priority_dead < priority_beef {
(id_dead, id_beef)
} else {
(id_beef, id_dead)
};

let data_provider = ProtoRegistryDataProvider::new();

// Insert the first two subnets.
for subnet_id in [low_subnet_id, high_subnet_id] {
data_provider
.add(
&make_subnet_record_key(subnet_id),
RegistryVersion::from(2),
Some(SubnetRecord {
replica_version_id: "".to_string(),
subnet_type: SubnetType::CloudEngine as i32,
..Default::default()
}),
)
.unwrap();
}

// The third subnet overrides, does not use the standard engine replica
// version.
data_provider
.add(
&make_subnet_record_key(subnet_id(0xCAFE)),
RegistryVersion::from(2),
Some(SubnetRecord {
replica_version_id: "override".to_string(),
subnet_type: SubnetType::CloudEngine as i32,
..Default::default()
}),
)
.unwrap();

// Standard engine replica version.
assert_ne!(priority_dead, priority_beef);
let deployment_progress = (priority_dead + priority_beef) / 2.0;
data_provider
.add(
&make_standard_engine_replica_version_record_key(),
RegistryVersion::from(2),
Some(StandardEngineReplicaVersionRecord {
new_replica_version_id: "new".to_string(),
old_replica_version_id: "old".to_string(),
deployment_progress,
}),
)
.unwrap();

// From the Registry data assembled above, create a RegistryClient.
let registry = FakeRegistryClient::new(Arc::new(data_provider));
registry.update_to_latest_version();

// Step 2: Run the code under test.
let low_priority_result = registry
.get_replica_version(low_subnet_id, RegistryVersion::from(2))
.unwrap();
let high_priority_result = registry
.get_replica_version(high_subnet_id, RegistryVersion::from(2))
.unwrap();
let override_result = registry
.get_replica_version(subnet_id(0xCAFE), RegistryVersion::from(2))
.unwrap();

// Step 3: Verify result(s).
assert_eq!(
low_priority_result,
Some(ReplicaVersion::try_from("new").unwrap())
);
assert_eq!(
high_priority_result,
Some(ReplicaVersion::try_from("old").unwrap())
);
assert_eq!(
override_result,
Some(ReplicaVersion::try_from("override").unwrap())
);
}

// This wouldn't occur in practice, so this test is "just" for completeness.
Comment thread
pierugo-dfinity marked this conversation as resolved.
#[test]
fn blank_replica_version_id_without_standard_engine_record_is_an_error() {
// Step 1: Prepare the world. A blank-replica_version_id subnet, but
// no StandardEngineReplicaVersionRecord at all.
let data_provider = ProtoRegistryDataProvider::new();
data_provider
.add(
&make_subnet_record_key(subnet_id(0xBABE)),
RegistryVersion::from(2),
Some(SubnetRecord {
replica_version_id: "".to_string(),
subnet_type: SubnetType::CloudEngine as i32,
..Default::default()
}),
)
.unwrap();
let registry = FakeRegistryClient::new(Arc::new(data_provider));
registry.update_to_latest_version();

// Step 2: Run the code under test.
let result = registry.get_replica_version(subnet_id(0xBABE), RegistryVersion::from(2));

// Step 3: Verify result(s).
assert_matches!(result, Err(RegistryClientError::DecodeError { .. }));
}

// This wouldn't occur in practice, so this test is "just" for completeness.
Comment thread
pierugo-dfinity marked this conversation as resolved.
#[test]
fn blank_replica_version_id_on_a_non_engine_subnet_is_an_error() {
// Step 1: Prepare the world.
let data_provider = ProtoRegistryDataProvider::new();

// Add just one normal (i.e. non engine) subnet.
data_provider
.add(
&make_subnet_record_key(subnet_id(0xBABE)),
RegistryVersion::from(2),
Some(SubnetRecord {
replica_version_id: "".to_string(),
subnet_type: SubnetType::Application as i32,
..Default::default()
}),
)
.unwrap();

// Assemble the above Registry data into a RegistryClient.
let registry = FakeRegistryClient::new(Arc::new(data_provider));
registry.update_to_latest_version();

// Step 2: Run the code under test.
let result = registry.get_replica_version(subnet_id(0xBABE), RegistryVersion::from(2));

// Step 3: Verify result(s).
assert_matches!(result, Err(RegistryClientError::DecodeError { .. }));
}

#[test]
fn can_get_is_halted_from_subnet() {
let subnet_id = subnet_id(4);
Expand Down
Loading