dfinity · daniel-wong-dfinity-org · Apr 24, 2026 · Apr 23, 2026 · Apr 23, 2026 · Apr 23, 2026
@@ -2065,6 +2065,7 @@ pub struct ProposalInfo {
 #[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, PartialEq, Debug)]
 pub enum SuccessfulProposalExecutionValue {
     CreateCanisterAndInstallCode(CreateCanisterAndInstallCodeOk),
+    TakeCanisterSnapshot(TakeCanisterSnapshotOk),
 }
 
 /// The result of a successful CreateCanisterAndInstallCode proposal execution.
@@ -2073,6 +2074,24 @@ pub struct CreateCanisterAndInstallCodeOk {
     pub canister_id: Option<PrincipalId>,
 }
 
+/// The result of a successful TakeCanisterSnapshot proposal execution.
+#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, PartialEq, Debug)]
+pub struct TakeCanisterSnapshotOk {
+    pub snapshot_id: Vec<u8>,
+}
+
+impl From<CreateCanisterAndInstallCodeOk> for SuccessfulProposalExecutionValue {
+    fn from(ok: CreateCanisterAndInstallCodeOk) -> Self {
+        Self::CreateCanisterAndInstallCode(ok)
+    }
+}
+
+impl From<TakeCanisterSnapshotOk> for SuccessfulProposalExecutionValue {
+    fn from(ok: TakeCanisterSnapshotOk) -> Self {
+        Self::TakeCanisterSnapshot(ok)
+    }
+}
+
 /// Network economics contains the parameters for several operations related
 /// to the economy of the network. When submitting a NetworkEconomics proposal
 /// default values (0) are considered unchanged, so a valid proposal only needs

@@ -251,8 +251,13 @@ type CreateCanisterAndInstallCodeOk = record {
   canister_id : opt principal;
 };
 
+type TakeCanisterSnapshotOk = record {
+  snapshot_id : blob;
+};
+
 type SuccessfulProposalExecutionValue = variant {
   CreateCanisterAndInstallCode : CreateCanisterAndInstallCodeOk;
+  TakeCanisterSnapshot : TakeCanisterSnapshotOk;
 };
 
 type CreateServiceNervousSystem = record {

@@ -1261,13 +1261,18 @@ message ProposalData {
 message SuccessfulProposalExecutionValue {
   oneof proposal_type {
     CreateCanisterAndInstallCodeOk create_canister_and_install_code = 1;
+    TakeCanisterSnapshotOk take_canister_snapshot = 2;
   }
 }
 
 message CreateCanisterAndInstallCodeOk {
   ic_base_types.pb.v1.PrincipalId canister_id = 1;
 }
 
+message TakeCanisterSnapshotOk {
+  bytes snapshot_id = 1;
+}
+
 // This structure contains data for settling the Neurons' Fund participation in an SNS token swap.
 message NeuronsFundData {
   // Initial Neurons' Fund reserves computed at the time of execution of the proposal through which

@@ -1542,7 +1542,7 @@ pub struct ProposalData {
 pub struct SuccessfulProposalExecutionValue {
     #[prost(
         oneof = "successful_proposal_execution_value::ProposalType",
-        tags = "1"
+        tags = "1, 2"
     )]
     pub proposal_type: ::core::option::Option<successful_proposal_execution_value::ProposalType>,
 }
@@ -1560,6 +1560,8 @@ pub mod successful_proposal_execution_value {
     pub enum ProposalType {
         #[prost(message, tag = "1")]
         CreateCanisterAndInstallCode(super::CreateCanisterAndInstallCodeOk),
+        #[prost(message, tag = "2")]
+        TakeCanisterSnapshot(super::TakeCanisterSnapshotOk),
     }
 }
 #[derive(
@@ -1575,6 +1577,19 @@ pub struct CreateCanisterAndInstallCodeOk {
     #[prost(message, optional, tag = "1")]
     pub canister_id: ::core::option::Option<::ic_base_types::PrincipalId>,
 }
+#[derive(
+    candid::CandidType,
+    candid::Deserialize,
+    serde::Serialize,
+    comparable::Comparable,
+    Clone,
+    PartialEq,
+    ::prost::Message,
+)]
+pub struct TakeCanisterSnapshotOk {
+    #[prost(bytes = "vec", tag = "1")]
+    pub snapshot_id: ::prost::alloc::vec::Vec<u8>,
+}
 /// This structure contains data for settling the Neurons' Fund participation in an SNS token swap.
 #[derive(
     candid::CandidType,

@@ -1771,20 +1771,18 @@ impl From<api::DerivedProposalInformation> for pb::DerivedProposalInformation {
     }
 }
 
-impl From<pb::SuccessfulProposalExecutionValue> for api::SuccessfulProposalExecutionValue {
+impl From<pb::SuccessfulProposalExecutionValue> for Option<api::SuccessfulProposalExecutionValue> {
     fn from(item: pb::SuccessfulProposalExecutionValue) -> Self {
-        match item.proposal_type {
-            Some(ProposalType::CreateCanisterAndInstallCode(ok)) => {
+        let result = match item.proposal_type? {
+            ProposalType::CreateCanisterAndInstallCode(ok) => {
                 api::SuccessfulProposalExecutionValue::CreateCanisterAndInstallCode(ok.into())
             }
-            None => {
-                // This shouldn't happen, but if it does, we need a fallback.
-                // Use a CreateCanisterAndInstallCode with None canister_id.
-                api::SuccessfulProposalExecutionValue::CreateCanisterAndInstallCode(
-                    api::CreateCanisterAndInstallCodeOk { canister_id: None },
-                )
+            ProposalType::TakeCanisterSnapshot(ok) => {
+                api::SuccessfulProposalExecutionValue::TakeCanisterSnapshot(ok.into())
             }
-        }
+        };
+
+        Some(result)
     }
 }
 
@@ -1830,6 +1828,35 @@ impl From<pb::CreateCanisterAndInstallCodeOk> for pb::SuccessfulProposalExecutio
     }
 }
 
+impl From<root::TakeCanisterSnapshotOk> for pb::TakeCanisterSnapshotOk {
+    fn from(ok: root::TakeCanisterSnapshotOk) -> Self {
+        Self { snapshot_id: ok.id }
+    }
+}
+
+impl From<root::TakeCanisterSnapshotOk> for pb::SuccessfulProposalExecutionValue {
+    fn from(ok: root::TakeCanisterSnapshotOk) -> Self {
+        Self::from(pb::TakeCanisterSnapshotOk::from(ok))
+    }
+}
+
+// Upgrade Ok to result.
+impl From<pb::TakeCanisterSnapshotOk> for pb::SuccessfulProposalExecutionValue {
+    fn from(ok: pb::TakeCanisterSnapshotOk) -> Self {
+        Self {
+            proposal_type: Some(ProposalType::TakeCanisterSnapshot(ok)),
+        }
+    }
+}
+
+impl From<pb::TakeCanisterSnapshotOk> for api::TakeCanisterSnapshotOk {
+    fn from(item: pb::TakeCanisterSnapshotOk) -> Self {
+        Self {
+            snapshot_id: item.snapshot_id,
+        }
+    }
+}
+
 impl From<pb::SwapBackgroundInformation> for api::SwapBackgroundInformation {
     fn from(item: pb::SwapBackgroundInformation) -> Self {
         Self {

@@ -566,7 +566,7 @@ pub(crate) fn proposal_data_to_info(
     let success_value = data
         .success_value
         .clone()
-        .map(|success_value| success_value.into());
+        .and_then(Option::<api::SuccessfulProposalExecutionValue>::from);
 
     let proposal = data
         .proposal

@@ -45,7 +45,6 @@ impl LoadCanisterSnapshot {
 }
 
 impl CallCanister for LoadCanisterSnapshot {
-    // TODO: Use a typed reply instead of () so that the result is recorded.
     type Reply = ();
 
     fn canister_and_function(&self) -> Result<(CanisterId, &str), GovernanceError> {

@@ -1,14 +1,18 @@
 use crate::{
-    pb::v1::{GovernanceError, TakeCanisterSnapshot, Topic},
+    pb::v1::{GovernanceError, TakeCanisterSnapshot, Topic, governance_error::ErrorType},
     proposals::{
-        call_canister::CallCanister, invalid_proposal_error, self_describing::DocumentedAction,
+        call_canister::{CallCanister, CallCanisterReply},
+        invalid_proposal_error,
+        self_describing::DocumentedAction,
         topic_to_manage_canister,
     },
 };
-use candid::Encode;
+use candid::{Decode, Encode};
 use ic_base_types::{CanisterId, PrincipalId};
 use ic_nns_constants::ROOT_CANISTER_ID;
-use ic_nns_handler_root_interface::TakeCanisterSnapshotRequest;
+use ic_nns_handler_root_interface::{
+    TakeCanisterSnapshotOk, TakeCanisterSnapshotRequest, TakeCanisterSnapshotResponse,
+};
 
 impl TakeCanisterSnapshot {
     pub fn validate(&self) -> Result<(), GovernanceError> {
@@ -33,8 +37,7 @@ impl TakeCanisterSnapshot {
 }
 
 impl CallCanister for TakeCanisterSnapshot {
-    // TODO: Use a typed reply instead of () so that the result is recorded.
-    type Reply = ();
+    type Reply = TakeCanisterSnapshotOk;
 
     fn canister_and_function(&self) -> Result<(CanisterId, &str), GovernanceError> {
         Ok((ROOT_CANISTER_ID, "take_canister_snapshot"))
@@ -47,6 +50,25 @@ impl CallCanister for TakeCanisterSnapshot {
     }
 }
 
+impl CallCanisterReply for TakeCanisterSnapshotOk {
+    fn try_decode(encoded_reply: &[u8]) -> Result<Option<Self>, GovernanceError> {
+        let response = Decode!(encoded_reply, TakeCanisterSnapshotResponse).map_err(|err| {
+            GovernanceError::new_with_message(
+                ErrorType::External,
+                format!("Failed to decode TakeCanisterSnapshotResponse: {err}"),
+            )
+        })?;
+
+        match response {
+            TakeCanisterSnapshotResponse::Ok(ok) => Ok(Some(ok)),
+            TakeCanisterSnapshotResponse::Err(err) => Err(GovernanceError::new_with_message(
+                ErrorType::External,
+                format!("Root returned error for TakeCanisterSnapshot: {:?}", err),
+            )),
+        }
+    }
+}
+
 pub fn convert_take_canister_snapshot_from_proposal_to_root_request(
     original: &TakeCanisterSnapshot,
 ) -> Result<TakeCanisterSnapshotRequest, GovernanceError> {

@@ -9,6 +9,9 @@ on the process that this file is part of, see
 
 ## Added
 
+* `TakeCanisterSnapshot` proposals now store the new snapshot ID in the
+  `success_value` field.
+
 ## Changed
 
 ## Deprecated

@@ -1,5 +1,5 @@
 use candid::{Nat, Principal};
-use ic_base_types::PrincipalId;
+use ic_base_types::{CanisterId, PrincipalId, SnapshotId};
 use ic_ledger_core::tokens::{CheckedAdd, CheckedSub};
 use ic_management_canister_types_private::CanisterSnapshotResponse;
 use ic_nervous_system_common::E8;
@@ -10,7 +10,10 @@ use ic_nervous_system_integration_tests::pocket_ic_helpers::{
 };
 use ic_nns_constants::{GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID, ROOT_CANISTER_ID};
 use ic_nns_governance::pb::v1::ProposalStatus;
-use ic_nns_governance_api::{MakeProposalRequest, Motion, ProposalActionRequest};
+use ic_nns_governance_api::{
+    MakeProposalRequest, Motion, ProposalActionRequest, SuccessfulProposalExecutionValue,
+    TakeCanisterSnapshotOk,
+};
 use icp_ledger::{AccountIdentifier, DEFAULT_TRANSFER_FEE, Tokens};
 use icrc_ledger_types::icrc1::{account::Account, transfer::TransferArg};
 use pocket_ic::{PocketIcBuilder, nonblocking::PocketIc};
@@ -181,6 +184,16 @@ async fn test_canister_snapshot() {
     }
     assert_snapshot_seems_reasonable(first_snapshot);
 
+    // Step 3A.2: Verify that the snapshot ID is recorded in success_value.
+    let snapshot_id_from_proposal = match &first_proposal_info.success_value {
+        Some(SuccessfulProposalExecutionValue::TakeCanisterSnapshot(ok)) => ok.snapshot_id.clone(),
+        other => panic!("Expected TakeCanisterSnapshot success value, got: {other:#?}"),
+    };
+    assert_eq!(
+        snapshot_id_from_proposal,
+        first_snapshot.snapshot_id().as_slice(),
+    );
+
     // Scenario B: Replace an existing snapshot.
 
     // Step 2(B): Run code under test.
@@ -237,6 +250,16 @@ async fn test_canister_snapshot() {
     let second_snapshot = &list_canister_snapshots_response[0];
     assert_ne!(second_snapshot.snapshot_id(), first_snapshot.snapshot_id());
 
+    // Also verify that the replace proposal's success_value records the new snapshot ID.
+    let snapshot_id_from_replace_proposal = match &replace_proposal_info.success_value {
+        Some(SuccessfulProposalExecutionValue::TakeCanisterSnapshot(ok)) => ok.snapshot_id.clone(),
+        other => panic!("Expected TakeCanisterSnapshot success value, got: {other:#?}"),
+    };
+    assert_eq!(
+        snapshot_id_from_replace_proposal,
+        second_snapshot.snapshot_id().as_slice(),
+    );
+
     // Generic checks of the second snapshot.
     assert_snapshot_seems_reasonable(second_snapshot);
 
@@ -450,7 +473,7 @@ async fn test_governance_canister_snapshot() {
             "Take a snapshot of the Governance canister.".to_string(),
         ],
     );
-    let _take_snapshot_proposal_id = extract_proposal_id(&output);
+    let take_canister_snapshot_proposal_id = extract_proposal_id(&output);
 
     // Wait for the effect of the proposal happen. This cannot be done in
     // the normal way (i.e. wait for the proposal status to become Executed,
@@ -474,6 +497,22 @@ async fn test_governance_canister_snapshot() {
     let snapshot = &snapshots[0];
     assert_eq!(snapshot.id.get_canister_id(), GOVERNANCE_CANISTER_ID);
 
+    // Verify that success_value is populated. When the target is Governance,
+    // Root returns a placeholder/optimistic response to avoid a deadlock
+    // (it cannot stop Governance while Governance is waiting for it). The
+    // snapshot ID in that placeholder is zeroed.
+    let take_canister_snapshot_proposal_info =
+        nns::governance::get_proposal_info(&pocket_ic, take_canister_snapshot_proposal_id)
+            .await
+            .unwrap();
+    let optimistic_snapshot_id = SnapshotId::from((CanisterId::from_u64(0), 0_u64)).to_vec();
+    assert_eq!(
+        take_canister_snapshot_proposal_info.success_value.unwrap(),
+        SuccessfulProposalExecutionValue::from(TakeCanisterSnapshotOk {
+            snapshot_id: optimistic_snapshot_id,
+        }),
+    );
+
     // Change Governance's state by creating ANOTHER (Motion) proposal.
     // This will get blown away at the end when we load the snapshot,
     // because the snapshot was taken before this proposal.