feat(sns): Automatically advance SNS target version upon opt-in (dfinity#3119)

This PR implements a simple extension of the AdvanceSnsTargetVersion
mechanism, allowing SNSs to opt-in for getting target versions advanced
to the latest available SNS version blessed by the NNS community.

Author: aterga

rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs

@@ -180,7 +180,8 @@ async fn test_get_upgrade_journal() {
 
     expected_upgrade_journal_entries.push(Event::TargetVersionSet(TargetVersionSet::new(
         None,
-        Some(new_sns_version_2.clone()),
+        new_sns_version_2.clone(),
+        false,
     )));
 
     sns::governance::assert_upgrade_journal(

rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs

@@ -1076,6 +1076,9 @@ pub struct NervousSystemParameters {
     /// that the PB default (bool fields are false) and our application default
     /// (enabled) agree.
     pub maturity_modulation_disabled: Option<bool>,
+    /// Whether to automatically advance the SNS target version after a new upgrade is published
+    /// by the NNS. If not specified, defaults to false for backward compatibility.
+    pub automatically_advance_target_version: Option<bool>,
 }
 #[derive(Default, candid::CandidType, candid::Deserialize, Debug, Clone, Copy, PartialEq)]
 pub struct VotingRewardsParameters {
@@ -2236,6 +2239,7 @@ pub mod upgrade_journal_entry {
     pub struct TargetVersionSet {
         pub old_target_version: Option<super::governance::Version>,
         pub new_target_version: Option<super::governance::Version>,
+        pub is_advanced_automatically: Option<bool>,
     }
     #[derive(
         candid::CandidType, candid::Deserialize, Debug, serde::Serialize, Clone, PartialEq,

rs/sns/governance/canister/governance.did

@@ -481,6 +481,7 @@ type NervousSystemParameters = record {
   voting_rewards_parameters : opt VotingRewardsParameters;
   maturity_modulation_disabled : opt bool;
   max_number_of_principals_per_neuron : opt nat64;
+  automatically_advance_target_version : opt bool;
 };
 
 type Neuron = record {
@@ -784,6 +785,7 @@ type UpgradeStepsReset = record {
 type TargetVersionSet = record {
   new_target_version : opt Version;
   old_target_version : opt Version;
+  is_advanced_automatically : opt bool;
 };
 
 type TargetVersionReset = record {

rs/sns/governance/canister/governance_test.did

@@ -495,6 +495,7 @@ type NervousSystemParameters = record {
   voting_rewards_parameters : opt VotingRewardsParameters;
   maturity_modulation_disabled : opt bool;
   max_number_of_principals_per_neuron : opt nat64;
+  automatically_advance_target_version : opt bool;
 };
 
 type Neuron = record {
@@ -798,6 +799,7 @@ type UpgradeStepsReset = record {
 type TargetVersionSet = record {
   new_target_version : opt Version;
   old_target_version : opt Version;
+  is_advanced_automatically : opt bool;
 };
 
 type TargetVersionReset = record {

rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto

@@ -1099,6 +1099,10 @@ message NervousSystemParameters {
   // that the PB default (bool fields are false) and our application default
   // (enabled) agree.
   optional bool maturity_modulation_disabled = 22;
+
+  // Whether to automatically advance the SNS target version after a new upgrade is published
+  // by the NNS. If not specified, defaults to false for backward compatibility.
+  optional bool automatically_advance_target_version = 23;
 }
 
 message VotingRewardsParameters {
@@ -2244,6 +2248,7 @@ message UpgradeJournalEntry {
   message TargetVersionSet {
     optional Governance.Version old_target_version = 1;
     optional Governance.Version new_target_version = 2;
+    optional bool is_advanced_automatically = 3;
   }
 
   message TargetVersionReset {

rs/sns/governance/src/cached_upgrade_steps.rs

@@ -476,7 +476,8 @@ impl Governance {
         true
     }
 
-    /// Refreshes the cached_upgrade_steps field
+    /// Attempts to refresh the cached_upgrade_steps field and (if this SNS wants automatic
+    /// deployment of upgrades), also the target_version.
     pub async fn refresh_cached_upgrade_steps(&mut self, deployed_version: Version) {
         let sns_governance_canister_id = self.env.canister_id().get();
 
@@ -495,6 +496,26 @@ impl Governance {
             }
         };
 
+        if self.should_automatically_advance_target_version()
+            && upgrade_steps.has_pending_upgrades()
+        {
+            let new_target = upgrade_steps.last().clone();
+
+            {
+                let old_version = self.proto.target_version.clone();
+                let new_target = new_target.clone();
+                if old_version.as_ref() != Some(&new_target) {
+                    self.push_to_upgrade_journal(upgrade_journal_entry::TargetVersionSet::new(
+                        old_version,
+                        new_target,
+                        true,
+                    ));
+                }
+            }
+
+            self.proto.target_version.replace(new_target);
+        }
+
         // This copy of the data would go to the upgrade journal for auditability.
         let versions = upgrade_steps.clone().into_iter().collect();
 

rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs

@@ -1473,6 +1473,10 @@ pub struct NervousSystemParameters {
     /// (enabled) agree.
     #[prost(bool, optional, tag = "22")]
     pub maturity_modulation_disabled: ::core::option::Option<bool>,
+    /// Whether to automatically advance the SNS target version after a new upgrade is published
+    /// by the NNS. If not specified, defaults to false for backward compatibility.
+    #[prost(bool, optional, tag = "23")]
+    pub automatically_advance_target_version: ::core::option::Option<bool>,
 }
 #[derive(
     candid::CandidType,
@@ -3547,6 +3551,8 @@ pub mod upgrade_journal_entry {
         pub old_target_version: ::core::option::Option<super::governance::Version>,
         #[prost(message, optional, tag = "2")]
         pub new_target_version: ::core::option::Option<super::governance::Version>,
+        #[prost(bool, optional, tag = "3")]
+        pub is_advanced_automatically: ::core::option::Option<bool>,
     }
     #[derive(
         candid::CandidType,

rs/sns/governance/src/governance.rs

@@ -3106,11 +3106,14 @@ impl Governance {
         let (_, target_version) = self
             .proto
             .validate_new_target_version(Some(new_target))
-            .map_err(|err| GovernanceError::new_with_message(ErrorType::InvalidProposal, err))?;
+            .map_err(|err: String| {
+                GovernanceError::new_with_message(ErrorType::InvalidProposal, err)
+            })?;
 
         self.push_to_upgrade_journal(upgrade_journal_entry::TargetVersionSet::new(
             self.proto.target_version.clone(),
-            Some(target_version.clone()),
+            target_version.clone(),
+            false,
         ));
 
         self.proto.target_version = Some(target_version);
@@ -3123,6 +3126,16 @@ impl Governance {
         self.proto.parameters.as_ref()
     }
 
+    pub fn should_automatically_advance_target_version(&self) -> bool {
+        self.nervous_system_parameters()
+            .map(|nervous_system_parameters| {
+                nervous_system_parameters
+                    .automatically_advance_target_version
+                    .unwrap_or_default()
+            })
+            .unwrap_or_default()
+    }
+
     /// Returns the NervousSystemParameters or panics
     fn nervous_system_parameters_or_panic(&self) -> &NervousSystemParameters {
         self.nervous_system_parameters()

rs/sns/governance/src/governance/advance_target_sns_version_tests.rs

@@ -926,6 +926,84 @@ fn get_or_reset_upgrade_steps_leads_to_should_refresh_cached_upgrade_steps() {
     assert!(gov.should_refresh_cached_upgrade_steps());
 }
 
+#[tokio::test]
+async fn test_refresh_cached_upgrade_steps_advances_target_version_automatically_is_required() {
+    let version_a = Version {
+        root_wasm_hash: vec![1, 2, 3],
+        governance_wasm_hash: vec![2, 3, 4],
+        ledger_wasm_hash: vec![3, 4, 5],
+        swap_wasm_hash: vec![4, 5, 6],
+        archive_wasm_hash: vec![5, 6, 7],
+        index_wasm_hash: vec![6, 7, 8],
+    };
+    let mut version_b = version_a.clone();
+    version_b.root_wasm_hash = vec![9, 9, 9];
+
+    // Smoke test.
+    assert_ne!(version_a, version_b);
+
+    let make_gov = || -> Governance {
+        let mut env = NativeEnvironment::new(Some(*TEST_GOVERNANCE_CANISTER_ID));
+        add_environment_mock_list_upgrade_steps_call(
+            &mut env,
+            vec![
+                SnsVersion::from(version_a.clone()),
+                SnsVersion::from(version_b.clone()),
+            ],
+        );
+        Governance::new(
+            GovernanceProto {
+                deployed_version: Some(version_a.clone()),
+                cached_upgrade_steps: None,
+                ..basic_governance_proto()
+            }
+            .try_into()
+            .unwrap(),
+            Box::new(env),
+            Box::new(DoNothingLedger {}),
+            Box::new(DoNothingLedger {}),
+            Box::new(FakeCmc::new()),
+        )
+    };
+
+    for (automatically_advance_target_version, expected_target_version) in [
+        (None, None),
+        (Some(false), None),
+        (Some(true), Some(version_b.clone())),
+    ] {
+        let mut gov = make_gov();
+
+        if let Some(parameters) = gov.proto.parameters.as_mut() {
+            parameters.automatically_advance_target_version = automatically_advance_target_version;
+        };
+
+        // Precondition
+        assert_eq!(gov.proto.cached_upgrade_steps, None);
+        assert_eq!(gov.proto.target_version, None);
+
+        // Run code under test and assert intermediate conditions.
+        let deployed_version = gov
+            .try_temporarily_lock_refresh_cached_upgrade_steps()
+            .unwrap();
+        gov.refresh_cached_upgrade_steps(deployed_version).await;
+
+        // Intermediate postcondition.
+        assert_eq!(
+            gov.proto
+                .cached_upgrade_steps
+                .clone()
+                .unwrap()
+                .upgrade_steps
+                .unwrap()
+                .versions,
+            vec![version_a.clone(), version_b.clone(),]
+        );
+
+        // Main postcondition.
+        assert_eq!(gov.proto.target_version, expected_target_version);
+    }
+}
+
 fn add_environment_mock_calls_for_initiate_upgrade(
     env: &mut NativeEnvironment,
     expected_wasm_hash_requested: Vec<u8>,

rs/sns/governance/src/pb/conversions.rs

@@ -1123,6 +1123,7 @@ impl From<pb::NervousSystemParameters> for pb_api::NervousSystemParameters {
             max_dissolve_delay_bonus_percentage: item.max_dissolve_delay_bonus_percentage,
             max_age_bonus_percentage: item.max_age_bonus_percentage,
             maturity_modulation_disabled: item.maturity_modulation_disabled,
+            automatically_advance_target_version: item.automatically_advance_target_version,
         }
     }
 }
@@ -1150,6 +1151,7 @@ impl From<pb_api::NervousSystemParameters> for pb::NervousSystemParameters {
             max_dissolve_delay_bonus_percentage: item.max_dissolve_delay_bonus_percentage,
             max_age_bonus_percentage: item.max_age_bonus_percentage,
             maturity_modulation_disabled: item.maturity_modulation_disabled,
+            automatically_advance_target_version: item.automatically_advance_target_version,
         }
     }
 }
@@ -3222,6 +3224,7 @@ impl From<pb::upgrade_journal_entry::TargetVersionSet>
         Self {
             old_target_version: item.old_target_version.map(|x| x.into()),
             new_target_version: item.new_target_version.map(|x| x.into()),
+            is_advanced_automatically: item.is_advanced_automatically,
         }
     }
 }
@@ -3232,6 +3235,7 @@ impl From<pb_api::upgrade_journal_entry::TargetVersionSet>
         Self {
             old_target_version: item.old_target_version.map(|x| x.into()),
             new_target_version: item.new_target_version.map(|x| x.into()),
+            is_advanced_automatically: item.is_advanced_automatically,
         }
     }
 }