Skip to content

Commit

Permalink
Return certificates from low-level update and request_status functions
Browse files Browse the repository at this point in the history
  • Loading branch information
adamspofford-dfinity committed Oct 17, 2024
1 parent a4d7d8d commit f1d8cd0
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 33 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

* The lower-level update call functions now return the certificate in addition to the parsed response data.
* Make ingress_expiry required and set the default value to 3 min.
* Changed `BasicIdentity`'s implmentation from `ring` to `ed25519-consensus`.
* Changed `BasicIdentity`'s implementation from `ring` to `ed25519-consensus`.
* Added `AgentBuilder::with_max_polling_time` to config the maximum time to wait for a response from the replica.
* `DelegatedIdentity::new` now checks the delegation chain. The old behavior is available under `new_unchecked`.

Expand Down
65 changes: 38 additions & 27 deletions ic-agent/src/agent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ type AgentFuture<'a, V> = Pin<Box<dyn Future<Output = Result<V, AgentError>> + '
/// ```
///
/// This agent does not understand Candid, and only acts on byte buffers.
///
/// Some methods return certificates. While there is a `verify_certificate` method, any certificate
/// you receive from a method has already been verified and you do not need to manually verify it.
#[derive(Clone)]
pub struct Agent {
nonce_factory: Arc<dyn NonceGenerator>,
Expand Down Expand Up @@ -517,7 +520,7 @@ impl Agent {
method_name: String,
arg: Vec<u8>,
ingress_expiry_datetime: Option<u64>,
) -> Result<CallResponse<Vec<u8>>, AgentError> {
) -> Result<CallResponse<(Vec<u8>, Certificate)>, AgentError> {
let nonce = self.nonce_factory.generate();
let content = self.update_content(
canister_id,
Expand All @@ -539,10 +542,12 @@ impl Agent {
serde_cbor::from_slice(&certificate).map_err(AgentError::InvalidCborData)?;

self.verify(&certificate, effective_canister_id)?;
let status = lookup_request_status(certificate, &request_id)?;
let status = lookup_request_status(&certificate, &request_id)?;

match status {
RequestStatusResponse::Replied(reply) => Ok(CallResponse::Response(reply.arg)),
RequestStatusResponse::Replied(reply) => {
Ok(CallResponse::Response((reply.arg, certificate)))
}
RequestStatusResponse::Rejected(reject_response) => {
Err(AgentError::CertifiedReject(reject_response))?
}
Expand Down Expand Up @@ -578,7 +583,7 @@ impl Agent {
serde_cbor::from_slice(&certificate).map_err(AgentError::InvalidCborData)?;

self.verify(&certificate, effective_canister_id)?;
let status = lookup_request_status(certificate, &request_id)?;
let status = lookup_request_status(&certificate, &request_id)?;

match status {
RequestStatusResponse::Replied(reply) => Ok(CallResponse::Response(reply.arg)),
Expand Down Expand Up @@ -628,19 +633,19 @@ impl Agent {
request_id: &RequestId,
effective_canister_id: Principal,
signed_request_status: Vec<u8>,
) -> Result<Vec<u8>, AgentError> {
) -> Result<(Vec<u8>, Certificate), AgentError> {
let mut retry_policy = self.get_retry_policy();

let mut request_accepted = false;
let (resp, cert) = self
.request_status_signed(
request_id,
effective_canister_id,
signed_request_status.clone(),
)
.await?;
loop {
match self
.request_status_signed(
request_id,
effective_canister_id,
signed_request_status.clone(),
)
.await?
{
match resp {
RequestStatusResponse::Unknown => {}

RequestStatusResponse::Received | RequestStatusResponse::Processing => {
Expand All @@ -650,7 +655,9 @@ impl Agent {
}
}

RequestStatusResponse::Replied(ReplyResponse { arg, .. }) => return Ok(arg),
RequestStatusResponse::Replied(ReplyResponse { arg, .. }) => {
return Ok((arg, cert))
}

RequestStatusResponse::Rejected(response) => {
return Err(AgentError::CertifiedReject(response))
Expand All @@ -676,15 +683,15 @@ impl Agent {
&self,
request_id: &RequestId,
effective_canister_id: Principal,
) -> Result<Vec<u8>, AgentError> {
) -> Result<(Vec<u8>, Certificate), AgentError> {
let mut retry_policy = self.get_retry_policy();

let mut request_accepted = false;
loop {
match self
let (resp, cert) = self
.request_status_raw(request_id, effective_canister_id)
.await?
{
.await?;
match resp {
RequestStatusResponse::Unknown => {}

RequestStatusResponse::Received | RequestStatusResponse::Processing => {
Expand All @@ -701,7 +708,9 @@ impl Agent {
}
}

RequestStatusResponse::Replied(ReplyResponse { arg, .. }) => return Ok(arg),
RequestStatusResponse::Replied(ReplyResponse { arg, .. }) => {
return Ok((arg, cert))
}

RequestStatusResponse::Rejected(response) => {
return Err(AgentError::CertifiedReject(response))
Expand Down Expand Up @@ -964,13 +973,13 @@ impl Agent {
&self,
request_id: &RequestId,
effective_canister_id: Principal,
) -> Result<RequestStatusResponse, AgentError> {
) -> Result<(RequestStatusResponse, Certificate), AgentError> {
let paths: Vec<Vec<Label>> =
vec![vec!["request_status".into(), request_id.to_vec().into()]];

let cert = self.read_state_raw(paths, effective_canister_id).await?;

lookup_request_status(cert, request_id)
Ok((lookup_request_status(&cert, request_id)?, cert))
}

/// Send the signed request_status to the network. Will return [`RequestStatusResponse`].
Expand All @@ -981,7 +990,7 @@ impl Agent {
request_id: &RequestId,
effective_canister_id: Principal,
signed_request_status: Vec<u8>,
) -> Result<RequestStatusResponse, AgentError> {
) -> Result<(RequestStatusResponse, Certificate), AgentError> {
let _envelope: Envelope =
serde_cbor::from_slice(&signed_request_status).map_err(AgentError::InvalidCborData)?;
let read_state_response: ReadStateResponse = self
Expand All @@ -991,7 +1000,7 @@ impl Agent {
let cert: Certificate = serde_cbor::from_slice(&read_state_response.certificate)
.map_err(AgentError::InvalidCborData)?;
self.verify(&cert, effective_canister_id)?;
lookup_request_status(cert, request_id)
Ok((lookup_request_status(&cert, request_id)?, cert))
}

/// Returns an UpdateBuilder enabling the construction of an update call without
Expand Down Expand Up @@ -1679,7 +1688,7 @@ impl<'agent> IntoFuture for QueryBuilder<'agent> {
/// An in-flight canister update call. Useful primarily as a `Future`.
pub struct UpdateCall<'agent> {
agent: &'agent Agent,
response_future: AgentFuture<'agent, CallResponse<Vec<u8>>>,
response_future: AgentFuture<'agent, CallResponse<(Vec<u8>, Certificate)>>,
effective_canister_id: Principal,
}

Expand All @@ -1693,13 +1702,15 @@ impl fmt::Debug for UpdateCall<'_> {
}

impl Future for UpdateCall<'_> {
type Output = Result<CallResponse<Vec<u8>>, AgentError>;
type Output = Result<CallResponse<(Vec<u8>, Certificate)>, AgentError>;
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
self.response_future.as_mut().poll(cx)
}
}

impl<'a> UpdateCall<'a> {
async fn and_wait(self) -> Result<Vec<u8>, AgentError> {
/// Waits for the update call to be completed, polling if necessary.
pub async fn and_wait(self) -> Result<(Vec<u8>, Certificate), AgentError> {
let response = self.response_future.await?;

match response {
Expand Down Expand Up @@ -1775,7 +1786,7 @@ impl<'agent> UpdateBuilder<'agent> {
/// Make an update call. This will call request_status on the RequestId in a loop and return
/// the response as a byte vector.
pub async fn call_and_wait(self) -> Result<Vec<u8>, AgentError> {
self.call().and_wait().await
self.call().and_wait().await.map(|x| x.0)
}

/// Make an update call. This will return a RequestId.
Expand Down
6 changes: 3 additions & 3 deletions ic-agent/src/agent/response_authentication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub(crate) fn lookup_subnet_metrics<Storage: AsRef<[u8]>>(
}

pub(crate) fn lookup_request_status<Storage: AsRef<[u8]>>(
certificate: Certificate<Storage>,
certificate: &Certificate<Storage>,
request_id: &RequestId,
) -> Result<RequestStatusResponse, AgentError> {
use AgentError::*;
Expand All @@ -94,8 +94,8 @@ pub(crate) fn lookup_request_status<Storage: AsRef<[u8]>>(
"done" => Ok(RequestStatusResponse::Done),
"processing" => Ok(RequestStatusResponse::Processing),
"received" => Ok(RequestStatusResponse::Received),
"rejected" => lookup_rejection(&certificate, request_id),
"replied" => lookup_reply(&certificate, request_id),
"rejected" => lookup_rejection(certificate, request_id),
"replied" => lookup_reply(certificate, request_id),
other => Err(InvalidRequestStatus(path_status.into(), other.to_string())),
},
LookupResult::Error => Err(LookupPathError(path_status.into())),
Expand Down
2 changes: 1 addition & 1 deletion ic-utils/src/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ where
/// See [`AsyncCall::call`].
pub async fn call(self) -> Result<CallResponse<Out>, AgentError> {
let response_bytes = match self.build_call()?.call().await? {
CallResponse::Response(response_bytes) => response_bytes,
CallResponse::Response((response_bytes, _)) => response_bytes,
CallResponse::Poll(request_id) => return Ok(CallResponse::Poll(request_id)),
};

Expand Down
5 changes: 4 additions & 1 deletion ic-utils/src/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ impl<'agent> Canister<'agent> {
&'canister self,
request_id: &RequestId,
) -> Result<Vec<u8>, AgentError> {
self.agent.wait(request_id, self.canister_id).await
self.agent
.wait(request_id, self.canister_id)
.await
.map(|x| x.0)
}

/// Creates a copy of this canister, changing the canister ID to the provided principal.
Expand Down

0 comments on commit f1d8cd0

Please sign in to comment.