diff --git a/rs/config/src/subnet_config.rs b/rs/config/src/subnet_config.rs index efdb882715fb..d1cc7f78a102 100644 --- a/rs/config/src/subnet_config.rs +++ b/rs/config/src/subnet_config.rs @@ -461,6 +461,12 @@ pub struct CyclesAccountManagerConfig { /// The default value of the reserved balance limit for the case when the /// canister doesn't have it set in the settings. pub default_reserved_balance_limit: Cycles, + + /// Base fee for fetching canister logs. + pub fetch_canister_logs_base_fee: Cycles, + + /// Fee per byte for fetching canister logs. + pub fetch_canister_logs_per_byte_fee: Cycles, } impl CyclesAccountManagerConfig { @@ -497,6 +503,8 @@ impl CyclesAccountManagerConfig { http_response_per_byte_fee: Cycles::new(800), max_storage_reservation_period: Duration::from_secs(300_000_000), default_reserved_balance_limit: DEFAULT_RESERVED_BALANCE_LIMIT, + fetch_canister_logs_base_fee: Cycles::new(1_000_000), + fetch_canister_logs_per_byte_fee: Cycles::new(800), } } @@ -537,6 +545,8 @@ impl CyclesAccountManagerConfig { // This effectively disables the storage reservation mechanism on system subnets. max_storage_reservation_period: Duration::from_secs(0), default_reserved_balance_limit: DEFAULT_RESERVED_BALANCE_LIMIT, + fetch_canister_logs_base_fee: Cycles::new(0), + fetch_canister_logs_per_byte_fee: Cycles::new(0), } } @@ -563,6 +573,8 @@ impl CyclesAccountManagerConfig { http_response_per_byte_fee: Cycles::zero(), max_storage_reservation_period: Duration::from_secs(u64::MAX), default_reserved_balance_limit: Cycles::zero(), + fetch_canister_logs_base_fee: Cycles::zero(), + fetch_canister_logs_per_byte_fee: Cycles::zero(), } } } diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index e14d401087e3..840072e175ab 100644 --- a/rs/cycles_account_manager/src/lib.rs +++ b/rs/cycles_account_manager/src/lib.rs @@ -29,6 +29,7 @@ use ic_types::{ PrincipalId, SubnetId, batch::CanisterCyclesCostSchedule, canister_http::MAX_CANISTER_HTTP_RESPONSE_BYTES, + canister_log::MAX_FETCH_CANISTER_LOGS_RESPONSE_BYTES, messages::{MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, Request, Response, SignedIngress}, }; use prometheus::IntCounter; @@ -1370,6 +1371,34 @@ impl CyclesAccountManager { pub fn default_reserved_balance_limit(&self) -> Cycles { self.config.default_reserved_balance_limit } + + pub fn fetch_canister_logs_fee( + &self, + response_size: NumBytes, + subnet_size: usize, + cost_schedule: CanisterCyclesCostSchedule, + ) -> Cycles { + match cost_schedule { + CanisterCyclesCostSchedule::Free => Cycles::new(0), + CanisterCyclesCostSchedule::Normal => { + (self.config.fetch_canister_logs_base_fee + + self.config.fetch_canister_logs_per_byte_fee * response_size.get()) + * subnet_size + } + } + } + + pub fn max_fetch_canister_logs_fee( + &self, + subnet_size: usize, + cost_schedule: CanisterCyclesCostSchedule, + ) -> Cycles { + self.fetch_canister_logs_fee( + NumBytes::new(MAX_FETCH_CANISTER_LOGS_RESPONSE_BYTES as u64), + subnet_size, + cost_schedule, + ) + } } /// Encapsulates the payer and cost of inducting an ingress messages. diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index 7e3d136b1906..554b58a5fdfe 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -1570,42 +1570,63 @@ impl ExecutionEnvironment { )), refund: msg.take_cycles(), }, - FlagStatus::Enabled => match &msg { - CanisterCall::Request(request) => { - let fetch_canister_logs_fee = Cycles::new(1_000_000); // TODO(EXC-2112): fix placeholder fees. - - let response = if request.payment < fetch_canister_logs_fee { - Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!( - "{} request sent with {} cycles, but {} cycles are required.", - Ic00Method::FetchCanisterLogs, - request.payment, - fetch_canister_logs_fee - ), - )) - } else { - FetchCanisterLogsRequest::decode(payload) - .and_then(|args| { - fetch_canister_logs( - *msg.sender(), - &state, - args, - self.config.fetch_canister_logs_filter, - ) - }) - .map(|resp| (Encode!(&resp).unwrap(), None)) - }; + FlagStatus::Enabled => { + let sender = *msg.sender(); + let payload = payload.to_vec(); + match &mut msg { + CanisterCall::Request(request) => { + let max_fetch_canister_logs_fee = + self.cycles_account_manager.max_fetch_canister_logs_fee( + registry_settings.subnet_size, + cost_schedule, + ); - ExecuteSubnetMessageResult::Finished { - response, - refund: msg.take_cycles(), + // Check there are sufficient cycles to cover the worst-case execution cost. + let response = if request.payment < max_fetch_canister_logs_fee { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!( + "{} request sent with {} cycles, but {} cycles are required.", + Ic00Method::FetchCanisterLogs, + request.payment, + max_fetch_canister_logs_fee + ), + )) + } else { + FetchCanisterLogsRequest::decode(&payload) + .and_then(|args| { + fetch_canister_logs( + sender, + &state, + args, + self.config.fetch_canister_logs_filter, + ) + }) + .map(|resp| { + let response_bytes = Encode!(&resp).unwrap(); + let actual_fee = self + .cycles_account_manager + .fetch_canister_logs_fee( + NumBytes::new(response_bytes.len() as u64), + registry_settings.subnet_size, + cost_schedule, + ); + // There are enough cycles, deduct the actual fee from paid cycles and refund the rest. + msg.deduct_cycles(actual_fee); + (response_bytes, None) + }) + }; + + ExecuteSubnetMessageResult::Finished { + response, + refund: msg.take_cycles(), + } + } + CanisterCall::Ingress(_) => { + self.reject_unexpected_ingress(Ic00Method::FetchCanisterLogs) } } - CanisterCall::Ingress(_) => { - self.reject_unexpected_ingress(Ic00Method::FetchCanisterLogs) - } - }, + } } } diff --git a/rs/execution_environment/tests/canister_logging.rs b/rs/execution_environment/tests/canister_logging.rs index 2596c110798e..86ab4b83da09 100644 --- a/rs/execution_environment/tests/canister_logging.rs +++ b/rs/execution_environment/tests/canister_logging.rs @@ -355,7 +355,7 @@ fn test_fetch_canister_logs_via_inter_canister_update_call_enabled() { call_args() .other_side(FetchCanisterLogsRequest::new(canister_b).encode()) .on_reject(wasm().reject_message().reject()), - Cycles::new(2_000_000), + Cycles::new(50_000_000_000), ) .build(), ); diff --git a/rs/execution_environment/tests/subnet_size_test.rs b/rs/execution_environment/tests/subnet_size_test.rs index c3a1d4281d04..8b99df565cf5 100644 --- a/rs/execution_environment/tests/subnet_size_test.rs +++ b/rs/execution_environment/tests/subnet_size_test.rs @@ -749,6 +749,8 @@ fn get_cycles_account_manager_config(subnet_type: SubnetType) -> CyclesAccountMa max_storage_reservation_period: Duration::from_secs(0), default_reserved_balance_limit: CyclesAccountManagerConfig::system_subnet() .default_reserved_balance_limit, + fetch_canister_logs_base_fee: Cycles::new(0), + fetch_canister_logs_per_byte_fee: Cycles::new(0), }, SubnetType::Application | SubnetType::VerifiedApplication => CyclesAccountManagerConfig { reference_subnet_size: DEFAULT_REFERENCE_SUBNET_SIZE, @@ -783,6 +785,8 @@ fn get_cycles_account_manager_config(subnet_type: SubnetType) -> CyclesAccountMa max_storage_reservation_period: Duration::from_secs(0), default_reserved_balance_limit: CyclesAccountManagerConfig::application_subnet() .default_reserved_balance_limit, + fetch_canister_logs_base_fee: Cycles::new(1_000_000), + fetch_canister_logs_per_byte_fee: Cycles::new(800), }, } } diff --git a/rs/types/types/src/canister_log.rs b/rs/types/types/src/canister_log.rs index 1b12a9f30fb1..505b5371d3a0 100644 --- a/rs/types/types/src/canister_log.rs +++ b/rs/types/types/src/canister_log.rs @@ -12,6 +12,9 @@ pub const MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE: usize = 4 * 1024; /// Prevents unbounded growth of `delta_log_sizes`. const DELTA_LOG_SIZES_CAP: usize = 100; +/// Maximum number of response bytes for a canister http request. +pub const MAX_FETCH_CANISTER_LOGS_RESPONSE_BYTES: usize = 2_000_000; + fn truncate_content(mut record: CanisterLogRecord) -> CanisterLogRecord { let max_content_size = MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE - std::mem::size_of::(); diff --git a/rs/types/types/src/messages.rs b/rs/types/types/src/messages.rs index bd0d455f8521..686411e4e081 100644 --- a/rs/types/types/src/messages.rs +++ b/rs/types/types/src/messages.rs @@ -409,6 +409,14 @@ impl CanisterCall { } } + /// Deducts the specified fee from the payment of this message. + pub fn deduct_cycles(&mut self, fee: Cycles) { + match self { + CanisterCall::Request(request) => Arc::make_mut(request).payment -= fee, + CanisterCall::Ingress(_) => {} // Ingress messages don't have payments + } + } + /// Extracts the cycles received with this message. pub fn take_cycles(&mut self) -> Cycles { match self {