refactor: decode Cycles from little-endian bytes via TryFrom (#11102)

`Cycles` was decoded from its little-endian byte representation through
an infallible `From<&Vec<u8>>` impl, which left it with no way to report
a length mismatch to its caller. Replace it with `TryFrom<&Vec<u8>>`.

The two protobuf conversions built on it, `state.queues.v1.Cycles` and
`canister_state_bits.v1.CyclesAccount`, become `TryFrom` impls returning
`ProxyDecodeError::ValueOutOfRange`, so a length mismatch is now
reported like any other proto decoding error. This matches how
`NominalCycles` and `CompoundCycles` already decode in the same crate.

The `try_from_option_field` call sites (`Request::cycles_payment`,
`Response::cycles_refund`, `CallContext::available_cycles`,
`CanisterStateBits::cycles_balance`, `Refund::amount`) need no change:
they already required a `TryFrom` whose error converts into
`ProxyDecodeError` and were satisfied through the blanket impl. The
callers that used the infallible impls directly now propagate the error
instead: `CompoundCycles::real`, `Callback::cycles_sent`, the three
`RefundStatus` fields, and four `CanisterStateBits` fields in
`state_layout`.

The encoding is unchanged, so this only affects the decoding failure
path.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: mraszyk

rs/embedders/tests/system_api.rs

@@ -1523,7 +1523,7 @@ fn cycles_burn128_clamps_to_available_cycles() {
     let mut heap = vec![0; 16];
     api.ic0_canister_liquid_cycle_balance128(0, &mut heap)
         .unwrap();
-    let liquid_cycles = Cycles::from(&heap);
+    let liquid_cycles = Cycles::try_from(&heap).unwrap();
     // Sanity check.
     assert!(liquid_cycles < INITIAL_CYCLES);
     let freeze_limit = INITIAL_CYCLES - liquid_cycles;
@@ -1534,7 +1534,7 @@ fn cycles_burn128_clamps_to_available_cycles() {
         .unwrap();
 
     // Only the available cycle balance was burned.
-    assert_eq!(liquid_cycles, Cycles::from(&heap));
+    assert_eq!(liquid_cycles, Cycles::try_from(&heap).unwrap());
 
     // The balance is equal to the freeze limit.
     let system_state_modifications = api.take_system_state_modifications();
@@ -1911,19 +1911,22 @@ fn test_ic0_cycles_burn() {
     for _ in 0..2 {
         let mut heap = vec![0; 16];
         api.ic0_cycles_burn128(removed, 0, &mut heap).unwrap();
-        assert_eq!(removed, Cycles::from(&heap));
+        assert_eq!(removed, Cycles::try_from(&heap).unwrap());
     }
 
     let mut heap = vec![0; 16];
     api.ic0_cycles_burn128(removed, 0, &mut heap).unwrap();
     // The remaining balance is lower than the amount requested to be burned,
     // hence the system will remove as many cycles as it can.
-    assert_eq!(Cycles::new(1_000_000_000_000), Cycles::from(&heap));
+    assert_eq!(
+        Cycles::new(1_000_000_000_000),
+        Cycles::try_from(&heap).unwrap()
+    );
 
     let mut heap = vec![0; 16];
     api.ic0_cycles_burn128(removed, 0, &mut heap).unwrap();
     // There are no more cycles that can be burned.
-    assert_eq!(Cycles::new(0), Cycles::from(&heap));
+    assert_eq!(Cycles::new(0), Cycles::try_from(&heap).unwrap());
 }
 
 #[test]

rs/execution_environment/tests/hypervisor.rs

@@ -9322,7 +9322,7 @@ fn invoke_cost_call() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost,);
 }
 
@@ -9342,7 +9342,7 @@ fn invoke_cost_create_canister() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 
@@ -9366,7 +9366,7 @@ fn invoke_cost_http_request() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 
@@ -9411,7 +9411,7 @@ fn invoke_cost_http_request_v2() {
         test.get_own_subnet_cycles_config(),
     );
     let bytes = get_reply(res);
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 
@@ -9479,7 +9479,7 @@ fn invoke_cost_sign_with_ecdsa() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 
@@ -9593,7 +9593,7 @@ fn invoke_cost_sign_with_schnorr() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 
@@ -9672,7 +9672,7 @@ fn invoke_cost_vetkd_derive_key() {
     let Ok(WasmResult::Reply(bytes)) = res else {
         panic!("Expected reply, got {res:?}");
     };
-    let actual_cost = Cycles::from(&bytes);
+    let actual_cost = Cycles::try_from(&bytes).unwrap();
     assert_eq!(actual_cost, expected_cost.real());
 }
 

rs/state_layout/src/state_layout/proto.rs

@@ -121,12 +121,14 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> for CanisterStateBits {
 
         let cycles_debit = value
             .cycles_debit
-            .map(|c| c.into())
+            .map(Cycles::try_from)
+            .transpose()?
             .unwrap_or_else(Cycles::zero);
 
         let reserved_balance = value
             .reserved_balance
-            .map(|c| c.into())
+            .map(Cycles::try_from)
+            .transpose()?
             .unwrap_or_else(Cycles::zero);
 
         let mut consumed_cycles_by_use_cases = BTreeMap::new();
@@ -178,10 +180,14 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> for CanisterStateBits {
             cycles_balance,
             cycles_debit,
             reserved_balance,
-            reserved_balance_limit: value.reserved_balance_limit.map(|v| v.into()),
+            reserved_balance_limit: value
+                .reserved_balance_limit
+                .map(Cycles::try_from)
+                .transpose()?,
             minimum_incoming_canister_call_cycles: value
                 .minimum_incoming_canister_call_cycles
-                .map(|v| v.into())
+                .map(Cycles::try_from)
+                .transpose()?
                 .unwrap_or_default(),
             status: try_from_option_field(
                 value.canister_status,

rs/types/cycles/src/compound_cycles.rs

@@ -215,7 +215,7 @@ impl<T: CyclesUseCaseKind> TryFrom<PbCompoundCycles> for CompoundCycles<T> {
                 .ok_or(ProxyDecodeError::MissingField("CompoundCycles::nominal"))?,
         )?;
         Ok(CompoundCycles {
-            real: Cycles::from(real),
+            real: Cycles::try_from(real)?,
             nominal,
             _cycles_use_case_marker: PhantomData,
         })

rs/types/cycles/src/cycles.rs

@@ -1,9 +1,10 @@
 use candid::{CandidType, Nat};
 use ic_heap_bytes::DeterministicHeapBytes;
+use ic_protobuf::proxy::ProxyDecodeError;
 use ic_protobuf::state::canister_state_bits::v1::CyclesAccount as pbCyclesAccount;
 use ic_protobuf::state::queues::v1::Cycles as PbCycles;
 use serde::{Deserialize, Serialize};
-use std::convert::TryInto;
+use std::array::TryFromSliceError;
 use std::iter::Sum;
 use std::{
     fmt,
@@ -92,9 +93,19 @@ impl From<u64> for Cycles {
     }
 }
 
-impl From<&Vec<u8>> for Cycles {
-    fn from(bytes: &Vec<u8>) -> Self {
-        Self::new(u128::from_le_bytes(bytes.as_slice().try_into().unwrap()))
+/// Decodes `Cycles` from their little-endian representation, as produced by
+/// `From<Cycles> for Vec<u8>`. Fails if `bytes` is not exactly 16 bytes long.
+///
+/// Takes a `&Vec<u8>` rather than a `&[u8]` because that is what all callers
+/// hold, and a `&[u8]` impl alone would force each of them to spell out an
+/// `as_slice()`. A blanket `impl<T: AsRef<[u8]>>` covering both is not
+/// possible: it would conflict with the `impl<T, U: Into<T>> TryFrom<U> for T`
+/// in `core`.
+impl TryFrom<&Vec<u8>> for Cycles {
+    type Error = TryFromSliceError;
+
+    fn try_from(bytes: &Vec<u8>) -> Result<Self, Self::Error> {
+        Ok(Self::new(u128::from_le_bytes(bytes.as_slice().try_into()?)))
     }
 }
 
@@ -212,9 +223,11 @@ impl From<Cycles> for PbCycles {
     }
 }
 
-impl From<PbCycles> for Cycles {
-    fn from(item: PbCycles) -> Self {
-        Self::from(&item.raw_cycles)
+impl TryFrom<PbCycles> for Cycles {
+    type Error = ProxyDecodeError;
+
+    fn try_from(item: PbCycles) -> Result<Self, Self::Error> {
+        try_from_le_bytes(item.raw_cycles)
     }
 }
 
@@ -226,12 +239,23 @@ impl From<Cycles> for pbCyclesAccount {
     }
 }
 
-impl From<pbCyclesAccount> for Cycles {
-    fn from(value: pbCyclesAccount) -> Self {
-        Self::from(&value.cycles_balance)
+impl TryFrom<pbCyclesAccount> for Cycles {
+    type Error = ProxyDecodeError;
+
+    fn try_from(value: pbCyclesAccount) -> Result<Self, Self::Error> {
+        try_from_le_bytes(value.cycles_balance)
     }
 }
 
+/// Decodes `Cycles` from the little-endian representation used by the protobuf
+/// encodings above, mapping a length mismatch onto a `ProxyDecodeError`.
+fn try_from_le_bytes(bytes: Vec<u8>) -> Result<Cycles, ProxyDecodeError> {
+    Cycles::try_from(&bytes).map_err(|_| ProxyDecodeError::ValueOutOfRange {
+        typ: "Cycles",
+        err: format!("expected 16 bytes, got {}", bytes.len()),
+    })
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -373,4 +397,43 @@ mod test {
             "Cycles(340282366920938463463374607431768211455)"
         );
     }
+
+    #[test]
+    fn test_le_bytes_roundtrip() {
+        for cycles in [Cycles::zero(), Cycles::new(1), Cycles::new(u128::MAX)] {
+            let bytes: Vec<u8> = cycles.into();
+            assert_eq!(bytes.len(), 16);
+            assert_eq!(Cycles::try_from(&bytes).unwrap(), cycles);
+        }
+    }
+
+    #[test]
+    fn test_try_from_le_bytes_of_wrong_length_fails() {
+        for len in [0, 15, 17] {
+            assert!(Cycles::try_from(&vec![0; len]).is_err());
+        }
+    }
+
+    #[test]
+    fn test_try_from_proto_with_wrong_length_fails() {
+        for len in [0, 15, 17] {
+            let err = Cycles::try_from(PbCycles {
+                raw_cycles: vec![0; len],
+            })
+            .unwrap_err();
+            assert!(
+                matches!(err, ProxyDecodeError::ValueOutOfRange { typ: "Cycles", .. }),
+                "unexpected error: {err:?}"
+            );
+
+            let err = Cycles::try_from(pbCyclesAccount {
+                cycles_balance: vec![0; len],
+            })
+            .unwrap_err();
+            assert!(
+                matches!(err, ProxyDecodeError::ValueOutOfRange { typ: "Cycles", .. }),
+                "unexpected error: {err:?}"
+            );
+        }
+    }
 }

rs/types/types/src/canister_http.rs

@@ -385,15 +385,18 @@ impl TryFrom<pb_metadata::CanisterHttpRequestContext> for CanisterHttpRequestCon
             Some(refund_status) => RefundStatus {
                 refundable_cycles: refund_status
                     .refundable_cycles
-                    .map(Into::into)
+                    .map(Cycles::try_from)
+                    .transpose()?
                     .unwrap_or_default(),
                 per_replica_allowance: refund_status
                     .per_replica_allowance
-                    .map(Into::into)
+                    .map(Cycles::try_from)
+                    .transpose()?
                     .unwrap_or_default(),
                 refunded_cycles: refund_status
                     .refunded_cycles
-                    .map(Into::into)
+                    .map(Cycles::try_from)
+                    .transpose()?
                     .unwrap_or_default(),
                 refunding_nodes: refund_status
                     .refunding_nodes

rs/types/types/src/methods.rs

@@ -381,7 +381,7 @@ impl TryFrom<pb::Callback> for Callback {
         Ok(Self {
             call_context_id: CallContextId::from(value.call_context_id),
             respondent: try_from_option_field(value.respondent, "Callback::respondent")?,
-            cycles_sent: Cycles::from(cycles_sent),
+            cycles_sent: Cycles::try_from(cycles_sent)?,
             prepayment_for_response_execution,
             prepayment_for_response_transmission,
             prepayment_for_call_transmission,