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

Updated prototype based on #2723 #3324

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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.

1 change: 1 addition & 0 deletions rs/nns/test_utils/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ impl NnsInitPayloadsBuilder {
.push_init_mutate_request(RegistryAtomicMutateRequest {
mutations: invariant_compliant_mutation(0),
preconditions: vec![],
timestamp: None
});
self
}
Expand Down
5 changes: 5 additions & 0 deletions rs/nns/test_utils/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ use std::{
collections::{BTreeMap, BTreeSet},
convert::TryFrom,
};
use ic_types::time::current_time;

/// ID used in multiple tests.
pub const TEST_ID: u64 = 999;
Expand Down Expand Up @@ -89,6 +90,7 @@ pub fn invariant_compliant_mutation_as_atomic_req(mutation_id: u8) -> RegistryAt
RegistryAtomicMutateRequest {
mutations: invariant_compliant_mutation(mutation_id),
preconditions: vec![],
timestamp: None
}
}
/// Returns a Result with either an Option(T) or a ic_registry_transport::Error
Expand Down Expand Up @@ -196,6 +198,7 @@ pub async fn get_node_operator_record(

/// Inserts a value into the registry.
pub async fn insert_value<T: Message + Default>(registry: &Canister<'_>, key: &[u8], value: &T) {
let mutate_ts = current_time().as_nanos_since_unix_epoch();
let response_bytes = registry
.update_(
"atomic_mutate",
Expand All @@ -206,6 +209,7 @@ pub async fn insert_value<T: Message + Default>(registry: &Canister<'_>, key: &[
key: key.to_vec(),
value: value.encode_to_vec(),
}],
timestamp: Some(mutate_ts),
preconditions: vec![],
}
.encode_to_vec(),
Expand Down Expand Up @@ -730,6 +734,7 @@ pub fn prepare_registry_with_two_node_sets(
let mutate_request = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp: None
};

(
Expand Down
4 changes: 3 additions & 1 deletion rs/registry/canister/src/common/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ pub fn invariant_compliant_registry(mutation_id: u8) -> Registry {
}

pub fn empty_mutation() -> Vec<u8> {
RegistryAtomicMutateRequest {
RegistryAtomicMutateRequest {
mutations: vec![RegistryMutation {
mutation_type: Type::Upsert as i32,
key: "_".into(),
value: "".into(),
}],
timestamp: None,
preconditions: vec![],
}
.encode_to_vec()
Expand Down Expand Up @@ -132,6 +133,7 @@ pub fn prepare_registry_with_nodes(
let mutate_request = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp: None
};
(mutate_request, node_ids_and_dkg_pks)
}
1 change: 1 addition & 0 deletions rs/registry/canister/src/invariants/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ mod tests {
key: "_".into(),
value: "".into(),
}],
timestamp: None,
preconditions: vec![],
}
.encode_to_vec()
Expand Down
100 changes: 41 additions & 59 deletions rs/registry/canister/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ use ic_registry_transport::{
use ic_types::messages::MAX_INTER_CANISTER_PAYLOAD_IN_BYTES_U64;
use prost::Message;
use std::{
cmp::max,
collections::{BTreeMap, VecDeque},
fmt,
};

#[cfg(target_arch = "wasm32")]
use dfn_core::println;
use ic_types::time::current_time;

/// The maximum size a registry delta, used to ensure that response payloads
/// stay under `MAX_INTER_CANISTER_PAYLOAD_IN_BYTES_U64`.
Expand Down Expand Up @@ -191,6 +191,7 @@ impl Registry {
&mut self,
mut mutations: Vec<RegistryMutation>,
version: Version,
timestamp: Option<u64>,
) {
// We sort entries by key to eliminate the difference between changelog
// produced by the new version of the registry canister starting from v1
Expand All @@ -217,6 +218,7 @@ impl Registry {

let req = RegistryAtomicMutateRequest {
mutations,
timestamp,
preconditions: vec![],
};
self.changelog_insert(version, &req);
Expand All @@ -242,8 +244,10 @@ impl Registry {
// global version is the max of all versions in the store.
return;
}
let mutations_ts = current_time().as_nanos_since_unix_epoch();

self.increment_version();
self.apply_mutations_as_version(mutations, self.version);
self.apply_mutations_as_version(mutations, self.version, Some(mutations_ts));
}

/// Verifies the implicit precondition corresponding to the mutation_type
Expand Down Expand Up @@ -400,7 +404,7 @@ impl Registry {
key: "_".into(),
value: "".into(),
}];
self.apply_mutations_as_version(mutations, i);
self.apply_mutations_as_version(mutations, i, None);
self.version = i;
}
// End code to fix ICSUP-2589
Expand All @@ -409,52 +413,12 @@ impl Registry {
.unwrap_or_else(|err| {
panic!("Failed to decode mutation@{}: {}", entry.version, err)
});
self.apply_mutations_as_version(req.mutations, entry.version);
self.apply_mutations_as_version(req.mutations, entry.version, req.timestamp);
self.version = entry.version;
current_version = self.version;
}
}
ReprVersion::Unspecified => {
let mut mutations_by_version = BTreeMap::<Version, Vec<RegistryMutation>>::new();
for delta in stable_repr.deltas.into_iter() {
self.version = max(
self.version,
delta
.values
.last()
.map(|registry_value| registry_value.version)
.unwrap_or(0),
);

for v in delta.values.iter() {
mutations_by_version
.entry(v.version)
.or_default()
.push(RegistryMutation {
mutation_type: if v.deletion_marker {
Type::Delete
} else {
Type::Upsert
} as i32,
key: delta.key.clone(),
value: v.value.clone(),
})
}

self.store.insert(delta.key, VecDeque::from(delta.values));
}
// We iterated over keys in ascending order, so the mutations
// must also be sorted by key, resulting in canonical encoding.
for (v, mutations) in mutations_by_version.into_iter() {
self.changelog_insert(
v,
&RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
},
);
}
}
ReprVersion::Unspecified => panic!("Unspecified version is not supported."),
}
}
}
Expand Down Expand Up @@ -760,8 +724,9 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let max_value = vec![0; max_mutation_value_size(version, key)];
let max_value = vec![0; max_mutation_value_size(version, timestamp, key)];

let mutation1 = upsert([90; 50], [1; 50]);
let mutation2 = upsert(key, max_value);
Expand Down Expand Up @@ -996,12 +961,14 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let max_value = vec![0; max_mutation_value_size(version, key)];
let max_value = vec![0; max_mutation_value_size(version, timestamp, key)];
let mutations = vec![upsert(key, max_value)];
let req = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp,
};
registry.changelog_insert(version, &req);

Expand All @@ -1019,12 +986,14 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let too_large_value = vec![0; max_mutation_value_size(version, key) + 1];
let too_large_value = vec![0; max_mutation_value_size(version, timestamp, key) + 1];
let mutations = vec![upsert(key, too_large_value)];
let req = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp,
};

registry.changelog_insert(1, &req);
Expand All @@ -1035,19 +1004,21 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let max_value = vec![0; max_mutation_value_size(version, key)];
let max_value = vec![0; max_mutation_value_size(version, timestamp, key)];
let mutations = vec![upsert(key, &max_value)];
apply_mutations_skip_invariant_checks(&mut registry, mutations);
let registry_value = registry.get(key, version).unwrap();

assert_eq!(registry.latest_version(), version);
assert_eq!(
registry.get(key, version),
Some(&RegistryValue {
registry_value,
&RegistryValue {
value: max_value,
version,
deletion_marker: false
})
deletion_marker: false,
}
);
}

Expand All @@ -1057,8 +1028,9 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let too_large_value = vec![0; max_mutation_value_size(version, key) + 1];
let too_large_value = vec![0; max_mutation_value_size(version, timestamp, key) + 1];
let mutations = vec![upsert(key, too_large_value)];

apply_mutations_skip_invariant_checks(&mut registry, mutations);
Expand All @@ -1074,13 +1046,17 @@ mod tests {
let mut registry = Registry::new();
let version = 1;
let key = b"key";
let timestamp = Some(current_time().as_nanos_since_unix_epoch());

let value =
vec![0; max_mutation_value_size(version, timestamp, key) + bytes_above_max_size];

let value = vec![0; max_mutation_value_size(version, key) + bytes_above_max_size];
let mutation = upsert(key, value);
let mutations = vec![mutation.clone()];
let req = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp,
};
// Circumvent `changelog_insert()` to insert potentially oversized mutations.
registry
Expand Down Expand Up @@ -1206,11 +1182,17 @@ Average length of the values: {} (desired: {})",

/// Computes the mutation value size (given the version and key) that will
/// result in a delta of exactly `MAX_REGISTRY_DELTAS_SIZE` bytes.
fn max_mutation_value_size(version: u64, key: &[u8]) -> usize {
fn delta_size(version: u64, key: &[u8], value_size: usize) -> usize {
fn max_mutation_value_size(version: u64, timestamp: Option<u64>, key: &[u8]) -> usize {
fn delta_size(
version: u64,
timestamp: Option<u64>,
key: &[u8],
value_size: usize,
) -> usize {
let req = RegistryAtomicMutateRequest {
mutations: vec![upsert(key, vec![0; value_size])],
preconditions: vec![],
timestamp,
};

let version = EncodedVersion::from(version);
Expand All @@ -1220,7 +1202,7 @@ Average length of the values: {} (desired: {})",
}

// Start off with an oversized delta.
let too_large_delta_size = delta_size(version, key, MAX_REGISTRY_DELTAS_SIZE);
let too_large_delta_size = delta_size(version, timestamp, key, MAX_REGISTRY_DELTAS_SIZE);

// Compoute the value size that will give us a delta of exactly
// MAX_REGISTRY_DELTAS_SIZE.
Expand All @@ -1229,7 +1211,7 @@ Average length of the values: {} (desired: {})",
// Ensure we actually get a MAX_REGISTRY_DELTAS_SIZE delta.
assert_eq!(
MAX_REGISTRY_DELTAS_SIZE,
delta_size(version, key, max_value_size)
delta_size(version, timestamp, key, max_value_size)
);

max_value_size
Expand Down
1 change: 1 addition & 0 deletions rs/registry/canister/tests/add_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,5 +390,6 @@ fn init_mutation_with_node_allowance(node_allowance: u64) -> RegistryAtomicMutat
value: node_operator_record.encode_to_vec(),
}],
preconditions: vec![],
timestamp: None,
}
}
3 changes: 3 additions & 0 deletions rs/registry/canister/tests/clear_provisional_whitelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ fn anonymous_user_cannot_clear_the_provisional_whitelist() {
.encode_to_vec(),
}],
preconditions: vec![],
timestamp: None,
})
.build(),
)
Expand Down Expand Up @@ -101,6 +102,7 @@ fn a_canister_other_than_the_governance_canister_cannot_change_the_provisional_w
.encode_to_vec(),
}],
preconditions: vec![],
timestamp: None,
})
.build(),
)
Expand Down Expand Up @@ -153,6 +155,7 @@ fn clear_provisional_whitelist_succeeds() {
.encode_to_vec(),
}],
preconditions: vec![],
timestamp: None,
})
.build(),
)
Expand Down
1 change: 1 addition & 0 deletions rs/registry/canister/tests/common/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ pub fn prepare_registry_with_nodes_from_template(
let mutate_request = RegistryAtomicMutateRequest {
mutations,
preconditions: vec![],
timestamp: None,
};

(mutate_request, node_ids_and_valid_pks)
Expand Down
2 changes: 2 additions & 0 deletions rs/registry/canister/tests/create_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ fn test_accepted_proposal_with_ecdsa_gets_keys_from_other_subnet_legacy() {
subnet_record.encode_to_vec(),
)],
preconditions: vec![],
timestamp: None,
};

let registry = setup_registry_synced_with_fake_client(
Expand Down Expand Up @@ -428,6 +429,7 @@ fn test_accepted_proposal_with_chain_key_gets_keys_from_other_subnet(key_id: Mas
subnet_record.encode_to_vec(),
)],
preconditions: vec![],
timestamp: None,
};

let registry = setup_registry_synced_with_fake_client(
Expand Down
Loading
Loading