clean up the tests

Author: roman-kashitsyn

Cargo.Bazel.lock

@@ -1,5 +1,5 @@
 {
-  "checksum": "fd7fe8ea967274d2f13641d3bbfad70cb48ad849c110fd74440b62e666029e40",
+  "checksum": "ad9783ce8e08ab1bcbcc853142e7d93648924113cbe1f4dffb641461593f5afe",
   "crates": {
     "addr2line 0.20.0": {
       "name": "addr2line",
@@ -5060,6 +5060,10 @@
             {
               "id": "serde 1.0.171",
               "target": "serde"
+            },
+            {
+              "id": "thiserror 1.0.44",
+              "target": "thiserror"
             }
           ],
           "selects": {}

Cargo.lock

@@ -21,11 +21,12 @@ rand = "0.8.5"
 ring = "0.16.20"
 serde = "=1.0.171"
 tempfile = "3.3"
+thiserror = "1"
 tokio = { version = "1.20.1", features = ["macros"] }
 
 [workspace.package]
 authors = ["DFINITY Stiftung"]
 edition = "2018"
 repository = "https://github.com/dfinity/ICRC-1"
 rust-version = "1.31.0"
-license = "Apache-2.0"
+license = "Apache-2.0"

Cargo.toml

@@ -142,7 +142,7 @@ actor class Ledger(init : { initial_mints : [{ account : { owner : Principal; su
         case (#Burn(args)) { total -= args.amount };
         case (#Mint(args)) { total += args.amount };
         case (#Transfer(_)) { total -= tx.fee };
-        case (#Approve(_)) {total -= tx.fee};
+        case (#Approve(_)) { total -= tx.fee };
       };
     };
     total;

ref/ICRC1.mo

@@ -12,6 +12,7 @@ path = "lib.rs"
 
 [dependencies]
 anyhow = { workspace = true }
+async-trait = { workspace = true } 
 candid = { workspace = true }
 serde = { workspace = true }
-async-trait = { workspace = true } 
+thiserror = { workspace = true }

test/env/Cargo.toml

@@ -3,7 +3,7 @@ use candid::utils::{ArgumentDecoder, ArgumentEncoder};
 use candid::Principal;
 use candid::{CandidType, Int, Nat};
 use serde::Deserialize;
-use std::fmt;
+use thiserror::Error;
 
 pub type Subaccount = [u8; 32];
 
@@ -36,58 +36,26 @@ pub enum Value {
     Int(Int),
 }
 
-#[derive(CandidType, Deserialize, PartialEq, Eq, Debug, Clone)]
+#[derive(CandidType, Deserialize, PartialEq, Eq, Debug, Clone, Error)]
 pub enum TransferError {
+    #[error("Invalid transfer fee, the ledger expected fee {expected_fee}")]
     BadFee { expected_fee: Nat },
+    #[error("Invalid burn amount, the minimal burn amount is {min_burn_amount}")]
     BadBurn { min_burn_amount: Nat },
+    #[error("The account owner doesn't have enough funds to for the transfer, balance: {balance}")]
     InsufficientFunds { balance: Nat },
+    #[error("created_at_time is too far in the past")]
     TooOld,
+    #[error("created_at_time is too far in the future, ledger time: {ledger_time}")]
     CreatedInFuture { ledger_time: u64 },
+    #[error("the transfer is a duplicate of transaction {duplicate_of}")]
     Duplicate { duplicate_of: Nat },
+    #[error("the ledger is temporarily unavailable")]
     TemporarilyUnavailable,
+    #[error("generic error (code {error_code}): {message}")]
     GenericError { error_code: Nat, message: String },
 }
 
-impl fmt::Display for TransferError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::BadFee { expected_fee } => write!(
-                f,
-                "Invalid transfer fee, the ledger expected fee {}",
-                expected_fee
-            ),
-            Self::BadBurn { min_burn_amount } => write!(
-                f,
-                "Invalid burn amount, the minimal burn amount is {}",
-                min_burn_amount
-            ),
-            Self::InsufficientFunds { balance } => write!(
-                f,
-                "The account owner doesn't have enough funds to for the transfer, balance: {}",
-                balance
-            ),
-            Self::TooOld => write!(f, "created_at_time is too far in the past"),
-            Self::CreatedInFuture { ledger_time } => write!(
-                f,
-                "created_at_time is too far in the future, ledger time: {}",
-                ledger_time
-            ),
-            Self::Duplicate { duplicate_of } => write!(
-                f,
-                "the transfer is a duplicate of transaction {}",
-                duplicate_of
-            ),
-            Self::TemporarilyUnavailable => write!(f, "the ledger is temporarily unavailable"),
-            Self::GenericError {
-                error_code,
-                message,
-            } => write!(f, "generic error (code {}): {}", error_code, message),
-        }
-    }
-}
-
-impl std::error::Error for TransferError {}
-
 #[derive(CandidType, Debug, Clone)]
 pub struct Transfer {
     from_subaccount: Option<Subaccount>,
@@ -183,16 +151,25 @@ impl ApproveArgs {
     }
 }
 
-#[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq)]
+#[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq, Error)]
 pub enum ApproveError {
+    #[error("Invalid transfer fee, the ledger expected fee {expected_fee}")]
     BadFee { expected_fee: Nat },
+    #[error("The account owner doesn't have enough funds to for the approval, balance: {balance}")]
     InsufficientFunds { balance: Nat },
+    #[error("The allowance changed, current allowance: {current_allowance}")]
     AllowanceChanged { current_allowance: Nat },
+    #[error("the approval expiration time is in the past, ledger time: {ledger_time}")]
     Expired { ledger_time: u64 },
+    #[error("created_at_time is too far in the past")]
     TooOld,
+    #[error("created_at_time is too far in the future, ledger time: {ledger_time}")]
     CreatedInFuture { ledger_time: u64 },
+    #[error("the transfer is a duplicate of transaction {duplicate_of}")]
     Duplicate { duplicate_of: Nat },
+    #[error("the ledger is temporarily unavailable")]
     TemporarilyUnavailable,
+    #[error("generic error (code {error_code}): {message}")]
     GenericError { error_code: Nat, message: String },
 }
 
@@ -245,16 +222,25 @@ impl TransferFromArgs {
     }
 }
 
-#[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq)]
+#[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq, Error)]
 pub enum TransferFromError {
+    #[error("Invalid transfer fee, the ledger expected fee {expected_fee}")]
     BadFee { expected_fee: Nat },
+    #[error("Invalid burn amount, the minimal burn amount is {min_burn_amount}")]
     BadBurn { min_burn_amount: Nat },
+    #[error("The account owner doesn't have enough funds to for the transfer, balance: {balance}")]
     InsufficientFunds { balance: Nat },
+    #[error("The account owner doesn't have allowance for the transfer, allowance: {allowance}")]
     InsufficientAllowance { allowance: Nat },
+    #[error("created_at_time is too far in the past")]
     TooOld,
+    #[error("created_at_time is too far in the future, ledger time: {ledger_time}")]
     CreatedInFuture { ledger_time: u64 },
+    #[error("the transfer is a duplicate of transaction {duplicate_of}")]
     Duplicate { duplicate_of: Nat },
+    #[error("the ledger is temporarily unavailable")]
     TemporarilyUnavailable,
+    #[error("generic error (code {error_code}): {message}")]
     GenericError { error_code: Nat, message: String },
 }
 
@@ -296,10 +282,7 @@ pub trait LedgerEnv {
 }
 
 pub mod icrc1 {
-    use crate::{
-        Account, Allowance, AllowanceArgs, ApproveArgs, ApproveError, LedgerEnv, SupportedStandard,
-        Transfer, TransferError, TransferFromArgs, TransferFromError, Value,
-    };
+    use crate::{Account, LedgerEnv, SupportedStandard, Transfer, TransferError, Value};
     use candid::Nat;
 
     pub async fn transfer(
@@ -354,6 +337,14 @@ pub mod icrc1 {
     pub async fn transfer_fee(ledger: &impl LedgerEnv) -> anyhow::Result<Nat> {
         ledger.query("icrc1_fee", ()).await.map(|(t,)| t)
     }
+}
+
+pub mod icrc2 {
+    use crate::{
+        Allowance, AllowanceArgs, ApproveArgs, ApproveError, LedgerEnv, TransferFromArgs,
+        TransferFromError,
+    };
+    use candid::Nat;
 
     pub async fn approve(
         ledger: &impl LedgerEnv,

test/env/lib.rs

@@ -1,14 +1,11 @@
 use anyhow::{bail, Context};
 use candid::Nat;
-use candid::Principal;
 use futures::StreamExt;
-use icrc1_test_env::icrc1::allowance;
-use icrc1_test_env::icrc1::approve;
-use icrc1_test_env::icrc1::transfer_from;
 use icrc1_test_env::icrc1::{
     balance_of, metadata, minting_account, supported_standards, token_decimals, token_name,
     token_symbol, transfer, transfer_fee,
 };
+use icrc1_test_env::icrc2::{allowance, approve, transfer_from};
 use icrc1_test_env::AllowanceArgs;
 use icrc1_test_env::ApproveArgs;
 use icrc1_test_env::TransferFromArgs;
@@ -78,64 +75,30 @@ async fn assert_balance(
     Ok(())
 }
 
-async fn transfer_or_fail(ledger_env: &impl LedgerEnv, amount: Nat, receiver: Principal) -> Nat {
-    transfer(ledger_env, Transfer::amount_to(amount.clone(), receiver))
-        .await
-        .with_context(|| format!("failed to transfer {} tokens to {}", amount, receiver))
-        .unwrap()
-        .unwrap()
-}
-
-async fn approve_or_err(ledger_env: &impl LedgerEnv, args: ApproveArgs) -> anyhow::Result<()> {
-    match approve(ledger_env, args.clone()).await? {
-        Ok(_) => Ok(()),
-        Err(err) => Err(anyhow::Error::msg(format!(
-            "Expected approval of {:?}, spender {:?} to succeed but received {:?}.",
-            args.amount, args.spender, err
-        ))),
-    }
-}
-
-async fn transfer_from_or_err(
-    ledger_env: &impl LedgerEnv,
-    args: TransferFromArgs,
-) -> anyhow::Result<()> {
-    match transfer_from(ledger_env, args.clone()).await.unwrap(){
-        Ok(_) => {
-            Ok(())
-        },
-        Err(err) => {
-             Err(anyhow::Error::msg(format!("Expected transfer from of {:?} from {:?},to {:?} with spender subaccount {:?} to succeed but received {:?}.",args.amount,args.from,args.to, args.spender_subaccount,err)))
-        },
-    }
-}
-
 async fn assert_allowance(
     ledger_env: &impl LedgerEnv,
     from: Account,
     spender: Account,
     expected_allowance: Nat,
 ) -> anyhow::Result<()> {
-    match allowance(
+    let allowance = allowance(
         ledger_env,
         AllowanceArgs {
             account: from.clone(),
             spender: spender.clone(),
         },
     )
-    .await
-    {
-        Ok(allowance) => {
-            if allowance.allowance != expected_allowance {
-                return Err(anyhow::Error::msg(format!("Expected the allowance returned by the allowance endpoint to be the {:?}, received {:?}",expected_allowance,allowance.allowance)));
-            }
-            Ok(())
-        }
-        Err(err) => Err(anyhow::Error::msg(format!(
-            "Expected allowance of {:?} from {:?}, spender {:?} to succeed but received {:?}.",
-            expected_allowance, from, spender, err
-        ))),
+    .await?;
+    if allowance.allowance != expected_allowance {
+        bail!(
+            "Expected the {:?} -> {:?} allowance to be {}, got {}",
+            from,
+            spender,
+            expected_allowance,
+            allowance.allowance
+        );
     }
+    Ok(())
 }
 
 async fn setup_test_account(
@@ -147,7 +110,7 @@ async fn setup_test_account(
     let receiver_env = ledger_env.fork();
     let receiver = receiver_env.principal();
     assert_balance(&receiver_env, receiver, 0).await?;
-    let _tx = transfer_or_fail(ledger_env, amount.clone(), receiver).await;
+    let _tx = transfer(ledger_env, Transfer::amount_to(amount.clone(), receiver)).await??;
     assert_balance(
         &receiver_env,
         Account {
@@ -170,7 +133,11 @@ pub async fn icrc1_test_transfer(ledger_env: impl LedgerEnv + LedgerEnv) -> Test
     let balance_p1 = balance_of(&p1_env, p1_env.principal()).await?;
     let balance_p2 = balance_of(&p2_env, p2_env.principal()).await?;
 
-    let _tx = transfer_or_fail(&p1_env, Nat::from(transfer_amount), p2_env.principal()).await;
+    let _tx = transfer(
+        &p1_env,
+        Transfer::amount_to(transfer_amount, p2_env.principal()),
+    )
+    .await??;
 
     assert_balance(
         &p2_env,
@@ -304,11 +271,11 @@ pub async fn icrc2_test_approve(ledger_env: impl LedgerEnv) -> anyhow::Result<Ou
     let p2_env = ledger_env.fork();
     let approve_amount = Nat::from(10_000);
 
-    approve_or_err(
+    approve(
         &p1_env,
         ApproveArgs::approve_amount(approve_amount.clone(), p2_env.principal()),
     )
-    .await?;
+    .await??;
 
     assert_balance(
         &ledger_env,
@@ -341,23 +308,24 @@ pub async fn icrc2_test_transfer_from(ledger_env: impl LedgerEnv) -> anyhow::Res
 
     let approve_amount = Nat::from(50_000);
 
-    approve_or_err(
+    approve(
         &p1_env,
         ApproveArgs::approve_amount(approve_amount.clone(), p2_env.principal()),
     )
-    .await?;
+    .await??;
 
     // Transferred amount has to be smaller than the approved amount minus the fee for transfering tokens
     let transfer_amount = approve_amount - fee.clone() - Nat::from(1);
-    transfer_from_or_err(
+    transfer_from(
         &p2_env,
         TransferFromArgs::transfer_from(
             transfer_amount.clone(),
             p3_env.principal(),
             p1_env.principal(),
         ),
     )
-    .await?;
+    .await??;
+
     assert_balance(
         &ledger_env,
         p1_env.principal(),