feat(ic-nervous-system-agent): Add submit_proposal helper (#3451)

Submitting proposals is a common pattern within the nervous system
integration tests, and we will soon be doing it even more within the SNS
CLI. So it makes sense to have a helper function for it in
`ic-nervous-system-agent`.

There is one nuance to this. `ic_agent::Agent` stores the identity of
the sender within it. But pocketic functions take the sender as an
argument. Previously, in `ic-nervous-system-agent`, we just assumed the
sender would always be the anonymous principal, but obviously this won't
work for submitting proposals. So this PR also introduces a way to
specify the sender for pocketic through the new `PocketIcAgent` type.

rs/nervous_system/agent/src/pocketic_impl.rs

@@ -5,6 +5,20 @@ use thiserror::Error;
 
 use crate::CallCanisters;
 
+/// A wrapper around PocketIc that specifies a sender for the requests.
+/// The name is an analogy for `ic_agent::Agent`, since each `ic_agent::Agent` specifies a sender.
+pub struct PocketIcAgent<'a> {
+    pub pocket_ic: &'a PocketIc,
+    pub sender: Principal,
+}
+
+impl<'a> PocketIcAgent<'a> {
+    pub fn new(pocket_ic: &'a PocketIc, sender: impl Into<Principal>) -> Self {
+        let sender = sender.into();
+        Self { pocket_ic, sender }
+    }
+}
+
 #[derive(Error, Debug)]
 pub enum PocketIcCallError {
     #[error("pocket_ic error: {0}")]
@@ -16,8 +30,9 @@ pub enum PocketIcCallError {
 }
 
 impl crate::sealed::Sealed for PocketIc {}
+impl crate::sealed::Sealed for PocketIcAgent<'_> {}
 
-impl CallCanisters for PocketIc {
+impl CallCanisters for PocketIcAgent<'_> {
     type Error = PocketIcCallError;
     async fn call<R: Request>(
         &self,
@@ -27,24 +42,29 @@ impl CallCanisters for PocketIc {
         let canister_id = canister_id.into();
         let request_bytes = request.payload().map_err(PocketIcCallError::CandidEncode)?;
         let response = if request.update() {
-            self.update_call(
-                canister_id,
-                Principal::anonymous(),
-                request.method(),
-                request_bytes,
-            )
-            .await
+            self.pocket_ic
+                .update_call(canister_id, self.sender, request.method(), request_bytes)
+                .await
         } else {
-            self.query_call(
-                canister_id,
-                Principal::anonymous(),
-                request.method(),
-                request_bytes,
-            )
-            .await
+            self.pocket_ic
+                .query_call(canister_id, self.sender, request.method(), request_bytes)
+                .await
         }
         .map_err(PocketIcCallError::PocketIc)?;
 
         candid::decode_one(response.as_slice()).map_err(PocketIcCallError::CandidDecode)
     }
 }
+
+impl CallCanisters for PocketIc {
+    type Error = PocketIcCallError;
+    async fn call<R: Request>(
+        &self,
+        canister_id: impl Into<Principal> + Send,
+        request: R,
+    ) -> Result<R::Response, Self::Error> {
+        PocketIcAgent::new(self, Principal::anonymous())
+            .call(canister_id, request)
+            .await
+    }
+}

rs/nervous_system/agent/src/sns/governance.rs

@@ -1,16 +1,28 @@
 use crate::{null_request::NullRequest, CallCanisters};
 use ic_base_types::PrincipalId;
 use ic_sns_governance::pb::v1::{
-    GetMetadataRequest, GetMetadataResponse, GetMode, GetModeResponse, GetRunningSnsVersionRequest,
-    GetRunningSnsVersionResponse, NervousSystemParameters,
+    manage_neuron, manage_neuron_response, GetMetadataRequest, GetMetadataResponse, GetMode,
+    GetModeResponse, GetRunningSnsVersionRequest, GetRunningSnsVersionResponse, GovernanceError,
+    ManageNeuron, ManageNeuronResponse, NervousSystemParameters, NeuronId, Proposal, ProposalId,
 };
 use serde::{Deserialize, Serialize};
+use std::error::Error;
 
 #[derive(Copy, Clone, Debug, Deserialize, Serialize)]
 pub struct GovernanceCanister {
     pub canister_id: PrincipalId,
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum SubmitProposalError<C: Error> {
+    #[error("Failed to call SNS Governance")]
+    CallGovernanceError(#[source] C),
+    #[error("SNS Governance returned an error")]
+    GovernanceError(#[source] GovernanceError),
+    #[error("SNS Governance did not confirm that the proposal was made: {0:?}")]
+    ProposalNotMade(ManageNeuronResponse),
+}
+
 impl GovernanceCanister {
     pub async fn metadata<C: CallCanisters>(
         &self,
@@ -39,6 +51,51 @@ impl GovernanceCanister {
         let request = NullRequest::new("get_nervous_system_parameters", false);
         agent.call(self.canister_id, request).await
     }
+
+    pub async fn manage_neuron<C: CallCanisters>(
+        &self,
+        agent: &C,
+        neuron_id: NeuronId,
+        command: manage_neuron::Command,
+    ) -> Result<ManageNeuronResponse, C::Error> {
+        let subaccount = neuron_id
+            .subaccount()
+            .expect("Valid SNS neuron IDs should be ICRC1 sub-accounts.")
+            .to_vec();
+        let request = ManageNeuron {
+            subaccount,
+            command: Some(command),
+        };
+        agent.call(self.canister_id, request).await
+    }
+
+    pub async fn submit_proposal<C: CallCanisters>(
+        &self,
+        agent: &C,
+        neuron_id: NeuronId,
+        proposal: Proposal,
+    ) -> Result<ProposalId, SubmitProposalError<C::Error>> {
+        let response = self
+            .manage_neuron(
+                agent,
+                neuron_id,
+                manage_neuron::Command::MakeProposal(proposal),
+            )
+            .await
+            .map_err(SubmitProposalError::CallGovernanceError)?;
+
+        match response.command {
+            Some(manage_neuron_response::Command::MakeProposal(
+                manage_neuron_response::MakeProposalResponse {
+                    proposal_id: Some(proposal_id),
+                },
+            )) => Ok(proposal_id),
+            Some(manage_neuron_response::Command::Error(e)) => {
+                Err(SubmitProposalError::GovernanceError(e))
+            }
+            _ => Err(SubmitProposalError::ProposalNotMade(response)),
+        }
+    }
 }
 
 impl GovernanceCanister {

rs/nervous_system/integration_tests/src/lib.rs

@@ -35,10 +35,14 @@ impl SectionTimer {
 
 impl Drop for SectionTimer {
     fn drop(&mut self) {
-        eprintln!(
-            "Executed `{}` in {:?}",
-            self.name,
-            self.start_time.elapsed()
-        );
+        if std::thread::panicking() {
+            eprintln!("Panicked during `{}`", self.name);
+        } else {
+            eprintln!(
+                "Executed `{}` in {:?}",
+                self.name,
+                self.start_time.elapsed()
+            );
+        }
     }
 }

rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs

@@ -4,7 +4,7 @@ use futures::stream;
 use futures::StreamExt;
 use ic_base_types::{CanisterId, PrincipalId, SubnetId};
 use ic_ledger_core::Tokens;
-use ic_nervous_system_agent::pocketic_impl::PocketIcCallError;
+use ic_nervous_system_agent::pocketic_impl::{PocketIcAgent, PocketIcCallError};
 use ic_nervous_system_agent::sns::Sns;
 use ic_nervous_system_agent::CallCanisters;
 use ic_nervous_system_common::{E8, ONE_DAY_SECONDS};
@@ -1365,7 +1365,7 @@ pub mod sns {
         use super::*;
         use assert_matches::assert_matches;
         use ic_crypto_sha2::Sha256;
-        use ic_nervous_system_agent::sns::governance::GovernanceCanister;
+        use ic_nervous_system_agent::sns::governance::{GovernanceCanister, SubmitProposalError};
         use ic_sns_governance::governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS;
         use ic_sns_governance::pb::v1::get_neuron_response;
         use pocket_ic::ErrorCode;
@@ -1425,26 +1425,16 @@ pub mod sns {
             neuron_id: sns_pb::NeuronId,
             proposal: sns_pb::Proposal,
         ) -> Result<sns_pb::ProposalData, sns_pb::GovernanceError> {
-            let response = manage_neuron(
-                pocket_ic,
-                canister_id,
-                sender,
-                neuron_id,
-                sns_pb::manage_neuron::Command::MakeProposal(proposal),
-            )
-            .await;
-            use sns_pb::manage_neuron_response::Command;
-            let response = match response.command {
-                Some(Command::MakeProposal(response)) => Ok(response),
-                Some(Command::Error(err)) => Err(err),
-                _ => panic!("Proposal failed unexpectedly: {:#?}", response),
-            }?;
-            let proposal_id = response.proposal_id.unwrap_or_else(|| {
-                panic!(
-                    "First SNS proposal response did not contain a proposal_id: {:#?}",
-                    response
-                )
-            });
+            let agent = PocketIcAgent::new(pocket_ic, sender);
+            let governance = GovernanceCanister::new(canister_id);
+            let proposal_id = governance
+                .submit_proposal(&agent, neuron_id, proposal)
+                .await
+                .map_err(|err| match err {
+                    SubmitProposalError::GovernanceError(e) => e,
+                    e => panic!("Unexpected error: {e}"),
+                })?;
+
             wait_for_proposal_execution(pocket_ic, canister_id, proposal_id).await
         }
 

rs/sns/governance/src/types.rs

@@ -969,6 +969,8 @@ impl fmt::Display for GovernanceError {
     }
 }
 
+impl std::error::Error for crate::pb::v1::GovernanceError {}
+
 impl From<NervousSystemError> for GovernanceError {
     fn from(nervous_system_error: NervousSystemError) -> Self {
         GovernanceError {