Skip to content

Commit 0515bc3

Browse files
authored
feat: [MR-591] Introduce a minimum supported certification version. (#1502)
This is a first step towards phasing out old certification versions. The choice of [V17](https://sourcegraph.com/github.com/dfinity/ic/-/blob/rs/canonical_state/certification_version/src/lib.rs?L43) for `MIN_SUPPORTED_CERTIFICATION_VERSION` is the last version that's not related to the new messaging model. This seems to be a good place to have it for now. Following this PR would be another PR where exception such as [this](https://sourcegraph.com/github.com/dfinity/ic@0441f40482386397f7c688bf508ddd901ca6c1b7/-/blob/rs/messaging/src/routing/stream_handler.rs?L869) are removed, as well as trimming down the test framework ([e.g.](https://sourcegraph.com/github.com/dfinity/ic@0441f40482386397f7c688bf508ddd901ca6c1b7/-/blob/rs/canonical_state/tests/compatibility.rs?L189)).
1 parent c5e6242 commit 0515bc3

File tree

6 files changed

+35
-47
lines changed

6 files changed

+35
-47
lines changed

rs/canonical_state/certification_version/src/lib.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ impl std::convert::TryFrom<u32> for CertificationVersion {
8080
/// computed states.
8181
pub const CURRENT_CERTIFICATION_VERSION: CertificationVersion = CertificationVersion::V19;
8282

83+
/// Minimum supported certification version.
84+
///
85+
/// The replica will panic if requested to certify using a version lower than
86+
/// this.
87+
pub const MIN_SUPPORTED_CERTIFICATION_VERSION: CertificationVersion = CertificationVersion::V17;
88+
8389
/// Maximum supported certification version.
8490
///
8591
/// The replica will panic if requested to certify using a version higher than
@@ -89,10 +95,13 @@ pub const MAX_SUPPORTED_CERTIFICATION_VERSION: CertificationVersion = Certificat
8995
/// Returns a list of all certification versions up to [MAX_SUPPORTED_CERTIFICATION_VERSION].
9096
pub fn all_supported_versions() -> impl std::iter::Iterator<Item = CertificationVersion> {
9197
use strum::IntoEnumIterator;
92-
CertificationVersion::iter().take_while(|v| *v <= MAX_SUPPORTED_CERTIFICATION_VERSION)
98+
CertificationVersion::iter().filter(|v| {
99+
MIN_SUPPORTED_CERTIFICATION_VERSION <= *v && *v <= MAX_SUPPORTED_CERTIFICATION_VERSION
100+
})
93101
}
94102

95103
#[test]
96-
fn supported_version_ge_current() {
104+
fn version_constants_consistent() {
105+
assert!(MIN_SUPPORTED_CERTIFICATION_VERSION <= CURRENT_CERTIFICATION_VERSION);
97106
assert!(CURRENT_CERTIFICATION_VERSION <= MAX_SUPPORTED_CERTIFICATION_VERSION);
98107
}

rs/canonical_state/src/lazy_tree_conversion.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
encode_controllers, encode_message, encode_metadata, encode_stream_header,
66
encode_subnet_canister_ranges, encode_subnet_metrics,
77
},
8-
CertificationVersion, MAX_SUPPORTED_CERTIFICATION_VERSION,
8+
CertificationVersion, MAX_SUPPORTED_CERTIFICATION_VERSION, MIN_SUPPORTED_CERTIFICATION_VERSION,
99
};
1010
use ic_canonical_state_tree_hash::lazy_tree::{blob, fork, num, string, Lazy, LazyFork, LazyTree};
1111
use ic_crypto_tree_hash::Label;
@@ -285,10 +285,11 @@ fn invert_routing_table(
285285
pub fn replicated_state_as_lazy_tree(state: &ReplicatedState) -> LazyTree<'_> {
286286
let certification_version = state.metadata.certification_version;
287287
assert!(
288-
certification_version <= MAX_SUPPORTED_CERTIFICATION_VERSION,
289-
"Unable to certify state with version {:?}. Maximum supported certification version is {:?}",
288+
MIN_SUPPORTED_CERTIFICATION_VERSION <= certification_version && certification_version <= MAX_SUPPORTED_CERTIFICATION_VERSION,
289+
"Unable to certify state with version {:?}. Supported certification versions are {:?}..={:?}",
290290
certification_version,
291-
MAX_SUPPORTED_CERTIFICATION_VERSION
291+
MIN_SUPPORTED_CERTIFICATION_VERSION,
292+
MAX_SUPPORTED_CERTIFICATION_VERSION,
292293
);
293294

294295
fork(

rs/canonical_state/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ pub mod visitor;
138138
pub use ic_certification_version::{
139139
all_supported_versions, CertificationVersion, UnsupportedCertificationVersion,
140140
CURRENT_CERTIFICATION_VERSION, MAX_SUPPORTED_CERTIFICATION_VERSION,
141+
MIN_SUPPORTED_CERTIFICATION_VERSION,
141142
};
142143

143144
#[cfg(test)]

rs/replicated_state/src/metadata_state.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -785,13 +785,7 @@ impl SystemMetadata {
785785
// committing each state.
786786
prev_state_hash: Default::default(),
787787
state_sync_version: CURRENT_STATE_SYNC_VERSION,
788-
// NB. State manager relies on the root hash of the hash tree
789-
// corresponding to the initial state to be a constant. Thus we fix
790-
// the certification version that we use for the initial state. If
791-
// we used CURRENT_CERTIFICATION_VERSION here, the state hash would
792-
// NOT be guaranteed to be constant, potentially leading to
793-
// hard-to-track bugs in state manager.
794-
certification_version: CertificationVersion::V0,
788+
certification_version: CURRENT_CERTIFICATION_VERSION,
795789
heap_delta_estimate: NumBytes::from(0),
796790
subnet_metrics: Default::default(),
797791
expected_compiled_wasms: BTreeSet::new(),

rs/state_manager/src/manifest/split/tests.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1+
use super::*;
12
use assert_matches::assert_matches;
23
use ic_base_types::{CanisterId, SnapshotId};
34
use ic_registry_routing_table::{CanisterIdRange, CanisterIdRanges};
45
use ic_state_layout::{CheckpointLayout, ReadOnly};
56
use ic_test_utilities_types::ids::{SUBNET_0, SUBNET_1};
67
use ic_types::{state_sync::CURRENT_STATE_SYNC_VERSION, Height};
78

8-
use super::*;
9-
109
/// Expected hash of a zero length file.
1110
const EMPTY_FILE_HASH: [u8; 32] = [
1211
152, 19, 5, 215, 192, 178, 172, 224, 245, 63, 232, 34, 244, 7, 82, 120, 250, 40, 81, 30, 140,
@@ -396,23 +395,26 @@ fn expected_split_marker() -> (FileInfo, ChunkInfo) {
396395

397396
/// Returns the expected `FileInfo` and `ChunkInfo` for the system metadata of
398397
/// `SUBNET_1`.
398+
///
399+
/// `SystemMetadata` encodes the `CURRENT_CERTIFICATION_VERSION`, therefore the hashes in this
400+
/// test must be updated every time the current certification version is bumped.
399401
fn expected_subnet_1_system_metadata() -> (FileInfo, ChunkInfo) {
400402
(
401403
FileInfo {
402404
relative_path: PathBuf::from(SYSTEM_METADATA_FILE),
403-
size_bytes: 63,
405+
size_bytes: 65,
404406
hash: [
405-
122, 238, 38, 137, 170, 83, 240, 133, 62, 48, 18, 112, 233, 148, 191, 115, 239,
406-
115, 135, 234, 25, 157, 24, 45, 161, 179, 219, 112, 242, 95, 10, 217,
407+
135, 141, 106, 99, 2, 246, 244, 239, 255, 254, 121, 198, 217, 0, 168, 217, 254,
408+
105, 171, 56, 159, 169, 160, 159, 87, 16, 106, 9, 168, 21, 6, 246,
407409
],
408410
},
409411
ChunkInfo {
410412
file_index: 13,
411-
size_bytes: 63,
413+
size_bytes: 65,
412414
offset: 0,
413415
hash: [
414-
167, 162, 133, 251, 148, 255, 80, 204, 229, 63, 140, 219, 43, 228, 234, 250, 69,
415-
49, 7, 200, 173, 216, 136, 186, 183, 255, 101, 117, 229, 161, 238, 156,
416+
48, 206, 112, 95, 239, 131, 62, 43, 197, 160, 224, 171, 236, 107, 12, 29, 127, 75,
417+
227, 118, 27, 199, 106, 203, 192, 91, 53, 67, 219, 53, 31, 173,
416418
],
417419
},
418420
)

rs/state_manager/src/tree_hash.rs

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ mod tests {
6161
use super::*;
6262
use hex::FromHex;
6363
use ic_base_types::{NumBytes, NumSeconds};
64-
use ic_canonical_state::CertificationVersion;
64+
use ic_canonical_state::{all_supported_versions, CertificationVersion};
6565
use ic_crypto_tree_hash::Digest;
6666
use ic_error_types::{ErrorCode, UserError};
6767
use ic_management_canister_types::{
@@ -99,7 +99,6 @@ mod tests {
9999
collections::{BTreeMap, BTreeSet},
100100
sync::Arc,
101101
};
102-
use strum::{EnumCount, IntoEnumIterator};
103102

104103
const INITIAL_CYCLES: Cycles = Cycles::new(1 << 36);
105104

@@ -367,35 +366,17 @@ mod tests {
367366
// PLEASE INCREMENT THE CERTIFICATION VERSION AND PROVIDE APPROPRIATE
368367
// BACKWARD COMPATIBILITY CODE FOR OLD CERTIFICATION VERSIONS THAT
369368
// NEED TO BE SUPPORTED.
370-
let expected_hashes: [&str; CertificationVersion::COUNT] = [
371-
"1B931426F36191153996B82CE305BE659AAE65D8AE75B4839736176C0453BDF3",
372-
"3B3F058CD6BAF16A990585223CDD9ED98BC5507B51403707E486B764F1FF5DAE",
373-
"5F17DC054CBE03F1887400247E4255764AAF3CDBFBA0AD4F414CB7ABAA4782C2",
374-
"2A615692EF107355C38439DC5AABDF85BAE4979C4135F761213485C655B6F196",
375-
"2A615692EF107355C38439DC5AABDF85BAE4979C4135F761213485C655B6F196",
376-
"2A615692EF107355C38439DC5AABDF85BAE4979C4135F761213485C655B6F196",
377-
"F86FEBBF994627432621BE7DEBD9D59BECEBD922C9C8B4F7F37BD34A5709F16B",
378-
"F86FEBBF994627432621BE7DEBD9D59BECEBD922C9C8B4F7F37BD34A5709F16B",
379-
"410BD1929B6884DE65DDBACC54749FDCC2A5FA3585898B9B2644147DE2760678",
380-
"410BD1929B6884DE65DDBACC54749FDCC2A5FA3585898B9B2644147DE2760678",
381-
"4BAB4FD35605188FDDCA534204C8E8852C9E450CEB6BE53129FB84DF109D8905",
382-
"1ED37E00D177681A4111B6D45F518DF3E414B0B614333BB6552EBC0D8492B687",
383-
"62B2E77DFCD17C7E0CE3E762FD37281776C4B0A38CE1B83A1316614C3F849E39",
384-
"80D4B528CC9E09C775273994261DD544D45EFFF90B655D90FC3A6E3F633ED718",
385-
"970BC5155AEB4B4F81E470CBF6748EFA7D8805B936998A54AE70B7DD21F5DDCC",
386-
"EA3B53B72150E3982CB0E6773F86634685EE7B153DCFE10D86D9927778409D97",
387-
"D13F75C42D3E2BDA2F742510029088A9ADB119E30241AC969DE24936489168B5",
369+
let expected_hashes: [&str; 3] = [
388370
"D13F75C42D3E2BDA2F742510029088A9ADB119E30241AC969DE24936489168B5",
389371
"E739B8EA1585E9BB97988C80ED0C0CDFDF064D4BC5A2B6B06EB414BFF6139CCE",
390372
"31F4593CC82CDB0B858F190E00112AF4599B5333F7AED9403EEAE88B656738D5",
391373
];
374+
assert_eq!(expected_hashes.len(), all_supported_versions().count());
392375

393-
for certification_version in CertificationVersion::iter() {
394-
assert_partial_state_hash_matches(
395-
certification_version,
396-
// expected_hash
397-
expected_hashes[certification_version as usize],
398-
);
376+
for (certification_version, expected_hash) in
377+
all_supported_versions().zip(expected_hashes.iter())
378+
{
379+
assert_partial_state_hash_matches(certification_version, expected_hash);
399380
}
400381
}
401382
}

0 commit comments

Comments
 (0)