Merge branch 'maciej-tryfrom' into 'master'

feat(FI-801): use TryFrom for ledger_core TransferError

When converting to TransferError or ApproveError we are not able to handle all match branches. 

See merge request dfinity-lab/public/ic!13347

rs/rosetta-api/icp_ledger/ledger/src/main.rs

@@ -294,7 +294,14 @@ async fn icrc1_send(
             created_at_time,
         };
         let (block_index, hash) = apply_transaction(&mut *ledger, tx, now, effective_fee)
-            .map_err(convert_transfer_error)?;
+            .map_err(convert_transfer_error)
+            .map_err(|err| {
+                let err: icrc_ledger_types::icrc1::transfer::TransferError = match err.try_into() {
+                    Ok(err) => err,
+                    Err(err) => trap_with(&err),
+                };
+                err
+            })?;
 
         set_certified_data(&hash.into_bytes());
 
@@ -746,7 +753,6 @@ fn send_() {
                 .await
                 .unwrap_or_else(|e| {
                     trap_with(&e.to_string());
-                    unreachable!()
                 })
         },
     );
@@ -800,7 +806,6 @@ fn notify_() {
 async fn transfer_candid(arg: TransferArgs) -> Result<BlockIndex, TransferError> {
     let to_account = AccountIdentifier::from_address(arg.to).unwrap_or_else(|e| {
         trap_with(&format!("Invalid account identifier: {}", e));
-        unreachable!()
     });
     send(
         arg.memo,
@@ -939,7 +944,6 @@ fn account_balance_() {
 fn account_balance_candid_(arg: BinaryAccountBalanceArgs) -> Tokens {
     let account = AccountIdentifier::from_address(arg.account).unwrap_or_else(|e| {
         trap_with(&format!("Invalid account identifier: {}", e));
-        unreachable!()
     });
     account_balance(account)
 }

rs/rosetta-api/icrc1/ledger/src/main.rs

@@ -348,8 +348,15 @@ async fn icrc1_transfer(arg: TransferArg) -> Result<Nat, TransferError> {
             )
         };
 
-        let (block_idx, _) =
-            apply_transaction(ledger, tx, now, effective_fee).map_err(convert_transfer_error)?;
+        let (block_idx, _) = apply_transaction(ledger, tx, now, effective_fee)
+            .map_err(convert_transfer_error)
+            .map_err(|err| {
+                let err: TransferError = match err.try_into() {
+                    Ok(err) => err,
+                    Err(err) => ic_cdk::trap(&err),
+                };
+                err
+            })?;
         Ok(block_idx)
     })?;
 
@@ -492,7 +499,14 @@ async fn icrc2_approve(arg: ApproveArgs) -> Result<Nat, ApproveError> {
         };
 
         let (block_idx, _) = apply_transaction(ledger, tx, now, expected_fee_tokens)
-            .map_err(convert_transfer_error)?;
+            .map_err(convert_transfer_error)
+            .map_err(|err| {
+                let err: ApproveError = match err.try_into() {
+                    Ok(err) => err,
+                    Err(err) => ic_cdk::trap(&err),
+                };
+                err
+            })?;
         Ok(block_idx)
     })?;
 

rs/rosetta-api/icrc1/src/endpoints.rs

@@ -18,12 +18,13 @@ pub fn convert_transfer_error<Tokens: TokensType>(
 
 pub struct EndpointsTransferError<Tokens>(pub CoreTransferError<Tokens>);
 
-impl<Tokens: TokensType> From<EndpointsTransferError<Tokens>> for TransferError {
-    fn from(err: EndpointsTransferError<Tokens>) -> Self {
+impl<Tokens: TokensType> TryFrom<EndpointsTransferError<Tokens>> for TransferError {
+    type Error = String;
+    fn try_from(err: EndpointsTransferError<Tokens>) -> Result<Self, Self::Error> {
         use ic_ledger_canister_core::ledger::TransferError as LTE;
         use TransferError as TE;
 
-        match err.0 {
+        Ok(match err.0 {
             LTE::BadFee { expected_fee } => TE::BadFee {
                 expected_fee: expected_fee.into(),
             },
@@ -39,27 +40,30 @@ impl<Tokens: TokensType> From<EndpointsTransferError<Tokens>> for TransferError
                 duplicate_of: Nat::from(duplicate_of),
             },
             LTE::InsufficientAllowance { .. } => {
-                unimplemented!("InsufficientAllowance error should not happen for transfer")
+                return Err(
+                    "InsufficientAllowance error should not happen for transfer".to_string()
+                );
             }
             LTE::ExpiredApproval { .. } => {
-                unimplemented!("ExpiredApproval error should not happen for transfer")
+                return Err("ExpiredApproval error should not happen for transfer".to_string());
             }
             LTE::AllowanceChanged { .. } => {
-                unimplemented!("AllowanceChanged error should not happen for transfer")
+                return Err("AllowanceChanged error should not happen for transfer".to_string());
             }
             LTE::SelfApproval { .. } => {
-                unimplemented!("SelfApproval error should not happen for transfer")
+                return Err("SelfApproval error should not happen for transfer".to_string());
             }
-        }
+        })
     }
 }
 
-impl<Tokens: TokensType> From<EndpointsTransferError<Tokens>> for ApproveError {
-    fn from(err: EndpointsTransferError<Tokens>) -> Self {
+impl<Tokens: TokensType> TryFrom<EndpointsTransferError<Tokens>> for ApproveError {
+    type Error = String;
+    fn try_from(err: EndpointsTransferError<Tokens>) -> Result<Self, Self::Error> {
         use ic_ledger_canister_core::ledger::TransferError as LTE;
         use ApproveError as AE;
 
-        match err.0 {
+        Ok(match err.0 {
             LTE::BadFee { expected_fee } => AE::BadFee {
                 expected_fee: expected_fee.into(),
             },
@@ -75,7 +79,9 @@ impl<Tokens: TokensType> From<EndpointsTransferError<Tokens>> for ApproveError {
                 duplicate_of: Nat::from(duplicate_of),
             },
             LTE::InsufficientAllowance { .. } => {
-                unimplemented!("InsufficientAllowance error should not happen for approval")
+                return Err(
+                    "InsufficientAllowance error should not happen for approval".to_string()
+                );
             }
             LTE::ExpiredApproval { ledger_time } => AE::Expired {
                 ledger_time: ledger_time.as_nanos_since_unix_epoch(),
@@ -84,9 +90,9 @@ impl<Tokens: TokensType> From<EndpointsTransferError<Tokens>> for ApproveError {
                 current_allowance: current_allowance.into(),
             },
             LTE::SelfApproval { .. } => {
-                unimplemented!("self approval not implemented for ApproveError")
+                return Err("self-approvals are not allowed".to_string());
             }
-        }
+        })
     }
 }
 

rs/rust_canisters/dfn_core/src/api.rs

@@ -708,9 +708,10 @@ pub fn print<S: std::convert::AsRef<str>>(s: S) {
 }
 
 /// Traps with the given message.
-pub fn trap_with(message: &str) {
+pub fn trap_with(message: &str) -> ! {
     unsafe {
         ic0::trap(message.as_ptr() as u32, message.len() as u32);
+        unreachable!()
     }
 }
 

rs/rust_canisters/memory_test/src/main.rs

@@ -80,7 +80,6 @@ fn operation_from_args() -> Operation {
                 String::from_utf8_lossy(&data),
                 err,
             ));
-            unreachable!("cannot reach here after the trap");
         }
     }
 }