feat!: add collateral cycles (#213)

* Add flat collateral amount * Lowercase 'main.mo' for consistency with 'lib.mo' * Change collateral amount to depend on number of nodes * Update tests * Adjust test output * Update 'TooFewCycles' return type to include collateral * Update dfx.json * Refactor * Misc
dfinity · May 23, 2024 · aa5cc2a · aa5cc2a
1 parent a6d2f9b
commit aa5cc2a
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 65 deletions.
diff --git a/dfx.json b/dfx.json
@@ -41,7 +41,7 @@
         "evm_rpc_staging_fiduciary"
       ],
       "type": "motoko",
-      "main": "e2e/motoko/Main.mo"
+      "main": "e2e/motoko/main.mo"
     }
   },
   "defaults": {

diff --git a/e2e/motoko/Main.mo → e2e/motoko/main.mo b/e2e/motoko/Main.mo → e2e/motoko/main.mo
@@ -3,8 +3,9 @@ import EvmRpcStaging13Node "canister:evm_rpc_staging_13_node";
 import EvmRpcStagingFidicuary "canister:evm_rpc_staging_fiduciary";
 
 import Buffer "mo:base/Buffer";
-import Debug "mo:base/Debug";
 import Cycles "mo:base/ExperimentalCycles";
+import Debug "mo:base/Debug";
+import Nat32 "mo:base/Nat32";
 import Principal "mo:base/Principal";
 import Text "mo:base/Text";
 import Evm "mo:evm";
@@ -15,6 +16,7 @@ shared ({ caller = installer }) actor class Main() {
 
     // (`subnet name`, `nodes in subnet`, `expected cycles for JSON-RPC call`)
     type SubnetTarget = (Text, Nat32, Nat);
+    let collateralCycles = 10_000_000;
     let defaultSubnet : SubnetTarget = ("13-node", 13, 99_330_400);
     let fiduciarySubnet : SubnetTarget = ("fiduciary", 28, 239_142_400);
 
@@ -75,8 +77,9 @@ shared ({ caller = installer }) actor class Main() {
                 };
             };
 
-            if (cycles != expectedCycles) {
-                addError("Unexpected number of cycles: " # debug_show cycles # " (expected " # debug_show expectedCycles # ")");
+            let expectedCyclesWithCollateral = expectedCycles + collateralCycles * Nat32.toNat(nodesInSubnet);
+            if (cycles != expectedCyclesWithCollateral) {
+                addError("Unexpected number of cycles: " # debug_show cycles # " (expected " # debug_show expectedCyclesWithCollateral # ")");
             };
 
             // `request()` without cycles
@@ -191,8 +194,8 @@ shared ({ caller = installer }) actor class Main() {
                         null,
                         {
                             addresses = ["0xB9B002e70AdF0F544Cd0F6b80BF12d4925B0695F"];
-                            fromBlock = ?#Number 19520540;
-                            toBlock = ?#Number 19520940;
+                            fromBlock = ? #Number 19520540;
+                            toBlock = ? #Number 19520940;
                             topics = ?[
                                 ["0x4d69d0bd4287b7f66c548f90154dc81bc98f65a1b362775df5ae171a2ccd262b"],
                                 [

diff --git a/src/accounting.rs b/src/accounting.rs
@@ -2,12 +2,12 @@ use cketh_common::eth_rpc_client::providers::RpcApi;
 
 use crate::{
     constants::{
-        CANISTER_OVERHEAD, HTTP_OUTCALL_REQUEST_BASE_COST, HTTP_OUTCALL_REQUEST_COST_PER_BYTE,
-        HTTP_OUTCALL_REQUEST_PER_NODE_COST, HTTP_OUTCALL_RESPONSE_COST_PER_BYTE,
-        INGRESS_MESSAGE_BYTE_RECEIVED_COST, INGRESS_MESSAGE_RECEIVED_COST, INGRESS_OVERHEAD_BYTES,
-        RPC_URL_MIN_COST_BYTES,
+        CANISTER_OVERHEAD, COLLATERAL_CYCLES_PER_NODE, HTTP_OUTCALL_REQUEST_BASE_COST,
+        HTTP_OUTCALL_REQUEST_COST_PER_BYTE, HTTP_OUTCALL_REQUEST_PER_NODE_COST,
+        HTTP_OUTCALL_RESPONSE_COST_PER_BYTE, INGRESS_MESSAGE_BYTE_RECEIVED_COST,
+        INGRESS_MESSAGE_RECEIVED_COST, INGRESS_OVERHEAD_BYTES, RPC_URL_MIN_COST_BYTES,
     },
-    memory::UNSTABLE_SUBNET_SIZE,
+    memory::get_nodes_in_subnet,
     types::{Provider, ResolvedRpcService},
 };
 
@@ -36,7 +36,7 @@ pub fn get_http_request_cost(
     payload_size_bytes: u64,
     max_response_bytes: u64,
 ) -> u128 {
-    let nodes_in_subnet = UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow());
+    let nodes_in_subnet = get_nodes_in_subnet();
     let ingress_bytes = payload_size_bytes as u128
         + u32::max(RPC_URL_MIN_COST_BYTES, api.url.len() as u32) as u128
         + INGRESS_OVERHEAD_BYTES;
@@ -52,65 +52,68 @@ pub fn get_http_request_cost(
 
 /// Calculate the additional cost for calling a registered JSON-RPC provider.
 pub fn get_provider_cost(provider: &Provider, payload_size_bytes: u64) -> u128 {
-    let nodes_in_subnet = UNSTABLE_SUBNET_SIZE.with(|m| *m.borrow());
+    let nodes_in_subnet = get_nodes_in_subnet();
     let cost_per_node = provider.cycles_per_call as u128
         + provider.cycles_per_message_byte as u128 * payload_size_bytes as u128;
     cost_per_node * (nodes_in_subnet as u128)
 }
 
-#[cfg(test)]
-use crate::constants::{NODES_IN_FIDUCIARY_SUBNET, NODES_IN_STANDARD_SUBNET};
-
-#[test]
-fn test_request_cost() {
-    for nodes_in_subnet in [1, NODES_IN_STANDARD_SUBNET, NODES_IN_FIDUCIARY_SUBNET] {
-        println!("Nodes in subnet: {nodes_in_subnet}");
-
-        UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow_mut() = nodes_in_subnet);
-
-        let url = "https://cloudflare-eth.com";
-        let payload = "{\"jsonrpc\":\"2.0\",\"method\":\"eth_gasPrice\",\"params\":[],\"id\":1}";
-        let base_cost = get_rpc_cost(
-            &ResolvedRpcService::Api(RpcApi {
-                url: url.to_string(),
-                headers: None,
-            }),
-            payload.len() as u64,
-            1000,
-        );
-        let base_cost_10_extra_bytes = get_rpc_cost(
-            &ResolvedRpcService::Api(RpcApi {
-                url: url.to_string(),
-                headers: None,
-            }),
-            payload.len() as u64 + 10,
-            1000,
-        );
-        let estimated_cost_10_extra_bytes = base_cost
-            + 10 * (INGRESS_MESSAGE_BYTE_RECEIVED_COST + HTTP_OUTCALL_REQUEST_COST_PER_BYTE)
-                * nodes_in_subnet as u128;
-        assert_eq!(base_cost_10_extra_bytes, estimated_cost_10_extra_bytes,);
-    }
+/// Calculate the cost + collateral cycles for an HTTP request.
+pub fn get_cost_with_collateral(cycles_cost: u128) -> u128 {
+    cycles_cost + COLLATERAL_CYCLES_PER_NODE * get_nodes_in_subnet() as u128
 }
 
 #[cfg(test)]
 mod test {
-    use candid::Principal;
-
+    use super::*;
     use crate::{
         accounting::{get_provider_cost, get_rpc_cost},
         constants::{NODES_IN_FIDUCIARY_SUBNET, NODES_IN_STANDARD_SUBNET},
-        memory::{PROVIDERS, UNSTABLE_SUBNET_SIZE},
+        memory::{set_nodes_in_subnet, PROVIDERS},
         providers::do_register_provider,
         types::{Provider, RegisterProviderArgs, ResolvedRpcService},
     };
+    use candid::Principal;
+
+    #[test]
+    fn test_request_cost() {
+        for nodes_in_subnet in [1, NODES_IN_STANDARD_SUBNET, NODES_IN_FIDUCIARY_SUBNET] {
+            println!("Nodes in subnet: {nodes_in_subnet}");
+
+            set_nodes_in_subnet(nodes_in_subnet);
+
+            let url = "https://cloudflare-eth.com";
+            let payload =
+                "{\"jsonrpc\":\"2.0\",\"method\":\"eth_gasPrice\",\"params\":[],\"id\":1}";
+            let base_cost = get_rpc_cost(
+                &ResolvedRpcService::Api(RpcApi {
+                    url: url.to_string(),
+                    headers: None,
+                }),
+                payload.len() as u64,
+                1000,
+            );
+            let base_cost_10_extra_bytes = get_rpc_cost(
+                &ResolvedRpcService::Api(RpcApi {
+                    url: url.to_string(),
+                    headers: None,
+                }),
+                payload.len() as u64 + 10,
+                1000,
+            );
+            let estimated_cost_10_extra_bytes = base_cost
+                + 10 * (INGRESS_MESSAGE_BYTE_RECEIVED_COST + HTTP_OUTCALL_REQUEST_COST_PER_BYTE)
+                    * nodes_in_subnet as u128;
+            assert_eq!(base_cost_10_extra_bytes, estimated_cost_10_extra_bytes,);
+        }
+    }
 
     #[test]
     fn test_provider_cost() {
         for nodes_in_subnet in [1, NODES_IN_STANDARD_SUBNET, NODES_IN_FIDUCIARY_SUBNET] {
             println!("Nodes in subnet: {nodes_in_subnet}");
 
-            UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow_mut() = nodes_in_subnet);
+            set_nodes_in_subnet(nodes_in_subnet);
 
             let provider = Provider {
                 provider_id: 0,
@@ -173,7 +176,7 @@ mod test {
         );
 
         // 13-node subnet
-        UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow_mut() = NODES_IN_STANDARD_SUBNET);
+        set_nodes_in_subnet(NODES_IN_STANDARD_SUBNET);
         assert_eq!(
             [
                 get_rpc_cost(&service, 0, 0),
@@ -185,7 +188,7 @@ mod test {
         );
 
         // Fiduciary subnet
-        UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow_mut() = NODES_IN_FIDUCIARY_SUBNET);
+        set_nodes_in_subnet(NODES_IN_FIDUCIARY_SUBNET);
         assert_eq!(
             [
                 get_rpc_cost(&service, 0, 0),

diff --git a/src/constants.rs b/src/constants.rs
@@ -13,6 +13,10 @@ pub const HTTP_OUTCALL_RESPONSE_COST_PER_BYTE: u128 = 800;
 // Additional cost of operating the canister per subnet node
 pub const CANISTER_OVERHEAD: u128 = 1_000_000;
 
+// Cycles which must be passed with each RPC request in case the
+// third-party JSON-RPC prices increase in the future (currently always refunded)
+pub const COLLATERAL_CYCLES_PER_NODE: u128 = 10_000_000;
+
 // Minimum number of bytes charged for a URL; improves consistency of costs between providers
 pub const RPC_URL_MIN_COST_BYTES: u32 = 256;
 

diff --git a/src/http.rs b/src/http.rs
@@ -7,7 +7,7 @@ use ic_cdk::api::management_canister::http_request::{
 use num_traits::ToPrimitive;
 
 use crate::{
-    accounting::{get_provider_cost, get_rpc_cost},
+    accounting::{get_cost_with_collateral, get_provider_cost, get_rpc_cost},
     add_metric, add_metric_entry,
     auth::{is_authorized, is_rpc_allowed},
     constants::{CONTENT_TYPE_HEADER, CONTENT_TYPE_VALUE, SERVICE_HOSTS_BLOCKLIST},
@@ -77,9 +77,10 @@ pub async fn do_http_request(
     }
     if !is_authorized(&caller, Auth::FreeRpc) {
         let cycles_available = ic_cdk::api::call::msg_cycles_available128();
-        if cycles_available < cycles_cost {
+        let cycles_cost_with_collateral = get_cost_with_collateral(cycles_cost);
+        if cycles_available < cycles_cost_with_collateral {
             return Err(ProviderError::TooFewCycles {
-                expected: cycles_cost,
+                expected: cycles_cost_with_collateral,
                 received: cycles_available,
             }
             .into());

diff --git a/src/main.rs b/src/main.rs
@@ -4,9 +4,10 @@ use cketh_common::eth_rpc::{Block, FeeHistory, LogEntry, RpcError};
 use cketh_common::eth_rpc_client::providers::RpcService;
 use cketh_common::eth_rpc_client::RpcConfig;
 use cketh_common::logs::INFO;
-use evm_rpc::accounting::get_rpc_cost;
+use evm_rpc::accounting::{get_cost_with_collateral, get_rpc_cost};
 use evm_rpc::candid_rpc::CandidRpcClient;
 use evm_rpc::http::get_http_response_body;
+use evm_rpc::memory::{get_nodes_in_subnet, set_nodes_in_subnet};
 use evm_rpc::metrics::encode_metrics;
 use evm_rpc::providers::{
     do_get_accumulated_cycle_count, do_withdraw_accumulated_cycles, find_provider,
@@ -26,9 +27,7 @@ use evm_rpc::{
     auth::{do_authorize, do_deauthorize, require_manage_or_controller, require_register_provider},
     constants::WASM_PAGE_SIZE,
     http::{do_json_rpc_request, do_transform_http_request},
-    memory::{
-        AUTH, METADATA, PROVIDERS, SERVICE_PROVIDER_MAP, UNSTABLE_METRICS, UNSTABLE_SUBNET_SIZE,
-    },
+    memory::{AUTH, METADATA, PROVIDERS, SERVICE_PROVIDER_MAP, UNSTABLE_METRICS},
     providers::{
         do_manage_provider, do_register_provider, do_unregister_provider, do_update_provider,
     },
@@ -145,11 +144,11 @@ fn request_cost(
     json_rpc_payload: String,
     max_response_bytes: u64,
 ) -> Result<u128, RpcError> {
-    Ok(get_rpc_cost(
+    Ok(get_cost_with_collateral(get_rpc_cost(
         &resolve_rpc_service(service)?,
         json_rpc_payload.len() as u64,
         max_response_bytes,
-    ))
+    )))
 }
 
 #[query(name = "getProviders")]
@@ -208,8 +207,8 @@ fn get_service_provider_map() -> Vec<(RpcService, u64)> {
 
 #[query(name = "getNodesInSubnet")]
 #[candid_method(query, rename = "getNodesInSubnet")]
-async fn get_nodes_in_subnet() -> u32 {
-    UNSTABLE_SUBNET_SIZE.with(|n| *n.borrow())
+async fn get_nodes_in_subnet_() -> u32 {
+    get_nodes_in_subnet()
 }
 
 #[query(name = "getAccumulatedCycleCount")]
@@ -254,7 +253,7 @@ fn init(args: InitArgs) {
 
 #[ic_cdk::post_upgrade]
 fn post_upgrade(args: InitArgs) {
-    UNSTABLE_SUBNET_SIZE.with(|m| *m.borrow_mut() = args.nodes_in_subnet);
+    set_nodes_in_subnet(args.nodes_in_subnet);
 }
 
 #[query]

diff --git a/src/memory.rs b/src/memory.rs
@@ -19,7 +19,7 @@ type Memory = VirtualMemory<DefaultMemoryImpl>;
 thread_local! {
     // Unstable static data: this is reset when the canister is upgraded.
     pub static UNSTABLE_METRICS: RefCell<Metrics> = RefCell::new(Metrics::default());
-    pub static UNSTABLE_SUBNET_SIZE: RefCell<u32> = RefCell::new(NODES_IN_FIDUCIARY_SUBNET);
+    static UNSTABLE_SUBNET_SIZE: RefCell<u32> = RefCell::new(NODES_IN_FIDUCIARY_SUBNET);
 
     // Stable static data: this is preserved when the canister is upgraded.
     #[cfg(not(target_arch = "wasm32"))]
@@ -38,3 +38,11 @@ thread_local! {
     pub static SERVICE_PROVIDER_MAP: RefCell<StableBTreeMap<StorableRpcService, u64, Memory>> = RefCell::new(
         StableBTreeMap::init(MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(3)))));
 }
+
+pub fn get_nodes_in_subnet() -> u32 {
+    UNSTABLE_SUBNET_SIZE.with_borrow(|n| *n)
+}
+
+pub fn set_nodes_in_subnet(nodes_in_subnet: u32) {
+    UNSTABLE_SUBNET_SIZE.with_borrow_mut(|n| *n = nodes_in_subnet)
+}