Natural subtraction underflow in burn_balance() causes trap instead of proper error handling

[wilaq]

icrc1.mo/src/ICRC1/Utils.mo

Line 306 in b451321

balance - amount;

icrc1.mo/src/ICRC1/Utils.mo

Line 268 in b451321

balance - amount;

The burn_balance() and transfer_balance() functions in src/ICRC1/Utils.mo use natural subtraction (balance - amount) without defensive checks, causing the canister to trap with "Natural subtraction underflow" when the balance is insufficient. This prevents proper error handling and results in a poor user experience.

Reproduction

1. Create an account with a balance of exactly or slightly less than the burn amount
2. Attempt to burn tokens via icrc1_transfer() with to set to the minting account
3. The transaction passes initial validation but traps during execution

Example: Attempting to burn 0.0027 tokens (270,000 base units) when the account has insufficient balance results in:

IC0503: Canister called `ic0.trap` with message: 'Natural subtraction underflow'

Affected Code

File: src/ICRC1/Utils.mo

Line 306 in burn_balance():

update_balance(
    state.accounts,
    account,
    func(balance) {
        balance - amount;  // ⚠️ Natural subtraction without check
    },
);

Line 268 in transfer_balance():

update_balance(
    state.accounts,
    from,
    func(balance) {
        balance - amount;  // ⚠️ Natural subtraction without check
    },
);

Root Cause

While the main transaction validation in lib.mo (line 2067) checks if (balance < tx_req.amount), there's a potential race condition or state inconsistency where the balance can change between the check and the actual subtraction. Additionally, the validation may not account for all edge cases involving fees or concurrent operations.

Proposed Fix

Add defensive balance checks before subtraction:

update_balance(
    state.accounts,
    account,
    func(balance) {
        if (balance < amount) {
            Debug.trap("Insufficient balance for operation: " # debug_show(balance) # " < " # debug_show(amount));
        };
        balance - amount;
    },
);

Better solution: Modify the functions to return Result<(), Text> instead of trapping, allowing the caller to handle insufficient balance errors gracefully:

public func burn_balance(
    state : MigrationTypes.Current.State,
    account : MigrationTypes.Current.Account,
    amount : MigrationTypes.Current.Balance,
) : Result.Result<(), Text> {
    let current_balance = get_balance(state.accounts, account);
    if (current_balance < amount) {
        return #err("Insufficient balance");
    };

    update_balance(
        state.accounts,
        account,
        func(balance) { balance - amount },
    );

    state._burned_tokens += amount;
    #ok(())
};

Impact

• Severity: High - Causes complete transaction failure with trap
• User Impact: Users see cryptic IC0503 errors instead of proper "Insufficient balance" messages
• Workaround: None - transactions simply fail

Environment

• Library: icrc1-mo v0.2.0
• Network: Internet Computer
• Motoko version: 0.11.0+