feat(sns): Implement AdvanceSnsTargetVersion proposal type (#2704)

This PR implements the new `AdvanceSnsTargetVersion` proposal type. This is part of an upcoming [Effortless SNS Updates](https://forum.dfinity.org/t/effortless-sns-updates/35751) feature. Example proposal rendering: ```markdown # Proposal to advance SNS target version | Canister | Current version's module hash | New target version's module hash | |------------|------------------------------------------------------------------|------------------------------------------------------------------| | Root | 94914e8dee53b50119b0d7ab3231d31a9edcbe64a551ab1bf7c64a384deb6d8e | f179a793617fbf243b8c40c550a7c55de192a78e94cf532c54f2afbb949bbd3b | | Governance | 56b4a7cd6000c0a5a25039eeefc6b8a1293584b6000d2d3337e767cfdfb898f9 | 56b4a7cd6000c0a5a25039eeefc6b8a1293584b6000d2d3337e767cfdfb898f9 | | Swap | 8d86919c99ce6ce9e84d886e57e34d03ba1dcd6146a9b9f0e093aaef75ec9d98 | 8d86919c99ce6ce9e84d886e57e34d03ba1dcd6146a9b9f0e093aaef75ec9d98 | | Index | 85a40c91c08f7e36fd0472c8ffce6b0a706a9327ba75c5026e9c666aa14c6030 | 85a40c91c08f7e36fd0472c8ffce6b0a706a9327ba75c5026e9c666aa14c6030 | | Ledger | faaae9a55533e2aafa262b7be8c8c72111ae7632a987672e42760a426088ca74 | faaae9a55533e2aafa262b7be8c8c72111ae7632a987672e42760a426088ca74 | | Archive | 5327fa6f81b69241d6250369d1085c693f4bb20310e96f762bcd3f49e628c998 | 5327fa6f81b69241d6250369d1085c693f4bb20310e96f762bcd3f49e628c998 | ### Upgrade steps | Step | Root | Governance | Swap | Index | Ledger | Archive | Changes | |------|------|------------|------|-------|--------|---------|---------| | 0 | 94914e | 56b4a7 | 8d8691 | 85a40c | faaae9 | 5327fa | Current version | | 1 | bfafb3 | 56b4a7 | 8d8691 | 85a40c | faaae9 | 5327fa | Root @ bfafb303f73c6a7e0b68b1c0ad8bc70a14b8f65bf68687348fa9be58192108eb | | 2 | f179a7 | 56b4a7 | 8d8691 | 85a40c | faaae9 | 5327fa | Root @ f179a793617fbf243b8c40c550a7c55de192a78e94cf532c54f2afbb949bbd3b | ### Monitoring the upgrade process Please note: the upgrade steps above (valid around timestamp 1620505059 seconds) might change during this proposal's voting period. Such changes are unlikely and are subject to NNS community's approval. The **upgrade journal** provides up-to-date information on this SNS's upgrade process: https://7uieb-cx777-77776-qaaaq-cai.raw.icp0.io/journal/json ``` < [Previous PR ](#2696) | [Next PR](#2715) >
dfinity · Nov 21, 2024 · a4da5f6 · a4da5f6
1 parent 2a7f5c9
commit a4da5f6
Show file tree

Hide file tree

Showing 14 changed files with 982 additions and 110 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel
@@ -29,6 +29,7 @@ BASE_DEPENDENCIES = [
     "@crate_index//:assert_matches",
     "@crate_index//:candid",
     "@crate_index//:futures",
+    "@crate_index//:itertools",
     "@crate_index//:lazy_static",
     "@crate_index//:prost",
     "@crate_index//:rust_decimal",

diff --git a/rs/nervous_system/integration_tests/Cargo.toml b/rs/nervous_system/integration_tests/Cargo.toml
@@ -28,6 +28,7 @@ ic-sns-root = { path = "../../sns/root" }
 ic-sns-swap = { path = "../../sns/swap" }
 icp-ledger = { path = "../../ledger_suite/icp" }
 icrc-ledger-types = { path = "../../../packages/icrc-ledger-types" }
+itertools = { workspace = true }
 lazy_static = { workspace = true }
 lifeline = { path = "../../nns/handlers/lifeline/impl" }
 pocket-ic = { path = "../../../packages/pocket-ic" }

diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs
@@ -58,6 +58,8 @@ use icrc_ledger_types::icrc1::{
     account::Account,
     transfer::{TransferArg, TransferError},
 };
+use itertools::EitherOrBoth;
+use itertools::Itertools;
 use maplit::btreemap;
 use pocket_ic::{
     management_canister::CanisterSettings, nonblocking::PocketIc, ErrorCode, PocketIcBuilder,
@@ -703,6 +705,59 @@ pub async fn upgrade_nns_canister_to_tip_of_master_or_panic(
     );
 }
 
+/// Advances time by up to `timeout_seconds` seconds and `timeout_seconds` tickets (1 tick = 1 second).
+/// Each tick, it observes the state using the provided `observe` function.
+/// If the observed state matches the `expected` state, it returns `Ok(())`.
+/// If the timeout is reached, it returns an error.
+///
+/// Example:
+/// ```
+/// let upgrade_journal_interval_seconds = 60 * 60;
+/// await_with_timeout(
+///     &pocket_ic,
+///     upgrade_journal_interval_seconds,
+///     |pocket_ic| async {
+///         sns::governance::get_upgrade_journal(pocket_ic, sns.governance.canister_id)
+///             .await
+///             .upgrade_steps
+///             .unwrap()
+///             .versions
+///     },
+///     &vec![initial_sns_version.clone()],
+/// )
+/// .await
+/// .unwrap();
+/// ```
+pub async fn await_with_timeout<'a, T, F, Fut>(
+    pocket_ic: &'a PocketIc,
+    timeout_seconds: u64,
+    observe: F,
+    expected: &T,
+) -> Result<(), String>
+where
+    T: std::cmp::PartialEq + std::fmt::Debug,
+    F: Fn(&'a PocketIc) -> Fut,
+    Fut: std::future::Future<Output = T>,
+{
+    let mut counter = 0;
+    loop {
+        pocket_ic.advance_time(Duration::from_secs(1)).await;
+        pocket_ic.tick().await;
+
+        let observed = observe(pocket_ic).await;
+        if observed == *expected {
+            return Ok(());
+        }
+        if counter == timeout_seconds {
+            return Err(format!(
+                "Observed state: {:?}\n!= Expected state {:?}\nafter {} seconds / rounds",
+                observed, expected, timeout_seconds,
+            ));
+        }
+        counter += 1;
+    }
+}
+
 pub mod nns {
     use super::*;
     pub mod governance {
@@ -1469,7 +1524,8 @@ pub mod sns {
             Decode!(&result, sns_pb::ListNeuronsResponse).unwrap()
         }
 
-        /// Searches for the ID and controller principal of an SNS neuron that can submit proposals.
+        /// Searches for the ID and controller principal of an SNS neuron that can submit proposals,
+        /// i.e., a neuron whose `dissolve_delay_seconds` is greater that or equal 6 months.
         pub async fn find_neuron_with_majority_voting_power(
             pocket_ic: &PocketIc,
             canister_id: PrincipalId,
@@ -1514,14 +1570,49 @@ pub mod sns {
             Decode!(&result, sns_pb::NervousSystemParameters).unwrap()
         }
 
+        pub async fn propose_to_advance_sns_target_version(
+            pocket_ic: &PocketIc,
+            sns_governance_canister_id: PrincipalId,
+        ) -> Result<sns_pb::ProposalData, String> {
+            // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this
+            // neuron either holds the majority of the voting power or the follow graph is set up
+            // s.t. when this neuron submits a proposal, that proposal gets through without the need
+            // for any voting.
+            let (sns_neuron_id, sns_neuron_principal_id) =
+                sns::governance::find_neuron_with_majority_voting_power(
+                    pocket_ic,
+                    sns_governance_canister_id,
+                )
+                .await
+                .expect("cannot find SNS neuron with dissolve delay over 6 months.");
+
+            sns::governance::propose_and_wait(
+                pocket_ic,
+                sns_governance_canister_id,
+                sns_neuron_principal_id,
+                sns_neuron_id.clone(),
+                sns_pb::Proposal {
+                    title: "Advance SNS target version.".to_string(),
+                    summary: "".to_string(),
+                    url: "".to_string(),
+                    action: Some(sns_pb::proposal::Action::AdvanceSnsTargetVersion(
+                        sns_pb::AdvanceSnsTargetVersion { new_target: None },
+                    )),
+                },
+            )
+            .await
+            .map_err(|err| err.to_string())
+        }
+
         // Upgrade; one canister at a time.
         pub async fn propose_to_upgrade_sns_to_next_version_and_wait(
             pocket_ic: &PocketIc,
             sns_governance_canister_id: PrincipalId,
         ) {
-            // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this neuron
-            // either holds the majority of the voting power or the follow graph is set up s.t. when this
-            // neuron submits a proposal, that proposal gets through without the need for any voting.
+            // Get an ID of an SNS neuron that can submit proposals. We rely on the fact that this
+            // neuron either holds the majority of the voting power or the follow graph is set up
+            // s.t. when this neuron submits a proposal, that proposal gets through without the need
+            // for any voting.
             let (sns_neuron_id, sns_neuron_principal_id) =
                 find_neuron_with_majority_voting_power(pocket_ic, sns_governance_canister_id)
                     .await
@@ -1626,6 +1717,47 @@ pub mod sns {
                 .await
                 .unwrap()
         }
+
+        /// Verifies that the upgrade journal has the expected entries.
+        pub async fn assert_upgrade_journal(
+            pocket_ic: &PocketIc,
+            sns_governance_canister_id: PrincipalId,
+            expected_entries: &[sns_pb::upgrade_journal_entry::Event],
+        ) {
+            let sns_pb::GetUpgradeJournalResponse {
+                upgrade_journal, ..
+            } = sns::governance::get_upgrade_journal(pocket_ic, sns_governance_canister_id).await;
+
+            let upgrade_journal = upgrade_journal.unwrap().entries;
+
+            for (index, either_or_both) in upgrade_journal
+                .iter()
+                .zip_longest(expected_entries.iter())
+                .enumerate()
+            {
+                let (actual, expected) = match either_or_both {
+                    EitherOrBoth::Both(actual, expected) => (actual, expected),
+                    EitherOrBoth::Left(actual) => panic!(
+                        "Observed an unexpected journal entry at index {}: {:?}",
+                        index, actual
+                    ),
+                    EitherOrBoth::Right(expected) => panic!(
+                        "Did not observe an expected entry at index {}: {:?}",
+                        index, expected
+                    ),
+                };
+                assert!(actual.timestamp_seconds.is_some());
+                assert_eq!(
+                    &actual
+                        .event
+                        .clone()
+                        .map(|event| event.redact_human_readable()),
+                    &Some(expected.clone().redact_human_readable()),
+                    "Upgrade journal entry at index {} does not match",
+                    index
+                );
+            }
+        }
     }
 
     pub mod index_ng {

diff --git a/...ion_tests/tests/advance_target_version.rs → ...tests/tests/advance_sns_target_version.rs b/...ion_tests/tests/advance_target_version.rs → ...tests/tests/advance_sns_target_version.rs
@@ -1,5 +1,4 @@
-use ic_nervous_system_agent::sns::governance::GovernanceCanister;
-use ic_nervous_system_integration_tests::pocket_ic_helpers::sns;
+use ic_nervous_system_integration_tests::pocket_ic_helpers::{await_with_timeout, sns};
 use ic_nervous_system_integration_tests::{
     create_service_nervous_system_builder::CreateServiceNervousSystemBuilder,
     pocket_ic_helpers::{
@@ -15,75 +14,8 @@ use ic_sns_governance::{
 };
 use ic_sns_swap::pb::v1::Lifecycle;
 use ic_sns_wasm::pb::v1::SnsCanisterType;
-use pocket_ic::nonblocking::PocketIc;
 use pocket_ic::PocketIcBuilder;
 use std::collections::BTreeMap;
-use std::time::Duration;
-
-/// Verifies that the upgrade journal has the expected entries.
-async fn assert_upgrade_journal(
-    pocket_ic: &PocketIc,
-    governance: GovernanceCanister,
-    expected_entries: &[sns_pb::upgrade_journal_entry::Event],
-) {
-    let sns_pb::GetUpgradeJournalResponse {
-        upgrade_journal, ..
-    } = sns::governance::get_upgrade_journal(pocket_ic, governance.canister_id).await;
-
-    let upgrade_journal = upgrade_journal.unwrap().entries;
-    assert_eq!(upgrade_journal.len(), expected_entries.len());
-
-    for (index, (actual, expected)) in upgrade_journal
-        .iter()
-        .zip(expected_entries.iter())
-        .enumerate()
-    {
-        assert!(actual.timestamp_seconds.is_some());
-        assert_eq!(
-            &actual
-                .event
-                .clone()
-                .map(|event| event.redact_human_readable()),
-            &Some(expected.clone().redact_human_readable()),
-            "Upgrade journal entry at index {} does not match",
-            index
-        );
-    }
-}
-
-/// Advances time by up to `timeout_seconds` seconds and `timeout_seconds` tickets (1 tick = 1 second).
-/// Each tick, it observes the state using the provided `observe` function.
-/// If the observed state matches the `expected` state, it returns `Ok(())`.
-/// If the timeout is reached, it returns an error.
-async fn await_with_timeout<'a, T, F, Fut>(
-    pocket_ic: &'a PocketIc,
-    timeout_seconds: u64,
-    observe: F,
-    expected: &T,
-) -> Result<(), String>
-where
-    T: std::cmp::PartialEq + std::fmt::Debug,
-    F: Fn(&'a PocketIc) -> Fut,
-    Fut: std::future::Future<Output = T>,
-{
-    let mut counter = 0;
-    loop {
-        pocket_ic.advance_time(Duration::from_secs(1)).await;
-        pocket_ic.tick().await;
-
-        let observed = observe(pocket_ic).await;
-        if observed == *expected {
-            return Ok(());
-        }
-        if counter == timeout_seconds {
-            return Err(format!(
-                "Observed state: {:?}\n!= Expected state {:?}\nafter {} seconds / rounds",
-                observed, expected, timeout_seconds,
-            ));
-        }
-        counter += 1;
-    }
-}
 
 #[tokio::test]
 async fn test_get_upgrade_journal() {
@@ -139,9 +71,9 @@ async fn test_get_upgrade_journal() {
             UpgradeStepsRefreshed::new(vec![initial_sns_version.clone()]),
         ));
 
-        assert_upgrade_journal(
+        sns::governance::assert_upgrade_journal(
             &pocket_ic,
-            sns.governance,
+            sns.governance.canister_id,
             &expected_upgrade_journal_entries,
         )
         .await;
@@ -228,30 +160,27 @@ async fn test_get_upgrade_journal() {
             ]),
         ));
 
-        assert_upgrade_journal(
+        sns::governance::assert_upgrade_journal(
             &pocket_ic,
-            sns.governance,
+            sns.governance.canister_id,
             &expected_upgrade_journal_entries,
         )
         .await;
     }
 
-    // State 3: Advance the target version.
-    sns::governance::advance_target_version(
-        &pocket_ic,
-        sns.governance.canister_id,
-        new_sns_version_2.clone(),
-    )
-    .await;
+    // State 3: Advance the target version via proposal.
+    sns::governance::propose_to_advance_sns_target_version(&pocket_ic, sns.governance.canister_id)
+        .await
+        .unwrap();
 
     expected_upgrade_journal_entries.push(Event::TargetVersionSet(TargetVersionSet::new(
         None,
         Some(new_sns_version_2.clone()),
     )));
 
-    assert_upgrade_journal(
+    sns::governance::assert_upgrade_journal(
         &pocket_ic,
-        sns.governance,
+        sns.governance.canister_id,
         &expected_upgrade_journal_entries,
     )
     .await;
@@ -318,9 +247,9 @@ async fn test_get_upgrade_journal() {
             ),
         );
 
-        assert_upgrade_journal(
+        sns::governance::assert_upgrade_journal(
             &pocket_ic,
-            sns.governance,
+            sns.governance.canister_id,
             &expected_upgrade_journal_entries,
         )
         .await;

diff --git a/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs b/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs
@@ -1903,6 +1903,8 @@ pub mod governance {
         candid::CandidType,
         candid::Deserialize,
         comparable::Comparable,
+        Eq,
+        std::hash::Hash,
         serde::Serialize,
         Clone,
         PartialEq,

diff --git a/rs/sns/governance/protobuf_generator/src/lib.rs b/rs/sns/governance/protobuf_generator/src/lib.rs
@@ -44,6 +44,10 @@ pub fn generate_prost_files(proto: ProtoPaths<'_>, out: &Path) {
         "ic_sns_governance.pb.v1.NeuronId",
         "#[derive(Eq, std::hash::Hash)]",
     );
+    config.type_attribute(
+        "ic_sns_governance.pb.v1.Governance.Version",
+        "#[derive(Eq, std::hash::Hash)]",
+    );
 
     let mut apply_attribute = |attribute, type_names| {
         for type_name in type_names {