Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide certificate for update calls #608

Merged
merged 3 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -516,7 +519,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 @@ -538,10 +541,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 @@ -577,7 +582,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 @@ -627,19 +632,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 @@ -649,7 +654,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 @@ -675,15 +682,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 @@ -700,7 +707,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 @@ -963,13 +972,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 @@ -980,7 +989,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 @@ -990,7 +999,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 @@ -1675,7 +1684,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 @@ -1689,13 +1698,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 @@ -1771,7 +1782,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
2 changes: 1 addition & 1 deletion icx/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ async fn main() -> Result<()> {
serde_json::from_str::<SignedRequestStatus>(&buffer)
{
fetch_root_key_from_non_ic(&agent, &opts.replica).await?;
let response = agent
let (response, _) = agent
.request_status_signed(
&signed_request_status.request_id,
signed_request_status.effective_canister_id,
Expand Down
4 changes: 2 additions & 2 deletions ref-tests/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fn wait_signed() {

let read_envelope_serialized = serialized_bytes(read_state_envelope);

let result = agent
let (result, _) = agent
.wait_signed(&call_request_id, canister_id, read_envelope_serialized)
.await
.unwrap();
Expand Down Expand Up @@ -629,7 +629,7 @@ mod sign_send {
let ten_secs = time::Duration::from_secs(10);
thread::sleep(ten_secs);

let response = agent
let (response, _) = agent
.request_status_signed(
&signed_request_status.request_id,
signed_request_status.effective_canister_id,
Expand Down
Loading