diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f34d25..59bf1367 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +* Changed `AgentError::ReplicaError` to `CertifiedReject` or `UncertifiedReject`. `CertifiedReject`s went through consensus, and `UncertifiedReject`s did not. If your code uses `ReplicaError`: + * for queries: use `UncertifiedReject` in all cases (for now) + * for updates: use `CertifiedReject` for errors raised after the message successfully reaches the canister, and `UncertifiedReject` otherwise * Added `Agent::fetch_api_boundary_nodes` for looking up API boundary nodes in the state tree. * Timestamps are now being checked in `Agent::verify` and `Agent::verify_for_subnet`. If you were using it with old certificates, increase the expiry timeout to continue to verify them. * Added node metrics, ECDSA, and Bitcoin functions to `MgmtMethod`. Most do not have wrappers in `ManagementCanister` because only canisters can call these functions. diff --git a/ic-agent/src/agent/agent_error.rs b/ic-agent/src/agent/agent_error.rs index 70477f05..08cf77a1 100644 --- a/ic-agent/src/agent/agent_error.rs +++ b/ic-agent/src/agent/agent_error.rs @@ -50,9 +50,13 @@ pub enum AgentError { #[error("Cannot parse Principal: {0}")] PrincipalError(#[from] crate::export::PrincipalError), - /// The replica rejected the message. - #[error("The replica returned a replica error: reject code {:?}, reject message {}, error code {:?}", .0.reject_code, .0.reject_message, .0.error_code)] - ReplicaError(RejectResponse), + /// The subnet rejected the message. + #[error("The replica returned a rejection error: reject code {:?}, reject message {}, error code {:?}", .0.reject_code, .0.reject_message, .0.error_code)] + CertifiedReject(RejectResponse), + + /// The replica rejected the message. This rejection cannot be verified as authentic. + #[error("The replica returned a rejection error: reject code {:?}, reject message {}, error code {:?}", .0.reject_code, .0.reject_message, .0.error_code)] + UncertifiedReject(RejectResponse), /// The replica returned an HTTP error. #[error("The replica returned an HTTP Error: {0}")] diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs index d6939d37..bf5715ca 100644 --- a/ic-agent/src/agent/agent_test.rs +++ b/ic-agent/src/agent/agent_test.rs @@ -141,7 +141,7 @@ async fn query_rejected() -> Result<(), AgentError> { assert_mock(query_mock).await; match result { - Err(AgentError::ReplicaError(replica_error)) => { + Err(AgentError::UncertifiedReject(replica_error)) => { assert_eq!(replica_error.reject_code, RejectCode::DestinationInvalid); assert_eq!(replica_error.reject_message, "Rejected Message"); assert_eq!(replica_error.error_code, Some("Error code".to_string())); @@ -202,7 +202,7 @@ async fn call_rejected() -> Result<(), AgentError> { assert_mock(call_mock).await; - let expected_response = Err(AgentError::ReplicaError(reject_body)); + let expected_response = Err(AgentError::UncertifiedReject(reject_body)); assert_eq!(expected_response, result); Ok(()) @@ -238,7 +238,7 @@ async fn call_rejected_without_error_code() -> Result<(), AgentError> { assert_mock(call_mock).await; - let expected_response = Err(AgentError::ReplicaError(reject_body)); + let expected_response = Err(AgentError::UncertifiedReject(reject_body)); assert_eq!(expected_response, result); Ok(()) diff --git a/ic-agent/src/agent/http_transport/reqwest_transport.rs b/ic-agent/src/agent/http_transport/reqwest_transport.rs index 194c9123..34802022 100644 --- a/ic-agent/src/agent/http_transport/reqwest_transport.rs +++ b/ic-agent/src/agent/http_transport/reqwest_transport.rs @@ -146,7 +146,7 @@ impl ReqwestTransport { serde_cbor::from_slice(&body); let agent_error = match cbor_decoded_body { - Ok(replica_error) => AgentError::ReplicaError(replica_error), + Ok(replica_error) => AgentError::UncertifiedReject(replica_error), Err(cbor_error) => AgentError::InvalidCborData(cbor_error), }; diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs index 3c859f04..28eaa429 100644 --- a/ic-agent/src/agent/mod.rs +++ b/ic-agent/src/agent/mod.rs @@ -550,7 +550,7 @@ impl Agent { match response { QueryResponse::Replied { reply, .. } => Ok(reply.arg), - QueryResponse::Rejected { reject, .. } => Err(AgentError::ReplicaError(reject)), + QueryResponse::Rejected { reject, .. } => Err(AgentError::UncertifiedReject(reject)), } } @@ -650,7 +650,7 @@ impl Agent { Ok(PollResult::Completed(arg)) } - RequestStatusResponse::Rejected(response) => Err(AgentError::ReplicaError(response)), + RequestStatusResponse::Rejected(response) => Err(AgentError::CertifiedReject(response)), RequestStatusResponse::Done => Err(AgentError::RequestStatusDoneNoReply(String::from( *request_id, @@ -698,7 +698,7 @@ impl Agent { RequestStatusResponse::Replied(ReplyResponse { arg, .. }) => return Ok(arg), RequestStatusResponse::Rejected(response) => { - return Err(AgentError::ReplicaError(response)) + return Err(AgentError::CertifiedReject(response)) } RequestStatusResponse::Done => { diff --git a/ic-utils/src/interfaces/wallet.rs b/ic-utils/src/interfaces/wallet.rs index 4bd327c1..ef4d7b9f 100644 --- a/ic-utils/src/interfaces/wallet.rs +++ b/ic-utils/src/interfaces/wallet.rs @@ -436,7 +436,7 @@ impl<'agent> WalletCanister<'agent> { let version: Result<(String,), _> = canister.query("wallet_api_version").build().call().await; let version = match version { - Err(AgentError::ReplicaError(replica_error)) + Err(AgentError::UncertifiedReject(replica_error)) if replica_error.reject_code == RejectCode::DestinationInvalid && (replica_error .reject_message diff --git a/ref-tests/tests/ic-ref.rs b/ref-tests/tests/ic-ref.rs index 45c9730b..79a9d08f 100644 --- a/ref-tests/tests/ic-ref.rs +++ b/ref-tests/tests/ic-ref.rs @@ -96,7 +96,7 @@ mod management_canister { .await; assert!(matches!(result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(ref error_code) @@ -136,7 +136,7 @@ mod management_canister { .call_and_wait() .await; - assert!(matches!(result, Err(AgentError::ReplicaError { .. }))); + assert!(matches!(result, Err(AgentError::CertifiedReject { .. }))); // Reinstall should succeed. ic00.install_code(&canister_id, &canister_wasm) @@ -157,7 +157,7 @@ mod management_canister { .with_mode(InstallMode::Reinstall) .call_and_wait() .await; - assert!(matches!(result, Err(AgentError::ReplicaError(..)))); + assert!(matches!(result, Err(AgentError::UncertifiedReject(..)))); // Upgrade should succeed. ic00.install_code(&canister_id, &canister_wasm) @@ -175,7 +175,7 @@ mod management_canister { }) .call_and_wait() .await; - assert!(matches!(result, Err(AgentError::ReplicaError(..)))); + assert!(matches!(result, Err(AgentError::UncertifiedReject(..)))); // Change controller. ic00.update_settings(&canister_id) @@ -190,7 +190,7 @@ mod management_canister { .call_and_wait() .await; assert!( - matches!(result, Err(AgentError::ReplicaError(RejectResponse{ + matches!(result, Err(AgentError::UncertifiedReject(RejectResponse{ reject_code: RejectCode::CanisterError, reject_message, error_code: Some(ref error_code), @@ -404,7 +404,7 @@ mod management_canister { ) { for expected_rc in &allowed_reject_codes { if matches!(result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code, .. })) if reject_code == *expected_rc) @@ -415,7 +415,7 @@ mod management_canister { assert!( matches!(result, Err(AgentError::HttpError(_))), - "expect an HttpError, or a ReplicaError with reject_code in {:?}", + "expect an HttpError, or a CertifiedReject with reject_code in {:?}", allowed_reject_codes ); } @@ -458,7 +458,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -473,7 +473,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -503,7 +503,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::CertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: None, @@ -517,7 +517,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(error_code), @@ -541,7 +541,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(error_code), @@ -556,7 +556,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(error_code), @@ -570,7 +570,7 @@ mod management_canister { let result = ic00.canister_status(&canister_id).call_and_wait().await; assert!( match &result { - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(error_code), @@ -590,7 +590,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse{ + Err(AgentError::UncertifiedReject(RejectResponse{ reject_code: RejectCode::DestinationInvalid, reject_message, error_code: Some(error_code), @@ -636,7 +636,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -651,7 +651,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -669,7 +669,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -687,7 +687,7 @@ mod management_canister { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::CanisterError, reject_message, error_code: Some(error_code), @@ -959,7 +959,7 @@ mod simple_calls { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::CertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, .. })), @@ -984,7 +984,7 @@ mod simple_calls { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::UncertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, .. })) @@ -1218,7 +1218,7 @@ mod extras { assert!( matches!( &result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::CertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: None, diff --git a/ref-tests/tests/integration.rs b/ref-tests/tests/integration.rs index a4fa7982..0f4085b2 100644 --- a/ref-tests/tests/integration.rs +++ b/ref-tests/tests/integration.rs @@ -187,7 +187,7 @@ fn canister_reject_call() { assert!(matches!( result, - Err(AgentError::ReplicaError(RejectResponse { + Err(AgentError::CertifiedReject(RejectResponse { reject_code: RejectCode::DestinationInvalid, reject_message, error_code: None,