refactor: Better separation between Cycles and NominalCycles

rs/canonical_state/src/encoding/tests/compatibility.rs

@@ -215,9 +215,9 @@ fn canonical_encoding_stream_header() {
 fn canonical_encoding_subnet_metrics() {
     for certification_version in all_supported_versions() {
         let mut metrics = SubnetMetrics::default();
-        metrics.consumed_cycles_by_deleted_canisters = NominalCycles::from(0);
-        metrics.consumed_cycles_http_outcalls = NominalCycles::from(50_000_000_000);
-        metrics.consumed_cycles_ecdsa_outcalls = NominalCycles::from(100_000_000_000);
+        metrics.observe_consumed_cycles_by_deleted_canisters(NominalCycles::from(0));
+        metrics.observe_consumed_cycles_http_outcalls(NominalCycles::from(50_000_000_000));
+        metrics.observe_consumed_cycles_ecdsa_outcalls(NominalCycles::from(100_000_000_000));
         metrics.num_canisters = 5;
         metrics.canister_state_bytes = NumBytes::from(5 * 1024 * 1024);
         metrics.update_transactions_total = 4200;

rs/cycles_account_manager/src/cycles_account_manager.rs

@@ -1357,7 +1357,8 @@ impl CyclesAccountManager {
         &self,
         system_state: &SystemState,
     ) -> NominalCycles {
-        NominalCycles::from(system_state.balance() + system_state.reserved_balance())
+        let raw_amount = (system_state.balance() + system_state.reserved_balance()).get();
+        NominalCycles::from(raw_amount)
     }
 
     // The fee for `UpdateSettings` is charged after applying

rs/execution_environment/src/canister_manager.rs

@@ -1297,7 +1297,7 @@ impl CanisterManager {
         state
             .metadata
             .subnet_metrics
-            .consumed_cycles_by_deleted_canisters += consumed_cycles_by_canister_to_delete;
+            .observe_consumed_cycles_by_deleted_canisters(consumed_cycles_by_canister_to_delete);
 
         for (use_case, cycles) in canister_to_delete
             .system_state

rs/execution_environment/src/canister_manager/tests.rs

@@ -2170,7 +2170,7 @@ fn delete_canister_consumed_cycles_observed() {
             .get_consumed_cycles_by_use_case()
             .get(&CyclesUseCase::DeletedCanisters)
             .unwrap(),
-        NominalCycles::from(initial_cycles)
+        NominalCycles::from(initial_cycles.get())
     );
 }
 

rs/execution_environment/src/execution_environment.rs

@@ -2000,8 +2000,11 @@ impl ExecutionEnvironment {
             ))
         } else {
             canister_http_request_context.request.payment -= http_request_fee;
-            let http_fee = NominalCycles::from(http_request_fee);
-            state.metadata.subnet_metrics.consumed_cycles_http_outcalls += http_fee;
+            let http_fee = NominalCycles::from(http_request_fee.get());
+            state
+                .metadata
+                .subnet_metrics
+                .observe_consumed_cycles_http_outcalls(http_fee);
             state
                 .metadata
                 .subnet_metrics
@@ -3565,10 +3568,13 @@ impl ExecutionEnvironment {
             } else {
                 // Charge for the request.
                 request.payment -= signature_fee;
-                let nominal_fee = NominalCycles::from(signature_fee);
+                let nominal_fee = NominalCycles::from(signature_fee.get());
                 let use_case = match args {
                     ThresholdArguments::Ecdsa(_) => {
-                        state.metadata.subnet_metrics.consumed_cycles_ecdsa_outcalls += nominal_fee;
+                        state
+                            .metadata
+                            .subnet_metrics
+                            .observe_consumed_cycles_ecdsa_outcalls(nominal_fee);
                         CyclesUseCase::ECDSAOutcalls
                     }
                     ThresholdArguments::Schnorr(_) => CyclesUseCase::SchnorrOutcalls,

rs/execution_environment/src/execution_environment/tests.rs

@@ -3054,15 +3054,15 @@ fn execute_canister_http_request() {
     assert_eq!(http_request_context.request.payment, payment - fee);
 
     assert_eq!(
-        NominalCycles::from(fee),
+        NominalCycles::from(fee.get()),
         test.state()
             .metadata
             .subnet_metrics
-            .consumed_cycles_http_outcalls
+            .get_consumed_cycles_http_outcalls()
     );
 
     assert_eq!(
-        NominalCycles::from(fee),
+        NominalCycles::from(fee.get()),
         *test
             .state()
             .metadata
@@ -3866,7 +3866,7 @@ fn replicated_query_can_burn_cycles() {
         .consumed_cycles_by_use_cases()
         .get(&CyclesUseCase::BurnedCycles)
         .unwrap();
-    assert_eq!(burned_cycles, NominalCycles::from(cycles_to_burn));
+    assert_eq!(burned_cycles, NominalCycles::from(cycles_to_burn.get()));
 }
 
 #[test]
@@ -4039,12 +4039,12 @@ fn test_consumed_cycles_by_use_case_with_refund() {
     // Check that consumed cycles are correct for both use cases.
     assert_eq!(
         transmission_consumption_after_response,
-        NominalCycles::from(transmission_cost)
+        NominalCycles::from(transmission_cost.get())
     );
 
     assert_eq!(
         instruction_consumption_after_response,
-        NominalCycles::from(execution_cost)
+        NominalCycles::from(execution_cost.get())
     );
 
     // Consumed cycles after the response should be smaller than before
@@ -4082,7 +4082,7 @@ fn test_consumed_cycles_by_use_case_with_refund() {
             .consumed_cycles_by_use_cases()
             .get(&CyclesUseCase::Instructions)
             .unwrap(),
-        NominalCycles::from(test.canister_execution_cost(b_id))
+        NominalCycles::from(test.canister_execution_cost(b_id).get())
     );
 }
 

rs/execution_environment/src/scheduler/tests/metrics.rs

@@ -921,7 +921,7 @@ fn consumed_cycles_ecdsa_outcalls_are_added_to_consumed_cycles_total() {
     );
 
     assert_eq!(
-        consumed_cycles_before + NominalCycles::from(fee),
+        consumed_cycles_before + NominalCycles::from(fee.get()),
         consumed_cycles_after
     );
 
@@ -1022,7 +1022,7 @@ fn consumed_cycles_http_outcalls_are_added_to_consumed_cycles_total() {
     );
 
     assert_eq!(
-        consumed_cycles_before + NominalCycles::from(fee),
+        consumed_cycles_before + NominalCycles::from(fee.get()),
         consumed_cycles_after
     );
 

rs/messaging/src/routing/stream_builder.rs

@@ -13,7 +13,7 @@ use ic_types::messages::{
     MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, MAX_REJECT_MESSAGE_LEN_BYTES, Payload, RejectContext,
     Request, RequestOrResponse, Response, StreamMessage,
 };
-use ic_types::{CountBytes, Cycles, SubnetId};
+use ic_types::{CountBytes, Cycles, SubnetId, nominal_cycles::NominalCycles};
 #[cfg(test)]
 use mockall::automock;
 use prometheus::{Histogram, IntCounter, IntCounterVec, IntGaugeVec};
@@ -256,7 +256,9 @@ impl StreamBuilderImpl {
                     response
                 );
                 self.metrics.critical_error_induct_response_failed.inc();
-                state.observe_lost_cycles_due_to_dropped_messages(req.payment);
+                state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                    req.payment.get(),
+                ));
             });
     }
 
@@ -720,7 +722,7 @@ impl StreamBuilderImpl {
                 }
             }
         });
-        state.observe_lost_cycles_due_to_dropped_messages(cycles_lost);
+        state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(cycles_lost.get()));
     }
 }
 

rs/messaging/src/routing/stream_handler.rs

@@ -22,7 +22,7 @@ use ic_types::messages::{
     RejectContext, Request, RequestOrResponse, Response, StreamMessage,
 };
 use ic_types::xnet::{RejectReason, RejectSignal, StreamIndex, StreamIndexedQueue, StreamSlice};
-use ic_types::{CanisterId, SubnetId};
+use ic_types::{CanisterId, SubnetId, nominal_cycles::NominalCycles};
 use prometheus::{Histogram, IntCounter, IntCounterVec, IntGaugeVec};
 use std::cell::RefCell;
 use std::collections::{BTreeMap, VecDeque};
@@ -819,7 +819,9 @@ impl StreamHandlerImpl {
                 self.metrics.critical_error_sender_subnet_mismatch.inc();
                 stream.push_accept_signal();
                 // Cycles are lost.
-                state.observe_lost_cycles_due_to_dropped_messages(rep.refund);
+                state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                    rep.refund.get(),
+                ));
             }
         }
     }
@@ -904,7 +906,9 @@ impl StreamHandlerImpl {
                                 );
                                 self.metrics.critical_error_induct_response_failed.inc();
                                 // Cycles are lost.
-                                state.observe_lost_cycles_due_to_dropped_messages(response.refund);
+                                state.observe_lost_cycles_due_to_dropped_messages(
+                                    NominalCycles::from(response.refund.get()),
+                                );
                                 Accept
                             }
                         }
@@ -1004,7 +1008,9 @@ impl StreamHandlerImpl {
                         LABEL_VALUE_TYPE_REFUND,
                         LABEL_VALUE_DROPPED,
                     );
-                    state.observe_lost_cycles_due_to_dropped_messages(refund.amount());
+                    state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                        refund.amount().get(),
+                    ));
                 }
             }
 

rs/messaging/src/routing/stream_handler/tests.rs

@@ -28,7 +28,7 @@ use ic_test_utilities_types::xnet::StreamHeaderBuilder;
 use ic_types::messages::{CallbackId, MAX_RESPONSE_COUNT_BYTES, NO_DEADLINE, Payload};
 use ic_types::time::{CoarseTime, UNIX_EPOCH};
 use ic_types::xnet::{RejectReason, RejectSignal, StreamFlags, StreamIndexedQueue};
-use ic_types::{CanisterId, CountBytes, Cycles};
+use ic_types::{CanisterId, CountBytes, Cycles, nominal_cycles::NominalCycles};
 use lazy_static::lazy_static;
 use maplit::btreemap;
 use pretty_assertions::assert_eq;
@@ -1257,9 +1257,11 @@ fn garbage_collect_local_state_with_reject_signals_for_request_from_absent_canis
             });
             expected_state.with_streams(btreemap![REMOTE_SUBNET => expected_stream]);
             // Cycles attached to the request / reject response are lost.
-            expected_state.observe_lost_cycles_due_to_dropped_messages(
-                message_in_stream(state.get_stream(&REMOTE_SUBNET), 21).cycles(),
-            );
+            expected_state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                message_in_stream(state.get_stream(&REMOTE_SUBNET), 21)
+                    .cycles()
+                    .get(),
+            ));
 
             // Act and compare to expected.
             let mut available_guaranteed_response_memory =
@@ -1554,7 +1556,9 @@ fn induct_stream_slices_reject_response_from_old_host_subnet_is_accepted() {
 
             // Cycles attached to the dropped reply are lost.
             let cycles_lost = message_in_slice(slices.get(&CANISTER_MIGRATION_SUBNET), 1).cycles();
-            expected_state.observe_lost_cycles_due_to_dropped_messages(cycles_lost);
+            expected_state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                cycles_lost.get(),
+            ));
 
             let mut available_guaranteed_response_memory =
                 stream_handler.available_guaranteed_response_memory(&state);
@@ -2065,7 +2069,10 @@ fn inducting_best_effort_response_addressed_to_non_existent_canister_does_not_ra
             expected_state
                 .metadata
                 .subnet_metrics
-                .observe_consumed_cycles_with_use_case(DroppedMessages, refund.into());
+                .observe_consumed_cycles_with_use_case(
+                    DroppedMessages,
+                    NominalCycles::from(refund.get()),
+                );
         },
     );
 }
@@ -2150,7 +2157,9 @@ fn induct_stream_slices_partial_success() {
             let cycles_lost = message_in_slice(slices.get(&REMOTE_SUBNET), 48).cycles()
                 + message_in_slice(slices.get(&REMOTE_SUBNET), 49).cycles()
                 + message_in_slice(slices.get(&REMOTE_SUBNET), 51).cycles();
-            expected_state.observe_lost_cycles_due_to_dropped_messages(cycles_lost);
+            expected_state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                cycles_lost.get(),
+            ));
 
             let initial_available_guaranteed_response_memory =
                 stream_handler.available_guaranteed_response_memory(&state);
@@ -2735,7 +2744,9 @@ fn induct_stream_slices_with_refunds() {
 
             // Cycles in refund @44 are lost
             let refund44 = refund_in_slice(slices.get(&REMOTE_SUBNET), 44);
-            expected_state.observe_lost_cycles_due_to_dropped_messages(refund44.amount());
+            expected_state.observe_lost_cycles_due_to_dropped_messages(NominalCycles::from(
+                refund44.amount().get(),
+            ));
 
             // Act.
             let mut available_guaranteed_response_memory =

rs/replicated_state/src/canister_state/system_state.rs

@@ -1953,7 +1953,7 @@ impl SystemState {
             .entry(use_case)
             .or_insert_with(|| NominalCycles::from(0));
 
-        let nominal_amount = amount.into();
+        let nominal_amount = NominalCycles::from(amount.get());
 
         match consuming_cycles {
             ConsumingCycles::Yes => {

rs/replicated_state/src/canister_state/tests.rs

@@ -856,16 +856,16 @@ fn canister_state_ingress_induction_cycles_debit() {
     // Check that 'ingress_induction_cycles_debit' is added
     // to consumed cycles.
     assert_eq!(
-        system_state.canister_metrics().consumed_cycles(),
-        ingress_induction_debit.into()
+        system_state.canister_metrics().consumed_cycles().get(),
+        ingress_induction_debit.get()
     );
     assert_eq!(
         *system_state
             .canister_metrics()
             .consumed_cycles_by_use_cases()
             .get(&CyclesUseCase::IngressInduction)
             .unwrap(),
-        ingress_induction_debit.into()
+        NominalCycles::from(ingress_induction_debit.get()),
     );
 }
 const INITIAL_CYCLES: Cycles = Cycles::new(1 << 36);
@@ -881,7 +881,7 @@ fn update_balance_and_consumed_cycles_correctly() {
     );
     assert_eq!(
         system_state.canister_metrics().consumed_cycles(),
-        NominalCycles::from(initial_consumed_cycles)
+        NominalCycles::from(initial_consumed_cycles.get())
     );
 
     let cycles = Cycles::new(100);
@@ -892,7 +892,7 @@ fn update_balance_and_consumed_cycles_correctly() {
     );
     assert_eq!(
         system_state.canister_metrics().consumed_cycles(),
-        NominalCycles::from(initial_consumed_cycles - cycles)
+        NominalCycles::from((initial_consumed_cycles - cycles).get())
     );
 }
 
@@ -914,7 +914,7 @@ fn update_balance_and_consumed_cycles_by_use_case_correctly() {
             .consumed_cycles_by_use_cases()
             .get(&CyclesUseCase::Memory)
             .unwrap(),
-        NominalCycles::from(cycles_to_consume - cycles_to_add)
+        NominalCycles::from((cycles_to_consume - cycles_to_add).get())
     );
 }
 

rs/replicated_state/src/metadata_state.rs

@@ -334,9 +334,9 @@ pub fn can_have_subnet_admins(
 
 #[derive(Clone, Eq, PartialEq, Debug, Default)]
 pub struct SubnetMetrics {
-    pub consumed_cycles_by_deleted_canisters: NominalCycles,
-    pub consumed_cycles_http_outcalls: NominalCycles,
-    pub consumed_cycles_ecdsa_outcalls: NominalCycles,
+    consumed_cycles_by_deleted_canisters: NominalCycles,
+    consumed_cycles_http_outcalls: NominalCycles,
+    consumed_cycles_ecdsa_outcalls: NominalCycles,
     consumed_cycles_by_use_case: BTreeMap<CyclesUseCase, NominalCycles>,
     pub threshold_signature_agreements: BTreeMap<MasterPublicKeyId, u64>,
     /// The number of canisters that exist on this subnet.
@@ -364,6 +364,30 @@ impl SubnetMetrics {
             .or_insert_with(|| NominalCycles::from(0)) += cycles;
     }
 
+    pub fn observe_consumed_cycles_by_deleted_canisters(&mut self, cycles: NominalCycles) {
+        self.consumed_cycles_by_deleted_canisters += cycles;
+    }
+
+    pub fn get_consumed_cycles_by_deleted_canisters(&self) -> NominalCycles {
+        self.consumed_cycles_by_deleted_canisters
+    }
+
+    pub fn observe_consumed_cycles_http_outcalls(&mut self, cycles: NominalCycles) {
+        self.consumed_cycles_http_outcalls += cycles;
+    }
+
+    pub fn get_consumed_cycles_http_outcalls(&self) -> NominalCycles {
+        self.consumed_cycles_http_outcalls
+    }
+
+    pub fn observe_consumed_cycles_ecdsa_outcalls(&mut self, cycles: NominalCycles) {
+        self.consumed_cycles_ecdsa_outcalls += cycles;
+    }
+
+    pub fn get_consumed_cycles_ecdsa_outcalls(&self) -> NominalCycles {
+        self.consumed_cycles_ecdsa_outcalls
+    }
+
     pub fn get_consumed_cycles_by_use_case(&self) -> &BTreeMap<CyclesUseCase, NominalCycles> {
         &self.consumed_cycles_by_use_case
     }