Skip to content

Commit

Permalink
feat: route all update calls to synchronous v3 endpoint (#595)
Browse files Browse the repository at this point in the history
  • Loading branch information
DSharifi authored Sep 19, 2024
1 parent 9973651 commit 4bd9a3a
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 102 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `Url` now implements `RouteProvider`.
* Add canister snapshot methods to `ManagementCanister`.
* Add `AllowedViewers` to `LogVisibility` enum.
* Remove the cargo feature, `experimental_sync_call`, and enable synchronous update calls by default.

## [0.37.1] - 2024-07-25

Expand Down
1 change: 0 additions & 1 deletion ic-agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ web-sys = { version = "0.3", features = [

[features]
default = ["pem"]
experimental_sync_call = []
pem = ["dep:pem"]
ic_ref_tests = [
"default",
Expand Down
5 changes: 0 additions & 5 deletions ic-agent/src/agent/agent_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ pub struct AgentConfig {
pub max_response_body_size: Option<usize>,
/// See [`with_max_tcp_error_retries`](super::AgentBuilder::with_max_tcp_error_retries).
pub max_tcp_error_retries: usize,
/// See [`with_call_v3_endpoint`](super::AgentBuilder::with_call_v3_endpoint).
#[cfg(feature = "experimental_sync_call")]
pub use_call_v3_endpoint: bool,
}

impl Default for AgentConfig {
Expand All @@ -45,8 +42,6 @@ impl Default for AgentConfig {
route_provider: None,
max_response_body_size: None,
max_tcp_error_retries: 0,
#[cfg(feature = "experimental_sync_call")]
use_call_v3_endpoint: false,
}
}
}
33 changes: 3 additions & 30 deletions ic-agent/src/agent/agent_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

fn make_agent(url: &str) -> Agent {
let builder = Agent::builder().with_url(url);
#[cfg(feature = "experimental_sync_call")]
let builder = builder.with_call_v3_endpoint();
builder.with_verify_query_signatures(false).build().unwrap()
}

Expand Down Expand Up @@ -168,20 +166,7 @@ async fn query_rejected() -> Result<(), AgentError> {
#[cfg_attr(not(target_family = "wasm"), tokio::test)]
#[cfg_attr(target_family = "wasm", wasm_bindgen_test)]
async fn call_error() -> Result<(), AgentError> {
let version = if cfg!(feature = "experimental_sync_call") {
"3"
} else {
"2"
};

let (call_mock, url) = mock(
"POST",
format!("/api/v{version}/canister/aaaaa-aa/call").as_str(),
500,
vec![],
None,
)
.await;
let (call_mock, url) = mock("POST", "/api/v3/canister/aaaaa-aa/call", 500, vec![], None).await;

let agent = make_agent(&url);

Expand Down Expand Up @@ -211,15 +196,9 @@ async fn call_rejected() -> Result<(), AgentError> {

let body = serde_cbor::to_vec(&reject_body).unwrap();

let version = if cfg!(feature = "experimental_sync_call") {
"3"
} else {
"2"
};

let (call_mock, url) = mock(
"POST",
format!("/api/v{version}/canister/aaaaa-aa/call").as_str(),
"/api/v3/canister/aaaaa-aa/call",
200,
body,
Some("application/cbor"),
Expand Down Expand Up @@ -257,15 +236,9 @@ async fn call_rejected_without_error_code() -> Result<(), AgentError> {

let body = serde_cbor::to_vec(&reject_body).unwrap();

let version = if cfg!(feature = "experimental_sync_call") {
"3"
} else {
"2"
};

let (call_mock, url) = mock(
"POST",
format!("/api/v{version}/canister/{}/call", canister_id_str).as_str(),
format!("/api/v3/canister/{}/call", canister_id_str).as_str(),
200,
body,
Some("application/cbor"),
Expand Down
13 changes: 0 additions & 13 deletions ic-agent/src/agent/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,6 @@ impl AgentBuilder {
self
}

/// Use call v3 endpoint for synchronous update calls.
/// __This is an experimental feature, and should not be used in production,
/// as the endpoint is not available yet on the mainnet IC.__
///
/// By enabling this feature, the agent will use the `v3` endpoint for update calls,
/// which is synchronous. This means the replica will wait for a certificate for the call,
/// meaning the agent will not need to poll for the certificate.
#[cfg(feature = "experimental_sync_call")]
pub fn with_call_v3_endpoint(mut self) -> Self {
self.config.use_call_v3_endpoint = true;
self
}

/// Retry up to the specified number of times upon encountering underlying TCP errors.
pub fn with_max_tcp_error_retries(mut self, retries: usize) -> Self {
self.config.max_tcp_error_retries = retries;
Expand Down
17 changes: 0 additions & 17 deletions ic-agent/src/agent/http_transport/reqwest_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ pub struct ReqwestTransport {
client: Client,
max_response_body_size: Option<usize>,
max_tcp_error_retries: usize,
#[allow(dead_code)]
use_call_v3_endpoint: bool,
}

impl ReqwestTransport {
Expand Down Expand Up @@ -69,7 +67,6 @@ impl ReqwestTransport {
client,
max_response_body_size: None,
max_tcp_error_retries: 0,
use_call_v3_endpoint: false,
})
}

Expand All @@ -96,16 +93,6 @@ impl ReqwestTransport {
..self
}
}

/// Equivalent to [`AgentBuilder::with_call_v3_endpoint`].
#[cfg(feature = "experimental_sync_call")]
#[deprecated(since = "0.38.0", note = "Use AgentBuilder::with_call_v3_endpoint")]
pub fn with_use_call_v3_endpoint(self) -> Self {
ReqwestTransport {
use_call_v3_endpoint: true,
..self
}
}
}

impl AgentBuilder {
Expand All @@ -119,10 +106,6 @@ impl AgentBuilder {
if let Some(max_size) = transport.max_response_body_size {
builder = builder.with_max_response_body_size(max_size);
}
#[cfg(feature = "experimental_sync_call")]
if transport.use_call_v3_endpoint {
builder = builder.with_call_v3_endpoint();
}
builder
}
#[doc(hidden)]
Expand Down
33 changes: 2 additions & 31 deletions ic-agent/src/agent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ pub struct Agent {
max_response_body_size: Option<usize>,
#[allow(dead_code)]
max_tcp_error_retries: usize,
use_call_v3_endpoint: bool,
}

impl fmt::Debug for Agent {
Expand Down Expand Up @@ -203,16 +202,6 @@ impl Agent {
concurrent_requests_semaphore: Arc::new(Semaphore::new(config.max_concurrent_requests)),
max_response_body_size: config.max_response_body_size,
max_tcp_error_retries: config.max_tcp_error_retries,
use_call_v3_endpoint: {
#[cfg(feature = "experimental_sync_call")]
{
config.use_call_v3_endpoint
}
#[cfg(not(feature = "experimental_sync_call"))]
{
false
}
},
})
}

Expand Down Expand Up @@ -348,17 +337,7 @@ impl Agent {
serialized_bytes: Vec<u8>,
) -> Result<TransportCallResponse, AgentError> {
let _permit = self.concurrent_requests_semaphore.acquire().await;
let api_version = if self.use_call_v3_endpoint {
"v3"
} else {
"v2"
};

let endpoint = format!(
"api/{}/canister/{}/call",
api_version,
effective_canister_id.to_text()
);
let endpoint = format!("api/v3/canister/{}/call", effective_canister_id.to_text());
let (status_code, response_body) = self
.execute(Method::POST, &endpoint, Some(serialized_bytes))
.await?;
Expand All @@ -367,15 +346,7 @@ impl Agent {
return Ok(TransportCallResponse::Accepted);
}

// status_code == OK (200)
if self.use_call_v3_endpoint {
serde_cbor::from_slice(&response_body).map_err(AgentError::InvalidCborData)
} else {
let reject_response = serde_cbor::from_slice::<RejectResponse>(&response_body)
.map_err(AgentError::InvalidCborData)?;

Err(AgentError::UncertifiedReject(reject_response))
}
serde_cbor::from_slice(&response_body).map_err(AgentError::InvalidCborData)
}

/// The simplest way to do a query call; sends a byte array and will return a byte vector.
Expand Down
3 changes: 0 additions & 3 deletions ref-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,3 @@ tokio = { workspace = true, features = ["full"] }
[dev-dependencies]
serde_cbor = { workspace = true }
ic-certification = { workspace = true }

[features]
experimental_sync_call = ["ic-agent/experimental_sync_call"]
2 changes: 0 additions & 2 deletions ref-tests/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ pub async fn create_agent(identity: impl Identity + 'static) -> Result<Agent, St
.parse::<u32>()
.expect("Could not parse the IC_REF_PORT environment variable as an integer.");
let builder = Agent::builder().with_url(format!("http://127.0.0.1:{port}"));
#[cfg(feature = "experimental_sync_call")]
let builder = builder.with_call_v3_endpoint();
builder
.with_identity(identity)
.build()
Expand Down

0 comments on commit 4bd9a3a

Please sign in to comment.