From 554b4e3d1d27514339447ffd97e5ace2975732cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 06:41:21 +0000 Subject: [PATCH 1/3] test(ledger): cover a subaccount-qualified ICRC-2 self-spend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A spend needs no allowance only when the spender is the account it spends from, and the ledger decides that on the whole account rather than the owner alone. The suite covered this only for the default subaccount, so neither half of the subaccount case was pinned: that naming the account's own subaccount succeeds, and that naming none fails for want of an allowance even though the owner matches. Callers rely on the rule — the ckETH minter burns from its own fee subaccount — so a regression should surface here rather than in a consumer's integration test. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ledger_suite/icp/ledger/tests/tests.rs | 8 ++++ rs/ledger_suite/icrc1/ledger/tests/tests.rs | 8 ++++ rs/ledger_suite/tests/sm-tests/src/lib.rs | 45 +++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/rs/ledger_suite/icp/ledger/tests/tests.rs b/rs/ledger_suite/icp/ledger/tests/tests.rs index d0d7f5e86724..ed14079b166f 100644 --- a/rs/ledger_suite/icp/ledger/tests/tests.rs +++ b/rs/ledger_suite/icp/ledger/tests/tests.rs @@ -1426,6 +1426,14 @@ fn test_transfer_from_self() { ic_ledger_suite_state_machine_tests::test_transfer_from_self(ledger_wasm(), encode_init_args); } +#[test] +fn test_transfer_from_self_subaccount() { + ic_ledger_suite_state_machine_tests::test_transfer_from_self_subaccount( + ledger_wasm(), + encode_init_args, + ); +} + #[test] fn test_transfer_from_minter() { ic_ledger_suite_state_machine_tests::test_transfer_from_minter(ledger_wasm(), encode_init_args); diff --git a/rs/ledger_suite/icrc1/ledger/tests/tests.rs b/rs/ledger_suite/icrc1/ledger/tests/tests.rs index bacca4f65559..6dc396939721 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -469,6 +469,14 @@ fn test_transfer_from_self() { ic_ledger_suite_state_machine_tests::test_transfer_from_self(ledger_wasm(), encode_init_args); } +#[test] +fn test_transfer_from_self_subaccount() { + ic_ledger_suite_state_machine_tests::test_transfer_from_self_subaccount( + ledger_wasm(), + encode_init_args, + ); +} + #[test] fn test_transfer_from_minter() { ic_ledger_suite_state_machine_tests::test_transfer_from_minter(ledger_wasm(), encode_init_args); diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 18192878ff67..a8151f82cd38 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -3397,6 +3397,51 @@ where assert_eq!(balance_of(&env, canister_id, to.0), 30_000); } +/// A spend needs no allowance only when the spender *is* the account it spends from — which the +/// ledger decides on the whole account, subaccount included. Owning the principal is not enough, +/// so a caller holding funds under a subaccount must name that subaccount to reach them. +pub fn test_transfer_from_self_subaccount( + ledger_wasm: Vec, + encode_init_args: fn(InitArgs) -> T, +) where + T: CandidType, +{ + const SUBACCOUNT: [u8; 32] = [42; 32]; + + let owner = PrincipalId::new_user_test_id(1); + let to = PrincipalId::new_user_test_id(2); + let from = Account { + owner: owner.0, + subaccount: Some(SUBACCOUNT), + }; + + let (env, canister_id) = setup(ledger_wasm, encode_init_args, vec![(from, 100_000)]); + + // Same owner, but the spender is {owner, None} while the funds are under {owner, SUBACCOUNT}: + // two different accounts, so this needs an allowance it does not have. + let mut transfer_from_args = default_transfer_from_args(from, to.0, 30_000); + transfer_from_args.spender_subaccount = None; + assert_eq!( + send_transfer_from(&env, canister_id, owner.0, &transfer_from_args), + Err(TransferFromError::InsufficientAllowance { + allowance: Nat::from(0_u8) + }) + ); + assert_eq!(balance_of(&env, canister_id, from), 100_000); + assert_eq!(balance_of(&env, canister_id, to.0), 0); + + // Naming the account's own subaccount makes it a self-spend, which needs no allowance. + transfer_from_args.spender_subaccount = Some(SUBACCOUNT); + let block_index = send_transfer_from(&env, canister_id, owner.0, &transfer_from_args) + .expect("transfer_from failed"); + assert_eq!( + block_index, 1, + "the rejected spend must not have written a block" + ); + assert_eq!(balance_of(&env, canister_id, from), 100_000 - 30_000 - FEE); + assert_eq!(balance_of(&env, canister_id, to.0), 30_000); +} + pub fn test_transfer_from_minter(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T) where T: CandidType, From 4d8dd79c96a203d5d4486144b8d619e3b31a78f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 07:23:59 +0000 Subject: [PATCH 2/3] test(ledger): cover a subaccount-qualified self-spend burn on ICRC ledgers Burning is how an account holding tokens under a subaccount gives them up, and it is a self-spend when the spender names that same subaccount, so the ledger charges no fee and reduces the supply. Kept separate from the allowance test and wired into the ICRC ledgers only: the ICP ledger checks an allowance for a burn even when the spender is the account itself, although it exempts that case when consuming one, so the same call fails there. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ledger_suite/icrc1/ledger/tests/tests.rs | 8 ++++ rs/ledger_suite/tests/sm-tests/src/lib.rs | 44 +++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/rs/ledger_suite/icrc1/ledger/tests/tests.rs b/rs/ledger_suite/icrc1/ledger/tests/tests.rs index 6dc396939721..8db6fa011047 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -477,6 +477,14 @@ fn test_transfer_from_self_subaccount() { ); } +#[test] +fn test_transfer_from_self_subaccount_burn() { + ic_ledger_suite_state_machine_tests::test_transfer_from_self_subaccount_burn( + ledger_wasm(), + encode_init_args, + ); +} + #[test] fn test_transfer_from_minter() { ic_ledger_suite_state_machine_tests::test_transfer_from_minter(ledger_wasm(), encode_init_args); diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index a8151f82cd38..a53f1f085851 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -3442,6 +3442,50 @@ pub fn test_transfer_from_self_subaccount( assert_eq!(balance_of(&env, canister_id, to.0), 30_000); } +/// A self-spend may also burn, which is how a caller holding tokens under a subaccount gives them +/// up: the destination is the minting account, so the ledger charges no fee and reduces the supply +/// instead of crediting anyone. +/// +/// ICRC ledgers only. The ICP ledger checks an allowance for a burn even when the spender is the +/// account itself (`rs/ledger_suite/icp/src/lib.rs`, `Operation::Burn`), so the same call fails +/// there with `InsufficientAllowance` — note that it exempts the self-spend when *consuming* the +/// allowance, just not when checking it. +pub fn test_transfer_from_self_subaccount_burn( + ledger_wasm: Vec, + encode_init_args: fn(InitArgs) -> T, +) where + T: CandidType, +{ + const SUBACCOUNT: [u8; 32] = [42; 32]; + + let owner = PrincipalId::new_user_test_id(1); + let from = Account { + owner: owner.0, + subaccount: Some(SUBACCOUNT), + }; + + let (env, canister_id) = setup(ledger_wasm, encode_init_args, vec![(from, 100_000)]); + + let minter = minting_account(&env, canister_id).expect("the ledger has a minting account"); + let supply_before = total_supply(&env, canister_id); + let mut burn_args = default_transfer_from_args(from, minter, 20_000); + burn_args.spender_subaccount = Some(SUBACCOUNT); + burn_args.fee = None; + + send_transfer_from(&env, canister_id, owner.0, &burn_args).expect("burn failed"); + + assert_eq!( + balance_of(&env, canister_id, from), + 100_000 - 20_000, + "a burn is fee-free, so only the burned amount leaves the account" + ); + assert_eq!( + total_supply(&env, canister_id), + supply_before - 20_000, + "burning must reduce the supply rather than move the tokens" + ); +} + pub fn test_transfer_from_minter(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T) where T: CandidType, From 31b137af59671d9255736e8f5ee0c5c82b5f4fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 07:32:59 +0000 Subject: [PATCH 3/3] test(ledger): pin how each ledger treats a self-spend burn The two ledgers disagree, so the shared test returns the outcome and each caller states what its own ledger owes: an ICRC ledger accepts the burn, the ICP ledger rejects it for want of an allowance it would never have consumed. Characterising the ICP behaviour beats omitting it. The rejected path is now asserted to leave balance and supply untouched, and closing the gap in Operation::Burn has to flip this assertion rather than pass unnoticed. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ledger_suite/icp/ledger/tests/tests.rs | 18 ++++++ rs/ledger_suite/icrc1/ledger/tests/tests.rs | 3 +- rs/ledger_suite/tests/sm-tests/src/lib.rs | 64 ++++++++++++++------- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/rs/ledger_suite/icp/ledger/tests/tests.rs b/rs/ledger_suite/icp/ledger/tests/tests.rs index ed14079b166f..dc7dbbd7ecc8 100644 --- a/rs/ledger_suite/icp/ledger/tests/tests.rs +++ b/rs/ledger_suite/icp/ledger/tests/tests.rs @@ -35,6 +35,7 @@ use icrc_ledger_types::icrc1::{ }; use icrc_ledger_types::icrc2::allowance::{Allowance, AllowanceArgs}; use icrc_ledger_types::icrc2::approve::{ApproveArgs, ApproveError}; +use icrc_ledger_types::icrc2::transfer_from::TransferFromError; use icrc_ledger_types::icrc21::errors::{ErrorInfo, Icrc21Error}; use icrc_ledger_types::icrc21::requests::ConsentMessageMetadata; use icrc_ledger_types::icrc21::requests::{ @@ -1434,6 +1435,23 @@ fn test_transfer_from_self_subaccount() { ); } +/// Unlike an ICRC ledger, this one requires an allowance to burn even when the spender is the +/// account itself: `Operation::Burn` in `rs/ledger_suite/icp/src/lib.rs` checks one for any +/// spender, though it exempts the self-spend when consuming one. Pinned rather than skipped, so +/// that closing the gap is a visible change here. +#[test] +fn test_transfer_from_self_subaccount_burn() { + assert_eq!( + ic_ledger_suite_state_machine_tests::test_transfer_from_self_subaccount_burn( + ledger_wasm(), + encode_init_args, + ), + Err(TransferFromError::InsufficientAllowance { + allowance: Nat::from(0_u8) + }) + ); +} + #[test] fn test_transfer_from_minter() { ic_ledger_suite_state_machine_tests::test_transfer_from_minter(ledger_wasm(), encode_init_args); diff --git a/rs/ledger_suite/icrc1/ledger/tests/tests.rs b/rs/ledger_suite/icrc1/ledger/tests/tests.rs index 8db6fa011047..ba9f0a47f2cc 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -482,7 +482,8 @@ fn test_transfer_from_self_subaccount_burn() { ic_ledger_suite_state_machine_tests::test_transfer_from_self_subaccount_burn( ledger_wasm(), encode_init_args, - ); + ) + .expect("an ICRC ledger accepts a self-spend burn, needing no allowance"); } #[test] diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index a53f1f085851..a4a60dc0916a 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -3442,21 +3442,26 @@ pub fn test_transfer_from_self_subaccount( assert_eq!(balance_of(&env, canister_id, to.0), 30_000); } -/// A self-spend may also burn, which is how a caller holding tokens under a subaccount gives them -/// up: the destination is the minting account, so the ledger charges no fee and reduces the supply -/// instead of crediting anyone. +/// Burns from `{P, Some(s)}` with the spender naming that same subaccount, and reports what the +/// ledger made of it — the two ledgers disagree, so the caller states which outcome its own owes. /// -/// ICRC ledgers only. The ICP ledger checks an allowance for a burn even when the spender is the -/// account itself (`rs/ledger_suite/icp/src/lib.rs`, `Operation::Burn`), so the same call fails -/// there with `InsufficientAllowance` — note that it exempts the self-spend when *consuming* the -/// allowance, just not when checking it. +/// ICRC ledgers accept it: burning is how an account holding tokens under a subaccount gives them +/// up, and naming the account's own subaccount makes it a self-spend. The ICP ledger rejects it, +/// because `Operation::Burn` (`rs/ledger_suite/icp/src/lib.rs`) checks an allowance for any spender +/// — exempting the self-spend only when *consuming* one, not when checking. +/// +/// Either way the books must stay consistent, which is asserted here: an accepted burn is fee-free +/// and reduces the supply, a rejected one moves nothing at all. pub fn test_transfer_from_self_subaccount_burn( ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T, -) where +) -> Result +where T: CandidType, { const SUBACCOUNT: [u8; 32] = [42; 32]; + const INITIAL_BALANCE: u64 = 100_000; + const BURN_AMOUNT: u64 = 20_000; let owner = PrincipalId::new_user_test_id(1); let from = Account { @@ -3464,26 +3469,43 @@ pub fn test_transfer_from_self_subaccount_burn( subaccount: Some(SUBACCOUNT), }; - let (env, canister_id) = setup(ledger_wasm, encode_init_args, vec![(from, 100_000)]); + let (env, canister_id) = setup(ledger_wasm, encode_init_args, vec![(from, INITIAL_BALANCE)]); let minter = minting_account(&env, canister_id).expect("the ledger has a minting account"); let supply_before = total_supply(&env, canister_id); - let mut burn_args = default_transfer_from_args(from, minter, 20_000); + let mut burn_args = default_transfer_from_args(from, minter, BURN_AMOUNT); burn_args.spender_subaccount = Some(SUBACCOUNT); burn_args.fee = None; - send_transfer_from(&env, canister_id, owner.0, &burn_args).expect("burn failed"); + let result = send_transfer_from(&env, canister_id, owner.0, &burn_args); - assert_eq!( - balance_of(&env, canister_id, from), - 100_000 - 20_000, - "a burn is fee-free, so only the burned amount leaves the account" - ); - assert_eq!( - total_supply(&env, canister_id), - supply_before - 20_000, - "burning must reduce the supply rather than move the tokens" - ); + match result { + Ok(_) => { + assert_eq!( + balance_of(&env, canister_id, from), + INITIAL_BALANCE - BURN_AMOUNT, + "a burn is fee-free, so only the burned amount leaves the account" + ); + assert_eq!( + total_supply(&env, canister_id), + supply_before - BURN_AMOUNT, + "burning must reduce the supply rather than move the tokens" + ); + } + Err(_) => { + assert_eq!( + balance_of(&env, canister_id, from), + INITIAL_BALANCE, + "a rejected burn must not move funds" + ); + assert_eq!( + total_supply(&env, canister_id), + supply_before, + "a rejected burn must not change the supply" + ); + } + } + result } pub fn test_transfer_from_minter(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T)