nervous-system-agent: extract 'is_canister_stopped_error' from 'CallCanisters'

'is_canister_stopped' is currently used by PocketIc and ic-agent
implementations. However, the 'CallCanisters' trait requires
implementing this function. As a result, some trait implementations have
dummy implementation for this method which panics in runtime.

To avoid runtime panic, this method is extracted into a dedicated
'CallCanistersWithStoppedCanisterError' trait that inherits
'CallCanisters'. This trait is only implemented for PocketIc and ic-agent's.
An attempt to use 'is_canister_stopped_error' for other implementation
will cause compilation error instead of runtime panic.

Author: rvem

rs/nervous_system/agent/src/agent_impl.rs

@@ -1,4 +1,4 @@
-use crate::{CallCanisters, ProgressNetwork};
+use crate::{CallCanisters, CallCanistersWithStoppedCanisterError, ProgressNetwork};
 use crate::{CanisterInfo, Request};
 use candid::Principal;
 use ic_agent::agent::{RejectCode, RejectResponse};
@@ -141,7 +141,9 @@ impl CallCanisters for Agent {
     fn caller(&self) -> Result<Principal, Self::Error> {
         self.get_principal().map_err(Self::Error::Identity)
     }
+}
 
+impl CallCanistersWithStoppedCanisterError for Agent {
     fn is_canister_stopped_error(&self, err: &Self::Error) -> bool {
         match err {
             AgentCallError::Agent(AgentError::CertifiedReject(RejectResponse {

rs/nervous_system/agent/src/helpers/nns.rs

@@ -14,11 +14,11 @@ use icp_ledger::{AccountIdentifier, Subaccount, Tokens, TransferArgs};
 use crate::nns::governance::{get_proposal_info, manage_neuron};
 use crate::nns::sns_wasm::get_deployed_sns_by_proposal_id;
 use crate::sns::Sns;
-use crate::{CallCanisters, ProgressNetwork};
+use crate::{CallCanisters, CallCanistersWithStoppedCanisterError, ProgressNetwork};
 
 // TODO @rvem: we probably need more meaningful error type rather than just 'String'
 
-pub async fn propose_and_wait<C: CallCanisters + ProgressNetwork>(
+pub async fn propose_and_wait<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     agent: &C,
     neuron_id: NeuronId,
     proposal: MakeProposalRequest,
@@ -40,7 +40,7 @@ pub async fn propose_and_wait<C: CallCanisters + ProgressNetwork>(
     }
 }
 
-pub async fn propose_to_deploy_sns_and_wait<C: CallCanisters + ProgressNetwork>(
+pub async fn propose_to_deploy_sns_and_wait<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     agent: &C,
     neuron_id: NeuronId,
     create_service_nervous_system: CreateServiceNervousSystem,
@@ -73,7 +73,7 @@ pub async fn propose_to_deploy_sns_and_wait<C: CallCanisters + ProgressNetwork>(
     Ok((sns, nns_proposal_id))
 }
 
-pub async fn wait_for_proposal_execution<C: CallCanisters + ProgressNetwork>(
+pub async fn wait_for_proposal_execution<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     agent: &C,
     proposal_id: ProposalId,
 ) -> Result<ProposalInfo, String> {

rs/nervous_system/agent/src/helpers/sns.rs

@@ -5,7 +5,7 @@ use thiserror::Error;
 use crate::nns::ledger::icrc1_transfer;
 use crate::sns::governance::{GovernanceCanister, ProposalSubmissionError, SubmittedProposal};
 use crate::sns::swap::SwapCanister;
-use crate::{CallCanisters, ProgressNetwork};
+use crate::{CallCanisters, CallCanistersWithStoppedCanisterError, ProgressNetwork};
 use candid::Nat;
 use ic_sns_governance_api::pb::v1::{
     get_proposal_response, ListNeurons, NeuronId, Proposal, ProposalData, ProposalId,
@@ -26,7 +26,7 @@ pub enum SnsProposalError {
     ProposalError(ProposalId, String),
 }
 
-pub async fn propose_and_wait<C: CallCanisters + ProgressNetwork>(
+pub async fn propose_and_wait<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     agent: &C,
     neuron_id: NeuronId,
     sns_governance_canister: GovernanceCanister,
@@ -42,7 +42,7 @@ pub async fn propose_and_wait<C: CallCanisters + ProgressNetwork>(
     wait_for_proposal_execution(agent, sns_governance_canister, proposal_id).await
 }
 
-pub async fn wait_for_proposal_execution<C: CallCanisters + ProgressNetwork>(
+pub async fn wait_for_proposal_execution<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     agent: &C,
     sns_governance_canister: GovernanceCanister,
     proposal_id: ProposalId,

rs/nervous_system/agent/src/lib.rs

@@ -54,12 +54,16 @@ pub trait CallCanisters: sealed::Sealed {
         &self,
         canister_id: impl Into<Principal> + Send,
     ) -> impl Future<Output = Result<CanisterInfo, Self::Error>> + Send;
+}
 
-    // Functions that use 'call' need to be able
-    // to determine if a call to the canister failed due to the canister being stopped.
-    // Matching on a specific error outside of the trait implementation is not viable
-    // since the 'Error' type is different for each trait implementation and thus we can
-    // only match on specific implementations errors in the trait implementation directly.
+// Functions that use 'call' need to be able
+// to determine if a call to the canister failed due to the canister being stopped.
+// Matching on a specific error outside of the trait implementation is not viable
+// since the 'Error' type is different for each trait implementation and thus we can
+// only match on specific implementations errors in the trait implementation directly.
+//
+// We're extending CallCanisters trait to allow this.
+pub trait CallCanistersWithStoppedCanisterError: CallCanisters {
     fn is_canister_stopped_error(&self, err: &Self::Error) -> bool;
 }
 

rs/nervous_system/agent/src/mock.rs

@@ -96,11 +96,6 @@ impl CallCanisters for MockCallCanisters {
         #[allow(unreachable_code)]
         std::future::ready(Err(MockCallCanistersError("UNREACHABLE".to_string())))
     }
-
-    /// This could be implemented later, but for now, it's not needed.
-    fn is_canister_stopped_error(&self, _err: &Self::Error) -> bool {
-        unimplemented!();
-    }
 }
 
 #[derive(Debug, PartialEq, Eq)]

rs/nervous_system/agent/src/pocketic_impl.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 use crate::management_canister::requests::{
     CanisterStatusArgs, DeleteCanisterArgs, StopCanisterArgs, StoredChunksArgs, UploadChunkArgs,
 };
-use crate::Request;
+use crate::{CallCanistersWithStoppedCanisterError, Request};
 use crate::{CallCanisters, CanisterInfo, ProgressNetwork};
 use candid::Principal;
 use ic_management_canister_types::{CanisterStatusResult, DefiniteCanisterSettings};
@@ -181,7 +181,9 @@ impl CallCanisters for PocketIcAgent<'_> {
     fn caller(&self) -> Result<Principal, Self::Error> {
         Ok(self.sender)
     }
+}
 
+impl CallCanistersWithStoppedCanisterError for PocketIcAgent<'_> {
     fn is_canister_stopped_error(&self, err: &Self::Error) -> bool {
         self.pocket_ic.is_canister_stopped_error(err)
     }
@@ -211,7 +213,9 @@ impl CallCanisters for PocketIc {
     fn caller(&self) -> Result<Principal, Self::Error> {
         Ok(Principal::anonymous())
     }
+}
 
+impl CallCanistersWithStoppedCanisterError for PocketIc {
     fn is_canister_stopped_error(&self, err: &Self::Error) -> bool {
         match err {
             PocketIcCallError::PocketIc(err) => {

rs/nervous_system/agent/src/state_machine_impl.rs

@@ -1,7 +1,6 @@
 use crate::{CallCanisters, CanisterInfo, Request};
 use candid::Principal;
 use ic_base_types::{CanisterId, PrincipalId};
-use ic_error_types::ErrorCode;
 use ic_state_machine_tests::{StateMachine, UserError, WasmResult};
 use std::time::Duration;
 use thiserror::Error;
@@ -99,10 +98,6 @@ impl CallCanisters for StateMachineAgent<'_> {
                 .collect(),
         })
     }
-
-    fn is_canister_stopped_error(&self, err: &Self::Error) -> bool {
-        self.state_machine.is_canister_stopped_error(err)
-    }
 }
 
 impl CallCanisters for StateMachine {
@@ -130,13 +125,4 @@ impl CallCanisters for StateMachine {
             .canister_info(canister_id)
             .await
     }
-
-    fn is_canister_stopped_error(&self, err: &Self::Error) -> bool {
-        match err {
-            StateMachineCallError::IngressStateError(err) => {
-                [ErrorCode::CanisterStopped, ErrorCode::CanisterStopping].contains(&err.code())
-            }
-            _ => false,
-        }
-    }
 }

rs/sns/testing/src/sns.rs

@@ -12,7 +12,7 @@ use ic_nervous_system_agent::{
     },
     nns::{ledger::transfer, sns_wasm::list_deployed_snses},
     sns::{governance::GovernanceCanister, swap::SwapCanister, Sns},
-    CallCanisters, ProgressNetwork,
+    CallCanisters, CallCanistersWithStoppedCanisterError, ProgressNetwork,
 };
 use ic_nervous_system_clients::canister_status::CanisterStatusType;
 use ic_nervous_system_integration_tests::{
@@ -48,17 +48,12 @@ pub const DEFAULT_SWAP_PARTICIPANTS_NUMBER: usize = 20;
 // Creates SNS using agents provided as arguments:
 // 1) neuron_agent - agent that controlls 'neuron_id'.
 // 2) neuron_id - ID of the neuron that has a sufficient amount of stake to propose the SNS creation and adopt the proposal.
-// 2) dev_participant_agent - Agent that will be used as an initial neuron in a newly created SNS. All other
+// 3) dev_participant_agent - Agent that will be used as an initial neuron in a newly created SNS. All other
 //    neurons will follow the dev neuron.
-// 3) swap_treasury_agent - Agent for the identity that has sufficient amout of ICP tokens to close the swap and
-//    pay for cycles needed canister upgrade as well as all associated costs.
-// 4) swap_participants_agents - Agents for the identities that will participate in the swap. The actual number of participants
-//    is determined by the swap parameters. So only some of these agents might be used. Each agent participates in the swap,
-//    follows the dev neuron for 'UpgradeSnsControlledCanister' proposals and increases its dissolve delay to be able to vote.
-// 5) dapp_canister_ids - Canister IDs of the DApps that will be added to the SNS.
+// 4) dapp_canister_ids - Canister IDs of the DApps that will be added to the SNS.
 //
 // Returns SNS canisters IDs.
-pub async fn create_sns<C: CallCanisters + ProgressNetwork + BuildEphemeralAgent>(
+pub async fn create_sns<C: CallCanistersWithStoppedCanisterError + ProgressNetwork + BuildEphemeralAgent>(
     neuron_agent: &C,
     neuron_id: NeuronId,
     dev_participant_agent: &C,
@@ -367,7 +362,7 @@ pub async fn complete_sns_swap<C: CallCanisters + ProgressNetwork + BuildEphemer
 // 2) sns - SNS canisters.
 // 3) canister_id - ID of the canister that will be upgraded.
 // 4) upgrade_arg - Arguments that will be passed to the canister during the upgrade.
-pub async fn upgrade_sns_controlled_test_canister<C: CallCanisters + ProgressNetwork>(
+pub async fn upgrade_sns_controlled_test_canister<C: CallCanistersWithStoppedCanisterError + ProgressNetwork>(
     dev_participant_agent: &C,
     sns: Sns,
     canister_id: CanisterId,