From 4701dfd9a36d87cd01de4dfcd45be2f20126233e Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 19 Jun 2026 12:57:05 +0000 Subject: [PATCH 01/36] dark launch tracker --- rs/https_outcalls/client/src/client.rs | 6 +- rs/https_outcalls/pricing/BUILD.bazel | 6 + rs/https_outcalls/pricing/Cargo.toml | 7 +- rs/https_outcalls/pricing/src/dark_launch.rs | 269 +++++++++++++++++++ rs/https_outcalls/pricing/src/legacy.rs | 7 + rs/https_outcalls/pricing/src/lib.rs | 62 ++++- rs/https_outcalls/pricing/src/metrics.rs | 36 +++ rs/https_outcalls/pricing/src/payg.rs | 268 ++++++++++++++++++ 8 files changed, 653 insertions(+), 8 deletions(-) create mode 100644 rs/https_outcalls/pricing/src/dark_launch.rs create mode 100644 rs/https_outcalls/pricing/src/metrics.rs create mode 100644 rs/https_outcalls/pricing/src/payg.rs diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 40b1f0a5ec23..e9e87e51a52e 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -65,6 +65,7 @@ pub struct CanisterHttpAdapterClientImpl { rx: Receiver<(CanisterHttpResponse, CanisterHttpPaymentReceipt)>, query_service: TransformExecutionService, metrics: Metrics, + pricing_factory: PricingFactory, log: ReplicaLogger, } @@ -79,6 +80,7 @@ impl CanisterHttpAdapterClientImpl { ) -> Self { let (tx, rx) = channel(inflight_requests); let metrics = Metrics::new(&metrics_registry); + let pricing_factory = PricingFactory::new(&metrics_registry, log.clone()); Self { rt_handle, grpc_channel, @@ -86,6 +88,7 @@ impl CanisterHttpAdapterClientImpl { rx, query_service, metrics, + pricing_factory, log, } } @@ -121,6 +124,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { let mut http_adapter_client = HttpsOutcallsServiceClient::new(self.grpc_channel.clone()); let query_handler = self.query_service.clone(); let metrics = self.metrics.clone(); + let pricing_factory = self.pricing_factory.clone(); let log = self.log.clone(); // Spawn an async task that sends the canister http request to the adapter and awaits the response. @@ -133,7 +137,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { socks_proxy_addrs, } = canister_http_request; - let mut budget = PricingFactory::new_tracker(&request_context); + let mut budget = pricing_factory.new_tracker(&request_context); let request_size = request_context.variable_parts_size(); let CanisterHttpRequestContext { diff --git a/rs/https_outcalls/pricing/BUILD.bazel b/rs/https_outcalls/pricing/BUILD.bazel index bb6fa3989628..2c948a5854c5 100644 --- a/rs/https_outcalls/pricing/BUILD.bazel +++ b/rs/https_outcalls/pricing/BUILD.bazel @@ -8,8 +8,14 @@ rust_library( crate_name = "ic_https_outcalls_pricing", version = "0.1.0", deps = [ + # Keep sorted. "//rs/config", + "//rs/monitoring/logger", + "//rs/monitoring/metrics", + "//rs/types/cycles", "//rs/types/types", + "@crate_index//:prometheus", + "@crate_index//:slog", ], ) diff --git a/rs/https_outcalls/pricing/Cargo.toml b/rs/https_outcalls/pricing/Cargo.toml index 1773aa69401f..3de7839e79db 100644 --- a/rs/https_outcalls/pricing/Cargo.toml +++ b/rs/https_outcalls/pricing/Cargo.toml @@ -8,4 +8,9 @@ documentation.workspace = true [dependencies] ic-config = { path = "../../config" } -ic-types = { path = "../../types/types" } \ No newline at end of file +ic-logger = { path = "../../monitoring/logger" } +ic-metrics = { path = "../../monitoring/metrics" } +ic-types = { path = "../../types/types" } +ic-types-cycles = { path = "../../types/cycles" } +prometheus = { workspace = true } +slog = { workspace = true } \ No newline at end of file diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs new file mode 100644 index 000000000000..be4475bf4195 --- /dev/null +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -0,0 +1,269 @@ +use ic_logger::{ReplicaLogger, warn}; +use ic_types::{CanisterId, NumBytes, NumInstructions, canister_http::CanisterHttpPaymentReceipt}; + +use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError, metrics::PricingMetrics}; + +/// A [`BudgetTracker`] that runs two child trackers side by side: a `real` +/// tracker whose results are the only ones returned (and therefore the only +/// ones that affect observable behaviour), and a `shadow` tracker whose results +/// are merely compared against the real one. +/// +/// Whenever the shadow tracker disagrees with the real tracker (e.g. it returns +/// a pricing error where the real tracker succeeded), the divergence is counted +/// in a metric and logged together with the canister id, so we can measure what +/// fraction of requests would not be backwards compatible under the shadow +/// pricing and which canisters would break. +pub struct DarkLaunchTracker { + real: Box, + shadow: Box, + canister_id: CanisterId, + metrics: PricingMetrics, + log: ReplicaLogger, + /// Whether an incompatibility has already been recorded for this request. + /// Ensures we count and log at most once per request. + reported: bool, +} + +impl DarkLaunchTracker { + pub fn new( + real: Box, + shadow: Box, + canister_id: CanisterId, + metrics: PricingMetrics, + log: ReplicaLogger, + ) -> Self { + Self { + real, + shadow, + canister_id, + metrics, + log, + reported: false, + } + } + + /// Compares the results of the real and shadow trackers for a given + /// accounting `step` and records a divergence if they disagree. + fn compare( + &mut self, + step: &str, + real: &Result<(), PricingError>, + shadow: &Result<(), PricingError>, + ) { + if real.is_ok() == shadow.is_ok() { + return; + } + if self.reported { + return; + } + self.reported = true; + self.metrics + .shadow_incompatible_total + .with_label_values(&[step]) + .inc(); + warn!( + self.log, + "Canister http request would not be backwards compatible under shadow pricing: \ + canister_id {}, step {}, real_result {:?}, shadow_result {:?}", + self.canister_id, + step, + real, + shadow, + ); + } +} + +impl BudgetTracker for DarkLaunchTracker { + fn get_adapter_limits(&self) -> AdapterLimits { + // Only the real tracker drives observable behaviour. + self.real.get_adapter_limits() + } + + fn subtract_network_usage(&mut self, network_usage: NetworkUsage) -> Result<(), PricingError> { + let real = self.real.subtract_network_usage(network_usage); + let shadow = self.shadow.subtract_network_usage(network_usage); + self.compare("network_usage", &real, &shadow); + real + } + + fn get_transform_limit(&self) -> NumInstructions { + self.real.get_transform_limit() + } + + fn subtract_transform_usage(&mut self, usage: NumInstructions) -> Result<(), PricingError> { + let real = self.real.subtract_transform_usage(usage); + let shadow = self.shadow.subtract_transform_usage(usage); + self.compare("transform_usage", &real, &shadow); + real + } + + fn subtract_transformed_response_usage( + &mut self, + transformed_response_size: NumBytes, + ) -> Result<(), PricingError> { + let real = self + .real + .subtract_transformed_response_usage(transformed_response_size); + let shadow = self + .shadow + .subtract_transformed_response_usage(transformed_response_size); + self.compare("transformed_response_usage", &real, &shadow); + real + } + + fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { + // Count every request that reaches the final accounting step so the + // incompatible counter can be expressed as a fraction of the total. + self.metrics.shadow_requests_total.inc(); + self.real.create_payment_receipt() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use ic_logger::no_op_logger; + use ic_metrics::MetricsRegistry; + use std::time::Duration; + + /// A [`BudgetTracker`] whose accounting steps return preconfigured results. + struct FakeTracker { + network: Result<(), PricingError>, + transform: Result<(), PricingError>, + transformed: Result<(), PricingError>, + } + + impl FakeTracker { + fn ok() -> Self { + Self { + network: Ok(()), + transform: Ok(()), + transformed: Ok(()), + } + } + } + + impl BudgetTracker for FakeTracker { + fn get_adapter_limits(&self) -> AdapterLimits { + AdapterLimits { + max_response_size: NumBytes::from(0), + max_response_time: Duration::ZERO, + } + } + fn subtract_network_usage(&mut self, _: NetworkUsage) -> Result<(), PricingError> { + self.network + } + fn get_transform_limit(&self) -> NumInstructions { + NumInstructions::from(0) + } + fn subtract_transform_usage(&mut self, _: NumInstructions) -> Result<(), PricingError> { + self.transform + } + fn subtract_transformed_response_usage(&mut self, _: NumBytes) -> Result<(), PricingError> { + self.transformed + } + fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { + CanisterHttpPaymentReceipt::default() + } + } + + fn dark_launch( + real: FakeTracker, + shadow: FakeTracker, + metrics: PricingMetrics, + ) -> DarkLaunchTracker { + DarkLaunchTracker::new( + Box::new(real), + Box::new(shadow), + CanisterId::from_u64(7), + metrics, + no_op_logger(), + ) + } + + fn network_usage() -> NetworkUsage { + NetworkUsage { + response_size: NumBytes::from(0), + response_time: Duration::ZERO, + } + } + + fn incompatible_count(metrics: &PricingMetrics) -> u64 { + [ + "network_usage", + "transform_usage", + "transformed_response_usage", + ] + .iter() + .map(|step| { + metrics + .shadow_incompatible_total + .with_label_values(&[*step]) + .get() + }) + .sum() + } + + #[test] + fn returns_real_result_and_counts_divergence() { + let metrics = PricingMetrics::new(&MetricsRegistry::new()); + let shadow = FakeTracker { + network: Err(PricingError::InsufficientCycles), + ..FakeTracker::ok() + }; + let mut tracker = dark_launch(FakeTracker::ok(), shadow, metrics.clone()); + + // The real (always-Ok) result is returned even though the shadow fails. + assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); + assert_eq!( + metrics + .shadow_incompatible_total + .with_label_values(&["network_usage"]) + .get(), + 1 + ); + + let _ = tracker.create_payment_receipt(); + assert_eq!(metrics.shadow_requests_total.get(), 1); + } + + #[test] + fn counts_divergence_at_most_once_per_request() { + let metrics = PricingMetrics::new(&MetricsRegistry::new()); + let shadow = FakeTracker { + network: Err(PricingError::InsufficientCycles), + transform: Err(PricingError::InsufficientCycles), + transformed: Err(PricingError::InsufficientCycles), + }; + let mut tracker = dark_launch(FakeTracker::ok(), shadow, metrics.clone()); + + assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); + assert_eq!( + tracker.subtract_transform_usage(NumInstructions::from(0)), + Ok(()) + ); + assert_eq!( + tracker.subtract_transformed_response_usage(NumBytes::from(0)), + Ok(()) + ); + + // Only the first divergence is recorded for the request. + assert_eq!(incompatible_count(&metrics), 1); + } + + #[test] + fn no_divergence_when_results_agree() { + let metrics = PricingMetrics::new(&MetricsRegistry::new()); + let mut tracker = dark_launch(FakeTracker::ok(), FakeTracker::ok(), metrics.clone()); + + assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); + assert_eq!( + tracker.subtract_transform_usage(NumInstructions::from(0)), + Ok(()) + ); + let _ = tracker.create_payment_receipt(); + + assert_eq!(incompatible_count(&metrics), 0); + assert_eq!(metrics.shadow_requests_total.get(), 1); + } +} diff --git a/rs/https_outcalls/pricing/src/legacy.rs b/rs/https_outcalls/pricing/src/legacy.rs index eb272bc8a37b..6ce72888a588 100644 --- a/rs/https_outcalls/pricing/src/legacy.rs +++ b/rs/https_outcalls/pricing/src/legacy.rs @@ -45,6 +45,13 @@ impl BudgetTracker for LegacyTracker { Ok(()) } + fn subtract_transformed_response_usage( + &mut self, + _transformed_response_size: NumBytes, + ) -> Result<(), PricingError> { + Ok(()) + } + fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { // Legacy pricing does not perform cycles accounting, so no cycles // are ever refunded. diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 827459a9bcdd..70654b653625 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -1,12 +1,21 @@ +mod dark_launch; mod legacy; +mod metrics; +mod payg; use std::time::Duration; +use ic_logger::ReplicaLogger; +use ic_metrics::MetricsRegistry; use ic_types::{ NumBytes, NumInstructions, - canister_http::{CanisterHttpPaymentReceipt, CanisterHttpRequestContext}, + canister_http::{CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion}, }; + +use dark_launch::DarkLaunchTracker; use legacy::LegacyTracker; +use metrics::PricingMetrics; +use payg::PayAsYouGoTracker; pub trait BudgetTracker: Send { /// Returns the maximum network resources the Adapter is allowed to consume. @@ -26,6 +35,15 @@ pub trait BudgetTracker: Send { /// # Invariants /// - This method returns `Ok(())` if and only if `usage <= get_transform_limit()`. fn subtract_transform_usage(&mut self, usage: NumInstructions) -> Result<(), PricingError>; + /// Deducts the cost of the final (post-transform) response that this replica + /// produced and that will be handed back to the caller. + /// + /// This is the last accounting step and is invoked once the size of the + /// response is known. + fn subtract_transformed_response_usage( + &mut self, + transformed_response_size: NumBytes, + ) -> Result<(), PricingError>; /// Produces the per-replica payment receipt that summarizes the cycles /// accounting outcome of the outcall, given the resources consumed so /// far via the `subtract_*` methods. @@ -39,6 +57,7 @@ pub struct AdapterLimits { pub max_response_time: Duration, } +#[derive(Clone, Copy)] pub struct NetworkUsage { /// The size of the HTTP response, including the headers and the body. pub response_size: NumBytes, @@ -46,16 +65,47 @@ pub struct NetworkUsage { pub response_time: Duration, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum PricingError { InsufficientCycles, } -pub struct PricingFactory; +/// Builds a [`BudgetTracker`] for each canister HTTP request. +/// +/// The factory is constructed once per replica and holds the shared metrics +/// and logger needed by the dark-launch tracker. +#[derive(Clone)] +pub struct PricingFactory { + metrics: PricingMetrics, + log: ReplicaLogger, +} impl PricingFactory { - pub fn new_tracker(context: &CanisterHttpRequestContext) -> Box { - // TODO(IC-1937): This should take into account context.pricing_version and a replica config. - // Currently, we only support the legacy pricing version. - Box::new(LegacyTracker::new(context.max_response_bytes)) + pub fn new(metrics_registry: &MetricsRegistry, log: ReplicaLogger) -> Self { + Self { + metrics: PricingMetrics::new(metrics_registry), + log, + } + } + + /// Creates the tracker for a request. The subnet size (`N`) needed by the + /// pay-as-you-go formula is read from `context.subnet_size`. + pub fn new_tracker(&self, context: &CanisterHttpRequestContext) -> Box { + match context.pricing_version { + // Legacy pricing is what is actually charged today. We run the + // PayAsYouGo tracker as a shadow next to it so we can measure how + // many requests would become backwards-incompatible under the new + // pricing without changing any observable behaviour. + PricingVersion::Legacy => Box::new(DarkLaunchTracker::new( + Box::new(LegacyTracker::new(context.max_response_bytes)), + Box::new(PayAsYouGoTracker::new(context)), + context.request.sender, + self.metrics.clone(), + self.log.clone(), + )), + // PayAsYouGo requests are not served yet (the client rejects them), + // but we still hand back the matching tracker for completeness. + PricingVersion::PayAsYouGo => Box::new(PayAsYouGoTracker::new(context)), + } } } diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs new file mode 100644 index 000000000000..df2c83b5829a --- /dev/null +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -0,0 +1,36 @@ +use ic_metrics::MetricsRegistry; +use prometheus::{IntCounter, IntCounterVec}; + +/// Label identifying the accounting step at which the shadow tracker diverged +/// from the real one. +pub const LABEL_STEP: &str = "step"; + +#[derive(Clone)] +pub struct PricingMetrics { + /// Total number of requests evaluated by the dark-launch budget tracker. + pub shadow_requests_total: IntCounter, + /// Number of requests that would be rejected (pricing error) under the + /// shadow pricing while succeeding under the real pricing, by the + /// accounting step at which the divergence was first observed. + /// + /// The fraction `shadow_incompatible_total / shadow_requests_total` is the + /// share of requests that would NOT be backwards compatible. + pub shadow_incompatible_total: IntCounterVec, +} + +impl PricingMetrics { + pub fn new(metrics_registry: &MetricsRegistry) -> Self { + Self { + shadow_requests_total: metrics_registry.int_counter( + "canister_http_pricing_shadow_requests_total", + "Total canister http requests evaluated by the dark-launch budget tracker.", + ), + shadow_incompatible_total: metrics_registry.int_counter_vec( + "canister_http_pricing_shadow_incompatible_total", + "Canister http requests that would be rejected (pricing error) under the shadow \ + pricing while succeeding under the real pricing, by accounting step.", + &[LABEL_STEP], + ), + } + } +} diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs new file mode 100644 index 000000000000..658df484e333 --- /dev/null +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -0,0 +1,268 @@ +use std::time::Duration; + +use ic_config::subnet_config::MAX_INSTRUCTIONS_PER_QUERY_MESSAGE; +use ic_types::{ + NumBytes, NumInstructions, + canister_http::{ + CanisterHttpPaymentReceipt, CanisterHttpRequestContext, MAX_CANISTER_HTTP_RESPONSE_BYTES, + Replication, + }, +}; +use ic_types_cycles::Cycles; + +use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; + +// Per-replica fee constants. +// +// A request's cost is split into three parts: +// 1. the base cost, subtracted up-front when the request context is created +// (and therefore reflected in `per_replica_allowance`); +// 2. the per-replica cost, accounted for here as-you-go; +// 3. the consensus cost, computed from the aggregated response in the block +// payload (ignored for now). +// +// This tracker only implements the per-replica part. The formula differs +// between fully/non-replicated and flexible outcalls: +// +// Fully/non-replicated per replica: +// 50 * downloaded_bytes_i + 300 * request_ms_i + transform_instructions_i / 13 +// +// Flexible per replica: +// 50 * downloaded_bytes_i + 300 * request_ms_i +// + 50 * transformed_response_bytes_i * N + transform_instructions_i / 13 +const PER_DOWNLOADED_BYTE_FEE: u128 = 50; +const PER_RESPONSE_MS_FEE: u128 = 300; +const TRANSFORM_INSTRUCTION_DIVISOR: u128 = 13; +const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; + +pub struct PayAsYouGoTracker { + /// Number of nodes (`N`) on the subnet. + n: u64, + /// Whether this is a flexible outcall (different per-replica formula). + is_flexible: bool, + /// The cycles budget available to this replica (already net of the base + /// cost, which was subtracted when the context was created). + allowance: u128, + /// The maximum size of the HTTP response, including headers and body. + max_response_size: NumBytes, + /// The cycles charged so far against `allowance`. + spent: u128, +} + +impl PayAsYouGoTracker { + pub fn new(context: &CanisterHttpRequestContext) -> Self { + Self { + n: 13, + is_flexible: matches!(context.replication, Replication::Flexible { .. }), + allowance: context.refund_status.per_replica_allowance.get(), + max_response_size: context + .max_response_bytes + .unwrap_or(NumBytes::from(MAX_CANISTER_HTTP_RESPONSE_BYTES)), + spent: 0, + } + } + + /// Charges `amount` against the budget. Returns an error if the total spent + /// now exceeds the available allowance. + fn charge(&mut self, amount: u128) -> Result<(), PricingError> { + self.spent = self.spent.saturating_add(amount); + if self.spent > self.allowance { + Err(PricingError::InsufficientCycles) + } else { + Ok(()) + } + } +} + +impl BudgetTracker for PayAsYouGoTracker { + fn get_adapter_limits(&self) -> AdapterLimits { + AdapterLimits { + max_response_size: self.max_response_size, + // Mirror the legacy limit: the server enforces a 30s timeout, so 60s + // here is just a safety margin. + max_response_time: Duration::from_secs(60), + } + } + + fn subtract_network_usage(&mut self, network_usage: NetworkUsage) -> Result<(), PricingError> { + let NetworkUsage { + response_size, + response_time, + } = network_usage; + let cost = PER_DOWNLOADED_BYTE_FEE + .saturating_mul(response_size.get() as u128) + .saturating_add(PER_RESPONSE_MS_FEE.saturating_mul(response_time.as_millis() as u128)); + self.charge(cost) + } + + fn get_transform_limit(&self) -> NumInstructions { + MAX_INSTRUCTIONS_PER_QUERY_MESSAGE + } + + fn subtract_transform_usage(&mut self, usage: NumInstructions) -> Result<(), PricingError> { + let cost = (usage.get() as u128) / TRANSFORM_INSTRUCTION_DIVISOR; + self.charge(cost) + } + + fn subtract_transformed_response_usage( + &mut self, + transformed_response_size: NumBytes, + ) -> Result<(), PricingError> { + // For fully/non-replicated outcalls the transformed-response term is a + // consensus cost (ignored here for now). For flexible outcalls each + // replica is charged 50 * transformed_response_bytes_i * N. + if !self.is_flexible { + return Ok(()); + } + let cost = FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE + .saturating_mul(transformed_response_size.get() as u128) + .saturating_mul(self.n as u128); + self.charge(cost) + } + + fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { + CanisterHttpPaymentReceipt { + refund: Cycles::new(self.allowance.saturating_sub(self.spent)), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use ic_types::{ + CanisterId, NodeId, PrincipalId, RegistryVersion, + canister_http::{CanisterHttpMethod, PricingVersion, RefundStatus}, + messages::{CallbackId, NO_DEADLINE, Request}, + time::UNIX_EPOCH, + }; + use std::collections::BTreeSet; + + fn context( + replication: Replication, + per_replica_allowance: u128, + ) -> CanisterHttpRequestContext { + CanisterHttpRequestContext { + request: Request { + receiver: CanisterId::from_u64(1), + sender: CanisterId::from_u64(1), + sender_reply_callback: CallbackId::from(1), + payment: Cycles::zero(), + method_name: String::new(), + method_payload: Vec::new(), + metadata: Default::default(), + deadline: NO_DEADLINE, + }, + url: String::new(), + max_response_bytes: None, + headers: vec![], + body: None, + http_method: CanisterHttpMethod::GET, + transform: None, + time: UNIX_EPOCH, + replication, + pricing_version: PricingVersion::Legacy, + refund_status: RefundStatus { + refundable_cycles: Cycles::new(per_replica_allowance), + per_replica_allowance: Cycles::new(per_replica_allowance), + refunded_cycles: Cycles::zero(), + refunding_nodes: BTreeSet::new(), + }, + registry_version: RegistryVersion::from(1), + } + } + + fn flexible(n: usize) -> Replication { + let committee: BTreeSet = (0..n as u64) + .map(|i| NodeId::from(PrincipalId::new_node_test_id(i))) + .collect(); + Replication::Flexible { + committee, + min_responses: 1, + max_responses: n as u32, + } + } + + #[test] + fn does_not_charge_base_cost() { + // The base cost is handled at context creation, so a freshly created + // tracker has spent nothing and a zero-usage request refunds everything. + let ctx = context(Replication::FullyReplicated, 1_000_000); + let tracker = PayAsYouGoTracker::new(&ctx); + assert_eq!(tracker.spent, 0); + assert_eq!( + tracker.create_payment_receipt().refund, + Cycles::new(1_000_000) + ); + } + + #[test] + fn charges_per_replica_cost_fully_replicated() { + let allowance = 1_000_000_000u128; + let ctx = context(Replication::FullyReplicated, allowance); + let mut tracker = PayAsYouGoTracker::new(&ctx); + + let response_size = 1_000u64; + let response_ms = 2_000u128; + assert_eq!( + tracker.subtract_network_usage(NetworkUsage { + response_size: NumBytes::from(response_size), + response_time: Duration::from_millis(response_ms as u64), + }), + Ok(()) + ); + let network = + PER_DOWNLOADED_BYTE_FEE * response_size as u128 + PER_RESPONSE_MS_FEE * response_ms; + + let instructions = 13_000u64; + assert_eq!( + tracker.subtract_transform_usage(NumInstructions::from(instructions)), + Ok(()) + ); + let transform = instructions as u128 / TRANSFORM_INSTRUCTION_DIVISOR; + + // For fully-replicated requests the transformed-response term is a + // consensus cost and must not be charged here. + assert_eq!( + tracker.subtract_transformed_response_usage(NumBytes::from(5_000)), + Ok(()) + ); + + assert_eq!(tracker.spent, network + transform); + assert_eq!( + tracker.create_payment_receipt().refund, + Cycles::new(allowance - network - transform) + ); + } + + #[test] + fn charges_transformed_response_for_flexible() { + let allowance = 1_000_000_000u128; + let n = 13; + let ctx = context(flexible(n), allowance); + let mut tracker = PayAsYouGoTracker::new(&ctx); + + let transformed_size = 500u64; + assert_eq!( + tracker.subtract_transformed_response_usage(NumBytes::from(transformed_size)), + Ok(()) + ); + let expected = + FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE * transformed_size as u128 * n as u128; + assert_eq!(tracker.spent, expected); + } + + #[test] + fn returns_pricing_error_when_budget_is_exceeded() { + let ctx = context(Replication::FullyReplicated, 100); + let mut tracker = PayAsYouGoTracker::new(&ctx); + assert_eq!( + tracker.subtract_network_usage(NetworkUsage { + response_size: NumBytes::from(1_000), + response_time: Duration::ZERO, + }), + Err(PricingError::InsufficientCycles) + ); + assert_eq!(tracker.create_payment_receipt().refund, Cycles::zero()); + } +} From 7d7cd45a8f13f80823261c29658a9b73be129bab Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Fri, 19 Jun 2026 12:58:45 +0000 Subject: [PATCH 02/36] Automatically updated Cargo*.lock --- Cargo.lock | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index c2e1fc00702c..29aa38ea3776 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10303,7 +10303,12 @@ name = "ic-https-outcalls-pricing" version = "0.9.0" dependencies = [ "ic-config", + "ic-logger", + "ic-metrics", "ic-types", + "ic-types-cycles", + "prometheus", + "slog", ] [[package]] From 0ec21dc5f103be35e98fe7df9f3ee6131ca9376c Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 19 Jun 2026 13:16:00 +0000 Subject: [PATCH 03/36] fix --- rs/https_outcalls/pricing/src/payg.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 658df484e333..4d2f808a0cd1 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -198,12 +198,12 @@ mod tests { #[test] fn charges_per_replica_cost_fully_replicated() { - let allowance = 1_000_000_000u128; + let allowance = 1_000_000_000_u128; let ctx = context(Replication::FullyReplicated, allowance); let mut tracker = PayAsYouGoTracker::new(&ctx); - let response_size = 1_000u64; - let response_ms = 2_000u128; + let response_size = 1_000_u64; + let response_ms = 2_000_u128; assert_eq!( tracker.subtract_network_usage(NetworkUsage { response_size: NumBytes::from(response_size), @@ -214,7 +214,7 @@ mod tests { let network = PER_DOWNLOADED_BYTE_FEE * response_size as u128 + PER_RESPONSE_MS_FEE * response_ms; - let instructions = 13_000u64; + let instructions = 13_000_u64; assert_eq!( tracker.subtract_transform_usage(NumInstructions::from(instructions)), Ok(()) @@ -237,12 +237,12 @@ mod tests { #[test] fn charges_transformed_response_for_flexible() { - let allowance = 1_000_000_000u128; + let allowance = 1_000_000_000_u128; let n = 13; let ctx = context(flexible(n), allowance); let mut tracker = PayAsYouGoTracker::new(&ctx); - let transformed_size = 500u64; + let transformed_size = 500_u64; assert_eq!( tracker.subtract_transformed_response_usage(NumBytes::from(transformed_size)), Ok(()) From cf87d44f574cb1cd7a1416b60546d9e84bf457ca Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 19 Jun 2026 13:22:55 +0000 Subject: [PATCH 04/36] fix --- rs/https_outcalls/pricing/src/payg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 4d2f808a0cd1..76171cdd376d 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -91,7 +91,7 @@ impl BudgetTracker for PayAsYouGoTracker { } = network_usage; let cost = PER_DOWNLOADED_BYTE_FEE .saturating_mul(response_size.get() as u128) - .saturating_add(PER_RESPONSE_MS_FEE.saturating_mul(response_time.as_millis() as u128)); + .saturating_add(PER_RESPONSE_MS_FEE.saturating_mul(response_time.as_millis())); self.charge(cost) } From 4923d681a8ac32e3a30ef6924d2b0994207886c4 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 08:58:16 +0000 Subject: [PATCH 05/36] charge for gossip --- rs/https_outcalls/client/src/client.rs | 14 ++++++++++++++ rs/https_outcalls/pricing/src/dark_launch.rs | 17 +++++------------ rs/https_outcalls/pricing/src/legacy.rs | 2 +- rs/https_outcalls/pricing/src/lib.rs | 4 ++-- rs/https_outcalls/pricing/src/payg.rs | 13 +++++-------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index e9e87e51a52e..b1d47af46962 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -266,6 +266,20 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { } .await; + // Account for the cost of gossiping the final (post-transform) + // response to peers before creating the receipt. + let response_size = match &payload { + Ok(response) => response.len(), + Err(reject) => reject.message.len(), + }; + let payload = budget + .subtract_gossip_usage(NumBytes::from(response_size as u64)) + .map_err(|PricingError::InsufficientCycles| CanisterHttpReject { + reject_code: RejectCode::SysFatal, + message: "Insufficient cycles".to_string(), + }) + .and(payload); + // Create the payment receipt after all processing is complete. let receipt = budget.create_payment_receipt(); diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index be4475bf4195..1964bec21461 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -97,16 +97,12 @@ impl BudgetTracker for DarkLaunchTracker { real } - fn subtract_transformed_response_usage( + fn subtract_gossip_usage( &mut self, transformed_response_size: NumBytes, ) -> Result<(), PricingError> { - let real = self - .real - .subtract_transformed_response_usage(transformed_response_size); - let shadow = self - .shadow - .subtract_transformed_response_usage(transformed_response_size); + let real = self.real.subtract_gossip_usage(transformed_response_size); + let shadow = self.shadow.subtract_gossip_usage(transformed_response_size); self.compare("transformed_response_usage", &real, &shadow); real } @@ -159,7 +155,7 @@ mod tests { fn subtract_transform_usage(&mut self, _: NumInstructions) -> Result<(), PricingError> { self.transform } - fn subtract_transformed_response_usage(&mut self, _: NumBytes) -> Result<(), PricingError> { + fn subtract_gossip_usage(&mut self, _: NumBytes) -> Result<(), PricingError> { self.transformed } fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { @@ -242,10 +238,7 @@ mod tests { tracker.subtract_transform_usage(NumInstructions::from(0)), Ok(()) ); - assert_eq!( - tracker.subtract_transformed_response_usage(NumBytes::from(0)), - Ok(()) - ); + assert_eq!(tracker.subtract_gossip_usage(NumBytes::from(0)), Ok(())); // Only the first divergence is recorded for the request. assert_eq!(incompatible_count(&metrics), 1); diff --git a/rs/https_outcalls/pricing/src/legacy.rs b/rs/https_outcalls/pricing/src/legacy.rs index 6ce72888a588..539299550f62 100644 --- a/rs/https_outcalls/pricing/src/legacy.rs +++ b/rs/https_outcalls/pricing/src/legacy.rs @@ -45,7 +45,7 @@ impl BudgetTracker for LegacyTracker { Ok(()) } - fn subtract_transformed_response_usage( + fn subtract_gossip_usage( &mut self, _transformed_response_size: NumBytes, ) -> Result<(), PricingError> { diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 70654b653625..c4225aa5bf1f 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -36,11 +36,11 @@ pub trait BudgetTracker: Send { /// - This method returns `Ok(())` if and only if `usage <= get_transform_limit()`. fn subtract_transform_usage(&mut self, usage: NumInstructions) -> Result<(), PricingError>; /// Deducts the cost of the final (post-transform) response that this replica - /// produced and that will be handed back to the caller. + /// produced and that will be gossiped to peers. /// /// This is the last accounting step and is invoked once the size of the /// response is known. - fn subtract_transformed_response_usage( + fn subtract_gossip_usage( &mut self, transformed_response_size: NumBytes, ) -> Result<(), PricingError>; diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 76171cdd376d..2de10ac0828e 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -104,11 +104,11 @@ impl BudgetTracker for PayAsYouGoTracker { self.charge(cost) } - fn subtract_transformed_response_usage( + fn subtract_gossip_usage( &mut self, transformed_response_size: NumBytes, ) -> Result<(), PricingError> { - // For fully/non-replicated outcalls the transformed-response term is a + // For fully replicated outcalls the gossip term is a // consensus cost (ignored here for now). For flexible outcalls each // replica is charged 50 * transformed_response_bytes_i * N. if !self.is_flexible { @@ -221,12 +221,9 @@ mod tests { ); let transform = instructions as u128 / TRANSFORM_INSTRUCTION_DIVISOR; - // For fully-replicated requests the transformed-response term is a + // For fully-replicated requests the gossip term is a // consensus cost and must not be charged here. - assert_eq!( - tracker.subtract_transformed_response_usage(NumBytes::from(5_000)), - Ok(()) - ); + assert_eq!(tracker.subtract_gossip_usage(NumBytes::from(5_000)), Ok(())); assert_eq!(tracker.spent, network + transform); assert_eq!( @@ -244,7 +241,7 @@ mod tests { let transformed_size = 500_u64; assert_eq!( - tracker.subtract_transformed_response_usage(NumBytes::from(transformed_size)), + tracker.subtract_gossip_usage(NumBytes::from(transformed_size)), Ok(()) ); let expected = From abe2b564b4693608fbfb99cb0e8defbb5fabc849 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 09:24:50 +0000 Subject: [PATCH 06/36] t --- rs/https_outcalls/client/src/client.rs | 3 +- rs/https_outcalls/pricing/src/dark_launch.rs | 78 +++++++++++++------- rs/https_outcalls/pricing/src/lib.rs | 28 +++---- rs/https_outcalls/pricing/src/metrics.rs | 8 +- rs/https_outcalls/pricing/src/payg.rs | 12 +-- 5 files changed, 79 insertions(+), 50 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index b1d47af46962..aac06984a8f1 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -137,7 +137,8 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { socks_proxy_addrs, } = canister_http_request; - let mut budget = pricing_factory.new_tracker(&request_context); + // TODO: thread through the actual subnet size; 13 is a placeholder. + let mut budget = pricing_factory.new_tracker(&request_context, 13); let request_size = request_context.variable_parts_size(); let CanisterHttpRequestContext { diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 1964bec21461..2b09665c0484 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -10,13 +10,12 @@ use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError, metrics::P /// /// Whenever the shadow tracker disagrees with the real tracker (e.g. it returns /// a pricing error where the real tracker succeeded), the divergence is counted -/// in a metric and logged together with the canister id, so we can measure what -/// fraction of requests would not be backwards compatible under the shadow -/// pricing and which canisters would break. +/// in a metric. pub struct DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, + non_replicated: bool, metrics: PricingMetrics, log: ReplicaLogger, /// Whether an incompatibility has already been recorded for this request. @@ -29,6 +28,7 @@ impl DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, + non_replicated: bool, metrics: PricingMetrics, log: ReplicaLogger, ) -> Self { @@ -36,6 +36,7 @@ impl DarkLaunchTracker { real, shadow, canister_id, + non_replicated, metrics, log, reported: false, @@ -59,14 +60,15 @@ impl DarkLaunchTracker { self.reported = true; self.metrics .shadow_incompatible_total - .with_label_values(&[step]) + .with_label_values(&[step, &self.non_replicated.to_string()]) .inc(); warn!( self.log, - "Canister http request would not be backwards compatible under shadow pricing: \ - canister_id {}, step {}, real_result {:?}, shadow_result {:?}", + "Canister http request would not be compatible under shadow pricing: \ + canister_id {}, step {}, non_replicated {}, real_result {:?}, shadow_result {:?}", self.canister_id, step, + self.non_replicated, real, shadow, ); @@ -75,7 +77,6 @@ impl DarkLaunchTracker { impl BudgetTracker for DarkLaunchTracker { fn get_adapter_limits(&self) -> AdapterLimits { - // Only the real tracker drives observable behaviour. self.real.get_adapter_limits() } @@ -103,7 +104,7 @@ impl BudgetTracker for DarkLaunchTracker { ) -> Result<(), PricingError> { let real = self.real.subtract_gossip_usage(transformed_response_size); let shadow = self.shadow.subtract_gossip_usage(transformed_response_size); - self.compare("transformed_response_usage", &real, &shadow); + self.compare("gossip_usage", &real, &shadow); real } @@ -166,12 +167,14 @@ mod tests { fn dark_launch( real: FakeTracker, shadow: FakeTracker, + non_replicated: bool, metrics: PricingMetrics, ) -> DarkLaunchTracker { DarkLaunchTracker::new( Box::new(real), Box::new(shadow), CanisterId::from_u64(7), + non_replicated, metrics, no_op_logger(), ) @@ -185,19 +188,16 @@ mod tests { } fn incompatible_count(metrics: &PricingMetrics) -> u64 { - [ - "network_usage", - "transform_usage", - "transformed_response_usage", - ] - .iter() - .map(|step| { - metrics - .shadow_incompatible_total - .with_label_values(&[*step]) - .get() - }) - .sum() + let mut total = 0; + for step in ["network_usage", "transform_usage", "gossip_usage"] { + for non_replicated in ["true", "false"] { + total += metrics + .shadow_incompatible_total + .with_label_values(&[step, non_replicated]) + .get(); + } + } + total } #[test] @@ -207,14 +207,14 @@ mod tests { network: Err(PricingError::InsufficientCycles), ..FakeTracker::ok() }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, metrics.clone()); + let mut tracker = dark_launch(FakeTracker::ok(), shadow, false, metrics.clone()); // The real (always-Ok) result is returned even though the shadow fails. assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( metrics .shadow_incompatible_total - .with_label_values(&["network_usage"]) + .with_label_values(&["network_usage", "false"]) .get(), 1 ); @@ -231,7 +231,7 @@ mod tests { transform: Err(PricingError::InsufficientCycles), transformed: Err(PricingError::InsufficientCycles), }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, metrics.clone()); + let mut tracker = dark_launch(FakeTracker::ok(), shadow, false, metrics.clone()); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( @@ -247,7 +247,7 @@ mod tests { #[test] fn no_divergence_when_results_agree() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); - let mut tracker = dark_launch(FakeTracker::ok(), FakeTracker::ok(), metrics.clone()); + let mut tracker = dark_launch(FakeTracker::ok(), FakeTracker::ok(), false, metrics.clone()); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( @@ -259,4 +259,32 @@ mod tests { assert_eq!(incompatible_count(&metrics), 0); assert_eq!(metrics.shadow_requests_total.get(), 1); } + + #[test] + fn labels_divergence_by_replication_type() { + let metrics = PricingMetrics::new(&MetricsRegistry::new()); + let shadow = FakeTracker { + network: Err(PricingError::InsufficientCycles), + ..FakeTracker::ok() + }; + let mut tracker = dark_launch(FakeTracker::ok(), shadow, true, metrics.clone()); + + assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); + + // The divergence is attributed to the non-replicated label. + assert_eq!( + metrics + .shadow_incompatible_total + .with_label_values(&["network_usage", "true"]) + .get(), + 1 + ); + assert_eq!( + metrics + .shadow_incompatible_total + .with_label_values(&["network_usage", "false"]) + .get(), + 0 + ); + } } diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index c4225aa5bf1f..4e95883999b8 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -9,7 +9,9 @@ use ic_logger::ReplicaLogger; use ic_metrics::MetricsRegistry; use ic_types::{ NumBytes, NumInstructions, - canister_http::{CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion}, + canister_http::{ + CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion, Replication, + }, }; use dark_launch::DarkLaunchTracker; @@ -70,10 +72,6 @@ pub enum PricingError { InsufficientCycles, } -/// Builds a [`BudgetTracker`] for each canister HTTP request. -/// -/// The factory is constructed once per replica and holds the shared metrics -/// and logger needed by the dark-launch tracker. #[derive(Clone)] pub struct PricingFactory { metrics: PricingMetrics, @@ -88,24 +86,22 @@ impl PricingFactory { } } - /// Creates the tracker for a request. The subnet size (`N`) needed by the - /// pay-as-you-go formula is read from `context.subnet_size`. - pub fn new_tracker(&self, context: &CanisterHttpRequestContext) -> Box { + /// Creates the tracker for a request. + pub fn new_tracker( + &self, + context: &CanisterHttpRequestContext, + subnet_size: u32, + ) -> Box { match context.pricing_version { - // Legacy pricing is what is actually charged today. We run the - // PayAsYouGo tracker as a shadow next to it so we can measure how - // many requests would become backwards-incompatible under the new - // pricing without changing any observable behaviour. PricingVersion::Legacy => Box::new(DarkLaunchTracker::new( Box::new(LegacyTracker::new(context.max_response_bytes)), - Box::new(PayAsYouGoTracker::new(context)), + Box::new(PayAsYouGoTracker::new(context, subnet_size)), context.request.sender, + matches!(context.replication, Replication::NonReplicated(_)), self.metrics.clone(), self.log.clone(), )), - // PayAsYouGo requests are not served yet (the client rejects them), - // but we still hand back the matching tracker for completeness. - PricingVersion::PayAsYouGo => Box::new(PayAsYouGoTracker::new(context)), + PricingVersion::PayAsYouGo => Box::new(PayAsYouGoTracker::new(context, subnet_size)), } } } diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index df2c83b5829a..bbdbce38c656 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -5,6 +5,9 @@ use prometheus::{IntCounter, IntCounterVec}; /// from the real one. pub const LABEL_STEP: &str = "step"; +/// Label indicating whether the diverging request was non-replicated. +pub const LABEL_NON_REPLICATED: &str = "non_replicated"; + #[derive(Clone)] pub struct PricingMetrics { /// Total number of requests evaluated by the dark-launch budget tracker. @@ -28,8 +31,9 @@ impl PricingMetrics { shadow_incompatible_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_incompatible_total", "Canister http requests that would be rejected (pricing error) under the shadow \ - pricing while succeeding under the real pricing, by accounting step.", - &[LABEL_STEP], + pricing while succeeding under the real pricing, by accounting step and whether \ + the request is non-replicated.", + &[LABEL_STEP, LABEL_NON_REPLICATED], ), } } diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 2de10ac0828e..8a4bc434ad0b 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -50,9 +50,9 @@ pub struct PayAsYouGoTracker { } impl PayAsYouGoTracker { - pub fn new(context: &CanisterHttpRequestContext) -> Self { + pub fn new(context: &CanisterHttpRequestContext, subnet_size: u32) -> Self { Self { - n: 13, + n: subnet_size as u64, is_flexible: matches!(context.replication, Replication::Flexible { .. }), allowance: context.refund_status.per_replica_allowance.get(), max_response_size: context @@ -188,7 +188,7 @@ mod tests { // The base cost is handled at context creation, so a freshly created // tracker has spent nothing and a zero-usage request refunds everything. let ctx = context(Replication::FullyReplicated, 1_000_000); - let tracker = PayAsYouGoTracker::new(&ctx); + let tracker = PayAsYouGoTracker::new(&ctx, 13); assert_eq!(tracker.spent, 0); assert_eq!( tracker.create_payment_receipt().refund, @@ -200,7 +200,7 @@ mod tests { fn charges_per_replica_cost_fully_replicated() { let allowance = 1_000_000_000_u128; let ctx = context(Replication::FullyReplicated, allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx); + let mut tracker = PayAsYouGoTracker::new(&ctx, 13); let response_size = 1_000_u64; let response_ms = 2_000_u128; @@ -237,7 +237,7 @@ mod tests { let allowance = 1_000_000_000_u128; let n = 13; let ctx = context(flexible(n), allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx); + let mut tracker = PayAsYouGoTracker::new(&ctx, n as u32); let transformed_size = 500_u64; assert_eq!( @@ -252,7 +252,7 @@ mod tests { #[test] fn returns_pricing_error_when_budget_is_exceeded() { let ctx = context(Replication::FullyReplicated, 100); - let mut tracker = PayAsYouGoTracker::new(&ctx); + let mut tracker = PayAsYouGoTracker::new(&ctx, 13); assert_eq!( tracker.subtract_network_usage(NetworkUsage { response_size: NumBytes::from(1_000), From 77e64855f3624bab513c4d731abf4b2703e765c7 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 09:41:51 +0000 Subject: [PATCH 07/36] clean --- rs/https_outcalls/pricing/src/dark_launch.rs | 38 ++++++++++++++++++-- rs/https_outcalls/pricing/src/metrics.rs | 16 +++++---- rs/https_outcalls/pricing/src/payg.rs | 23 ++++++------ 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 2b09665c0484..5388de71d669 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -111,7 +111,10 @@ impl BudgetTracker for DarkLaunchTracker { fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { // Count every request that reaches the final accounting step so the // incompatible counter can be expressed as a fraction of the total. - self.metrics.shadow_requests_total.inc(); + self.metrics + .shadow_requests_total + .with_label_values(&[&self.non_replicated.to_string()]) + .inc(); self.real.create_payment_receipt() } } @@ -220,7 +223,13 @@ mod tests { ); let _ = tracker.create_payment_receipt(); - assert_eq!(metrics.shadow_requests_total.get(), 1); + assert_eq!( + metrics + .shadow_requests_total + .with_label_values(&["false"]) + .get(), + 1 + ); } #[test] @@ -257,7 +266,13 @@ mod tests { let _ = tracker.create_payment_receipt(); assert_eq!(incompatible_count(&metrics), 0); - assert_eq!(metrics.shadow_requests_total.get(), 1); + assert_eq!( + metrics + .shadow_requests_total + .with_label_values(&["false"]) + .get(), + 1 + ); } #[test] @@ -286,5 +301,22 @@ mod tests { .get(), 0 ); + + // The total is attributed to the same label, so per-type rates are exact. + let _ = tracker.create_payment_receipt(); + assert_eq!( + metrics + .shadow_requests_total + .with_label_values(&["true"]) + .get(), + 1 + ); + assert_eq!( + metrics + .shadow_requests_total + .with_label_values(&["false"]) + .get(), + 0 + ); } } diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index bbdbce38c656..bf8b0d2a25de 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -1,5 +1,5 @@ use ic_metrics::MetricsRegistry; -use prometheus::{IntCounter, IntCounterVec}; +use prometheus::IntCounterVec; /// Label identifying the accounting step at which the shadow tracker diverged /// from the real one. @@ -10,11 +10,13 @@ pub const LABEL_NON_REPLICATED: &str = "non_replicated"; #[derive(Clone)] pub struct PricingMetrics { - /// Total number of requests evaluated by the dark-launch budget tracker. - pub shadow_requests_total: IntCounter, + /// Total number of requests evaluated by the dark-launch budget tracker, + /// by whether the request is non-replicated. + pub shadow_requests_total: IntCounterVec, /// Number of requests that would be rejected (pricing error) under the /// shadow pricing while succeeding under the real pricing, by the - /// accounting step at which the divergence was first observed. + /// accounting step at which the divergence was first observed and whether + /// the request is non-replicated. /// /// The fraction `shadow_incompatible_total / shadow_requests_total` is the /// share of requests that would NOT be backwards compatible. @@ -24,9 +26,11 @@ pub struct PricingMetrics { impl PricingMetrics { pub fn new(metrics_registry: &MetricsRegistry) -> Self { Self { - shadow_requests_total: metrics_registry.int_counter( + shadow_requests_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_requests_total", - "Total canister http requests evaluated by the dark-launch budget tracker.", + "Total canister http requests evaluated by the dark-launch budget tracker, by \ + whether the request is non-replicated.", + &[LABEL_NON_REPLICATED], ), shadow_incompatible_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_incompatible_total", diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 8a4bc434ad0b..68c113e39c02 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -19,15 +19,15 @@ use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; // (and therefore reflected in `per_replica_allowance`); // 2. the per-replica cost, accounted for here as-you-go; // 3. the consensus cost, computed from the aggregated response in the block -// payload (ignored for now). +// payload. // -// This tracker only implements the per-replica part. The formula differs -// between fully/non-replicated and flexible outcalls: +// This tracker implements the per-replica part. The formula differs +// between fully-replicated and non-replicated/flexible outcalls: // -// Fully/non-replicated per replica: +// Fully-replicated per replica: // 50 * downloaded_bytes_i + 300 * request_ms_i + transform_instructions_i / 13 // -// Flexible per replica: +// None-replicated/Flexible per replica: // 50 * downloaded_bytes_i + 300 * request_ms_i // + 50 * transformed_response_bytes_i * N + transform_instructions_i / 13 const PER_DOWNLOADED_BYTE_FEE: u128 = 50; @@ -38,8 +38,8 @@ const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; pub struct PayAsYouGoTracker { /// Number of nodes (`N`) on the subnet. n: u64, - /// Whether this is a flexible outcall (different per-replica formula). - is_flexible: bool, + /// Whether this is outcall uses flexible pricing. + is_flexible_pricing: bool, /// The cycles budget available to this replica (already net of the base /// cost, which was subtracted when the context was created). allowance: u128, @@ -53,7 +53,10 @@ impl PayAsYouGoTracker { pub fn new(context: &CanisterHttpRequestContext, subnet_size: u32) -> Self { Self { n: subnet_size as u64, - is_flexible: matches!(context.replication, Replication::Flexible { .. }), + is_flexible_pricing: match context.replication { + Replication::Flexible { .. } | Replication::NonReplicated(_) => true, + Replication::FullyReplicated => false, + }, allowance: context.refund_status.per_replica_allowance.get(), max_response_size: context .max_response_bytes @@ -111,7 +114,7 @@ impl BudgetTracker for PayAsYouGoTracker { // For fully replicated outcalls the gossip term is a // consensus cost (ignored here for now). For flexible outcalls each // replica is charged 50 * transformed_response_bytes_i * N. - if !self.is_flexible { + if !self.is_flexible_pricing { return Ok(()); } let cost = FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE @@ -233,7 +236,7 @@ mod tests { } #[test] - fn charges_transformed_response_for_flexible() { + fn charges_gossip_usage_for_flexible() { let allowance = 1_000_000_000_u128; let n = 13; let ctx = context(flexible(n), allowance); From 937a9bc19857f630c1f9c347471adee100890ea8 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 09:54:18 +0000 Subject: [PATCH 08/36] schedule --- rs/https_outcalls/client/src/client.rs | 12 +++-- rs/https_outcalls/pricing/src/lib.rs | 8 +++- rs/https_outcalls/pricing/src/payg.rs | 61 +++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index aac06984a8f1..070a99183431 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -2,7 +2,8 @@ use crate::metrics::Metrics; use candid::Encode; use ic_error_types::{RejectCode, UserError}; use ic_https_outcalls_pricing::{ - AdapterLimits, BudgetTracker, NetworkUsage, PricingError, PricingFactory, + AdapterLimits, BudgetTracker, CanisterCyclesCostSchedule, NetworkUsage, PricingError, + PricingFactory, }; use ic_https_outcalls_service::{ CanisterHttpErrorKind, HttpHeader, HttpMethod, HttpsOutcallRequest, HttpsOutcallResponse, @@ -137,8 +138,13 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { socks_proxy_addrs, } = canister_http_request; - // TODO: thread through the actual subnet size; 13 is a placeholder. - let mut budget = pricing_factory.new_tracker(&request_context, 13); + // TODO: thread through the actual subnet size and cost schedule; + // 13 and Normal are placeholders. + let mut budget = pricing_factory.new_tracker( + &request_context, + 13, + CanisterCyclesCostSchedule::Normal, + ); let request_size = request_context.variable_parts_size(); let CanisterHttpRequestContext { diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 4e95883999b8..22c6d38dd8cb 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -13,6 +13,7 @@ use ic_types::{ CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion, Replication, }, }; +pub use ic_types_cycles::CanisterCyclesCostSchedule; use dark_launch::DarkLaunchTracker; use legacy::LegacyTracker; @@ -91,17 +92,20 @@ impl PricingFactory { &self, context: &CanisterHttpRequestContext, subnet_size: u32, + cost_schedule: CanisterCyclesCostSchedule, ) -> Box { match context.pricing_version { PricingVersion::Legacy => Box::new(DarkLaunchTracker::new( Box::new(LegacyTracker::new(context.max_response_bytes)), - Box::new(PayAsYouGoTracker::new(context, subnet_size)), + Box::new(PayAsYouGoTracker::new(context, subnet_size, cost_schedule)), context.request.sender, matches!(context.replication, Replication::NonReplicated(_)), self.metrics.clone(), self.log.clone(), )), - PricingVersion::PayAsYouGo => Box::new(PayAsYouGoTracker::new(context, subnet_size)), + PricingVersion::PayAsYouGo => { + Box::new(PayAsYouGoTracker::new(context, subnet_size, cost_schedule)) + } } } } diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 68c113e39c02..645c47d9e575 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -8,7 +8,7 @@ use ic_types::{ Replication, }, }; -use ic_types_cycles::Cycles; +use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles}; use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; @@ -40,6 +40,9 @@ pub struct PayAsYouGoTracker { n: u64, /// Whether this is outcall uses flexible pricing. is_flexible_pricing: bool, + /// Whether the subnet uses a free cost schedule. When `true` the tracker + /// charges nothing and refunds the full allowance. + is_free: bool, /// The cycles budget available to this replica (already net of the base /// cost, which was subtracted when the context was created). allowance: u128, @@ -50,13 +53,21 @@ pub struct PayAsYouGoTracker { } impl PayAsYouGoTracker { - pub fn new(context: &CanisterHttpRequestContext, subnet_size: u32) -> Self { + pub fn new( + context: &CanisterHttpRequestContext, + subnet_size: u32, + cost_schedule: CanisterCyclesCostSchedule, + ) -> Self { Self { n: subnet_size as u64, is_flexible_pricing: match context.replication { Replication::Flexible { .. } | Replication::NonReplicated(_) => true, Replication::FullyReplicated => false, }, + is_free: match cost_schedule { + CanisterCyclesCostSchedule::Free => true, + CanisterCyclesCostSchedule::Normal => false, + }, allowance: context.refund_status.per_replica_allowance.get(), max_response_size: context .max_response_bytes @@ -68,6 +79,10 @@ impl PayAsYouGoTracker { /// Charges `amount` against the budget. Returns an error if the total spent /// now exceeds the available allowance. fn charge(&mut self, amount: u128) -> Result<(), PricingError> { + // A free cost schedule means the subnet charges nothing for resources. + if self.is_free { + return Ok(()); + } self.spent = self.spent.saturating_add(amount); if self.spent > self.allowance { Err(PricingError::InsufficientCycles) @@ -191,7 +206,7 @@ mod tests { // The base cost is handled at context creation, so a freshly created // tracker has spent nothing and a zero-usage request refunds everything. let ctx = context(Replication::FullyReplicated, 1_000_000); - let tracker = PayAsYouGoTracker::new(&ctx, 13); + let tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); assert_eq!(tracker.spent, 0); assert_eq!( tracker.create_payment_receipt().refund, @@ -203,7 +218,7 @@ mod tests { fn charges_per_replica_cost_fully_replicated() { let allowance = 1_000_000_000_u128; let ctx = context(Replication::FullyReplicated, allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx, 13); + let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); let response_size = 1_000_u64; let response_ms = 2_000_u128; @@ -240,7 +255,8 @@ mod tests { let allowance = 1_000_000_000_u128; let n = 13; let ctx = context(flexible(n), allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx, n as u32); + let mut tracker = + PayAsYouGoTracker::new(&ctx, n as u32, CanisterCyclesCostSchedule::Normal); let transformed_size = 500_u64; assert_eq!( @@ -255,7 +271,7 @@ mod tests { #[test] fn returns_pricing_error_when_budget_is_exceeded() { let ctx = context(Replication::FullyReplicated, 100); - let mut tracker = PayAsYouGoTracker::new(&ctx, 13); + let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); assert_eq!( tracker.subtract_network_usage(NetworkUsage { response_size: NumBytes::from(1_000), @@ -265,4 +281,37 @@ mod tests { ); assert_eq!(tracker.create_payment_receipt().refund, Cycles::zero()); } + + #[test] + fn charges_nothing_on_free_cost_schedule() { + // On a free subnet the tracker charges nothing, even for usage that + // would otherwise exceed the allowance, so the full allowance is + // refunded. A flexible request is used so the gossip term (which would + // normally be charged) is also exercised. + let allowance = 1_000_000_u128; + let ctx = context(flexible(13), allowance); + let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Free); + + assert_eq!( + tracker.subtract_network_usage(NetworkUsage { + response_size: NumBytes::from(1_000_000), + response_time: Duration::from_secs(30), + }), + Ok(()) + ); + assert_eq!( + tracker.subtract_transform_usage(NumInstructions::from(1_000_000_000)), + Ok(()) + ); + assert_eq!( + tracker.subtract_gossip_usage(NumBytes::from(1_000_000)), + Ok(()) + ); + + assert_eq!(tracker.spent, 0); + assert_eq!( + tracker.create_payment_receipt().refund, + Cycles::new(allowance) + ); + } } From e265c2f549b4c992998ff895296be513aff482cf Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 11:20:36 +0000 Subject: [PATCH 09/36] add --- rs/https_outcalls/client/src/client.rs | 19 ++++---- .../consensus/src/pool_manager.rs | 47 ++++++++++++++++++- rs/https_outcalls/pricing/src/lib.rs | 5 +- rs/https_outcalls/pricing/src/payg.rs | 41 +++++++++++----- rs/pocket_ic_server/src/pocket_ic.rs | 7 ++- rs/types/cycles/src/cycles_cost_schedule.rs | 4 +- rs/types/types/src/canister_http.rs | 10 +++- 7 files changed, 103 insertions(+), 30 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 070a99183431..2d23f9f30c6a 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -2,8 +2,7 @@ use crate::metrics::Metrics; use candid::Encode; use ic_error_types::{RejectCode, UserError}; use ic_https_outcalls_pricing::{ - AdapterLimits, BudgetTracker, CanisterCyclesCostSchedule, NetworkUsage, PricingError, - PricingFactory, + AdapterLimits, BudgetTracker, NetworkUsage, PricingError, PricingFactory, }; use ic_https_outcalls_service::{ CanisterHttpErrorKind, HttpHeader, HttpMethod, HttpsOutcallRequest, HttpsOutcallResponse, @@ -136,15 +135,12 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { id: request_id, context: request_context, socks_proxy_addrs, + cost_schedule, + subnet_size, } = canister_http_request; - // TODO: thread through the actual subnet size and cost schedule; - // 13 and Normal are placeholders. - let mut budget = pricing_factory.new_tracker( - &request_context, - 13, - CanisterCyclesCostSchedule::Normal, - ); + let mut budget = + pricing_factory.new_tracker(&request_context, subnet_size, cost_schedule); let request_size = request_context.variable_parts_size(); let CanisterHttpRequestContext { @@ -578,6 +574,7 @@ where #[cfg(test)] mod tests { use super::*; + use ic_https_outcalls_pricing::CanisterCyclesCostSchedule; use ic_https_outcalls_service::{ HttpsOutcallRequest, HttpsOutcallResponse, HttpsOutcallResult, https_outcalls_service_server::{HttpsOutcallsService, HttpsOutcallsServiceServer}, @@ -586,7 +583,7 @@ mod tests { use ic_logger::replica_logger::no_op_logger; use ic_test_utilities_types::messages::RequestBuilder; use ic_types::{ - RegistryVersion, + NumberOfNodes, RegistryVersion, canister_http::{ MAX_CANISTER_HTTP_RESPONSE_BYTES, PricingVersion, RefundStatus, Replication, Transform, }, @@ -685,6 +682,8 @@ mod tests { registry_version: RegistryVersion::from(1), }, socks_proxy_addrs: vec![], + cost_schedule: CanisterCyclesCostSchedule::Normal, + subnet_size: NumberOfNodes::from(13), } } diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index 62d1dc638c56..8cc436bc9a66 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -15,15 +15,17 @@ use ic_interfaces_registry::RegistryClient; use ic_interfaces_state_manager::StateReader; use ic_logger::*; use ic_metrics::MetricsRegistry; +use ic_protobuf::registry::subnet::v1::CanisterCyclesCostSchedule as CanisterCyclesCostScheduleProto; use ic_registry_client_helpers::api_boundary_node::ApiBoundaryNodeRegistry; use ic_registry_client_helpers::node::NodeRegistry; use ic_registry_client_helpers::subnet::SubnetRegistry; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::ReplicatedState; use ic_types::{ - CountBytes, Height, NumBytes, ReplicaVersion, canister_http::*, consensus::HasHeight, - crypto::Signed, messages::CallbackId, replica_config::ReplicaConfig, + CountBytes, Height, NumBytes, NumberOfNodes, RegistryVersion, ReplicaVersion, canister_http::*, + consensus::HasHeight, crypto::Signed, messages::CallbackId, replica_config::ReplicaConfig, }; +use ic_types_cycles::CanisterCyclesCostSchedule; use ic_utils::str::StrEllipsize; use std::{ cell::RefCell, @@ -248,6 +250,29 @@ impl CanisterHttpPoolManagerImpl { .collect::>() } + /// Reads the subnet's pricing inputs (number of nodes and cycles cost + /// schedule) from the registry at `registry_version`. Returns `None` if + /// they cannot be determined. + fn pricing_inputs( + &self, + registry_version: RegistryVersion, + ) -> Option<(NumberOfNodes, CanisterCyclesCostSchedule)> { + let record = self + .registry_client + .get_subnet_record(self.replica_config.subnet_id, registry_version) + .ok() + .flatten()?; + let subnet_size = u32::try_from(record.membership.len()).ok()?; + if subnet_size == 0 { + return None; + } + let cost_schedule = + CanisterCyclesCostScheduleProto::try_from(record.canister_cycles_cost_schedule) + .ok() + .map(CanisterCyclesCostSchedule::from)?; + Some((NumberOfNodes::from(subnet_size), cost_schedule)) + } + /// Inform the HttpAdapterShim of any new requests that must be made. fn make_new_requests(&self, canister_http_pool: &dyn CanisterHttpPool) { let _time = self @@ -293,6 +318,20 @@ impl CanisterHttpPoolManagerImpl { } if !request_ids_already_made.contains(id) { + let Some((subnet_size, cost_schedule)) = + self.pricing_inputs(context.registry_version) + else { + warn!( + every_n_seconds => 10, + self.log, + "Skipping canister http request {} because the subnet size or cost \ + schedule could not be determined at registry version {}", + id, + context.registry_version + ); + continue; + }; + if let Err(err) = self .http_adapter_shim .lock() @@ -301,6 +340,8 @@ impl CanisterHttpPoolManagerImpl { id: *id, context: context.clone(), socks_proxy_addrs: socks_proxy_addrs.clone(), + cost_schedule, + subnet_size, }) { warn!( @@ -2729,6 +2770,8 @@ pub mod test { id: CallbackId::from(7), context: request.clone(), socks_proxy_addrs: vec![], + cost_schedule: CanisterCyclesCostSchedule::Normal, + subnet_size: NumberOfNodes::from(4), })) .times(1) .return_const(Ok(())); diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 22c6d38dd8cb..919b335e86d8 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -8,7 +8,7 @@ use std::time::Duration; use ic_logger::ReplicaLogger; use ic_metrics::MetricsRegistry; use ic_types::{ - NumBytes, NumInstructions, + NumBytes, NumInstructions, NumberOfNodes, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion, Replication, }, @@ -87,11 +87,10 @@ impl PricingFactory { } } - /// Creates the tracker for a request. pub fn new_tracker( &self, context: &CanisterHttpRequestContext, - subnet_size: u32, + subnet_size: NumberOfNodes, cost_schedule: CanisterCyclesCostSchedule, ) -> Box { match context.pricing_version { diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 645c47d9e575..1b2763e7cf5d 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -2,7 +2,7 @@ use std::time::Duration; use ic_config::subnet_config::MAX_INSTRUCTIONS_PER_QUERY_MESSAGE; use ic_types::{ - NumBytes, NumInstructions, + NumBytes, NumInstructions, NumberOfNodes, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpRequestContext, MAX_CANISTER_HTTP_RESPONSE_BYTES, Replication, @@ -37,7 +37,7 @@ const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; pub struct PayAsYouGoTracker { /// Number of nodes (`N`) on the subnet. - n: u64, + subnet_size: NumberOfNodes, /// Whether this is outcall uses flexible pricing. is_flexible_pricing: bool, /// Whether the subnet uses a free cost schedule. When `true` the tracker @@ -55,11 +55,11 @@ pub struct PayAsYouGoTracker { impl PayAsYouGoTracker { pub fn new( context: &CanisterHttpRequestContext, - subnet_size: u32, + subnet_size: NumberOfNodes, cost_schedule: CanisterCyclesCostSchedule, ) -> Self { Self { - n: subnet_size as u64, + subnet_size, is_flexible_pricing: match context.replication { Replication::Flexible { .. } | Replication::NonReplicated(_) => true, Replication::FullyReplicated => false, @@ -134,7 +134,7 @@ impl BudgetTracker for PayAsYouGoTracker { } let cost = FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE .saturating_mul(transformed_response_size.get() as u128) - .saturating_mul(self.n as u128); + .saturating_mul(self.subnet_size.get() as u128); self.charge(cost) } @@ -206,7 +206,11 @@ mod tests { // The base cost is handled at context creation, so a freshly created // tracker has spent nothing and a zero-usage request refunds everything. let ctx = context(Replication::FullyReplicated, 1_000_000); - let tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); + let tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(13), + CanisterCyclesCostSchedule::Normal, + ); assert_eq!(tracker.spent, 0); assert_eq!( tracker.create_payment_receipt().refund, @@ -218,7 +222,11 @@ mod tests { fn charges_per_replica_cost_fully_replicated() { let allowance = 1_000_000_000_u128; let ctx = context(Replication::FullyReplicated, allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); + let mut tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(13), + CanisterCyclesCostSchedule::Normal, + ); let response_size = 1_000_u64; let response_ms = 2_000_u128; @@ -255,8 +263,11 @@ mod tests { let allowance = 1_000_000_000_u128; let n = 13; let ctx = context(flexible(n), allowance); - let mut tracker = - PayAsYouGoTracker::new(&ctx, n as u32, CanisterCyclesCostSchedule::Normal); + let mut tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(n as u32), + CanisterCyclesCostSchedule::Normal, + ); let transformed_size = 500_u64; assert_eq!( @@ -271,7 +282,11 @@ mod tests { #[test] fn returns_pricing_error_when_budget_is_exceeded() { let ctx = context(Replication::FullyReplicated, 100); - let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Normal); + let mut tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(13), + CanisterCyclesCostSchedule::Normal, + ); assert_eq!( tracker.subtract_network_usage(NetworkUsage { response_size: NumBytes::from(1_000), @@ -290,7 +305,11 @@ mod tests { // normally be charged) is also exercised. let allowance = 1_000_000_u128; let ctx = context(flexible(13), allowance); - let mut tracker = PayAsYouGoTracker::new(&ctx, 13, CanisterCyclesCostSchedule::Free); + let mut tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(13), + CanisterCyclesCostSchedule::Free, + ); assert_eq!( tracker.subtract_network_usage(NetworkUsage { diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index c89b27b2dd73..18b0119bdb71 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -120,7 +120,8 @@ use ic_types::messages::{ CertificateDelegationFormat, CertificateDelegationMetadata, SignedSenderInfo, }; use ic_types::{ - CanisterId, Height, NumInstructions, PrincipalId, RegistryVersion, SnapshotId, SubnetId, + CanisterId, Height, NumInstructions, NumberOfNodes, PrincipalId, RegistryVersion, SnapshotId, + SubnetId, artifact::UnvalidatedArtifactMutation, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpReject, @@ -3759,6 +3760,8 @@ impl Operation for ProcessCanisterHttpInternal { id, context, socks_proxy_addrs: vec![], + cost_schedule: CanisterCyclesCostSchedule::Normal, + subnet_size: NumberOfNodes::from(sm.nodes.len() as u32), }) { canister_http.pending.insert(id); } @@ -3933,6 +3936,8 @@ fn process_mock_canister_https_response( id: canister_http_request_id, context: context.clone(), socks_proxy_addrs: vec![], + cost_schedule: CanisterCyclesCostSchedule::Normal, + subnet_size: NumberOfNodes::from(subnet.nodes.len() as u32), }) .unwrap(); let response = loop { diff --git a/rs/types/cycles/src/cycles_cost_schedule.rs b/rs/types/cycles/src/cycles_cost_schedule.rs index b9f4bb119e28..738dbd603710 100644 --- a/rs/types/cycles/src/cycles_cost_schedule.rs +++ b/rs/types/cycles/src/cycles_cost_schedule.rs @@ -3,7 +3,9 @@ use serde::{Deserialize, Serialize}; /// How to charge canisters for their use of computational resources (such as /// executing instructions, storing data, network, etc.) -#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[derive( + Clone, Copy, Debug, Default, Serialize, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, +)] pub enum CanisterCyclesCostSchedule { #[default] Normal, diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index 7a09536d538c..7596af6d7f44 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -42,7 +42,7 @@ //! the timestamp of a request plus the timeout interval. This condition is verifiable by the other nodes in the network. //! Once a timeout has made it into a finalized block, the request is answered with an error message. use crate::{ - CanisterId, CountBytes, RegistryVersion, ReplicaVersion, Time, + CanisterId, CountBytes, NumberOfNodes, RegistryVersion, ReplicaVersion, Time, artifact::{CanisterHttpResponseId, IdentifiableArtifact, PbArtifact}, crypto::{BasicSigOf, CryptoHashOf}, messages::{CallbackId, RejectContext, Request}, @@ -62,7 +62,7 @@ use ic_protobuf::{ proxy::{ProxyDecodeError, try_from_option_field}, state::system_metadata::v1 as pb_metadata, }; -use ic_types_cycles::Cycles; +use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles}; use rand::RngCore; use rand::seq::IteratorRandom; use serde::{Deserialize, Serialize}; @@ -814,6 +814,12 @@ pub struct CanisterHttpRequest { /// The addresses should be sent in the following format: `socks5://[]:`, for example: /// `socks5://[2602:fb2b:110:10:506f:cff:feff:fe69]:1080` pub socks_proxy_addrs: Vec, + /// The subnet's cycles cost schedule in effect at the request context's + /// registry version. + pub cost_schedule: CanisterCyclesCostSchedule, + /// The number of nodes on the subnet at the request context's registry + /// version. + pub subnet_size: NumberOfNodes, } /// The content of a response after the transformation From dd28e1550e2aa4ec3f1f2cba3fc8b941e4a2439e Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 11:32:44 +0000 Subject: [PATCH 10/36] comment --- rs/https_outcalls/pricing/src/payg.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 1b2763e7cf5d..d2fc8d331980 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -61,6 +61,8 @@ impl PayAsYouGoTracker { Self { subnet_size, is_flexible_pricing: match context.replication { + // Non-replicated outcalls gossip the reponse, so thay are charged + // the same way as flexible outcalls. Replication::Flexible { .. } | Replication::NonReplicated(_) => true, Replication::FullyReplicated => false, }, From 3631e42795e2c6b7d7e4d858459916be48b52a39 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 12:37:47 +0000 Subject: [PATCH 11/36] review --- rs/https_outcalls/client/src/client.rs | 6 ++ .../consensus/src/pool_manager.rs | 37 +++++++----- rs/https_outcalls/pricing/Cargo.toml | 2 +- rs/https_outcalls/pricing/src/dark_launch.rs | 56 +++++++++++-------- rs/https_outcalls/pricing/src/lib.rs | 15 ++++- rs/https_outcalls/pricing/src/metrics.rs | 24 ++++---- rs/https_outcalls/pricing/src/payg.rs | 30 +++++++++- 7 files changed, 114 insertions(+), 56 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 2d23f9f30c6a..bce1f0610a7b 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -529,6 +529,12 @@ async fn transform_adapter_response( if let Err(PricingError::InsufficientCycles) = budget.subtract_transform_usage(NumInstructions::from(instructions_used)) { + // NOTE: This assertion assumes a tracker whose transform charge fails + // only when the transform itself failed. That holds for the legacy + // tracker (used as the "real" tracker today, which never returns an + // error), but NOT for a bare `PayAsYouGoTracker`, where a successful + // transform can still overrun the per-replica budget. Revisit before + // enabling pay-as-you-go pricing on the live path. debug_assert!( execution_result.is_err(), "Transform should fail if insufficient cycles" diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index 9f8d9b466ac2..8c92e1fbf0b1 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -317,19 +317,30 @@ impl CanisterHttpPoolManagerImpl { } if !request_ids_already_made.contains(id) { - let Some((subnet_size, cost_schedule)) = - self.pricing_inputs(context.registry_version) - else { - warn!( - every_n_seconds => 10, - self.log, - "Skipping canister http request {} because the subnet size or cost \ - schedule could not be determined at registry version {}", - id, - context.registry_version - ); - continue; - }; + // The subnet size and cost schedule are only consumed by the + // dark-launch shadow tracker, which is observability-only. If + // they cannot be read from the registry we must NOT skip the + // request: that would alter the real (legacy) charging flow. + // Instead, fall back to defaults so the request is still + // dispatched; only the shadow comparison for this request is + // affected. + let (subnet_size, cost_schedule) = self + .pricing_inputs(context.registry_version) + .unwrap_or_else(|| { + warn!( + every_n_seconds => 10, + self.log, + "Using default pricing inputs for canister http request {} because \ + the subnet size or cost schedule could not be determined at registry \ + version {}", + id, + context.registry_version + ); + ( + NumberOfNodes::from(FALLBACK_SHADOW_SUBNET_SIZE), + CanisterCyclesCostSchedule::Normal, + ) + }); if let Err(err) = self .http_adapter_shim diff --git a/rs/https_outcalls/pricing/Cargo.toml b/rs/https_outcalls/pricing/Cargo.toml index 3de7839e79db..c3eeb858fe1e 100644 --- a/rs/https_outcalls/pricing/Cargo.toml +++ b/rs/https_outcalls/pricing/Cargo.toml @@ -13,4 +13,4 @@ ic-metrics = { path = "../../monitoring/metrics" } ic-types = { path = "../../types/types" } ic-types-cycles = { path = "../../types/cycles" } prometheus = { workspace = true } -slog = { workspace = true } \ No newline at end of file +slog = { workspace = true } diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 5388de71d669..2fa8a1f1167e 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -15,7 +15,7 @@ pub struct DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, - non_replicated: bool, + replication: &'static str, metrics: PricingMetrics, log: ReplicaLogger, /// Whether an incompatibility has already been recorded for this request. @@ -28,7 +28,7 @@ impl DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, - non_replicated: bool, + replication: &'static str, metrics: PricingMetrics, log: ReplicaLogger, ) -> Self { @@ -36,7 +36,7 @@ impl DarkLaunchTracker { real, shadow, canister_id, - non_replicated, + replication, metrics, log, reported: false, @@ -60,15 +60,15 @@ impl DarkLaunchTracker { self.reported = true; self.metrics .shadow_incompatible_total - .with_label_values(&[step, &self.non_replicated.to_string()]) + .with_label_values(&[step, self.replication]) .inc(); warn!( self.log, "Canister http request would not be compatible under shadow pricing: \ - canister_id {}, step {}, non_replicated {}, real_result {:?}, shadow_result {:?}", + canister_id {}, step {}, replication {}, real_result {:?}, shadow_result {:?}", self.canister_id, step, - self.non_replicated, + self.replication, real, shadow, ); @@ -113,7 +113,7 @@ impl BudgetTracker for DarkLaunchTracker { // incompatible counter can be expressed as a fraction of the total. self.metrics .shadow_requests_total - .with_label_values(&[&self.non_replicated.to_string()]) + .with_label_values(&[self.replication]) .inc(); self.real.create_payment_receipt() } @@ -170,14 +170,14 @@ mod tests { fn dark_launch( real: FakeTracker, shadow: FakeTracker, - non_replicated: bool, + replication: &'static str, metrics: PricingMetrics, ) -> DarkLaunchTracker { DarkLaunchTracker::new( Box::new(real), Box::new(shadow), CanisterId::from_u64(7), - non_replicated, + replication, metrics, no_op_logger(), ) @@ -193,10 +193,10 @@ mod tests { fn incompatible_count(metrics: &PricingMetrics) -> u64 { let mut total = 0; for step in ["network_usage", "transform_usage", "gossip_usage"] { - for non_replicated in ["true", "false"] { + for replication in ["fully_replicated", "flexible", "non_replicated"] { total += metrics .shadow_incompatible_total - .with_label_values(&[step, non_replicated]) + .with_label_values(&[step, replication]) .get(); } } @@ -210,14 +210,19 @@ mod tests { network: Err(PricingError::InsufficientCycles), ..FakeTracker::ok() }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, false, metrics.clone()); + let mut tracker = dark_launch( + FakeTracker::ok(), + shadow, + "fully_replicated", + metrics.clone(), + ); // The real (always-Ok) result is returned even though the shadow fails. assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( metrics .shadow_incompatible_total - .with_label_values(&["network_usage", "false"]) + .with_label_values(&["network_usage", "fully_replicated"]) .get(), 1 ); @@ -226,7 +231,7 @@ mod tests { assert_eq!( metrics .shadow_requests_total - .with_label_values(&["false"]) + .with_label_values(&["fully_replicated"]) .get(), 1 ); @@ -240,7 +245,7 @@ mod tests { transform: Err(PricingError::InsufficientCycles), transformed: Err(PricingError::InsufficientCycles), }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, false, metrics.clone()); + let mut tracker = dark_launch(FakeTracker::ok(), shadow, "flexible", metrics.clone()); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( @@ -256,7 +261,12 @@ mod tests { #[test] fn no_divergence_when_results_agree() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); - let mut tracker = dark_launch(FakeTracker::ok(), FakeTracker::ok(), false, metrics.clone()); + let mut tracker = dark_launch( + FakeTracker::ok(), + FakeTracker::ok(), + "fully_replicated", + metrics.clone(), + ); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( @@ -269,7 +279,7 @@ mod tests { assert_eq!( metrics .shadow_requests_total - .with_label_values(&["false"]) + .with_label_values(&["fully_replicated"]) .get(), 1 ); @@ -282,22 +292,22 @@ mod tests { network: Err(PricingError::InsufficientCycles), ..FakeTracker::ok() }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, true, metrics.clone()); + let mut tracker = dark_launch(FakeTracker::ok(), shadow, "non_replicated", metrics.clone()); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); - // The divergence is attributed to the non-replicated label. + // The divergence is attributed to the non_replicated label. assert_eq!( metrics .shadow_incompatible_total - .with_label_values(&["network_usage", "true"]) + .with_label_values(&["network_usage", "non_replicated"]) .get(), 1 ); assert_eq!( metrics .shadow_incompatible_total - .with_label_values(&["network_usage", "false"]) + .with_label_values(&["network_usage", "fully_replicated"]) .get(), 0 ); @@ -307,14 +317,14 @@ mod tests { assert_eq!( metrics .shadow_requests_total - .with_label_values(&["true"]) + .with_label_values(&["non_replicated"]) .get(), 1 ); assert_eq!( metrics .shadow_requests_total - .with_label_values(&["false"]) + .with_label_values(&["fully_replicated"]) .get(), 0 ); diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 919b335e86d8..e04aec342c58 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -23,7 +23,7 @@ use payg::PayAsYouGoTracker; pub trait BudgetTracker: Send { /// Returns the maximum network resources the Adapter is allowed to consume. fn get_adapter_limits(&self) -> AdapterLimits; - /// Deducts the actual network resources consumed. + /// Deducts the cost of the network resources consumed by the request. /// /// # Invariants /// - This method returns `Ok(())` if `network_usage <= get_adapter_limits()`. @@ -33,7 +33,7 @@ pub trait BudgetTracker: Send { fn subtract_network_usage(&mut self, network_usage: NetworkUsage) -> Result<(), PricingError>; /// Returns the maximum instructions allowed for the transformation function. fn get_transform_limit(&self) -> NumInstructions; - /// Deducts the actual instructions consumed by the transformation. + /// Deducts the cost of the instructions consumed by the transformation. /// /// # Invariants /// - This method returns `Ok(())` if and only if `usage <= get_transform_limit()`. @@ -98,7 +98,7 @@ impl PricingFactory { Box::new(LegacyTracker::new(context.max_response_bytes)), Box::new(PayAsYouGoTracker::new(context, subnet_size, cost_schedule)), context.request.sender, - matches!(context.replication, Replication::NonReplicated(_)), + replication_label(&context.replication), self.metrics.clone(), self.log.clone(), )), @@ -108,3 +108,12 @@ impl PricingFactory { } } } + +/// Returns the metric label for a request's replication type. +fn replication_label(replication: &Replication) -> &'static str { + match replication { + Replication::FullyReplicated => "fully_replicated", + Replication::Flexible { .. } => "flexible", + Replication::NonReplicated(_) => "non_replicated", + } +} diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index bf8b0d2a25de..ea5a7a7d88ac 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -5,18 +5,17 @@ use prometheus::IntCounterVec; /// from the real one. pub const LABEL_STEP: &str = "step"; -/// Label indicating whether the diverging request was non-replicated. -pub const LABEL_NON_REPLICATED: &str = "non_replicated"; +/// Label identifying the replication type of the request. +pub const LABEL_REPLICATION: &str = "replication"; #[derive(Clone)] pub struct PricingMetrics { /// Total number of requests evaluated by the dark-launch budget tracker, - /// by whether the request is non-replicated. + /// by replication type. pub shadow_requests_total: IntCounterVec, - /// Number of requests that would be rejected (pricing error) under the - /// shadow pricing while succeeding under the real pricing, by the - /// accounting step at which the divergence was first observed and whether - /// the request is non-replicated. + /// Number of requests for which the shadow tracker disagreed with the real + /// tracker, by the accounting step at which the divergence was first + /// observed and by replication type. /// /// The fraction `shadow_incompatible_total / shadow_requests_total` is the /// share of requests that would NOT be backwards compatible. @@ -29,15 +28,14 @@ impl PricingMetrics { shadow_requests_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_requests_total", "Total canister http requests evaluated by the dark-launch budget tracker, by \ - whether the request is non-replicated.", - &[LABEL_NON_REPLICATED], + replication type.", + &[LABEL_REPLICATION], ), shadow_incompatible_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_incompatible_total", - "Canister http requests that would be rejected (pricing error) under the shadow \ - pricing while succeeding under the real pricing, by accounting step and whether \ - the request is non-replicated.", - &[LABEL_STEP, LABEL_NON_REPLICATED], + "Canister http requests for which the shadow tracker disagreed with the real \ + tracker, by accounting step and replication type.", + &[LABEL_STEP, LABEL_REPLICATION], ), } } diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index d2fc8d331980..ca23acaa3293 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -27,7 +27,7 @@ use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; // Fully-replicated per replica: // 50 * downloaded_bytes_i + 300 * request_ms_i + transform_instructions_i / 13 // -// None-replicated/Flexible per replica: +// Non-replicated/Flexible per replica: // 50 * downloaded_bytes_i + 300 * request_ms_i // + 50 * transformed_response_bytes_i * N + transform_instructions_i / 13 const PER_DOWNLOADED_BYTE_FEE: u128 = 50; @@ -38,7 +38,7 @@ const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; pub struct PayAsYouGoTracker { /// Number of nodes (`N`) on the subnet. subnet_size: NumberOfNodes, - /// Whether this is outcall uses flexible pricing. + /// Whether this outcall uses flexible pricing. is_flexible_pricing: bool, /// Whether the subnet uses a free cost schedule. When `true` the tracker /// charges nothing and refunds the full allowance. @@ -61,7 +61,7 @@ impl PayAsYouGoTracker { Self { subnet_size, is_flexible_pricing: match context.replication { - // Non-replicated outcalls gossip the reponse, so thay are charged + // Non-replicated outcalls gossip the response, so they are charged // the same way as flexible outcalls. Replication::Flexible { .. } | Replication::NonReplicated(_) => true, Replication::FullyReplicated => false, @@ -281,6 +281,30 @@ mod tests { assert_eq!(tracker.spent, expected); } + #[test] + fn charges_gossip_usage_for_non_replicated() { + // Non-replicated outcalls use the same (flexible) pricing as flexible + // outcalls, so the gossip term is charged over the full subnet size. + let allowance = 1_000_000_000_u128; + let subnet_size = 13_u32; + let node = NodeId::from(PrincipalId::new_node_test_id(0)); + let ctx = context(Replication::NonReplicated(node), allowance); + let mut tracker = PayAsYouGoTracker::new( + &ctx, + NumberOfNodes::from(subnet_size), + CanisterCyclesCostSchedule::Normal, + ); + + let transformed_size = 500_u64; + assert_eq!( + tracker.subtract_gossip_usage(NumBytes::from(transformed_size)), + Ok(()) + ); + let expected = + FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE * transformed_size as u128 * subnet_size as u128; + assert_eq!(tracker.spent, expected); + } + #[test] fn returns_pricing_error_when_budget_is_exceeded() { let ctx = context(Replication::FullyReplicated, 100); From 37abafff9f43459be082cd96ffd1b231efd1e7f0 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 12:40:36 +0000 Subject: [PATCH 12/36] fix --- rs/https_outcalls/client/src/client.rs | 6 --- .../consensus/src/pool_manager.rs | 37 +++++++------------ 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index bce1f0610a7b..2d23f9f30c6a 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -529,12 +529,6 @@ async fn transform_adapter_response( if let Err(PricingError::InsufficientCycles) = budget.subtract_transform_usage(NumInstructions::from(instructions_used)) { - // NOTE: This assertion assumes a tracker whose transform charge fails - // only when the transform itself failed. That holds for the legacy - // tracker (used as the "real" tracker today, which never returns an - // error), but NOT for a bare `PayAsYouGoTracker`, where a successful - // transform can still overrun the per-replica budget. Revisit before - // enabling pay-as-you-go pricing on the live path. debug_assert!( execution_result.is_err(), "Transform should fail if insufficient cycles" diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index 8c92e1fbf0b1..9f8d9b466ac2 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -317,30 +317,19 @@ impl CanisterHttpPoolManagerImpl { } if !request_ids_already_made.contains(id) { - // The subnet size and cost schedule are only consumed by the - // dark-launch shadow tracker, which is observability-only. If - // they cannot be read from the registry we must NOT skip the - // request: that would alter the real (legacy) charging flow. - // Instead, fall back to defaults so the request is still - // dispatched; only the shadow comparison for this request is - // affected. - let (subnet_size, cost_schedule) = self - .pricing_inputs(context.registry_version) - .unwrap_or_else(|| { - warn!( - every_n_seconds => 10, - self.log, - "Using default pricing inputs for canister http request {} because \ - the subnet size or cost schedule could not be determined at registry \ - version {}", - id, - context.registry_version - ); - ( - NumberOfNodes::from(FALLBACK_SHADOW_SUBNET_SIZE), - CanisterCyclesCostSchedule::Normal, - ) - }); + let Some((subnet_size, cost_schedule)) = + self.pricing_inputs(context.registry_version) + else { + warn!( + every_n_seconds => 10, + self.log, + "Skipping canister http request {} because the subnet size or cost \ + schedule could not be determined at registry version {}", + id, + context.registry_version + ); + continue; + }; if let Err(err) = self .http_adapter_shim From 2930059106c20b7a8a8c6069b2755a947dab2db4 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 13:05:09 +0000 Subject: [PATCH 13/36] move truncation --- rs/https_outcalls/client/BUILD.bazel | 2 + rs/https_outcalls/client/Cargo.toml | 1 + rs/https_outcalls/client/src/client.rs | 89 +++++- .../consensus/src/pool_manager.rs | 278 +----------------- rs/types/types/src/canister_http.rs | 3 + 5 files changed, 94 insertions(+), 279 deletions(-) diff --git a/rs/https_outcalls/client/BUILD.bazel b/rs/https_outcalls/client/BUILD.bazel index a091c40d9a78..c1825ddb1af1 100644 --- a/rs/https_outcalls/client/BUILD.bazel +++ b/rs/https_outcalls/client/BUILD.bazel @@ -25,6 +25,7 @@ rust_library( "//rs/monitoring/metrics", "//rs/types/management_canister_types", "//rs/types/types", + "//rs/utils", "@crate_index//:candid", "@crate_index//:futures", "@crate_index//:hyper-util", @@ -56,6 +57,7 @@ rust_test( "//rs/test_utilities/types", "//rs/types/management_canister_types", "//rs/types/types", + "//rs/utils", "@crate_index//:candid", "@crate_index//:futures", "@crate_index//:hyper-util", diff --git a/rs/https_outcalls/client/Cargo.toml b/rs/https_outcalls/client/Cargo.toml index bb9b7eec0758..3875900224da 100644 --- a/rs/https_outcalls/client/Cargo.toml +++ b/rs/https_outcalls/client/Cargo.toml @@ -23,6 +23,7 @@ ic-interfaces-adapter-client = { path = "../../interfaces/adapter_client" } ic-logger = { path = "../../monitoring/logger" } ic-metrics = { path = "../../monitoring/metrics" } ic-types = { path = "../../types/types" } +ic-utils = { path = "../../utils" } prometheus = { workspace = true } slog = { workspace = true } tokio = { workspace = true } diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 2d23f9f30c6a..d6b7476bc91a 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -19,11 +19,13 @@ use ic_types::{ canister_http::{ CanisterHttpHeader, CanisterHttpMethod, CanisterHttpPaymentReceipt, CanisterHttpReject, CanisterHttpRequest, CanisterHttpRequestContext, CanisterHttpResponse, - CanisterHttpResponseContent, Transform, validate_http_headers_and_body, + CanisterHttpResponseContent, MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, Transform, + validate_http_headers_and_body, }, ingress::WasmResult, messages::{Query, QuerySource, Request}, }; +use ic_utils::str::StrEllipsize; use std::{ sync::{Arc, atomic::AtomicU64}, time::{Duration, Instant}, @@ -184,7 +186,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { return; } - let payload = async { + let mut payload = async { // Execute the HTTP request and get the adapter response. let (adapter_response, downloaded_bytes, elapsed) = execute_http_request( &mut http_adapter_client, @@ -269,6 +271,24 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { } .await; + // Truncate an oversized reject message before pricing and gossiping + // it, so the gossip cost reflects what is actually gossiped. + if let Err(reject) = &mut payload + && reject.message.len() > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES + { + warn!( + log, + "Pruning oversized reject message for request {}: \ + original size {}, new size {}", + request_id, + reject.message.len(), + MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, + ); + reject.message = reject + .message + .ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); + } + // Account for the cost of gossiping the final (post-transform) // response to peers before creating the receipt. let response_size = match &payload { @@ -1152,6 +1172,71 @@ mod tests { assert_eq!(client.try_receive(), Err(TryReceiveError::Empty)); } + // Test that an oversized reject message is truncated (char-boundary-safe) + // before being returned, so that what is priced and gossiped is bounded. + #[tokio::test] + async fn test_oversized_reject_message_is_truncated() { + // Adapter mock setup. Not relevant; the transform produces the reject. + let response = HttpsOutcallResponse { + status: 200, + headers: Vec::new(), + content: Vec::new(), + }; + let mock_grpc_channel = setup_adapter_mock(Ok(create_result_from_response(response))).await; + let (svc, mut handle) = setup_system_query_mock(); + + // A multi-byte message whose 1200 bytes exceed the 1024-byte limit, with + // emoji straddling the truncation boundary to exercise char safety. + let oversized_message = "😀".repeat(300); + let oversized_len = oversized_message.len(); + assert!(oversized_len > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES); + // The client must apply exactly this truncation before pricing the response. + let expected_message = oversized_message.ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); + tokio::spawn(async move { + let (_, rsp) = handle.next_request().await.unwrap(); + rsp.send_response(Ok(( + Ok(WasmResult::Reject(oversized_message)), + current_time(), + ))); + }); + + let mut client = CanisterHttpAdapterClientImpl::new( + tokio::runtime::Handle::current(), + mock_grpc_channel, + svc, + 100, + MetricsRegistry::default(), + no_op_logger(), + ); + + assert_eq!( + client.send(build_mock_canister_http_request( + 420, + Some("transform".to_string()) + )), + Ok(()) + ); + loop { + match client.try_receive() { + Err(_) => tokio::time::sleep(Duration::from_millis(10)).await, + Ok((r, _payment_receipt)) => { + let CanisterHttpResponseContent::Reject(reject) = r.content else { + panic!("expected a reject response"); + }; + // Ellipsized exactly as the limit dictates: this pins the size, + // the ellipsize parameters, and char-boundary safety in one check. + assert_eq!( + reject.message, expected_message, + "reject message should be ellipsized to the allowed size" + ); + assert!(reject.message.len() <= MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES); + assert!(reject.message.len() < oversized_len); + break; + } + } + } + } + // Test client capacity. The capicity of the client is specified by the channel size. #[tokio::test] async fn test_client_at_capacity() { diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index 9f8d9b466ac2..b069c013c4a5 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -26,7 +26,6 @@ use ic_types::{ consensus::HasHeight, crypto::Signed, messages::CallbackId, replica_config::ReplicaConfig, }; use ic_types_cycles::CanisterCyclesCostSchedule; -use ic_utils::str::StrEllipsize; use std::{ cell::RefCell, collections::{BTreeSet, HashSet}, @@ -62,7 +61,6 @@ pub struct CanisterHttpPoolManagerImpl { log: ReplicaLogger, } -const MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES: usize = 1024; // 1KB const CANDID_OVERHEAD_RESERVE_BYTES: u64 = 1024; // 1KB // Checks that the response size is within the allowed limits. @@ -384,7 +382,7 @@ impl CanisterHttpPoolManagerImpl { loop { match self.http_adapter_shim.lock().unwrap().try_receive() { Err(TryReceiveError::Empty) => break, - Ok((mut response, payment_receipt)) => { + Ok((response, payment_receipt)) => { // Drop the response if its context is no longer present in the replicated state // (e.g. the request has timed out or has already been answered by enough other nodes). let Some(context) = active_contexts.get(&response.id) else { @@ -398,28 +396,6 @@ impl CanisterHttpPoolManagerImpl { continue; }; - // Truncate the reject message if it's too long. - // - // The "happy path" response is organically bounded by max_response_bytes, however we need to set a - // limit for the error message as well. - // - // The current limit is 1KB, which should be reasonable for an error message. - if let CanisterHttpResponseContent::Reject(reject) = &mut response.content - && reject.message.len() > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES - { - let original_len = reject.message.len(); - warn!( - self.log, - "Pruning oversized reject message for request ID {}. Original size: {}, New size: {}", - response.id, - original_len, - MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES - ); - reject.message = reject - .message - .ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); - } - let receipt_share = CanisterHttpResponseReceipt { metadata: CanisterHttpResponseMetadata { id: response.id, @@ -1881,258 +1857,6 @@ pub mod test { }); } - #[test] - fn test_oversized_reject_message_is_pruned_not_invalidated() { - ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - with_test_replica_logger(|log| { - // 1. SETUP: Standard dependencies. - let Dependencies { - pool, - replica_config, - crypto, - state_manager, - registry, - .. - } = dependencies(pool_config.clone(), 4); - - let callback_id = CallbackId::from(5); - let delegated_node_id = ic_test_utilities_types::ids::node_test_id(1); - - // 2. CONTEXT: Set up a NonReplicated request context in the state manager. - // This ensures we test the gossiping code path. - let request_context = test_request_context( - Replication::NonReplicated(delegated_node_id), - PricingVersion::Legacy, - None, - ); - state_manager - .get_mut() - .expect_get_latest_state() - .return_const(Labeled::new( - Height::from(1), - Arc::new(state_with_pending_http_calls(BTreeMap::from([( - callback_id, - request_context, - )]))), - )); - - // 3. OVERSIZED RESPONSE: Define an error message that is intentionally too large. - let oversized_len = MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES + 100; - let max_len = MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES; - let oversized_message = "a".repeat(oversized_len); - - let oversized_response = CanisterHttpResponse { - id: callback_id, - content: CanisterHttpResponseContent::Reject(CanisterHttpReject { - reject_code: RejectCode::SysFatal, - message: oversized_message, - }), - ..empty_canister_http_response(callback_id.get()) - }; - - // 4. MOCK ADAPTER: Mock the adapter to return the oversized response once. - let mut shim_mock = MockNonBlockingChannel::::new(); - let mut sequence = Sequence::new(); - shim_mock - .expect_try_receive() - .times(1) - .returning(move || { - Ok(( - oversized_response.clone(), - CanisterHttpPaymentReceipt::default(), - )) - }) - .in_sequence(&mut sequence); - shim_mock - .expect_try_receive() - .times(1) - .returning(|| Err(TryReceiveError::Empty)) - .in_sequence(&mut sequence); - - let shim: Arc> = - Arc::new(Mutex::new(Box::new(shim_mock))); - - let pool_manager = CanisterHttpPoolManagerImpl::new( - state_manager, - shim, - crypto, - pool.get_cache(), - replica_config, - SubnetType::Application, - Arc::clone(®istry) as Arc<_>, - MetricsRegistry::new(), - log, - ); - - // 5. ACTION: Call the function to generate shares from responses. - let change_set = pool_manager.create_shares_from_responses(Height::from(1)); - - // 6. ASSERTIONS: - assert_eq!( - change_set.len(), - 1, - "A change action should have been created" - ); - - // Check that the action contains the *pruned* response and a share with the correct hash. - assert_matches!( - &change_set[0], - CanisterHttpChangeAction::AddToValidatedAndGossipResponse(share, response) => { - // Assert that the response message was indeed truncated. - let pruned_message_len = if let CanisterHttpResponseContent::Reject(r) = &response.content { - r.message.len() - } else { - panic!("Test failed: Expected a Reject response content"); - }; - assert_eq!( - pruned_message_len, max_len, - "The reject message should have been truncated to the maximum allowed length" - ); - - // Assert that the hash in the share matches the hash of the *truncated* response. - let expected_hash = ic_types::crypto::crypto_hash(response); - assert_eq!( - share.content.content_hash(), &expected_hash, - "The share's content hash must match the pruned response" - ); - } - ); - }) - }); - } - - #[test] - fn test_oversized_reject_with_multibyte_char_is_pruned_safely() { - // This test ensures that when pruning an oversized reject message, - // the logic correctly handles multi-byte UTF-8 characters that - // straddle the byte limit, preventing a panic that a simple `truncate()` would cause. - ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - with_test_replica_logger(|log| { - // 1. SETUP: Standard dependencies. - let Dependencies { - pool, - replica_config, - crypto, - state_manager, - registry, - .. - } = dependencies(pool_config.clone(), 4); - - let callback_id = CallbackId::from(0); // Use ID that will pass validation filter - let delegated_node_id = ic_test_utilities_types::ids::node_test_id(1); - - // 2. CONTEXT: Set up a NonReplicated request context. - let request_context = test_request_context( - Replication::NonReplicated(delegated_node_id), - PricingVersion::Legacy, - None, - ); - state_manager - .get_mut() - .expect_get_latest_state() - .return_const(Labeled::new( - Height::from(1), - Arc::new(state_with_pending_http_calls(BTreeMap::from([( - callback_id, - request_context, - )]))), - )); - - // 3. MALICIOUS RESPONSE: Construct a string where a 4-byte emoji ('👍') - // starts 2 bytes before the limit, causing it to cross the boundary. - let max_len = MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES; - let padding = "a".repeat(max_len - 2); - let emoji = "👍"; // A 4-byte character - let oversized_message = format!("{}{}", padding, emoji); - - // Verify the setup: the message is oversized and the emoji crosses the boundary. - assert!(oversized_message.len() > max_len); - assert_eq!(padding.len(), max_len - 2); - - let oversized_response = CanisterHttpResponse { - id: callback_id, - content: CanisterHttpResponseContent::Reject(CanisterHttpReject { - reject_code: RejectCode::SysFatal, - message: oversized_message, - }), - ..empty_canister_http_response(callback_id.get()) - }; - - // 4. MOCK ADAPTER: Mock the adapter to return this specific response. - let mut shim_mock = MockNonBlockingChannel::::new(); - shim_mock - .expect_try_receive() - .times(1) - .return_once(move || { - Ok(( - oversized_response.clone(), - CanisterHttpPaymentReceipt::default(), - )) - }); - shim_mock - .expect_try_receive() - .return_const(Err(TryReceiveError::Empty)); - - let shim: Arc> = - Arc::new(Mutex::new(Box::new(shim_mock))); - - let pool_manager = CanisterHttpPoolManagerImpl::new( - state_manager, - shim, - crypto, - pool.get_cache(), - replica_config, - SubnetType::Application, - Arc::clone(®istry) as Arc<_>, - MetricsRegistry::new(), - log, - ); - - // 5. ACTION: Call the function. The test will fail with a panic if - // the pruning logic is incorrect (i.e., not UTF-8 aware). - let change_set = pool_manager.create_shares_from_responses(Height::from(1)); - - // 6. ASSERTIONS: - assert_eq!( - change_set.len(), - 1, - "A change action should have been created" - ); - - assert_matches!( - &change_set[0], - CanisterHttpChangeAction::AddToValidatedAndGossipResponse(share, response) => { - let pruned_message = if let CanisterHttpResponseContent::Reject(r) = &response.content { - &r.message - } else { - panic!("Test failed: Expected a Reject response content"); - }; - - // Assert that the pruned message is now within the byte limit. - assert!( - pruned_message.len() <= max_len, - "The pruned message (len: {}) should not exceed the max length ({})", - pruned_message.len(), max_len - ); - - // Assert the emoji is still present because ellipsize preserves the end of the string. - assert!( - pruned_message.contains(emoji), - "Pruned message should still contain the emoji from the suffix" - ); - - // Assert that the hash in the share matches the hash of the now-pruned response. - let expected_hash = ic_types::crypto::crypto_hash(response); - assert_eq!( - share.content.content_hash(), &expected_hash, - "The share's content hash must match the pruned response" - ); - } - ); - }) - }); - } - #[test] fn test_dishonest_oversized_reject_is_invalidated() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index 6f66cb7521fc..0b4265802d67 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -90,6 +90,9 @@ pub const MAX_CANISTER_HTTP_REQUEST_BYTES: u64 = 2_000_000; /// Maximum number of response bytes for a canister http request. pub const MAX_CANISTER_HTTP_RESPONSE_BYTES: u64 = 2_000_000; +/// Maximum size of a canister http reject message. +pub const MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES: usize = 1024; // 1KB + /// Maximum number of bytes to represent URL for a canister http request. pub const MAX_CANISTER_HTTP_URL_SIZE: usize = 8192; From ed19c451e990bf767f2be6e826af986556aa06cd Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Fri, 26 Jun 2026 13:07:27 +0000 Subject: [PATCH 14/36] Automatically updated Cargo*.lock --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index c1a31854977e..958041a9dfd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10247,6 +10247,7 @@ dependencies = [ "ic-test-utilities-time", "ic-test-utilities-types", "ic-types", + "ic-utils 0.9.0", "prometheus", "slog", "tokio", From cc6be298bcb2cf15054721b1a18798e34477e862 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 13:16:00 +0000 Subject: [PATCH 15/36] import --- rs/https_outcalls/consensus/BUILD.bazel | 2 -- rs/https_outcalls/consensus/Cargo.toml | 1 - 2 files changed, 3 deletions(-) diff --git a/rs/https_outcalls/consensus/BUILD.bazel b/rs/https_outcalls/consensus/BUILD.bazel index e0c353fd4870..dab8b0b219cb 100644 --- a/rs/https_outcalls/consensus/BUILD.bazel +++ b/rs/https_outcalls/consensus/BUILD.bazel @@ -32,7 +32,6 @@ rust_library( "//rs/types/cycles", "//rs/types/management_canister_types", "//rs/types/types", - "//rs/utils", "@crate_index//:candid", "@crate_index//:hex", "@crate_index//:prometheus", @@ -80,7 +79,6 @@ rust_test( "//rs/types/cycles", "//rs/types/management_canister_types", "//rs/types/types", - "//rs/utils", "@crate_index//:assert_matches", "@crate_index//:candid", "@crate_index//:hex", diff --git a/rs/https_outcalls/consensus/Cargo.toml b/rs/https_outcalls/consensus/Cargo.toml index a005039b5896..9a21f85adfcc 100644 --- a/rs/https_outcalls/consensus/Cargo.toml +++ b/rs/https_outcalls/consensus/Cargo.toml @@ -24,7 +24,6 @@ ic-registry-subnet-type = { path = "../../registry/subnet_type" } ic-replicated-state = { path = "../../replicated_state" } ic-types = { path = "../../types/types" } ic-types-cycles = { path = "../../types/cycles" } -ic-utils = { path = "../../utils" } prometheus = { workspace = true } rand = { workspace = true } slog = { workspace = true } From 2e7b712a1e3209016d7e7ad76f8b66f464785bef Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Fri, 26 Jun 2026 13:19:49 +0000 Subject: [PATCH 16/36] Automatically updated Cargo*.lock --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 958041a9dfd8..437bbdcb6edf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10294,7 +10294,6 @@ dependencies = [ "ic-test-utilities-types", "ic-types", "ic-types-cycles", - "ic-utils 0.9.0", "mockall", "prometheus", "proptest", From d8ec86a875782ede62b54ab0eb06ce42ce9a4c93 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 13:28:22 +0000 Subject: [PATCH 17/36] count_bytes --- rs/https_outcalls/client/src/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index d6b7476bc91a..0037a6148fbc 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -15,7 +15,7 @@ use ic_logger::{ReplicaLogger, info, warn}; use ic_management_canister_types_private::{CanisterHttpResponsePayload, TransformArgs}; use ic_metrics::MetricsRegistry; use ic_types::{ - CanisterId, NumBytes, NumInstructions, + CanisterId, CountBytes, NumBytes, NumInstructions, canister_http::{ CanisterHttpHeader, CanisterHttpMethod, CanisterHttpPaymentReceipt, CanisterHttpReject, CanisterHttpRequest, CanisterHttpRequestContext, CanisterHttpResponse, @@ -293,7 +293,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { // response to peers before creating the receipt. let response_size = match &payload { Ok(response) => response.len(), - Err(reject) => reject.message.len(), + Err(reject) => reject.count_bytes(), }; let payload = budget .subtract_gossip_usage(NumBytes::from(response_size as u64)) From 5ed23a83569fbe53d11c135bbb348be9de438ea6 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:19:43 +0000 Subject: [PATCH 18/36] consistent CanisterReject --- rs/https_outcalls/client/src/client.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 0037a6148fbc..7eed9270083f 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -298,7 +298,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { let payload = budget .subtract_gossip_usage(NumBytes::from(response_size as u64)) .map_err(|PricingError::InsufficientCycles| CanisterHttpReject { - reject_code: RejectCode::SysFatal, + reject_code: RejectCode::CanisterReject, message: "Insufficient cycles".to_string(), }) .and(payload); @@ -413,7 +413,7 @@ async fn execute_http_request( response_time: elapsed, }) .map_err(|PricingError::InsufficientCycles| CanisterHttpReject { - reject_code: RejectCode::SysFatal, + reject_code: RejectCode::CanisterReject, message: "Insufficient cycles".to_string(), })?; @@ -555,7 +555,7 @@ async fn transform_adapter_response( ); ( Err(CanisterHttpReject { - reject_code: RejectCode::SysFatal, + reject_code: RejectCode::CanisterReject, message: "Insufficient cycles".to_string(), }), instructions_used, From 5132054cabf4fc47708e8e35d656dbf3071af8f3 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:21:20 +0000 Subject: [PATCH 19/36] remove unneeded check --- rs/https_outcalls/consensus/src/pool_manager.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index b069c013c4a5..a47307dacab8 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -260,9 +260,6 @@ impl CanisterHttpPoolManagerImpl { .ok() .flatten()?; let subnet_size = u32::try_from(record.membership.len()).ok()?; - if subnet_size == 0 { - return None; - } let cost_schedule = CanisterCyclesCostScheduleProto::try_from(record.canister_cycles_cost_schedule) .ok() From 35c202a29401c50303a36a5c05848e05abc8e840 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:21:54 +0000 Subject: [PATCH 20/36] MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES --- rs/https_outcalls/client/src/client.rs | 15 ++++++++------- rs/https_outcalls/consensus/src/pool_manager.rs | 12 ++++++------ rs/types/types/src/canister_http.rs | 2 +- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index 7eed9270083f..e09a2c9a3893 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -19,7 +19,7 @@ use ic_types::{ canister_http::{ CanisterHttpHeader, CanisterHttpMethod, CanisterHttpPaymentReceipt, CanisterHttpReject, CanisterHttpRequest, CanisterHttpRequestContext, CanisterHttpResponse, - CanisterHttpResponseContent, MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, Transform, + CanisterHttpResponseContent, MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, Transform, validate_http_headers_and_body, }, ingress::WasmResult, @@ -274,7 +274,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { // Truncate an oversized reject message before pricing and gossiping // it, so the gossip cost reflects what is actually gossiped. if let Err(reject) = &mut payload - && reject.message.len() > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES + && reject.message.len() > MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES { warn!( log, @@ -282,11 +282,11 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { original size {}, new size {}", request_id, reject.message.len(), - MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, + MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, ); reject.message = reject .message - .ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); + .ellipsize(MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, 90); } // Account for the cost of gossiping the final (post-transform) @@ -1189,9 +1189,10 @@ mod tests { // emoji straddling the truncation boundary to exercise char safety. let oversized_message = "😀".repeat(300); let oversized_len = oversized_message.len(); - assert!(oversized_len > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES); + assert!(oversized_len > MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES); // The client must apply exactly this truncation before pricing the response. - let expected_message = oversized_message.ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); + let expected_message = + oversized_message.ellipsize(MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, 90); tokio::spawn(async move { let (_, rsp) = handle.next_request().await.unwrap(); rsp.send_response(Ok(( @@ -1229,7 +1230,7 @@ mod tests { reject.message, expected_message, "reject message should be ellipsized to the allowed size" ); - assert!(reject.message.len() <= MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES); + assert!(reject.message.len() <= MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES); assert!(reject.message.len() < oversized_len); break; } diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index a47307dacab8..6162bc7a6a0c 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -94,9 +94,9 @@ fn validate_response_size( } CanisterHttpResponseContent::Reject(reject) => { let response_size = reject.message.len(); - if response_size > MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES { + if response_size > MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES { Err(format!( - "Reject message size {response_size} exceeds the maximum allowed size of {MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES}" + "Reject message size {response_size} exceeds the maximum allowed size of {MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES}" )) } else { Ok(()) @@ -1794,7 +1794,7 @@ pub mod test { ); // 3. ARTIFACT: Create a Reject response. Its message size is valid - // (i.e., less than MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES), so it should pass + // (i.e., less than MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES), so it should pass // validation despite the context's zero-byte limit for success responses. let mut canister_http_pool = CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); @@ -1890,7 +1890,7 @@ pub mod test { )); // 3. DISHONEST ARTIFACT: - let oversized_len = MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES + 1; + let oversized_len = MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES + 1; let dishonest_response = CanisterHttpResponse { id: callback_id, content: CanisterHttpResponseContent::Reject(CanisterHttpReject { @@ -1958,7 +1958,7 @@ pub mod test { let expected_error = format!( "Http Response for request ID {} is too large: Reject message size {} exceeds the maximum allowed size of {}", - callback_id, oversized_len, MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES + callback_id, oversized_len, MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES ); assert_matches!( &change_set[0], @@ -2030,7 +2030,7 @@ pub mod test { let reject_message = "This error message is definitely longer than 10 bytes.".to_string(); assert!(reject_message.len() as u64 > LOW_MAX_RESPONSE_BYTES); - assert!(reject_message.len() <= MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES); + assert!(reject_message.len() <= MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES); let reject_content = CanisterHttpReject { reject_code: RejectCode::SysFatal, diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index 0b4265802d67..7acfcdd569be 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -91,7 +91,7 @@ pub const MAX_CANISTER_HTTP_REQUEST_BYTES: u64 = 2_000_000; pub const MAX_CANISTER_HTTP_RESPONSE_BYTES: u64 = 2_000_000; /// Maximum size of a canister http reject message. -pub const MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES: usize = 1024; // 1KB +pub const MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES: usize = 1024; // 1KB /// Maximum number of bytes to represent URL for a canister http request. pub const MAX_CANISTER_HTTP_URL_SIZE: usize = 8192; From b8df347aedcdd5f1b97c78eecd187fb98dd39b31 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:25:14 +0000 Subject: [PATCH 21/36] rm diverged/disagreed --- rs/https_outcalls/pricing/src/metrics.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index ea5a7a7d88ac..d08affb41e08 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -1,8 +1,8 @@ use ic_metrics::MetricsRegistry; use prometheus::IntCounterVec; -/// Label identifying the accounting step at which the shadow tracker diverged -/// from the real one. +/// Label identifying the accounting step at which the shadow tracker ran out of cycles +/// before the real one. pub const LABEL_STEP: &str = "step"; /// Label identifying the replication type of the request. @@ -13,8 +13,8 @@ pub struct PricingMetrics { /// Total number of requests evaluated by the dark-launch budget tracker, /// by replication type. pub shadow_requests_total: IntCounterVec, - /// Number of requests for which the shadow tracker disagreed with the real - /// tracker, by the accounting step at which the divergence was first + /// Number of requests for which the shadow tracker ran out of cycles before the + /// real tracker, by the accounting step at which the divergence was first /// observed and by replication type. /// /// The fraction `shadow_incompatible_total / shadow_requests_total` is the From a91a45b3ef23cb841c282cc1be204b2b4ccb6071 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:26:49 +0000 Subject: [PATCH 22/36] comment --- rs/https_outcalls/pricing/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index e04aec342c58..74dd9a4b6ae2 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -39,7 +39,8 @@ pub trait BudgetTracker: Send { /// - This method returns `Ok(())` if and only if `usage <= get_transform_limit()`. fn subtract_transform_usage(&mut self, usage: NumInstructions) -> Result<(), PricingError>; /// Deducts the cost of the final (post-transform) response that this replica - /// produced and that will be gossiped to peers. + /// produced and that will be gossiped to peers. This cost does not apply to fully-replicated + /// requests, which doesn't gossip responses. /// /// This is the last accounting step and is invoked once the size of the /// response is known. From 591ececda428cedf7fd5b8873f44e9f435471bba Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:27:25 +0000 Subject: [PATCH 23/36] error_reported --- rs/https_outcalls/pricing/src/dark_launch.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 2fa8a1f1167e..44de4e272107 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -20,7 +20,7 @@ pub struct DarkLaunchTracker { log: ReplicaLogger, /// Whether an incompatibility has already been recorded for this request. /// Ensures we count and log at most once per request. - reported: bool, + error_reported: bool, } impl DarkLaunchTracker { @@ -39,7 +39,7 @@ impl DarkLaunchTracker { replication, metrics, log, - reported: false, + error_reported: false, } } @@ -54,10 +54,10 @@ impl DarkLaunchTracker { if real.is_ok() == shadow.is_ok() { return; } - if self.reported { + if self.error_reported { return; } - self.reported = true; + self.error_reported = true; self.metrics .shadow_incompatible_total .with_label_values(&[step, self.replication]) From 47738078bbc0571278bffc08f718e76d6c8585b1 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:32:44 +0000 Subject: [PATCH 24/36] is_gossiping --- rs/https_outcalls/pricing/src/payg.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index ca23acaa3293..59dadeb94e85 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -38,8 +38,8 @@ const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; pub struct PayAsYouGoTracker { /// Number of nodes (`N`) on the subnet. subnet_size: NumberOfNodes, - /// Whether this outcall uses flexible pricing. - is_flexible_pricing: bool, + /// Whether this responses to this outcalls are gossiped (only flexible and non-replicated). + is_gossiping: bool, /// Whether the subnet uses a free cost schedule. When `true` the tracker /// charges nothing and refunds the full allowance. is_free: bool, @@ -60,7 +60,7 @@ impl PayAsYouGoTracker { ) -> Self { Self { subnet_size, - is_flexible_pricing: match context.replication { + is_gossiping: match context.replication { // Non-replicated outcalls gossip the response, so they are charged // the same way as flexible outcalls. Replication::Flexible { .. } | Replication::NonReplicated(_) => true, @@ -131,7 +131,7 @@ impl BudgetTracker for PayAsYouGoTracker { // For fully replicated outcalls the gossip term is a // consensus cost (ignored here for now). For flexible outcalls each // replica is charged 50 * transformed_response_bytes_i * N. - if !self.is_flexible_pricing { + if !self.is_gossiping { return Ok(()); } let cost = FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE From 6d32da54b9f9cb6f3a2e570f96b8dc6c0872520c Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:37:47 +0000 Subject: [PATCH 25/36] constant --- rs/https_outcalls/pricing/src/legacy.rs | 8 ++------ rs/https_outcalls/pricing/src/lib.rs | 6 ++++++ rs/https_outcalls/pricing/src/payg.rs | 8 ++------ 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/rs/https_outcalls/pricing/src/legacy.rs b/rs/https_outcalls/pricing/src/legacy.rs index 539299550f62..3cde14d3284d 100644 --- a/rs/https_outcalls/pricing/src/legacy.rs +++ b/rs/https_outcalls/pricing/src/legacy.rs @@ -1,12 +1,10 @@ -use std::time::Duration; - use ic_config::subnet_config::MAX_INSTRUCTIONS_PER_QUERY_MESSAGE; use ic_types::{ NumBytes, NumInstructions, canister_http::{CanisterHttpPaymentReceipt, MAX_CANISTER_HTTP_RESPONSE_BYTES}, }; -use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; +use crate::{AdapterLimits, BudgetTracker, MAX_RESPONSE_TIME, NetworkUsage, PricingError}; pub struct LegacyTracker { max_response_size: NumBytes, @@ -25,9 +23,7 @@ impl BudgetTracker for LegacyTracker { fn get_adapter_limits(&self) -> AdapterLimits { AdapterLimits { max_response_size: self.max_response_size, - // Note: there is already a timeout limit on the server itself (30 seconds, see DEFAULT_HTTP_REQUEST_TIMEOUT_SECS). - // Setting higher than that just to be safe. - max_response_time: Duration::from_secs(60), + max_response_time: MAX_RESPONSE_TIME, } } diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 74dd9a4b6ae2..23da9d1c4895 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -54,6 +54,12 @@ pub trait BudgetTracker: Send { fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt; } +/// The maximum duration the adapter is allowed to take to fully receive a +/// response, as measured by the client. The server already enforces a 30s +/// timeout (see `DEFAULT_HTTP_REQUEST_TIMEOUT_SECS`), so this is a safety margin +/// above it. +pub(crate) const MAX_RESPONSE_TIME: Duration = Duration::from_secs(60); + pub struct AdapterLimits { /// The maximum size of the HTTP response, including the headers and the body. pub max_response_size: NumBytes, diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 59dadeb94e85..9a8d38ad2bce 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -1,5 +1,3 @@ -use std::time::Duration; - use ic_config::subnet_config::MAX_INSTRUCTIONS_PER_QUERY_MESSAGE; use ic_types::{ NumBytes, NumInstructions, NumberOfNodes, @@ -10,7 +8,7 @@ use ic_types::{ }; use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles}; -use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError}; +use crate::{AdapterLimits, BudgetTracker, MAX_RESPONSE_TIME, NetworkUsage, PricingError}; // Per-replica fee constants. // @@ -98,9 +96,7 @@ impl BudgetTracker for PayAsYouGoTracker { fn get_adapter_limits(&self) -> AdapterLimits { AdapterLimits { max_response_size: self.max_response_size, - // Mirror the legacy limit: the server enforces a 30s timeout, so 60s - // here is just a safety margin. - max_response_time: Duration::from_secs(60), + max_response_time: MAX_RESPONSE_TIME, } } From 22c5465fbfd07a6e7583ff72a822d8a7835e81b0 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:38:43 +0000 Subject: [PATCH 26/36] rm for now --- rs/https_outcalls/pricing/src/payg.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 9a8d38ad2bce..7cc2bcbe2020 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -125,7 +125,7 @@ impl BudgetTracker for PayAsYouGoTracker { transformed_response_size: NumBytes, ) -> Result<(), PricingError> { // For fully replicated outcalls the gossip term is a - // consensus cost (ignored here for now). For flexible outcalls each + // consensus cost (ignored here). For flexible outcalls each // replica is charged 50 * transformed_response_bytes_i * N. if !self.is_gossiping { return Ok(()); @@ -153,6 +153,7 @@ mod tests { time::UNIX_EPOCH, }; use std::collections::BTreeSet; + use std::time::Duration; fn context( replication: Replication, From ebd2a9be2c362cece767caa33960f390d8cfcc82 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:43:24 +0000 Subject: [PATCH 27/36] extract helper --- rs/https_outcalls/pricing/src/payg.rs | 80 +++++++++++---------------- 1 file changed, 31 insertions(+), 49 deletions(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 7cc2bcbe2020..722612ef6bb4 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -200,16 +200,38 @@ mod tests { } } + /// Builds a tracker on a `Normal` cost schedule with the given subnet size. + fn make_tracker(ctx: &CanisterHttpRequestContext, subnet_size: u32) -> PayAsYouGoTracker { + PayAsYouGoTracker::new( + ctx, + NumberOfNodes::from(subnet_size), + CanisterCyclesCostSchedule::Normal, + ) + } + + /// Asserts that a gossiping outcall (flexible or non-replicated) charges the + /// gossip term over the full subnet size. + fn assert_gossip_charged_over_subnet_size(replication: Replication) { + let subnet_size = 13_u32; + let ctx = context(replication, 1_000_000_000); + let mut tracker = make_tracker(&ctx, subnet_size); + + let transformed_size = 500_u64; + assert_eq!( + tracker.subtract_gossip_usage(NumBytes::from(transformed_size)), + Ok(()) + ); + let expected = + FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE * transformed_size as u128 * subnet_size as u128; + assert_eq!(tracker.spent, expected); + } + #[test] fn does_not_charge_base_cost() { // The base cost is handled at context creation, so a freshly created // tracker has spent nothing and a zero-usage request refunds everything. let ctx = context(Replication::FullyReplicated, 1_000_000); - let tracker = PayAsYouGoTracker::new( - &ctx, - NumberOfNodes::from(13), - CanisterCyclesCostSchedule::Normal, - ); + let tracker = make_tracker(&ctx, 13); assert_eq!(tracker.spent, 0); assert_eq!( tracker.create_payment_receipt().refund, @@ -221,11 +243,7 @@ mod tests { fn charges_per_replica_cost_fully_replicated() { let allowance = 1_000_000_000_u128; let ctx = context(Replication::FullyReplicated, allowance); - let mut tracker = PayAsYouGoTracker::new( - &ctx, - NumberOfNodes::from(13), - CanisterCyclesCostSchedule::Normal, - ); + let mut tracker = make_tracker(&ctx, 13); let response_size = 1_000_u64; let response_ms = 2_000_u128; @@ -259,57 +277,21 @@ mod tests { #[test] fn charges_gossip_usage_for_flexible() { - let allowance = 1_000_000_000_u128; - let n = 13; - let ctx = context(flexible(n), allowance); - let mut tracker = PayAsYouGoTracker::new( - &ctx, - NumberOfNodes::from(n as u32), - CanisterCyclesCostSchedule::Normal, - ); - - let transformed_size = 500_u64; - assert_eq!( - tracker.subtract_gossip_usage(NumBytes::from(transformed_size)), - Ok(()) - ); - let expected = - FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE * transformed_size as u128 * n as u128; - assert_eq!(tracker.spent, expected); + assert_gossip_charged_over_subnet_size(flexible(13)); } #[test] fn charges_gossip_usage_for_non_replicated() { // Non-replicated outcalls use the same (flexible) pricing as flexible // outcalls, so the gossip term is charged over the full subnet size. - let allowance = 1_000_000_000_u128; - let subnet_size = 13_u32; let node = NodeId::from(PrincipalId::new_node_test_id(0)); - let ctx = context(Replication::NonReplicated(node), allowance); - let mut tracker = PayAsYouGoTracker::new( - &ctx, - NumberOfNodes::from(subnet_size), - CanisterCyclesCostSchedule::Normal, - ); - - let transformed_size = 500_u64; - assert_eq!( - tracker.subtract_gossip_usage(NumBytes::from(transformed_size)), - Ok(()) - ); - let expected = - FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE * transformed_size as u128 * subnet_size as u128; - assert_eq!(tracker.spent, expected); + assert_gossip_charged_over_subnet_size(Replication::NonReplicated(node)); } #[test] fn returns_pricing_error_when_budget_is_exceeded() { let ctx = context(Replication::FullyReplicated, 100); - let mut tracker = PayAsYouGoTracker::new( - &ctx, - NumberOfNodes::from(13), - CanisterCyclesCostSchedule::Normal, - ); + let mut tracker = make_tracker(&ctx, 13); assert_eq!( tracker.subtract_network_usage(NetworkUsage { response_size: NumBytes::from(1_000), From 608b767688ee8aa9cee78d736e6d8fc3fcfe184e Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 18:43:59 +0000 Subject: [PATCH 28/36] comment --- rs/https_outcalls/pricing/src/payg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 722612ef6bb4..905459f73221 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -307,7 +307,7 @@ mod tests { // On a free subnet the tracker charges nothing, even for usage that // would otherwise exceed the allowance, so the full allowance is // refunded. A flexible request is used so the gossip term (which would - // normally be charged) is also exercised. + // not be charged for fully-replicated requests) is also exercised. let allowance = 1_000_000_u128; let ctx = context(flexible(13), allowance); let mut tracker = PayAsYouGoTracker::new( From 81c0f8194fcb5c89b57a32f6db7a4d9097b54640 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Fri, 26 Jun 2026 19:20:33 +0000 Subject: [PATCH 29/36] metric --- rs/https_outcalls/client/src/client.rs | 9 ++- rs/https_outcalls/client/src/metrics.rs | 3 +- rs/https_outcalls/pricing/src/dark_launch.rs | 74 ++++++-------------- rs/https_outcalls/pricing/src/lib.rs | 15 +--- rs/https_outcalls/pricing/src/metrics.rs | 18 +---- rs/types/types/src/canister_http.rs | 27 +++++++ 6 files changed, 65 insertions(+), 81 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index e09a2c9a3893..aa455fdbfd92 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -158,6 +158,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { http_method: request_http_method, transform: request_transform, pricing_version: request_pricing_version, + replication: request_replication, .. } = request_context; @@ -306,6 +307,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { // Create the payment receipt after all processing is complete. let receipt = budget.create_payment_receipt(); + let replication = request_replication.kind(); permit.send(( CanisterHttpResponse { id: request_id, @@ -314,7 +316,11 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { Ok(resp) => { metrics .request_total - .with_label_values(&["success", request_http_method.as_str()]) + .with_label_values(&[ + "success", + request_http_method.as_str(), + replication.as_str(), + ]) .inc(); CanisterHttpResponseContent::Success(resp) } @@ -324,6 +330,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { .with_label_values(&[ reject.reject_code.as_str(), request_http_method.as_str(), + replication.as_str(), ]) .inc(); CanisterHttpResponseContent::Reject(reject) diff --git a/rs/https_outcalls/client/src/metrics.rs b/rs/https_outcalls/client/src/metrics.rs index fad3eae4dedd..ff82b7bcb9d9 100644 --- a/rs/https_outcalls/client/src/metrics.rs +++ b/rs/https_outcalls/client/src/metrics.rs @@ -5,6 +5,7 @@ use prometheus::{Histogram, HistogramVec, IntCounterVec}; const LABEL_STATUS_CODE: &str = "status_code"; const LABEL_HTTP_METHOD: &str = "http_method"; const LABEL_STATUS: &str = "status"; +const LABEL_REPLICATION: &str = "replication"; #[derive(Clone)] pub struct Metrics { @@ -35,7 +36,7 @@ impl Metrics { request_total: metrics_registry.int_counter_vec( "canister_http_requests_total", "Canister http request results returned to consensus.", - &[LABEL_STATUS, LABEL_HTTP_METHOD], + &[LABEL_STATUS, LABEL_HTTP_METHOD, LABEL_REPLICATION], ), } } diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 44de4e272107..81e922df90cb 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -1,5 +1,8 @@ use ic_logger::{ReplicaLogger, warn}; -use ic_types::{CanisterId, NumBytes, NumInstructions, canister_http::CanisterHttpPaymentReceipt}; +use ic_types::{ + CanisterId, NumBytes, NumInstructions, + canister_http::{CanisterHttpPaymentReceipt, ReplicationKind}, +}; use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError, metrics::PricingMetrics}; @@ -15,7 +18,7 @@ pub struct DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, - replication: &'static str, + replication: ReplicationKind, metrics: PricingMetrics, log: ReplicaLogger, /// Whether an incompatibility has already been recorded for this request. @@ -28,7 +31,7 @@ impl DarkLaunchTracker { real: Box, shadow: Box, canister_id: CanisterId, - replication: &'static str, + replication: ReplicationKind, metrics: PricingMetrics, log: ReplicaLogger, ) -> Self { @@ -60,7 +63,7 @@ impl DarkLaunchTracker { self.error_reported = true; self.metrics .shadow_incompatible_total - .with_label_values(&[step, self.replication]) + .with_label_values(&[step, self.replication.as_str()]) .inc(); warn!( self.log, @@ -68,7 +71,7 @@ impl DarkLaunchTracker { canister_id {}, step {}, replication {}, real_result {:?}, shadow_result {:?}", self.canister_id, step, - self.replication, + self.replication.as_str(), real, shadow, ); @@ -109,12 +112,6 @@ impl BudgetTracker for DarkLaunchTracker { } fn create_payment_receipt(&self) -> CanisterHttpPaymentReceipt { - // Count every request that reaches the final accounting step so the - // incompatible counter can be expressed as a fraction of the total. - self.metrics - .shadow_requests_total - .with_label_values(&[self.replication]) - .inc(); self.real.create_payment_receipt() } } @@ -170,7 +167,7 @@ mod tests { fn dark_launch( real: FakeTracker, shadow: FakeTracker, - replication: &'static str, + replication: ReplicationKind, metrics: PricingMetrics, ) -> DarkLaunchTracker { DarkLaunchTracker::new( @@ -213,7 +210,7 @@ mod tests { let mut tracker = dark_launch( FakeTracker::ok(), shadow, - "fully_replicated", + ReplicationKind::FullyReplicated, metrics.clone(), ); @@ -226,15 +223,6 @@ mod tests { .get(), 1 ); - - let _ = tracker.create_payment_receipt(); - assert_eq!( - metrics - .shadow_requests_total - .with_label_values(&["fully_replicated"]) - .get(), - 1 - ); } #[test] @@ -245,7 +233,12 @@ mod tests { transform: Err(PricingError::InsufficientCycles), transformed: Err(PricingError::InsufficientCycles), }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, "flexible", metrics.clone()); + let mut tracker = dark_launch( + FakeTracker::ok(), + shadow, + ReplicationKind::Flexible, + metrics.clone(), + ); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); assert_eq!( @@ -264,7 +257,7 @@ mod tests { let mut tracker = dark_launch( FakeTracker::ok(), FakeTracker::ok(), - "fully_replicated", + ReplicationKind::FullyReplicated, metrics.clone(), ); @@ -273,16 +266,7 @@ mod tests { tracker.subtract_transform_usage(NumInstructions::from(0)), Ok(()) ); - let _ = tracker.create_payment_receipt(); - assert_eq!(incompatible_count(&metrics), 0); - assert_eq!( - metrics - .shadow_requests_total - .with_label_values(&["fully_replicated"]) - .get(), - 1 - ); } #[test] @@ -292,7 +276,12 @@ mod tests { network: Err(PricingError::InsufficientCycles), ..FakeTracker::ok() }; - let mut tracker = dark_launch(FakeTracker::ok(), shadow, "non_replicated", metrics.clone()); + let mut tracker = dark_launch( + FakeTracker::ok(), + shadow, + ReplicationKind::NonReplicated, + metrics.clone(), + ); assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); @@ -311,22 +300,5 @@ mod tests { .get(), 0 ); - - // The total is attributed to the same label, so per-type rates are exact. - let _ = tracker.create_payment_receipt(); - assert_eq!( - metrics - .shadow_requests_total - .with_label_values(&["non_replicated"]) - .get(), - 1 - ); - assert_eq!( - metrics - .shadow_requests_total - .with_label_values(&["fully_replicated"]) - .get(), - 0 - ); } } diff --git a/rs/https_outcalls/pricing/src/lib.rs b/rs/https_outcalls/pricing/src/lib.rs index 23da9d1c4895..c1b5f555f826 100644 --- a/rs/https_outcalls/pricing/src/lib.rs +++ b/rs/https_outcalls/pricing/src/lib.rs @@ -9,9 +9,7 @@ use ic_logger::ReplicaLogger; use ic_metrics::MetricsRegistry; use ic_types::{ NumBytes, NumInstructions, NumberOfNodes, - canister_http::{ - CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion, Replication, - }, + canister_http::{CanisterHttpPaymentReceipt, CanisterHttpRequestContext, PricingVersion}, }; pub use ic_types_cycles::CanisterCyclesCostSchedule; @@ -105,7 +103,7 @@ impl PricingFactory { Box::new(LegacyTracker::new(context.max_response_bytes)), Box::new(PayAsYouGoTracker::new(context, subnet_size, cost_schedule)), context.request.sender, - replication_label(&context.replication), + context.replication.kind(), self.metrics.clone(), self.log.clone(), )), @@ -115,12 +113,3 @@ impl PricingFactory { } } } - -/// Returns the metric label for a request's replication type. -fn replication_label(replication: &Replication) -> &'static str { - match replication { - Replication::FullyReplicated => "fully_replicated", - Replication::Flexible { .. } => "flexible", - Replication::NonReplicated(_) => "non_replicated", - } -} diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index d08affb41e08..bef4a8b21a6c 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -5,36 +5,24 @@ use prometheus::IntCounterVec; /// before the real one. pub const LABEL_STEP: &str = "step"; -/// Label identifying the replication type of the request. +/// Label identifying the replication kind of the request. pub const LABEL_REPLICATION: &str = "replication"; #[derive(Clone)] pub struct PricingMetrics { - /// Total number of requests evaluated by the dark-launch budget tracker, - /// by replication type. - pub shadow_requests_total: IntCounterVec, /// Number of requests for which the shadow tracker ran out of cycles before the /// real tracker, by the accounting step at which the divergence was first - /// observed and by replication type. - /// - /// The fraction `shadow_incompatible_total / shadow_requests_total` is the - /// share of requests that would NOT be backwards compatible. + /// observed and by replication kind. pub shadow_incompatible_total: IntCounterVec, } impl PricingMetrics { pub fn new(metrics_registry: &MetricsRegistry) -> Self { Self { - shadow_requests_total: metrics_registry.int_counter_vec( - "canister_http_pricing_shadow_requests_total", - "Total canister http requests evaluated by the dark-launch budget tracker, by \ - replication type.", - &[LABEL_REPLICATION], - ), shadow_incompatible_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_incompatible_total", "Canister http requests for which the shadow tracker disagreed with the real \ - tracker, by accounting step and replication type.", + tracker, by accounting step and replication kind.", &[LABEL_STEP, LABEL_REPLICATION], ), } diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index 7acfcdd569be..4e16ac9cceda 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -194,6 +194,33 @@ impl Replication { Replication::Flexible { committee, .. } => committee.contains(signer), } } + + /// Returns the [`ReplicationKind`] of this request. + pub fn kind(&self) -> ReplicationKind { + match self { + Replication::FullyReplicated => ReplicationKind::FullyReplicated, + Replication::Flexible { .. } => ReplicationKind::Flexible, + Replication::NonReplicated(_) => ReplicationKind::NonReplicated, + } + } +} + +/// The kind of replication of a request. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ReplicationKind { + FullyReplicated, + Flexible, + NonReplicated, +} + +impl ReplicationKind { + pub fn as_str(&self) -> &'static str { + match self { + ReplicationKind::FullyReplicated => "fully_replicated", + ReplicationKind::Flexible => "flexible", + ReplicationKind::NonReplicated => "non_replicated", + } + } } #[derive(Clone, Eq, PartialEq, Hash, Debug, Deserialize, Serialize, FromRepr)] From 8e46e6ba5dd21f56a96dfb81d9b24ee20b7422a4 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 08:59:27 +0000 Subject: [PATCH 30/36] remove disagree/divergence --- rs/https_outcalls/pricing/src/dark_launch.rs | 21 ++++++++++---------- rs/https_outcalls/pricing/src/metrics.rs | 6 +++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 81e922df90cb..85205e0e7692 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -11,9 +11,9 @@ use crate::{AdapterLimits, BudgetTracker, NetworkUsage, PricingError, metrics::P /// ones that affect observable behaviour), and a `shadow` tracker whose results /// are merely compared against the real one. /// -/// Whenever the shadow tracker disagrees with the real tracker (e.g. it returns -/// a pricing error where the real tracker succeeded), the divergence is counted -/// in a metric. +/// A counter metric is increased whenever the shadow tracker computes a different +/// result than the real tracker. This could happen if the shadow tracker returns +/// a pricing error where the real tracker succeeded, meaning it ran out of cycles. pub struct DarkLaunchTracker { real: Box, shadow: Box, @@ -47,7 +47,8 @@ impl DarkLaunchTracker { } /// Compares the results of the real and shadow trackers for a given - /// accounting `step` and records a divergence if they disagree. + /// accounting `step` and increment the shadow_incompatible_total metric + /// if they differ. fn compare( &mut self, step: &str, @@ -201,7 +202,7 @@ mod tests { } #[test] - fn returns_real_result_and_counts_divergence() { + fn returns_real_result_and_increments_counter() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); let shadow = FakeTracker { network: Err(PricingError::InsufficientCycles), @@ -226,7 +227,7 @@ mod tests { } #[test] - fn counts_divergence_at_most_once_per_request() { + fn counts_incompatible_requests_at_most_once_per_request() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); let shadow = FakeTracker { network: Err(PricingError::InsufficientCycles), @@ -247,12 +248,12 @@ mod tests { ); assert_eq!(tracker.subtract_gossip_usage(NumBytes::from(0)), Ok(())); - // Only the first divergence is recorded for the request. + // Only the first occurance is recorded for the request. assert_eq!(incompatible_count(&metrics), 1); } #[test] - fn no_divergence_when_results_agree() { + fn do_not_increase_counter_when_results_agree() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); let mut tracker = dark_launch( FakeTracker::ok(), @@ -270,7 +271,7 @@ mod tests { } #[test] - fn labels_divergence_by_replication_type() { + fn labels_incompatibility_by_replication_type() { let metrics = PricingMetrics::new(&MetricsRegistry::new()); let shadow = FakeTracker { network: Err(PricingError::InsufficientCycles), @@ -285,7 +286,7 @@ mod tests { assert_eq!(tracker.subtract_network_usage(network_usage()), Ok(())); - // The divergence is attributed to the non_replicated label. + // The incompatibility is attributed to the non_replicated label. assert_eq!( metrics .shadow_incompatible_total diff --git a/rs/https_outcalls/pricing/src/metrics.rs b/rs/https_outcalls/pricing/src/metrics.rs index bef4a8b21a6c..f4565d03f843 100644 --- a/rs/https_outcalls/pricing/src/metrics.rs +++ b/rs/https_outcalls/pricing/src/metrics.rs @@ -11,7 +11,7 @@ pub const LABEL_REPLICATION: &str = "replication"; #[derive(Clone)] pub struct PricingMetrics { /// Number of requests for which the shadow tracker ran out of cycles before the - /// real tracker, by the accounting step at which the divergence was first + /// real tracker, by the accounting step at which the incompatibility was first /// observed and by replication kind. pub shadow_incompatible_total: IntCounterVec, } @@ -21,8 +21,8 @@ impl PricingMetrics { Self { shadow_incompatible_total: metrics_registry.int_counter_vec( "canister_http_pricing_shadow_incompatible_total", - "Canister http requests for which the shadow tracker disagreed with the real \ - tracker, by accounting step and replication kind.", + "Canister http requests that attached enough cycles to be compatible with the + real tracker, but not for the shadow tracker.", &[LABEL_STEP, LABEL_REPLICATION], ), } From a89e86b1632be4f2a73346929bc178a34b1af47f Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 09:00:49 +0000 Subject: [PATCH 31/36] typo --- rs/https_outcalls/pricing/src/payg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 905459f73221..79abb0643199 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -36,7 +36,7 @@ const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; pub struct PayAsYouGoTracker { /// Number of nodes (`N`) on the subnet. subnet_size: NumberOfNodes, - /// Whether this responses to this outcalls are gossiped (only flexible and non-replicated). + /// Whether responses to this outcalls are gossiped (only flexible and non-replicated). is_gossiping: bool, /// Whether the subnet uses a free cost schedule. When `true` the tracker /// charges nothing and refunds the full allowance. From eaeaa56964f0629b0757dacce654ac6f64b6e2ff Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 12:02:11 +0000 Subject: [PATCH 32/36] clarify ellipsize test --- rs/https_outcalls/client/src/client.rs | 51 ++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index aa455fdbfd92..0e072520f956 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -1192,14 +1192,31 @@ mod tests { let mock_grpc_channel = setup_adapter_mock(Ok(create_result_from_response(response))).await; let (svc, mut handle) = setup_system_query_mock(); - // A multi-byte message whose 1200 bytes exceed the 1024-byte limit, with - // emoji straddling the truncation boundary to exercise char safety. - let oversized_message = "😀".repeat(300); + // 300 four-byte emoji (1200 bytes) followed by a single one-byte 'x' + // (1201 bytes total). `ellipsize` keeps a prefix + "..." + suffix; the + // trailing byte makes the total length not a multiple of 4 so that the + // both suffix and prefix cut, fall *inside* an emoji. + const PREFIX_PERCENTAGE: usize = 90; + let oversized_message = format!("{}x", "😀".repeat(300)); let oversized_len = oversized_message.len(); assert!(oversized_len > MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES); + + // The exact byte offsets `ellipsize` cuts at. + let budget = MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES - "...".len(); + let prefix_cut = MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES * PREFIX_PERCENTAGE / 100; + let suffix_cut = oversized_len - (budget - prefix_cut); + assert!( + !oversized_message.is_char_boundary(prefix_cut), + "prefix cut at byte {prefix_cut} must fall inside a multi-byte emoji" + ); + assert!( + !oversized_message.is_char_boundary(suffix_cut), + "suffix cut at byte {suffix_cut} must fall inside a multi-byte emoji" + ); + // The client must apply exactly this truncation before pricing the response. - let expected_message = - oversized_message.ellipsize(MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, 90); + let expected_message = oversized_message + .ellipsize(MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES, PREFIX_PERCENTAGE); tokio::spawn(async move { let (_, rsp) = handle.next_request().await.unwrap(); rsp.send_response(Ok(( @@ -1231,14 +1248,34 @@ mod tests { let CanisterHttpResponseContent::Reject(reject) = r.content else { panic!("expected a reject response"); }; - // Ellipsized exactly as the limit dictates: this pins the size, - // the ellipsize parameters, and char-boundary safety in one check. + // Ellipsized exactly as the limit dictates: this pins the size + // and the ellipsize parameters. assert_eq!( reject.message, expected_message, "reject message should be ellipsized to the allowed size" ); assert!(reject.message.len() <= MAXIMUM_CANISTER_HTTP_ERROR_MESSAGE_BYTES); assert!(reject.message.len() < oversized_len); + + // Although both cuts fell inside an emoji (asserted above), the + // returned message is well-formed: no emoji was split. Every + // character retained on either side of the ellipsis is a whole + // emoji (plus the preserved trailing 'x' at the very end). + let (head, tail) = reject + .message + .split_once("...") + .expect("ellipsized message must contain the ellipsis"); + assert!( + !head.is_empty() && head.chars().all(|c| c == '😀'), + "prefix must consist of whole emoji, got {head:?}" + ); + assert!( + tail.strip_suffix('x') + .expect("trailing byte should be preserved") + .chars() + .all(|c| c == '😀'), + "suffix must be whole emoji followed by the trailing byte, got {tail:?}" + ); break; } } From f8d302f1a8329a4f38cb4cf8965650a0a86767ed Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 12:10:00 +0000 Subject: [PATCH 33/36] add todo --- rs/https_outcalls/pricing/src/payg.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index 79abb0643199..c8a8d74de530 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -94,6 +94,7 @@ impl PayAsYouGoTracker { impl BudgetTracker for PayAsYouGoTracker { fn get_adapter_limits(&self) -> AdapterLimits { + // TODO: Adjust limits based on remaining budget. AdapterLimits { max_response_size: self.max_response_size, max_response_time: MAX_RESPONSE_TIME, @@ -112,6 +113,7 @@ impl BudgetTracker for PayAsYouGoTracker { } fn get_transform_limit(&self) -> NumInstructions { + // TODO: Adjust limits based on remaining budget. MAX_INSTRUCTIONS_PER_QUERY_MESSAGE } From 63ee45a609074c75770f578e1ccdb035663e1169 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 12:28:40 +0000 Subject: [PATCH 34/36] add TODO for reference subnet size --- rs/https_outcalls/pricing/src/payg.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/https_outcalls/pricing/src/payg.rs b/rs/https_outcalls/pricing/src/payg.rs index c8a8d74de530..5b38aba2c377 100644 --- a/rs/https_outcalls/pricing/src/payg.rs +++ b/rs/https_outcalls/pricing/src/payg.rs @@ -30,6 +30,7 @@ use crate::{AdapterLimits, BudgetTracker, MAX_RESPONSE_TIME, NetworkUsage, Prici // + 50 * transformed_response_bytes_i * N + transform_instructions_i / 13 const PER_DOWNLOADED_BYTE_FEE: u128 = 50; const PER_RESPONSE_MS_FEE: u128 = 300; +// TODO: Determine and use the reference subnet size instead. const TRANSFORM_INSTRUCTION_DIVISOR: u128 = 13; const FLEXIBLE_PER_TRANSFORMED_BYTE_NODE_FEE: u128 = 50; From 50efb978028a3f7317318872cdd3d2768b15a264 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 12:32:25 +0000 Subject: [PATCH 35/36] adjust condition --- rs/https_outcalls/pricing/src/dark_launch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 85205e0e7692..3b11d26ba2b9 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -55,7 +55,7 @@ impl DarkLaunchTracker { real: &Result<(), PricingError>, shadow: &Result<(), PricingError>, ) { - if real.is_ok() == shadow.is_ok() { + if shadow.is_ok() { return; } if self.error_reported { From a7b8261c7b3a8ce3a3b2a850d69d30b4e9ac7fb1 Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 6 Jul 2026 12:35:30 +0000 Subject: [PATCH 36/36] fix --- rs/https_outcalls/pricing/src/dark_launch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/https_outcalls/pricing/src/dark_launch.rs b/rs/https_outcalls/pricing/src/dark_launch.rs index 3b11d26ba2b9..cab05b73617a 100644 --- a/rs/https_outcalls/pricing/src/dark_launch.rs +++ b/rs/https_outcalls/pricing/src/dark_launch.rs @@ -55,7 +55,7 @@ impl DarkLaunchTracker { real: &Result<(), PricingError>, shadow: &Result<(), PricingError>, ) { - if shadow.is_ok() { + if real.is_err() || shadow.is_ok() { return; } if self.error_reported {