From c704ed2572cf47b6f9868ee91b9608ba25fc7f34 Mon Sep 17 00:00:00 2001 From: Daniel Sharifi Date: Wed, 18 Sep 2024 14:11:45 +0200 Subject: [PATCH 1/2] . --- CHANGELOG.md | 1 + ic-agent/Cargo.toml | 1 - ic-agent/src/agent/agent_config.rs | 5 --- ic-agent/src/agent/agent_test.rs | 26 ++------------- ic-agent/src/agent/builder.rs | 13 -------- .../agent/http_transport/reqwest_transport.rs | 17 ---------- ic-agent/src/agent/mod.rs | 33 ++----------------- ref-tests/Cargo.toml | 3 -- ref-tests/src/utils.rs | 2 -- 9 files changed, 6 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf64bc12..469a40b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/ic-agent/Cargo.toml b/ic-agent/Cargo.toml index 9c5462b7..fa4e650f 100644 --- a/ic-agent/Cargo.toml +++ b/ic-agent/Cargo.toml @@ -84,7 +84,6 @@ web-sys = { version = "0.3", features = [ [features] default = ["pem"] -experimental_sync_call = [] pem = ["dep:pem"] ic_ref_tests = [ "default", diff --git a/ic-agent/src/agent/agent_config.rs b/ic-agent/src/agent/agent_config.rs index 54bd51df..6899e05f 100644 --- a/ic-agent/src/agent/agent_config.rs +++ b/ic-agent/src/agent/agent_config.rs @@ -28,9 +28,6 @@ pub struct AgentConfig { pub max_response_body_size: Option, /// 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 { @@ -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, } } } diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs index 227c4672..37750c4c 100644 --- a/ic-agent/src/agent/agent_test.rs +++ b/ic-agent/src/agent/agent_test.rs @@ -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() } @@ -168,15 +166,9 @@ 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(), + format!("/api/v3/canister/aaaaa-aa/call").as_str(), 500, vec![], None, @@ -211,15 +203,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(), + format!("/api/v3/canister/aaaaa-aa/call").as_str(), 200, body, Some("application/cbor"), @@ -257,15 +243,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"), diff --git a/ic-agent/src/agent/builder.rs b/ic-agent/src/agent/builder.rs index 7fbc8559..6c4e491c 100644 --- a/ic-agent/src/agent/builder.rs +++ b/ic-agent/src/agent/builder.rs @@ -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; diff --git a/ic-agent/src/agent/http_transport/reqwest_transport.rs b/ic-agent/src/agent/http_transport/reqwest_transport.rs index 4368e38d..5876c281 100644 --- a/ic-agent/src/agent/http_transport/reqwest_transport.rs +++ b/ic-agent/src/agent/http_transport/reqwest_transport.rs @@ -20,8 +20,6 @@ pub struct ReqwestTransport { client: Client, max_response_body_size: Option, max_tcp_error_retries: usize, - #[allow(dead_code)] - use_call_v3_endpoint: bool, } impl ReqwestTransport { @@ -69,7 +67,6 @@ impl ReqwestTransport { client, max_response_body_size: None, max_tcp_error_retries: 0, - use_call_v3_endpoint: false, }) } @@ -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 { @@ -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)] diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs index a739b9db..ec44e482 100644 --- a/ic-agent/src/agent/mod.rs +++ b/ic-agent/src/agent/mod.rs @@ -156,7 +156,6 @@ pub struct Agent { max_response_body_size: Option, #[allow(dead_code)] max_tcp_error_retries: usize, - use_call_v3_endpoint: bool, } impl fmt::Debug for Agent { @@ -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 - } - }, }) } @@ -348,17 +337,7 @@ impl Agent { serialized_bytes: Vec, ) -> Result { 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?; @@ -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::(&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. diff --git a/ref-tests/Cargo.toml b/ref-tests/Cargo.toml index f1a490e2..c4505767 100644 --- a/ref-tests/Cargo.toml +++ b/ref-tests/Cargo.toml @@ -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"] diff --git a/ref-tests/src/utils.rs b/ref-tests/src/utils.rs index 85e30e87..d6938dfc 100644 --- a/ref-tests/src/utils.rs +++ b/ref-tests/src/utils.rs @@ -98,8 +98,6 @@ pub async fn create_agent(identity: impl Identity + 'static) -> Result() .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() From 79acbf5f37c064c525bf5a4ae4cb62dfe8271295 Mon Sep 17 00:00:00 2001 From: Daniel Sharifi Date: Wed, 18 Sep 2024 14:19:55 +0200 Subject: [PATCH 2/2] fix lint --- ic-agent/src/agent/agent_test.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs index 37750c4c..07416536 100644 --- a/ic-agent/src/agent/agent_test.rs +++ b/ic-agent/src/agent/agent_test.rs @@ -166,14 +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 (call_mock, url) = mock( - "POST", - format!("/api/v3/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); @@ -205,7 +198,7 @@ async fn call_rejected() -> Result<(), AgentError> { let (call_mock, url) = mock( "POST", - format!("/api/v3/canister/aaaaa-aa/call").as_str(), + "/api/v3/canister/aaaaa-aa/call", 200, body, Some("application/cbor"),