feat(nns): Self describing value conversion for AddWasm and SubnetRentalRequest (#8322)

Implement conversion from AddWasm/SubnetRentalRequest to
SelfDescribingValue, following a similar pattern as BitcoinSetConfig

Author: jasonz-dfinity

rs/nns/governance/src/proposals/execute_nns_function.rs

@@ -7,12 +7,13 @@ use crate::{
     },
     proposals::{
         decode_candid_args_to_self_describing_value::decode_candid_args_to_self_describing_value,
-        self_describing::ValueBuilder,
+        self_describing::{SelfDescribingProstEnum, ValueBuilder},
     },
 };
 
 use candid::{Decode, Encode};
 use ic_base_types::CanisterId;
+use ic_crypto_sha2::Sha256;
 use ic_management_canister_types_private::{CanisterMetadataRequest, CanisterMetadataResponse};
 use ic_nns_common::types::CallCanisterRequest;
 use ic_nns_constants::{
@@ -22,7 +23,7 @@ use ic_nns_constants::{
 };
 use ic_nns_governance_api::bitcoin::{BitcoinNetwork, BitcoinSetConfigProposal};
 use ic_nns_governance_api::subnet_rental::{SubnetRentalProposalPayload, SubnetRentalRequest};
-use ic_sns_wasm::pb::v1::{AddWasmRequest, SnsWasm};
+use ic_sns_wasm::pb::v1::{AddWasmRequest, SnsCanisterType, SnsWasm};
 use std::sync::Arc;
 
 /// A partial Candid interface for the management canister (ic_00) that contains the necessary
@@ -67,6 +68,16 @@ impl ValidExecuteNnsFunction {
                 let request = get_request_for_bitcoin_set_config(&self.payload)?;
                 return Ok(SelfDescribingValue::from(request));
             }
+            ValidNnsFunction::SubnetRentalRequest => {
+                let subnet_rental_request =
+                    decode_subnet_rental_request(&self.payload).map_err(|e| e.error_message)?;
+                return Ok(SelfDescribingValue::from(subnet_rental_request));
+            }
+            ValidNnsFunction::AddSnsWasm => {
+                let add_wasm_request =
+                    decode_add_wasm_request(&self.payload).map_err(|e| e.error_message)?;
+                return Ok(SelfDescribingValue::from(add_wasm_request));
+            }
             _ => {}
         };
 
@@ -142,39 +153,19 @@ impl ValidExecuteNnsFunction {
                 Ok(encoded_request)
             }
             ValidNnsFunction::SubnetRentalRequest => {
-                // Decode the payload to `SubnetRentalRequest`.
-                let decoded_payload =
-                    Decode!([decoder_config()]; &self.payload, SubnetRentalRequest).map_err(
-                        |_| {
-                            GovernanceError::new_with_message(
-                                ErrorType::InvalidProposal,
-                                "Unable to decode SubnetRentalRequest proposal: {e}",
-                            )
-                        },
-                    )?;
-
-                // Convert the payload to `SubnetRentalProposalPayload`.
-                let SubnetRentalRequest {
-                    user,
-                    rental_condition_id,
-                } = decoded_payload;
-                let proposal_creation_time_seconds = proposal_timestamp_seconds;
-                let encoded_payload = Encode!(&SubnetRentalProposalPayload {
-                    user,
-                    rental_condition_id,
+                let decoded_payload = decode_subnet_rental_request(&self.payload)?;
+                let encoded_payload = encode_subnet_rental_proposal_payload(
+                    decoded_payload,
                     proposal_id,
-                    proposal_creation_time_seconds,
-                })
-                .unwrap();
-
+                    proposal_timestamp_seconds,
+                )?;
                 Ok(encoded_payload)
             }
 
             ValidNnsFunction::AddSnsWasm => {
-                let transformed_payload =
-                    add_proposal_id_to_add_wasm_request(&self.payload, proposal_id)?;
-
-                Ok(transformed_payload)
+                let decoded_payload = decode_add_wasm_request(&self.payload)?;
+                let encoded_payload = encode_add_wasm_request(decoded_payload, proposal_id)?;
+                Ok(encoded_payload)
             }
 
             // Most NNS functions don't require any transformation of the payload, and for new NNS
@@ -203,35 +194,54 @@ fn get_request_for_bitcoin_set_config(payload: &[u8]) -> Result<CallCanisterRequ
     })
 }
 
-impl From<CallCanisterRequest> for SelfDescribingValue {
-    fn from(request: CallCanisterRequest) -> Self {
-        let CallCanisterRequest {
-            canister_id,
-            method_name,
-            payload,
-        } = request;
-        ValueBuilder::new()
-            .add_field("canister_id", canister_id)
-            .add_field("method_name", method_name)
-            .add_field("payload", payload)
-            .build()
-    }
+fn decode_subnet_rental_request(payload: &[u8]) -> Result<SubnetRentalRequest, GovernanceError> {
+    Decode!([decoder_config()]; payload, SubnetRentalRequest).map_err(|_| {
+        GovernanceError::new_with_message(
+            ErrorType::InvalidProposal,
+            "Unable to decode SubnetRentalRequest proposal: {e}",
+        )
+    })
 }
 
-fn add_proposal_id_to_add_wasm_request(
-    payload: &[u8],
+fn encode_subnet_rental_proposal_payload(
+    subnet_rental_request: SubnetRentalRequest,
     proposal_id: u64,
+    proposal_creation_time_seconds: u64,
 ) -> Result<Vec<u8>, GovernanceError> {
-    let add_wasm_request = match Decode!([decoder_config()]; payload, AddWasmRequest) {
-        Ok(add_wasm_request) => add_wasm_request,
-        Err(e) => {
-            return Err(GovernanceError::new_with_message(
-                ErrorType::InvalidProposal,
-                format!("Payload must be a valid AddWasmRequest. Error: {e}"),
-            ));
-        }
-    };
+    let SubnetRentalRequest {
+        user,
+        rental_condition_id,
+    } = subnet_rental_request;
+
+    let encoded_payload = Encode!(&SubnetRentalProposalPayload {
+        user,
+        rental_condition_id,
+        proposal_id,
+        proposal_creation_time_seconds,
+    })
+    .map_err(|_| {
+        GovernanceError::new_with_message(
+            ErrorType::InvalidProposal,
+            "Unable to encode SubnetRentalProposalPayload proposal: {e}",
+        )
+    })?;
+
+    Ok(encoded_payload)
+}
 
+fn decode_add_wasm_request(payload: &[u8]) -> Result<AddWasmRequest, GovernanceError> {
+    Decode!([decoder_config()]; payload, AddWasmRequest).map_err(|_| {
+        GovernanceError::new_with_message(
+            ErrorType::InvalidProposal,
+            "Unable to decode AddWasmRequest proposal: {e}",
+        )
+    })
+}
+
+fn encode_add_wasm_request(
+    add_wasm_request: AddWasmRequest,
+    proposal_id: u64,
+) -> Result<Vec<u8>, GovernanceError> {
     let wasm = add_wasm_request
         .wasm
         .ok_or(GovernanceError::new_with_message(
@@ -247,9 +257,74 @@ fn add_proposal_id_to_add_wasm_request(
         ..add_wasm_request
     };
 
-    let payload = Encode!(&add_wasm_request).unwrap();
+    Encode!(&add_wasm_request).map_err(|_| {
+        GovernanceError::new_with_message(
+            ErrorType::InvalidProposal,
+            "Unable to encode AddWasmRequest proposal: {e}",
+        )
+    })
+}
 
-    Ok(payload)
+impl From<CallCanisterRequest> for SelfDescribingValue {
+    fn from(request: CallCanisterRequest) -> Self {
+        let CallCanisterRequest {
+            canister_id,
+            method_name,
+            payload,
+        } = request;
+        ValueBuilder::new()
+            .add_field("canister_id", canister_id)
+            .add_field("method_name", method_name)
+            .add_field("payload", payload)
+            .build()
+    }
+}
+
+impl From<SubnetRentalRequest> for SelfDescribingValue {
+    fn from(request: SubnetRentalRequest) -> Self {
+        let SubnetRentalRequest {
+            user,
+            rental_condition_id,
+        } = request;
+        ValueBuilder::new()
+            .add_field("user", user)
+            .add_field("rental_condition_id", format!("{:?}", rental_condition_id))
+            .build()
+    }
+}
+
+impl From<AddWasmRequest> for SelfDescribingValue {
+    fn from(payload: AddWasmRequest) -> Self {
+        let AddWasmRequest {
+            wasm,
+            hash,
+            skip_update_latest_version,
+        } = payload;
+
+        ValueBuilder::new()
+            .add_field("wasm", wasm)
+            .add_field("hash", hash)
+            .add_field("skip_update_latest_version", skip_update_latest_version)
+            .build()
+    }
+}
+
+impl From<SnsWasm> for SelfDescribingValue {
+    fn from(wasm: SnsWasm) -> Self {
+        let SnsWasm {
+            wasm,
+            canister_type,
+            proposal_id: _,
+        } = wasm;
+
+        let wasm_hash = Sha256::hash(&wasm).to_vec();
+        let canister_type = SelfDescribingProstEnum::<SnsCanisterType>::new(canister_type);
+
+        ValueBuilder::new()
+            .add_field("wasm_hash", wasm_hash)
+            .add_field("canister_type", canister_type)
+            .build()
+    }
 }
 
 #[derive(Debug, Clone, PartialEq)]

rs/nns/governance/src/proposals/execute_nns_function_tests.rs

@@ -7,14 +7,16 @@ use crate::{
     test_utils::{ExpectedCallCanisterMethodCallArguments, MockEnvironment},
 };
 use candid::{Decode, Encode};
-use ic_base_types::CanisterId;
+use ic_base_types::{CanisterId, PrincipalId};
+use ic_crypto_sha2::Sha256;
 use ic_management_canister_types_private::{CanisterMetadataRequest, CanisterMetadataResponse};
 use ic_nns_constants::{BITCOIN_MAINNET_CANISTER_ID, CYCLES_MINTING_CANISTER_ID};
 use ic_nns_governance_api::{
     SelfDescribingValue,
     bitcoin::{BitcoinNetwork, BitcoinSetConfigProposal},
+    subnet_rental::{RentalConditionId, SubnetRentalRequest},
 };
-use ic_sns_wasm::pb::v1::{AddWasmRequest, SnsWasm};
+use ic_sns_wasm::pb::v1::{AddWasmRequest, SnsCanisterType, SnsWasm};
 use maplit::hashmap;
 use std::sync::Arc;
 
@@ -271,6 +273,84 @@ async fn test_to_self_describing_bitcoin_set_config() {
     );
 }
 
+#[tokio::test]
+async fn test_to_self_describing_subnet_rental_request() {
+    let user = PrincipalId::new_user_test_id(123);
+    let rental_condition_id = RentalConditionId::App13CH;
+
+    let arg = SubnetRentalRequest {
+        user,
+        rental_condition_id,
+    };
+    let payload = Encode!(&arg).unwrap();
+
+    let execute_nns_function = ValidExecuteNnsFunction {
+        nns_function: ValidNnsFunction::SubnetRentalRequest,
+        payload,
+    };
+
+    // No canister_metadata call expected - SubnetRentalRequest uses static conversion.
+    let env = Arc::new(MockEnvironment::new(vec![], 0));
+
+    let proposal_action = ValidProposalAction::ExecuteNnsFunction(execute_nns_function);
+    let result = proposal_action.to_self_describing(env).await.unwrap();
+
+    assert_eq!(result.type_name, "Subnet Rental Request");
+    assert!(
+        result
+            .type_description
+            .contains("A proposal to rent a subnet on the Internet Computer")
+    );
+    assert_eq!(
+        SelfDescribingValue::from(result.value.unwrap()),
+        SelfDescribingValue::Map(hashmap! {
+            "user".to_string() => SelfDescribingValue::Text(user.to_string()),
+            "rental_condition_id".to_string() => SelfDescribingValue::Text("App13CH".to_string()),
+        })
+    );
+}
+
+#[tokio::test]
+async fn test_to_self_describing_add_sns_wasm() {
+    let wasm_bytes = vec![0, 0x61, 0x73, 0x6D, 1, 0, 0, 0];
+    let canister_type = SnsCanisterType::Root as i32;
+    let hash = vec![1, 2, 3, 4];
+
+    let arg = AddWasmRequest {
+        wasm: Some(SnsWasm {
+            wasm: wasm_bytes.clone(),
+            canister_type,
+            proposal_id: None, // Will be set by the NNS function
+        }),
+        hash: hash.clone(),
+        skip_update_latest_version: Some(false),
+    };
+    let payload = Encode!(&arg).unwrap();
+
+    let execute_nns_function = ValidExecuteNnsFunction {
+        nns_function: ValidNnsFunction::AddSnsWasm,
+        payload,
+    };
+
+    // No canister_metadata call expected - AddSnsWasm uses static conversion.
+    let env = Arc::new(MockEnvironment::new(vec![], 0));
+
+    let proposal_action = ValidProposalAction::ExecuteNnsFunction(execute_nns_function);
+    let result = proposal_action.to_self_describing(env).await.unwrap();
+
+    assert_eq!(
+        SelfDescribingValue::from(result.value.unwrap()),
+        SelfDescribingValue::Map(hashmap! {
+            "wasm".to_string() => SelfDescribingValue::Map(hashmap! {
+                "wasm_hash".to_string() => SelfDescribingValue::from(Sha256::hash(&wasm_bytes).to_vec()),
+                "canister_type".to_string() => SelfDescribingValue::from("Root"),
+            }),
+            "hash".to_string() => SelfDescribingValue::from(hash),
+            "skip_update_latest_version".to_string() => SelfDescribingValue::from(false),
+        })
+    );
+}
+
 #[test]
 fn test_re_encode_payload_to_target_canister_sets_proposal_id_for_add_wasm() {
     let proposal_id = 42;