fix: Do not charge pay-as-you-go base fee on system subnets

rs/embedders/src/wasmtime_embedder/system_api.rs

@@ -28,7 +28,7 @@ use ic_types::{
     messages::{CallContextId, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, RejectContext, SenderInfo},
     methods::{SystemMethod, WasmClosure},
 };
-use ic_types_cycles::Cycles;
+use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles};
 use ic_utils::deterministic_operations::deterministic_copy_from_slice;
 use ic_wasm_types::doc_ref;
 use request_in_prep::{RequestInPrep, into_output_request};
@@ -4456,7 +4456,11 @@ impl SystemApi for SystemApiImpl {
                 }
             })?;
 
-        let subnet_cycles_config = self.sandbox_safe_system_state.subnet_cycles_config;
+        // HTTP outcalls are also free on system subnets, despite their normal cost schedule.
+        let mut subnet_cycles_config = self.sandbox_safe_system_state.subnet_cycles_config;
+        if self.sandbox_safe_system_state.subnet_type == SubnetType::System {
+            subnet_cycles_config.cost_schedule = CanisterCyclesCostSchedule::Free;
+        }
         let replication_kind = cost_params_v2
             .replication_kind(NumberOfNodes::from(subnet_cycles_config.subnet_size as u32));
         let cost = self

rs/execution_environment/src/execution_environment.rs

@@ -1269,12 +1269,25 @@ impl ExecutionEnvironment {
                 // paying subnets flexible outcalls remain unavailable until the
                 // flag is enabled, since legacy pricing would overcharge them
                 // (it charges the maximum response size up front).
-                let http_outcalls_are_free =
-                    self.http_outcalls_are_free(state.get_own_cost_schedule());
-                let pricing_version = match self.config.flexible_http_requests {
-                    FlagStatus::Enabled => Some(PricingVersion::PayAsYouGo),
-                    FlagStatus::Disabled if http_outcalls_are_free => Some(PricingVersion::Legacy),
-                    FlagStatus::Disabled => None,
+                let cost_schedule = match self.own_subnet_type {
+                    SubnetType::System => CanisterCyclesCostSchedule::Free,
+                    SubnetType::Application
+                    | SubnetType::VerifiedApplication
+                    | SubnetType::CloudEngine => state.get_own_cost_schedule(),
+                };
+                // And, just like non-flexible outcalls, flexible outcalls are
+                // only offered on subnets where the `http_requests` subnet
+                // feature is enabled.
+                let pricing_version = if state.subnet_features().http_requests {
+                    match (self.config.flexible_http_requests, cost_schedule) {
+                        (FlagStatus::Enabled, _) => Some(PricingVersion::PayAsYouGo),
+                        (FlagStatus::Disabled, CanisterCyclesCostSchedule::Free) => {
+                            Some(PricingVersion::Legacy)
+                        }
+                        (FlagStatus::Disabled, CanisterCyclesCostSchedule::Normal) => None,
+                    }
+                } else {
+                    None
                 };
                 match pricing_version {
                     None => ExecuteSubnetMessageResult::Finished {
@@ -1292,12 +1305,6 @@ impl ExecutionEnvironment {
                                     refund: msg.take_cycles(),
                                 },
                                 Ok(args) => {
-                                    let cost_schedule = match self.own_subnet_type {
-                                        SubnetType::System => CanisterCyclesCostSchedule::Free,
-                                        SubnetType::Application
-                                        | SubnetType::VerifiedApplication
-                                        | SubnetType::CloudEngine => state.get_own_cost_schedule(),
-                                    };
                                     match CanisterHttpRequestContext::generate_from_flexible_args(
                                         state.time(),
                                         request.as_ref(),
@@ -2192,14 +2199,6 @@ impl ExecutionEnvironment {
         }
     }
 
-    /// Returns whether HTTP outcalls are free on this subnet, i.e. the subnet
-    /// charges nothing for them. This is true on a free cost schedule, and on
-    /// system subnets.
-    fn http_outcalls_are_free(&self, cost_schedule: CanisterCyclesCostSchedule) -> bool {
-        cost_schedule == CanisterCyclesCostSchedule::Free
-            || self.own_subnet_type == SubnetType::System
-    }
-
     fn try_add_http_context_to_replicated_state(
         &self,
         mut canister_http_request_context: CanisterHttpRequestContext,
@@ -2208,8 +2207,11 @@ impl ExecutionEnvironment {
         since: Instant,
     ) -> Result<(), UserError> {
         let variable_parts_size = canister_http_request_context.variable_parts_size();
-        let cycles_config = state.get_own_subnet_cycles_config();
-        let cost_schedule = cycles_config.cost_schedule;
+        // HTTP outcalls are also free on system subnets, despite their normal cost schedule.
+        let cost_schedule = canister_http_request_context.cost_schedule;
+        let mut cycles_config = state.get_own_subnet_cycles_config();
+        cycles_config.cost_schedule = cost_schedule;
+
         let legacy_fee = self.cycles_account_manager.http_request_fee(
             variable_parts_size,
             canister_http_request_context.max_response_bytes,
@@ -2269,7 +2271,7 @@ impl ExecutionEnvironment {
             ));
         }
 
-        let http_outcalls_are_free = self.http_outcalls_are_free(cost_schedule);
+        let http_outcalls_are_free = cost_schedule == CanisterCyclesCostSchedule::Free;
 
         // The refundable payment is everything the payment covers beyond the
         // base fee; when the outcall is free nothing is charged, so nothing is

rs/execution_environment/src/execution_environment/tests.rs

@@ -10,10 +10,10 @@ use ic_management_canister_types_private::{
     CanisterIdRecord, CanisterMetadataRequest, CanisterMetadataResponse, CanisterStatusResultV2,
     CanisterStatusType, CreateCanisterArgs, DerivationPath, EcdsaCurve, EcdsaKeyId, EmptyBlob,
     FetchCanisterLogsRequest, FlexibleCanisterHttpRequestArgs, HttpMethod, IC_00, LogVisibilityV2,
-    MasterPublicKeyId, Method, Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs,
-    ProvisionalTopUpCanisterArgs, ReplicationCounts, SchnorrAlgorithm, SchnorrKeyId,
-    TakeCanisterSnapshotArgs, TransformContext, TransformFunc, UploadChunkArgs, VetKdCurve,
-    VetKdKeyId,
+    MasterPublicKeyId, Method, PRICING_VERSION_LEGACY, PRICING_VERSION_PAY_AS_YOU_GO,
+    Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs, ProvisionalTopUpCanisterArgs,
+    ReplicationCounts, SchnorrAlgorithm, SchnorrKeyId, TakeCanisterSnapshotArgs, TransformContext,
+    TransformFunc, UploadChunkArgs, VetKdCurve, VetKdKeyId,
 };
 use ic_registry_routing_table::{CanisterIdRange, RoutingTable, canister_id_into_u64};
 use ic_registry_subnet_type::SubnetType;
@@ -3425,6 +3425,110 @@ fn execute_canister_http_request_disabled() {
     assert_eq!(canister_http_request_contexts.len(), 0);
 }
 
+/// The two ways HTTP outcalls come for free: a free cost schedule, and a system
+/// subnet, which charges nothing for outcalls despite its normal schedule.
+#[derive(Copy, Clone, Debug)]
+enum FreeOutcalls {
+    FreeCostSchedule,
+    SystemSubnet,
+}
+
+#[test]
+fn execute_canister_http_request_free_subnet_accepts_zero_cycles() {
+    // Where HTTP outcalls are free nothing is charged for them, so a caller must
+    // not have to attach any cycles — under pay-as-you-go just as under legacy
+    // pricing, and for flexible outcalls (always pay-as-you-go) just as for
+    // fully replicated ones.
+    //
+    // Both flavours of free are covered because they used to differ: outcalls are
+    // priced off the cost schedule pinned in the request context, and a system
+    // subnet reaches that through a mapping (normal schedule, free outcalls)
+    // rather than by carrying a free schedule to begin with.
+    let own_subnet = subnet_test_id(1);
+    let caller_canister = canister_test_id(10);
+    let build_test = |free: FreeOutcalls| {
+        let builder = ExecutionTestBuilder::new()
+            .with_own_subnet_id(own_subnet)
+            .with_caller(own_subnet, caller_canister);
+        match free {
+            FreeOutcalls::FreeCostSchedule => {
+                builder.with_cost_schedule(CanisterCyclesCostSchedule::Free)
+            }
+            FreeOutcalls::SystemSubnet => builder.with_subnet_type(SubnetType::System),
+        }
+        .build()
+    };
+    let http_request_args = |pricing_version| CanisterHttpRequestArgs {
+        url: "https://example.com".to_string(),
+        max_response_bytes: Some(1_000_000),
+        headers: BoundedHttpHeaders::new(vec![]),
+        body: None,
+        method: HttpMethod::GET,
+        transform: Some(TransformContext {
+            function: TransformFunc(candid::Func {
+                principal: caller_canister.get().0,
+                method: "transform".to_string(),
+            }),
+            context: vec![0, 1, 2],
+        }),
+        is_replicated: None,
+        pricing_version,
+    };
+
+    for free in [FreeOutcalls::FreeCostSchedule, FreeOutcalls::SystemSubnet] {
+        let calls: [(&str, Method, Vec<u8>); 3] = [
+            (
+                "legacy",
+                Method::HttpRequest,
+                http_request_args(Some(PRICING_VERSION_LEGACY)).encode(),
+            ),
+            (
+                "pay-as-you-go",
+                Method::HttpRequest,
+                http_request_args(Some(PRICING_VERSION_PAY_AS_YOU_GO)).encode(),
+            ),
+            (
+                "flexible",
+                Method::FlexibleHttpRequest,
+                flexible_http_request_args(caller_canister).encode(),
+            ),
+        ];
+        for (label, method, payload) in calls {
+            let mut test = build_test(free);
+            test.inject_call_to_ic00(method, payload, Cycles::zero());
+            test.execute_all();
+
+            let contexts = &test
+                .state()
+                .metadata
+                .subnet_call_context_manager
+                .canister_http_request_contexts;
+            assert_eq!(
+                contexts.len(),
+                1,
+                "a {label} outcall with no cycles attached was not accepted on {free:?}: {:?}",
+                test.xnet_messages()
+                    .first()
+                    .cloned()
+                    .map(get_reject_message),
+            );
+            // Nothing is charged, so nothing is withheld as an allowance and the
+            // whole (empty) payment is left to be refunded with the response.
+            let context = contexts.get(&CallbackId::from(0)).unwrap();
+            assert_eq!(
+                context.request.payment,
+                Cycles::zero(),
+                "a {label} outcall on {free:?} charged something out of an empty payment",
+            );
+            assert_eq!(
+                context.refund_status.per_replica_allowance,
+                Cycles::zero(),
+                "a {label} outcall on {free:?} withheld an allowance out of an empty payment",
+            );
+        }
+    }
+}
+
 #[test]
 fn execute_canister_http_request_insufficient_payment() {
     // Under legacy pricing the *full* request fee is charged upfront, not just
@@ -4073,6 +4177,75 @@ fn execute_flexible_canister_http_request_disabled() {
     );
 }
 
+#[test]
+fn execute_flexible_canister_http_request_disabled_by_subnet_feature() {
+    /// The configurations in which flexible outcalls would be available if the
+    /// `http_requests` subnet feature were enabled.
+    #[derive(Copy, Clone, Debug)]
+    enum Available {
+        /// The `flexible_http_requests` feature flag is enabled.
+        FeatureFlag,
+        /// The subnet is on a free cost schedule, so pricing is moot.
+        FreeCostSchedule,
+        /// A system subnet charges nothing for outcalls despite a normal
+        /// cost schedule.
+        SystemSubnet,
+    }
+
+    // Just like non-flexible outcalls, flexible outcalls are unavailable on a
+    // subnet where the `http_requests` subnet feature is disabled — in every
+    // configuration that would otherwise offer them.
+    for available in [
+        Available::FeatureFlag,
+        Available::FreeCostSchedule,
+        Available::SystemSubnet,
+    ] {
+        let own_subnet = subnet_test_id(1);
+        let caller_canister = canister_test_id(10);
+        let builder = ExecutionTestBuilder::new()
+            .with_own_subnet_id(own_subnet)
+            .with_caller(own_subnet, caller_canister);
+        let mut test = match available {
+            Available::FeatureFlag => builder.with_flexible_http_requests_enabled(),
+            Available::FreeCostSchedule => {
+                builder.with_cost_schedule(CanisterCyclesCostSchedule::Free)
+            }
+            Available::SystemSubnet => builder.with_subnet_type(SubnetType::System),
+        }
+        .build();
+        std::sync::Arc::make_mut(&mut test.state_mut().metadata.own_subnet_info)
+            .subnet_features
+            .http_requests = false;
+
+        let args = flexible_http_request_args(caller_canister);
+        test.inject_call_to_ic00(
+            Method::FlexibleHttpRequest,
+            args.encode(),
+            Cycles::new(1_000_000_000),
+        );
+        test.execute_all();
+
+        // No context is added and the request is rejected specifically because
+        // the feature is not available on this subnet (as opposed to any other
+        // rejection reason).
+        let canister_http_request_contexts = &test
+            .state()
+            .metadata
+            .subnet_call_context_manager
+            .canister_http_request_contexts;
+        assert_eq!(
+            canister_http_request_contexts.len(),
+            0,
+            "unexpected context for {available:?}"
+        );
+        assert_eq!(
+            get_reject_message(test.xnet_messages()[0].clone()),
+            "This API is not enabled on this subnet",
+            "unexpected rejection message for {available:?}"
+        );
+    }
+}
+
 fn get_reject_message(response: RequestOrResponse) -> String {
     match response {
         RequestOrResponse::Request(_) => panic!("Expected Response"),

rs/execution_environment/tests/hypervisor.rs

@@ -52,7 +52,7 @@ use ic_types::{
     ingress::{IngressState, IngressStatus, WasmResult},
     methods::WasmMethod,
 };
-use ic_types_cycles::Cycles;
+use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles};
 use ic_universal_canister::{CallArgs, UNIVERSAL_CANISTER_WASM, call_args, wasm};
 use more_asserts::{assert_ge, assert_gt, assert_le, assert_lt};
 #[cfg(not(all(target_arch = "aarch64", target_vendor = "apple")))]
@@ -9563,6 +9563,45 @@ fn invoke_cost_http_request_v2_flexible_without_counts_uses_the_defaults() {
     );
 }
 
+#[test]
+fn cost_http_request_v2_is_free_on_system_subnet() {
+    cost_http_request_v2_is_free_on(
+        ExecutionTestBuilder::new()
+            .with_subnet_type(SubnetType::System)
+            .build(),
+    );
+}
+
+#[test]
+fn cost_http_request_v2_is_free_on_free_cost_schedule() {
+    cost_http_request_v2_is_free_on(
+        ExecutionTestBuilder::new()
+            .with_cost_schedule(CanisterCyclesCostSchedule::Free)
+            .build(),
+    );
+}
+
+fn cost_http_request_v2_is_free_on(mut test: ExecutionTest) {
+    let canister_id = test.universal_canister().unwrap();
+    let params_blob = Encode!(&CostHttpRequestV2Params {
+        request_bytes: 1000,
+        http_roundtrip_time_ms: 2_000,
+        raw_response_bytes: 500_000,
+        transformed_response_bytes: 1_000,
+        transform_instructions: 1_000_000,
+        outcall_type: None,
+    })
+    .unwrap();
+
+    let payload = wasm()
+        .cost_http_request_v2(&params_blob)
+        .reply_data_append()
+        .reply()
+        .build();
+    let bytes = get_reply(test.ingress(canister_id, "update", payload));
+    assert_eq!(Cycles::try_from(&bytes).unwrap(), Cycles::zero());
+}
+
 #[test]
 fn cost_http_request_v2_accepts_maximal_params() {
     // The largest params a caller can send: every value at its maximum, with the