Skip to content

token transfer: use to_token_unit_amount to avoid silent truncation / u128 overflow in amount scaling #638

Description

@marc0olo

Summary

icp token transfer converts the user-supplied decimal amount into the ledger's smallest unit using a manual 10^decimals multiplication followed by to_bigint(), which can silently truncate non-representable amounts and can overflow u128 for ledgers reporting large decimals.

This was identified during review of #637 (which adds token approve/allowance). The new approve command was fixed to use the validated helper icp::parsers::to_token_unit_amount; transfer was intentionally left untouched to keep that PR focused. This issue tracks bringing transfer in line.

Note: #637 has not been merged and it is not yet decided whether the new approve/allowance commands will land. This issue concerns the pre-existing transfer code path regardless of that outcome — the helper to_token_unit_amount already exists in crates/icp/src/parsers.rs and is used elsewhere (e.g. parse_cycles_str).

Location

crates/icp-cli/src/operations/token/transfer.rs (in transfer()), currently:

let ledger_amount_decimal = amount.clone() * 10u128.pow(decimals);
let ledger_amount = ledger_amount_decimal
    .to_bigint()
    .ok_or(TokenTransferError::InvalidAmount)?
    .to_biguint()
    .ok_or(TokenTransferError::InvalidAmount)
    .map(Nat::from)?;

Problems

  1. Silent truncation. If the amount has more fractional digits than the token's icrc1_decimals (e.g. 0.000000001 for an 8-decimal token), to_bigint() truncates toward zero — the user transfers a different amount than they typed, with no error.
  2. Overflow. 10u128.pow(decimals) overflows u128 for decimals >= 39 (panics in debug, wraps in release), producing an incorrect amount for exotic/malicious ledgers.

Suggested fix

Replace the manual scaling with the existing helper, which uses arbitrary-precision math and rejects non-representable amounts:

let ledger_amount = to_token_unit_amount(amount.clone(), decimals)
    .map(Nat::from)
    .map_err(|message| TokenTransferError::InvalidAmount { message })?;

This likely means changing TokenTransferError::InvalidAmount from a unit variant to InvalidAmount { message: String } so the precision error surfaces to the user (mirroring what #637 did for TokenApproveError).

Acceptance criteria

  • icp token transfer rejects amounts that are not exactly representable at the token's decimals, with a clear error.
  • No u128 overflow path in amount conversion.
  • Add/extend integration coverage for the rejection case (an over-precise amount fails cleanly).
  • Consider auditing other call sites for the same 10^decimals pattern while here.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions