From aec4a1a0af04e6cc00096389d2191af70d7768a9 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 29 Sep 2025 12:36:49 +0000 Subject: [PATCH 01/67] handle incorrect blocks, don't schedule timer if bad block --- rs/ledger_suite/icrc1/index-ng/BUILD.bazel | 2 + rs/ledger_suite/icrc1/index-ng/src/main.rs | 11 ++- .../icrc1/index-ng/tests/common/mod.rs | 14 +++ rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 94 ++++++++++++++++++- 4 files changed, 116 insertions(+), 5 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/BUILD.bazel b/rs/ledger_suite/icrc1/index-ng/BUILD.bazel index ccd245e4cd02..61c3ccc9ba5c 100644 --- a/rs/ledger_suite/icrc1/index-ng/BUILD.bazel +++ b/rs/ledger_suite/icrc1/index-ng/BUILD.bazel @@ -102,12 +102,14 @@ rust_test( data = [ conf["index_wasm"], conf["ledger_wasm"], + "//rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger:icrc3_test_ledger_canister.wasm.gz", ], env = { "RUST_TEST_THREADS": "4", "CARGO_MANIFEST_DIR": "rs/ledger_suite/icrc1/index-ng", "IC_ICRC1_INDEX_NG_WASM_PATH": "$(rootpath " + conf["index_wasm"] + ")", "IC_ICRC1_LEDGER_WASM_PATH": "$(rootpath " + conf["ledger_wasm"] + ")", + "ICRC3_TEST_LEDGER_CANISTER_WASM_PATH": "$(rootpath //rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger:icrc3_test_ledger_canister.wasm.gz)", }, extra_srcs = ["tests/common/mod.rs"], tags = ["cpu:4"], diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index d3c09472e300..1d74cd8c238f 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -584,6 +584,10 @@ pub async fn build_index() -> Option<()> { num_indexed, retrieve_blocks_from_ledger_interval ); + // Schedule the next index timer + set_build_index_timer(with_state(|state| { + state.retrieve_blocks_from_ledger_interval() + })); Some(()) } @@ -684,7 +688,7 @@ async fn fetch_blocks_via_icrc3() -> Option { } fn set_build_index_timer(after: Duration) -> TimerId { - ic_cdk_timers::set_timer_interval(after, || { + ic_cdk_timers::set_timer(after, || { ic_cdk::spawn(async { let _ = build_index().await; }) @@ -693,6 +697,7 @@ fn set_build_index_timer(after: Duration) -> TimerId { fn append_block(block_index: BlockIndex64, block: GenericBlock) { measure_span(&PROFILING_DATA, "append_blocks", move || { + let original_hash = block.hash(); let block = generic_block_to_encoded_block_or_trap(block_index, block); // append the encoded block to the block log @@ -703,6 +708,10 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) { }); let decoded_block = decode_encoded_block_or_trap(block_index, block); + let decoded_hash = Block::::block_hash(&decoded_block.clone().encode()); + if original_hash != decoded_hash.as_slice() { + trap(format!("Unknown block at index {block_index}.")); + } // add the block idx to the indices with_account_block_ids(|account_block_ids| { diff --git a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs index 901e902bb6c4..16a530fc8d82 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs @@ -97,6 +97,20 @@ pub fn install_ledger( .unwrap() } +fn icrc3_test_ledger() -> Vec { + std::fs::read(std::env::var("ICRC3_TEST_LEDGER_CANISTER_WASM_PATH").unwrap()).unwrap() +} + +pub fn install_icrc3_test_ledger(env: &StateMachine) -> CanisterId { + env.install_canister_with_cycles( + icrc3_test_ledger(), + Encode!(&()).unwrap(), + None, + ic_types::Cycles::new(STARTING_CYCLES_PER_CANISTER), + ) + .unwrap() +} + #[allow(dead_code)] pub fn install_index_ng(env: &StateMachine, init_arg: IndexInitArg) -> CanisterId { let args = IndexArg::Init(init_arg); diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index c3b6b181f681..40bd6e8eef00 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -1,7 +1,7 @@ use crate::common::{ ARCHIVE_TRIGGER_THRESHOLD, FEE, MAX_BLOCKS_FROM_ARCHIVE, account, default_archive_options, - index_ng_wasm, install_index_ng, install_ledger, ledger_get_all_blocks, ledger_wasm, - wait_until_sync_is_completed, + index_ng_wasm, install_icrc3_test_ledger, install_index_ng, install_ledger, + ledger_get_all_blocks, ledger_wasm, wait_until_sync_is_completed, }; use candid::{Decode, Encode, Nat, Principal}; use ic_agent::identity::Identity; @@ -11,12 +11,16 @@ use ic_icrc1_index_ng::{ GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksResponse, IndexArg, InitArg as IndexInitArg, ListSubaccountsArgs, TransactionWithId, }; -use ic_icrc1_ledger::{ChangeFeeCollector, LedgerArgument, UpgradeArgs as LedgerUpgradeArgs}; +use ic_icrc1_ledger::{ + ChangeFeeCollector, LedgerArgument, Tokens, UpgradeArgs as LedgerUpgradeArgs, +}; use ic_icrc1_test_utils::{ - ArgWithCaller, LedgerEndpointArg, minter_identity, valid_transactions_strategy, + ArgWithCaller, LedgerEndpointArg, icrc3::BlockBuilder, minter_identity, + valid_transactions_strategy, }; use ic_ledger_suite_state_machine_tests::test_http_request_decoding_quota; use ic_state_machine_tests::StateMachine; +use icrc_ledger_types::icrc::generic_value::ICRC3Value; use icrc_ledger_types::icrc1::account::{Account, Subaccount}; use icrc_ledger_types::icrc1::transfer::{BlockIndex, TransferArg, TransferError}; use icrc_ledger_types::icrc2::approve::{ApproveArgs, ApproveError}; @@ -222,6 +226,22 @@ fn transfer( icrc1_transfer(env, ledger_id, owner.into(), req) } +fn add_block(env: &StateMachine, ledger_id: CanisterId, block: &ICRC3Value) -> BlockIndex { + let req = Encode!(block).expect("Failed to encode block"); + let res = env + .execute_ingress_as( + ic_base_types::PrincipalId(Principal::anonymous()), + ledger_id, + "add_block", + req, + ) + .unwrap_or_else(|e| panic!("Failed to add block. arg:{:?} error:{}", block, e)) + .bytes(); + Decode!(&res, Result) + .expect("Failed to decode Result") + .unwrap_or_else(|e| panic!("Failed to add block. arg:{:?} error:{}", block, e)) +} + fn icrc2_approve( env: &StateMachine, ledger_id: CanisterId, @@ -454,6 +474,72 @@ fn test_ledger_growing() { ); } +#[test] +fn test_unknown_block() { + // check that the index canister can incrementally get the blocks from the ledger. + + let env = &StateMachine::new(); + let ledger_id = install_icrc3_test_ledger(env); + let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); + + // Test initial mint block. + wait_until_sync_is_completed(env, index_id, ledger_id); + assert_ledger_index_parity(env, ledger_id, index_id); + + // Test first transfer block. + + const TEST_ACCOUNT: Account = Account { + owner: PrincipalId::new_user_test_id(44).0, + subaccount: None, + }; + + const TA1: Account = Account { + owner: PrincipalId::new_user_test_id(55).0, + subaccount: None, + }; + + const TA2: Account = Account { + owner: PrincipalId::new_user_test_id(66).0, + subaccount: None, + }; + + let block0 = BlockBuilder::new(0, 1000) + .with_fee(Tokens::from(50u64)) + .mint(TEST_ACCOUNT, Tokens::from(1_000u64)) + .build(); + let block1 = BlockBuilder::new(1, 2000) + .with_parent_hash(block0.clone().hash().to_vec()) + .mint(TA1, Tokens::from(1_000u64)) + .build(); + let block2 = BlockBuilder::new(2, 3000) + .with_parent_hash(block1.clone().hash().to_vec()) + .with_fee(Tokens::from(50u64)) + .transfer(TEST_ACCOUNT, TA2, Tokens::from(50u64)) + .build(); + let mut block2_map = match block2 { + ICRC3Value::Map(btree_map) => btree_map, + _ => panic!("block should be a map"), + }; + block2_map.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); + let block2 = ICRC3Value::Map(block2_map); + + add_block(env, ledger_id, &block0); + add_block(env, ledger_id, &block1); + + wait_until_sync_is_completed(env, index_id, ledger_id); + assert_ledger_index_parity(env, ledger_id, index_id); + + add_block(env, ledger_id, &block2); + + for _i in 0..3 { + env.advance_time(Duration::from_secs(60)); + env.tick(); + } + let ledger_blocks = ledger_get_all_blocks(env, ledger_id, 0, u64::MAX); + let index_blocks = index_get_all_blocks(env, index_id, 0, u64::MAX); + assert_eq!(ledger_blocks.chain_length, index_blocks.chain_length + 1); +} + #[test] fn test_archive_indexing() { let env = &StateMachine::new(); From 21a9c3bad6c48ce3b1f2eb1b34ac831b30ebfacd Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 29 Sep 2025 13:34:02 +0000 Subject: [PATCH 02/67] clippy --- rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs index 16a530fc8d82..b9c2f7feb696 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs @@ -101,6 +101,7 @@ fn icrc3_test_ledger() -> Vec { std::fs::read(std::env::var("ICRC3_TEST_LEDGER_CANISTER_WASM_PATH").unwrap()).unwrap() } +#[allow(dead_code)] pub fn install_icrc3_test_ledger(env: &StateMachine) -> CanisterId { env.install_canister_with_cycles( icrc3_test_ledger(), From 725b25b380e25cfa74df3c862030a734461ec13c Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 12:41:51 +0000 Subject: [PATCH 03/67] stop sync with error --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 115 ++++++++++++++------- 1 file changed, 76 insertions(+), 39 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 1d74cd8c238f..cf5448b5ab29 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -111,6 +111,12 @@ thread_local! { /// Cache of the canister, i.e. ephemeral data that doesn't need to be /// persistent between upgrades static CACHE: RefCell = RefCell::new(Cache::default()); + + /// The ID of the block sync timer. `None` means the sync is stopped due to an error. + static TIMER_ID: RefCell> = RefCell::new(None); + + /// The description of the error encountered during block sync. + static SYNC_ERROR: RefCell> = RefCell::new(None); } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -584,13 +590,13 @@ pub async fn build_index() -> Option<()> { num_indexed, retrieve_blocks_from_ledger_interval ); - // Schedule the next index timer - set_build_index_timer(with_state(|state| { - state.retrieve_blocks_from_ledger_interval() - })); Some(()) } +fn sync_stopped() -> bool { + TIMER_ID.with(|tid| *tid.borrow()).is_none() +} + async fn fetch_blocks_via_get_blocks() -> Option { let mut num_indexed = 0; let next_id = with_blocks(|blocks| blocks.len()); @@ -606,14 +612,15 @@ async fn fetch_blocks_via_get_blocks() -> Option { }; let res = get_blocks_from_archive(&archived).await?; next_archived_txid += res.blocks.len(); - num_indexed += res.blocks.len(); remaining -= res.blocks.len(); - append_blocks(res.blocks); + num_indexed += append_blocks(res.blocks); + if sync_stopped() { + return Some(num_indexed); + } } } - num_indexed += res.blocks.len(); - append_blocks(res.blocks); - Some(num_indexed as u64) + num_indexed += append_blocks(res.blocks); + Some(num_indexed) } async fn fetch_blocks_via_icrc3() -> Option { @@ -687,18 +694,56 @@ async fn fetch_blocks_via_icrc3() -> Option { } } -fn set_build_index_timer(after: Duration) -> TimerId { - ic_cdk_timers::set_timer(after, || { +fn set_build_index_timer(after: Duration) { + let timer_id = ic_cdk_timers::set_timer_interval(after, || { ic_cdk::spawn(async { let _ = build_index().await; }) - }) + }); + TIMER_ID.with(|tid| *tid.borrow_mut() = Some(timer_id)); } -fn append_block(block_index: BlockIndex64, block: GenericBlock) { +fn stop_timer_with_error(error: String) { + log!(P0, "{}", error); + if let Some(timer_id) = TIMER_ID.with(|tid| *tid.borrow()) { + ic_cdk_timers::clear_timer(timer_id); + TIMER_ID.with(|tid| *tid.borrow_mut() = None); + } + SYNC_ERROR.with(|sync_error| *sync_error.borrow_mut() = Some(error)); +} + +fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { measure_span(&PROFILING_DATA, "append_blocks", move || { + assert!( + !sync_stopped(), + "Trying to append a block, even though the sync is stopped." + ); + let original_hash = block.hash(); - let block = generic_block_to_encoded_block_or_trap(block_index, block); + let block = match generic_block_to_encoded_block(block) { + Ok(block) => block, + Err(e) => { + stop_timer_with_error(format!( + "Unable to decode generic block at index {block_index}. Error: {e}" + )); + return false; + } + }; + + let decoded_block = match Block::::decode(block.clone()) { + Ok(block) => block, + Err(e) => { + stop_timer_with_error(format!( + "Unable to decode encoded block at index {block_index}. Error: {e}" + )); + return false; + } + }; + let decoded_hash = Block::::block_hash(&decoded_block.clone().encode()); + if original_hash != decoded_hash.as_slice() { + stop_timer_with_error(format!("Unknown block at index {block_index}.")); + return false; + } // append the encoded block to the block log with_blocks(|blocks| { @@ -707,12 +752,6 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) { .unwrap_or_else(|_| trap("no space left")) }); - let decoded_block = decode_encoded_block_or_trap(block_index, block); - let decoded_hash = Block::::block_hash(&decoded_block.clone().encode()); - if original_hash != decoded_hash.as_slice() { - trap(format!("Unknown block at index {block_index}.")); - } - // add the block idx to the indices with_account_block_ids(|account_block_ids| { for account in get_accounts(&decoded_block) { @@ -725,17 +764,25 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) { // change the balance of the involved accounts process_balance_changes(block_index, &decoded_block); - }); + + // Adding block was successful + true + }) } -fn append_blocks(new_blocks: Vec) { +fn append_blocks(new_blocks: Vec) -> u64 { + let mut num_indexed = 0; // the index of the next block that we // are going to append let mut block_index = with_blocks(|blocks| blocks.len()); for block in new_blocks { - append_block(block_index, block); + if !append_block(block_index, block) { + break; + } + num_indexed += 1; block_index += 1; } + num_indexed } fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { @@ -745,19 +792,20 @@ fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { // sanity check let expected_id = start_id + blocks.len() as u64; if id != expected_id { - log!( - P0, + let error = format!( "[fetch_blocks_via_icrc3]: wrong block index returned by ledger. Expected: {} actual: {}", - expected_id, - id, + expected_id, id ); + stop_timer_with_error(error); return None; } // This conversion is safe as `Value` // can represent any `ICRC3Value`. blocks.push(Value::from(block)); } - append_blocks(blocks); + if blocks.len() as u64 > append_blocks(blocks) { + return None; + } Some(()) } @@ -876,17 +924,6 @@ fn credit(block_index: BlockIndex64, account: Account, amount: Tokens) { }); } -fn generic_block_to_encoded_block_or_trap( - block_index: BlockIndex64, - block: GenericBlock, -) -> EncodedBlock { - generic_block_to_encoded_block(block).unwrap_or_else(|e| { - trap(format!( - "Unable to decode generic block at index {block_index}. Error: {e}" - )) - }) -} - fn decode_encoded_block_or_trap(block_index: BlockIndex64, block: EncodedBlock) -> Block { Block::::decode(block).unwrap_or_else(|e| { trap(format!( From e93399043baf44cbcae52b3ee13fefe3fafafb66 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 12:56:44 +0000 Subject: [PATCH 04/67] handle append blocks failure --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 45 ++++++++++++---------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index cf5448b5ab29..ecd5dba6c250 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -579,17 +579,19 @@ pub async fn build_index() -> Option<()> { }); }); let num_indexed = match find_get_blocks_method().await { - GetBlocksMethod::GetBlocks => fetch_blocks_via_get_blocks().await?, - GetBlocksMethod::ICRC3GetBlocks => fetch_blocks_via_icrc3().await?, + GetBlocksMethod::GetBlocks => fetch_blocks_via_get_blocks().await, + GetBlocksMethod::ICRC3GetBlocks => fetch_blocks_via_icrc3().await, }; - let retrieve_blocks_from_ledger_interval = - with_state(|state| state.retrieve_blocks_from_ledger_interval()); - log!( - P1, - "Indexed: {} waiting : {:?}", - num_indexed, - retrieve_blocks_from_ledger_interval - ); + if let Some(num_indexed) = num_indexed { + let retrieve_blocks_from_ledger_interval = + with_state(|state| state.retrieve_blocks_from_ledger_interval()); + log!( + P1, + "Indexed: {} waiting : {:?}", + num_indexed, + retrieve_blocks_from_ledger_interval + ); + } Some(()) } @@ -612,15 +614,18 @@ async fn fetch_blocks_via_get_blocks() -> Option { }; let res = get_blocks_from_archive(&archived).await?; next_archived_txid += res.blocks.len(); + num_indexed += res.blocks.len(); remaining -= res.blocks.len(); - num_indexed += append_blocks(res.blocks); - if sync_stopped() { - return Some(num_indexed); + if !append_blocks(res.blocks) { + return None; } } } - num_indexed += append_blocks(res.blocks); - Some(num_indexed) + num_indexed += res.blocks.len(); + if !append_blocks(res.blocks) { + return None; + } + Some(num_indexed as u64) } async fn fetch_blocks_via_icrc3() -> Option { @@ -770,19 +775,17 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { }) } -fn append_blocks(new_blocks: Vec) -> u64 { - let mut num_indexed = 0; +fn append_blocks(new_blocks: Vec) -> bool { // the index of the next block that we // are going to append let mut block_index = with_blocks(|blocks| blocks.len()); for block in new_blocks { if !append_block(block_index, block) { - break; + return false; } - num_indexed += 1; block_index += 1; } - num_indexed + true } fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { @@ -803,7 +806,7 @@ fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { // can represent any `ICRC3Value`. blocks.push(Value::from(block)); } - if blocks.len() as u64 > append_blocks(blocks) { + if !append_blocks(blocks) { return None; } Some(()) From 08af8d750bf2748ac39343a7a8e2fcce502797a8 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 13:08:37 +0000 Subject: [PATCH 05/67] use results --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 74 ++++++++++------------ 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index ecd5dba6c250..0dd2ef905d91 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -582,7 +582,7 @@ pub async fn build_index() -> Option<()> { GetBlocksMethod::GetBlocks => fetch_blocks_via_get_blocks().await, GetBlocksMethod::ICRC3GetBlocks => fetch_blocks_via_icrc3().await, }; - if let Some(num_indexed) = num_indexed { + if let Ok(num_indexed) = num_indexed { let retrieve_blocks_from_ledger_interval = with_state(|state| state.retrieve_blocks_from_ledger_interval()); log!( @@ -599,7 +599,7 @@ fn sync_stopped() -> bool { TIMER_ID.with(|tid| *tid.borrow()).is_none() } -async fn fetch_blocks_via_get_blocks() -> Option { +async fn fetch_blocks_via_get_blocks() -> Result { let mut num_indexed = 0; let next_id = with_blocks(|blocks| blocks.len()); let res = get_blocks_from_ledger(next_id).await?; @@ -616,19 +616,15 @@ async fn fetch_blocks_via_get_blocks() -> Option { next_archived_txid += res.blocks.len(); num_indexed += res.blocks.len(); remaining -= res.blocks.len(); - if !append_blocks(res.blocks) { - return None; - } + append_blocks(res.blocks)?; } } num_indexed += res.blocks.len(); - if !append_blocks(res.blocks) { - return None; - } - Some(num_indexed as u64) + append_blocks(res.blocks)?; + Ok(num_indexed as u64) } -async fn fetch_blocks_via_icrc3() -> Option { +async fn fetch_blocks_via_icrc3() -> Result { // The current number of blocks is also the id of the next // block to query from the Ledger. let previous_num_blocks = with_blocks(|blocks| blocks.len()); @@ -653,13 +649,12 @@ async fn fetch_blocks_via_icrc3() -> Option { // one, i.e. next_id + num_indexed let expected_id = with_blocks(|blocks| blocks.len()); if arg.start != expected_id { - log!( - P0, + let error = format!( "[fetch_blocks_via_icrc3]: wrong start index in archive args. Expected: {} actual: {}", - expected_id, - arg.start, + expected_id, arg.start, ); - return None; + stop_timer_with_error(error); + return Err(()); } let archived = ArchivedBlocks { @@ -670,14 +665,14 @@ async fn fetch_blocks_via_icrc3() -> Option { // sanity check: the index does not support nested archives if !res.archived_blocks.is_empty() { - log!( - P0, + let error = format!( "[fetch_blocks_via_icrc3]: The archive callback {:?} with arg {:?} returned one or more archived blocks and the index is currently not supporting nested archived blocks. Archived blocks returned are {:?}", callback.clone(), arg.clone(), res.archived_blocks, ); - return None; + stop_timer_with_error(error); + return Err(()); } // change `arg` for the next iteration @@ -691,11 +686,15 @@ async fn fetch_blocks_via_icrc3() -> Option { append_icrc3_blocks(res.blocks)?; let num_blocks = with_blocks(|blocks| blocks.len()); match num_blocks.checked_sub(previous_num_blocks) { - None => panic!( - "The number of blocks {} is smaller than the number of blocks before indexing {}. This is impossible. I'm trapping to reset the state", - num_blocks, previous_num_blocks - ), - Some(new_blocks_indexed) => Some(new_blocks_indexed), + None => { + let error = format!( + "The number of blocks {} is smaller than the number of blocks before indexing {}. This is impossible. I'm trapping to reset the state", + num_blocks, previous_num_blocks + ); + stop_timer_with_error(error); + Err(()) + } + Some(new_blocks_indexed) => Ok(new_blocks_indexed), } } @@ -717,7 +716,7 @@ fn stop_timer_with_error(error: String) { SYNC_ERROR.with(|sync_error| *sync_error.borrow_mut() = Some(error)); } -fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { +fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), ()> { measure_span(&PROFILING_DATA, "append_blocks", move || { assert!( !sync_stopped(), @@ -731,7 +730,7 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { stop_timer_with_error(format!( "Unable to decode generic block at index {block_index}. Error: {e}" )); - return false; + return Err(()); } }; @@ -741,13 +740,13 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { stop_timer_with_error(format!( "Unable to decode encoded block at index {block_index}. Error: {e}" )); - return false; + return Err(()); } }; let decoded_hash = Block::::block_hash(&decoded_block.clone().encode()); if original_hash != decoded_hash.as_slice() { stop_timer_with_error(format!("Unknown block at index {block_index}.")); - return false; + return Err(()); } // append the encoded block to the block log @@ -770,25 +769,22 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> bool { // change the balance of the involved accounts process_balance_changes(block_index, &decoded_block); - // Adding block was successful - true + Ok(()) }) } -fn append_blocks(new_blocks: Vec) -> bool { +fn append_blocks(new_blocks: Vec) -> Result<(), ()> { // the index of the next block that we // are going to append let mut block_index = with_blocks(|blocks| blocks.len()); for block in new_blocks { - if !append_block(block_index, block) { - return false; - } + append_block(block_index, block)?; block_index += 1; } - true + Ok(()) } -fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { +fn append_icrc3_blocks(new_blocks: Vec) -> Result<(), ()> { let mut blocks = vec![]; let start_id = with_blocks(|blocks| blocks.len()); for BlockWithId { id, block } in new_blocks { @@ -800,16 +796,14 @@ fn append_icrc3_blocks(new_blocks: Vec) -> Option<()> { expected_id, id ); stop_timer_with_error(error); - return None; + return Err(()); } // This conversion is safe as `Value` // can represent any `ICRC3Value`. blocks.push(Value::from(block)); } - if !append_blocks(blocks) { - return None; - } - Some(()) + append_blocks(blocks)?; + Ok(()) } fn index_fee_collector(block_index: BlockIndex64, block: &Block) { From 6d211a89894989144c14c1b03c541e46f977cb0b Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 13:21:59 +0000 Subject: [PATCH 06/67] change more options to results --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 42 +++++++++++----------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 0dd2ef905d91..eccc7fba6577 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -450,7 +450,7 @@ where .map_err(|err| format!("failed to candid decode the output: {err}")) } -async fn get_blocks_from_ledger(start: u64) -> Option { +async fn get_blocks_from_ledger(start: u64) -> Result { let (ledger_id, length) = with_state(|state| (state.ledger_id, state.max_blocks_per_response)); let req = GetBlocksRequest { start: Nat::from(start), @@ -466,17 +466,18 @@ async fn get_blocks_from_ledger(start: u64) -> Option { ) .await; match res { - Ok(res) => Some(res), + Ok(res) => Ok(res), Err(err) => { - log!(P0, "[get_blocks_from_ledger] failed to get blocks: {}", err); - None + let error = format!("[get_blocks_from_ledger] failed to get blocks: {}", err); + stop_timer_with_error(error); + Err(()) } } } async fn get_blocks_from_archive( archived: &ArchivedRange, -) -> Option { +) -> Result { let req = GetBlocksRequest { start: archived.start.clone(), length: archived.length.clone(), @@ -490,19 +491,16 @@ async fn get_blocks_from_archive( ) .await; match res { - Ok(res) => Some(res), + Ok(res) => Ok(res), Err(err) => { - log!( - P0, - "[get_blocks_from_archive] failed to get blocks: {}", - err - ); - None + let error = format!("[get_blocks_from_archive] failed to get blocks: {}", err); + stop_timer_with_error(error); + Err(()) } } } -async fn icrc3_get_blocks_from_ledger(start: u64) -> Option { +async fn icrc3_get_blocks_from_ledger(start: u64) -> Result { let (ledger_id, length) = with_state(|state| (state.ledger_id, state.max_blocks_per_response)); let req = vec![GetBlocksRequest { start: Nat::from(start), @@ -518,19 +516,19 @@ async fn icrc3_get_blocks_from_ledger(start: u64) -> Option { ) .await; match res { - Ok(res) => Some(res), + Ok(res) => Ok(res), Err(err) => { - log!( - P0, + let error = format!( "[icrc3_get_blocks_from_ledger] failed to get blocks: {}", err ); - None + stop_timer_with_error(error); + Err(()) } } } -async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Option { +async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Result { let res = measured_call( "build_index.icrc3_get_blocks_from_archive.encode", "build_index.icrc3_get_blocks_from_archive.decode", @@ -540,14 +538,14 @@ async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Option Some(res), + Ok(res) => Ok(res), Err(err) => { - log!( - P0, + let error = format!( "[icrc3_get_blocks_from_archive] failed to get blocks: {}", err ); - None + stop_timer_with_error(error); + Err(()) } } } From 6f952f84412a468b51f3f826ae685396b81c8de7 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 13:34:47 +0000 Subject: [PATCH 07/67] don't stop timer if failed to contact ledger or archive --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index eccc7fba6577..3a2504b0888a 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -468,8 +468,7 @@ async fn get_blocks_from_ledger(start: u64) -> Result { match res { Ok(res) => Ok(res), Err(err) => { - let error = format!("[get_blocks_from_ledger] failed to get blocks: {}", err); - stop_timer_with_error(error); + log!(P0, "[get_blocks_from_ledger] failed to get blocks: {}", err); Err(()) } } @@ -493,8 +492,11 @@ async fn get_blocks_from_archive( match res { Ok(res) => Ok(res), Err(err) => { - let error = format!("[get_blocks_from_archive] failed to get blocks: {}", err); - stop_timer_with_error(error); + log!( + P0, + "[get_blocks_from_archive] failed to get blocks: {}", + err + ); Err(()) } } @@ -518,11 +520,11 @@ async fn icrc3_get_blocks_from_ledger(start: u64) -> Result match res { Ok(res) => Ok(res), Err(err) => { - let error = format!( + log!( + P0, "[icrc3_get_blocks_from_ledger] failed to get blocks: {}", err ); - stop_timer_with_error(error); Err(()) } } @@ -540,11 +542,11 @@ async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Result Ok(res), Err(err) => { - let error = format!( + log!( + P0, "[icrc3_get_blocks_from_archive] failed to get blocks: {}", err ); - stop_timer_with_error(error); Err(()) } } From d73981bb6e3e4bb531600bc7d6a3771eb496edbe Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 13:39:10 +0000 Subject: [PATCH 08/67] remove fn used only once --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 3a2504b0888a..72fdd66c6993 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -595,10 +595,6 @@ pub async fn build_index() -> Option<()> { Some(()) } -fn sync_stopped() -> bool { - TIMER_ID.with(|tid| *tid.borrow()).is_none() -} - async fn fetch_blocks_via_get_blocks() -> Result { let mut num_indexed = 0; let next_id = with_blocks(|blocks| blocks.len()); @@ -719,7 +715,7 @@ fn stop_timer_with_error(error: String) { fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), ()> { measure_span(&PROFILING_DATA, "append_blocks", move || { assert!( - !sync_stopped(), + TIMER_ID.with(|tid| *tid.borrow()).is_some(), "Trying to append a block, even though the sync is stopped." ); From 2e45d6148b4e75fa65cd2fd5815ba71abcf74191 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 15:33:00 +0000 Subject: [PATCH 09/67] sync_status --- rs/ledger_suite/icrc1/index-ng/src/lib.rs | 6 ++ rs/ledger_suite/icrc1/index-ng/src/main.rs | 10 +++- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 58 ++++++++++++------- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/lib.rs b/rs/ledger_suite/icrc1/index-ng/src/lib.rs index 829790f55535..5f5d669e9b9f 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/lib.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/lib.rs @@ -83,6 +83,12 @@ pub struct Status { pub num_blocks_synced: BlockIndex, } +#[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] +pub struct SyncStatus { + pub sync_active: bool, + pub sync_error: Option, +} + #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] pub struct FeeCollectorRanges { pub ranges: Vec<(Account, Vec<(BlockIndex, BlockIndex)>)>, diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 72fdd66c6993..dfbce5f067b8 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -13,7 +13,7 @@ use ic_icrc1::{Block, Operation}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksMethod, IndexArg, - InitArg, ListSubaccountsArgs, Log, LogEntry, Status, TransactionWithId, UpgradeArg, + InitArg, ListSubaccountsArgs, Log, LogEntry, Status, SyncStatus, TransactionWithId, UpgradeArg, }; use ic_ledger_canister_core::runtime::heap_memory_size_bytes; use ic_ledger_core::block::{BlockIndex as BlockIndex64, BlockType, EncodedBlock}; @@ -1087,6 +1087,14 @@ fn status() -> Status { Status { num_blocks_synced } } +#[query] +fn sync_status() -> SyncStatus { + SyncStatus { + sync_active: TIMER_ID.with(|tid| *tid.borrow()).is_some(), + sync_error: SYNC_ERROR.with(|error| error.borrow().clone()), + } +} + #[query] fn list_subaccounts(args: ListSubaccountsArgs) -> Vec { let start_key = balance_key(Account { diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 40bd6e8eef00..7db17c6636a4 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -9,7 +9,7 @@ use ic_base_types::{CanisterId, PrincipalId}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksResponse, IndexArg, - InitArg as IndexInitArg, ListSubaccountsArgs, TransactionWithId, + InitArg as IndexInitArg, ListSubaccountsArgs, SyncStatus, TransactionWithId, }; use ic_icrc1_ledger::{ ChangeFeeCollector, LedgerArgument, Tokens, UpgradeArgs as LedgerUpgradeArgs, @@ -86,6 +86,14 @@ fn icrc1_balance_of(env: &StateMachine, canister_id: CanisterId, account: Accoun .expect("Balance must be a u64!") } +fn sync_status(env: &StateMachine, canister_id: CanisterId) -> SyncStatus { + let res = env + .execute_ingress(canister_id, "sync_status", Encode!(&()).unwrap()) + .expect("Failed to query sync_status") + .bytes(); + Decode!(&res, SyncStatus).expect("Failed to decode sync_status response") +} + fn index_get_blocks( env: &StateMachine, index_id: CanisterId, @@ -488,13 +496,8 @@ fn test_unknown_block() { // Test first transfer block. - const TEST_ACCOUNT: Account = Account { - owner: PrincipalId::new_user_test_id(44).0, - subaccount: None, - }; - const TA1: Account = Account { - owner: PrincipalId::new_user_test_id(55).0, + owner: PrincipalId::new_user_test_id(44).0, subaccount: None, }; @@ -504,31 +507,36 @@ fn test_unknown_block() { }; let block0 = BlockBuilder::new(0, 1000) - .with_fee(Tokens::from(50u64)) - .mint(TEST_ACCOUNT, Tokens::from(1_000u64)) + .mint(TA1, Tokens::from(1_000u64)) .build(); let block1 = BlockBuilder::new(1, 2000) .with_parent_hash(block0.clone().hash().to_vec()) - .mint(TA1, Tokens::from(1_000u64)) - .build(); - let block2 = BlockBuilder::new(2, 3000) - .with_parent_hash(block1.clone().hash().to_vec()) - .with_fee(Tokens::from(50u64)) - .transfer(TEST_ACCOUNT, TA2, Tokens::from(50u64)) + .transfer(TA1, TA2, Tokens::from(50u64)) .build(); - let mut block2_map = match block2 { + let mut block1_map = match block1 { ICRC3Value::Map(btree_map) => btree_map, _ => panic!("block should be a map"), }; - block2_map.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); - let block2 = ICRC3Value::Map(block2_map); + block1_map.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); + let block1 = ICRC3Value::Map(block1_map); + let block2 = BlockBuilder::new(2, 3000) + .with_parent_hash(block1.clone().hash().to_vec()) + .mint(TA1, Tokens::from(1_000u64)) + .build(); add_block(env, ledger_id, &block0); - add_block(env, ledger_id, &block1); wait_until_sync_is_completed(env, index_id, ledger_id); assert_ledger_index_parity(env, ledger_id, index_id); + let status = sync_status(&env, index_id); + assert!(status.sync_active); + assert!(status.sync_error.is_none()); + + assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); + assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); + + add_block(env, ledger_id, &block1); add_block(env, ledger_id, &block2); for _i in 0..3 { @@ -537,7 +545,17 @@ fn test_unknown_block() { } let ledger_blocks = ledger_get_all_blocks(env, ledger_id, 0, u64::MAX); let index_blocks = index_get_all_blocks(env, index_id, 0, u64::MAX); - assert_eq!(ledger_blocks.chain_length, index_blocks.chain_length + 1); + assert_eq!(ledger_blocks.chain_length, index_blocks.chain_length + 2); + + assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); + assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); + + let status = sync_status(&env, index_id); + assert_eq!(status.sync_active, false); + assert_eq!( + status.sync_error, + Some("Unknown block at index 1.".to_string()) + ); } #[test] From d3780892536ad67b77a0936c5998cc0f45444609 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 15:50:20 +0000 Subject: [PATCH 10/67] clippy --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 4 ++-- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index dfbce5f067b8..d9cfcaf1a48c 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -113,10 +113,10 @@ thread_local! { static CACHE: RefCell = RefCell::new(Cache::default()); /// The ID of the block sync timer. `None` means the sync is stopped due to an error. - static TIMER_ID: RefCell> = RefCell::new(None); + static TIMER_ID: RefCell> = const { RefCell::new(None) }; /// The description of the error encountered during block sync. - static SYNC_ERROR: RefCell> = RefCell::new(None); + static SYNC_ERROR: RefCell> = const { RefCell::new(None) }; } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 7db17c6636a4..40b6c6c0c069 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -529,7 +529,7 @@ fn test_unknown_block() { wait_until_sync_is_completed(env, index_id, ledger_id); assert_ledger_index_parity(env, ledger_id, index_id); - let status = sync_status(&env, index_id); + let status = sync_status(env, index_id); assert!(status.sync_active); assert!(status.sync_error.is_none()); @@ -550,8 +550,8 @@ fn test_unknown_block() { assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); - let status = sync_status(&env, index_id); - assert_eq!(status.sync_active, false); + let status = sync_status(env, index_id); + assert!(!status.sync_active); assert_eq!( status.sync_error, Some("Unknown block at index 1.".to_string()) From e6c58818890dd6d07eb1acc066553ee7ba72c8c4 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 1 Oct 2025 15:56:45 +0000 Subject: [PATCH 11/67] fix did file --- rs/ledger_suite/icrc1/index-ng/index-ng.did | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/index-ng.did b/rs/ledger_suite/icrc1/index-ng/index-ng.did index 53ad6d473735..0f54f6bcc642 100644 --- a/rs/ledger_suite/icrc1/index-ng/index-ng.did +++ b/rs/ledger_suite/icrc1/index-ng/index-ng.did @@ -137,6 +137,11 @@ type Status = record { num_blocks_synced : BlockIndex }; +type SyncStatus = record { + sync_active : bool; + sync_error : opt text +}; + type FeeCollectorRanges = record { ranges : vec record { Account; vec record { BlockIndex; BlockIndex } } } @@ -148,5 +153,6 @@ service : (index_arg : opt IndexArg) -> { icrc1_balance_of : (Account) -> (Tokens) query; ledger_id : () -> (principal) query; list_subaccounts : (ListSubaccountsArgs) -> (vec SubAccount) query; - status : () -> (Status) query + status : () -> (Status) query; + sync_status : () -> (SyncStatus) query } From 3ada93241e0effc4b11c0157199d3ad830afb59d Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 7 Oct 2025 12:37:49 +0000 Subject: [PATCH 12/67] extract test function --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 12 ++- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 93 ++++++++++++------- 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index d9cfcaf1a48c..dd61eb636352 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -583,6 +583,10 @@ pub async fn build_index() -> Option<()> { GetBlocksMethod::ICRC3GetBlocks => fetch_blocks_via_icrc3().await, }; if let Ok(num_indexed) = num_indexed { + assert!( + sync_active(), + "Indexing succeeded but the sync timer is stopped." + ); let retrieve_blocks_from_ledger_interval = with_state(|state| state.retrieve_blocks_from_ledger_interval()); log!( @@ -715,7 +719,7 @@ fn stop_timer_with_error(error: String) { fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), ()> { measure_span(&PROFILING_DATA, "append_blocks", move || { assert!( - TIMER_ID.with(|tid| *tid.borrow()).is_some(), + sync_active(), "Trying to append a block, even though the sync is stopped." ); @@ -1087,10 +1091,14 @@ fn status() -> Status { Status { num_blocks_synced } } +fn sync_active() -> bool { + TIMER_ID.with(|tid| *tid.borrow()).is_some() +} + #[query] fn sync_status() -> SyncStatus { SyncStatus { - sync_active: TIMER_ID.with(|tid| *tid.borrow()).is_some(), + sync_active: sync_active(), sync_error: SYNC_ERROR.with(|error| error.borrow().clone()), } } diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 40b6c6c0c069..e6389dfa3076 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -355,6 +355,16 @@ fn get_fee_collectors_ranges(env: &StateMachine, index: CanisterId) -> FeeCollec .expect("failed to decode get_fee_collectors_ranges response") } +fn set_icrc3_enabled(env: &StateMachine, canister_id: CanisterId, enabled: bool) { + Decode!( + &env.execute_ingress(canister_id, "set_icrc3_enabled", Encode!(&enabled).unwrap()) + .expect("failed to set_icrc3_enabled") + .bytes(), + () + ) + .expect("failed to decode set_icrc3_enabled response") +} + // Assert that the index canister contains the same blocks as the ledger. #[track_caller] fn assert_ledger_index_parity(env: &StateMachine, ledger_id: CanisterId, index_id: CanisterId) { @@ -482,20 +492,7 @@ fn test_ledger_growing() { ); } -#[test] -fn test_unknown_block() { - // check that the index canister can incrementally get the blocks from the ledger. - - let env = &StateMachine::new(); - let ledger_id = install_icrc3_test_ledger(env); - let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); - - // Test initial mint block. - wait_until_sync_is_completed(env, index_id, ledger_id); - assert_ledger_index_parity(env, ledger_id, index_id); - - // Test first transfer block. - +fn verify_unknown_block_handling(env: &StateMachine, ledger_id: CanisterId, index_id: CanisterId) { const TA1: Account = Account { owner: PrincipalId::new_user_test_id(44).0, subaccount: None, @@ -506,11 +503,15 @@ fn test_unknown_block() { subaccount: None, }; - let block0 = BlockBuilder::new(0, 1000) - .mint(TA1, Tokens::from(1_000u64)) - .build(); - let block1 = BlockBuilder::new(1, 2000) - .with_parent_hash(block0.clone().hash().to_vec()) + let mut blocks = vec![]; + + blocks.push( + BlockBuilder::new(0, 0) + .mint(TA1, Tokens::from(1_000u64)) + .build(), + ); + let block1 = BlockBuilder::new(1, 1) + .with_parent_hash(blocks[0].clone().hash().to_vec()) .transfer(TA1, TA2, Tokens::from(50u64)) .build(); let mut block1_map = match block1 { @@ -518,13 +519,20 @@ fn test_unknown_block() { _ => panic!("block should be a map"), }; block1_map.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); - let block1 = ICRC3Value::Map(block1_map); - let block2 = BlockBuilder::new(2, 3000) - .with_parent_hash(block1.clone().hash().to_vec()) - .mint(TA1, Tokens::from(1_000u64)) - .build(); + blocks.push(ICRC3Value::Map(block1_map)); + + const NUM_BLOCKS: u64 = 10; - add_block(env, ledger_id, &block0); + for i in 2..NUM_BLOCKS { + blocks.push( + BlockBuilder::new(i, i) + .with_parent_hash(blocks[i as usize - 1].clone().hash().to_vec()) + .mint(TA1, Tokens::from(1_000u64)) + .build(), + ); + } + + add_block(env, ledger_id, &blocks[0]); wait_until_sync_is_completed(env, index_id, ledger_id); assert_ledger_index_parity(env, ledger_id, index_id); @@ -536,16 +544,17 @@ fn test_unknown_block() { assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); - add_block(env, ledger_id, &block1); - add_block(env, ledger_id, &block2); - - for _i in 0..3 { - env.advance_time(Duration::from_secs(60)); - env.tick(); + for i in 1..NUM_BLOCKS { + add_block(env, ledger_id, &blocks[i as usize]); } + + env.advance_time(Duration::from_secs(60)); + env.tick(); + let ledger_blocks = ledger_get_all_blocks(env, ledger_id, 0, u64::MAX); let index_blocks = index_get_all_blocks(env, index_id, 0, u64::MAX); - assert_eq!(ledger_blocks.chain_length, index_blocks.chain_length + 2); + assert_eq!(ledger_blocks.chain_length, NUM_BLOCKS); + assert_eq!(index_blocks.chain_length, 1); assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); @@ -558,6 +567,26 @@ fn test_unknown_block() { ); } +#[test] +fn test_ledger_unknown_block_icrc3() { + let env = &StateMachine::new(); + let ledger_id = install_icrc3_test_ledger(env); + let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); + + verify_unknown_block_handling(env, ledger_id, index_id); +} + +#[test] +fn test_ledger_unknown_block_legacy() { + let env = &StateMachine::new(); + let ledger_id = install_icrc3_test_ledger(env); + let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); + + set_icrc3_enabled(env, ledger_id, false); + + verify_unknown_block_handling(env, ledger_id, index_id); +} + #[test] fn test_archive_indexing() { let env = &StateMachine::new(); From 969915d71dba33218ad65c82228c034a0e66f41a Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 11:30:59 +0000 Subject: [PATCH 13/67] improve test --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 125 +++++++++--------- 1 file changed, 60 insertions(+), 65 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index e6389dfa3076..23a70e08055f 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -492,60 +492,44 @@ fn test_ledger_growing() { ); } -fn verify_unknown_block_handling(env: &StateMachine, ledger_id: CanisterId, index_id: CanisterId) { - const TA1: Account = Account { - owner: PrincipalId::new_user_test_id(44).0, - subaccount: None, - }; +const NUM_BLOCKS: u64 = 6; - const TA2: Account = Account { - owner: PrincipalId::new_user_test_id(66).0, +fn verify_unknown_block_handling( + env: &StateMachine, + ledger_id: CanisterId, + index_id: CanisterId, + bad_block_index: u64, +) { + const TEST_ACCOUNT: Account = Account { + owner: PrincipalId::new_user_test_id(44).0, subaccount: None, }; - let mut blocks = vec![]; + let mut prev_hash = None; - blocks.push( - BlockBuilder::new(0, 0) - .mint(TA1, Tokens::from(1_000u64)) - .build(), - ); - let block1 = BlockBuilder::new(1, 1) - .with_parent_hash(blocks[0].clone().hash().to_vec()) - .transfer(TA1, TA2, Tokens::from(50u64)) - .build(); - let mut block1_map = match block1 { - ICRC3Value::Map(btree_map) => btree_map, - _ => panic!("block should be a map"), - }; - block1_map.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); - blocks.push(ICRC3Value::Map(block1_map)); - - const NUM_BLOCKS: u64 = 10; - - for i in 2..NUM_BLOCKS { - blocks.push( + for i in 0..NUM_BLOCKS { + let block = if let Some(prev_hash) = prev_hash { BlockBuilder::new(i, i) - .with_parent_hash(blocks[i as usize - 1].clone().hash().to_vec()) - .mint(TA1, Tokens::from(1_000u64)) - .build(), - ); - } - - add_block(env, ledger_id, &blocks[0]); - - wait_until_sync_is_completed(env, index_id, ledger_id); - assert_ledger_index_parity(env, ledger_id, index_id); - - let status = sync_status(env, index_id); - assert!(status.sync_active); - assert!(status.sync_error.is_none()); - - assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); - assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); - - for i in 1..NUM_BLOCKS { - add_block(env, ledger_id, &blocks[i as usize]); + .with_parent_hash(prev_hash) + .mint(TEST_ACCOUNT, Tokens::from(1u64)) + .build() + } else { + BlockBuilder::new(i, i) + .mint(TEST_ACCOUNT, Tokens::from(1u64)) + .build() + }; + let block = if i == bad_block_index { + let mut bad_block = match block { + ICRC3Value::Map(btree_map) => btree_map, + _ => panic!("block should be a map"), + }; + bad_block.insert("unknown_key".to_string(), ICRC3Value::Nat(Nat::from(0u64))); + ICRC3Value::Map(bad_block) + } else { + block + }; + prev_hash = Some(block.clone().hash().to_vec()); + add_block(env, ledger_id, &block); } env.advance_time(Duration::from_secs(60)); @@ -554,37 +538,48 @@ fn verify_unknown_block_handling(env: &StateMachine, ledger_id: CanisterId, inde let ledger_blocks = ledger_get_all_blocks(env, ledger_id, 0, u64::MAX); let index_blocks = index_get_all_blocks(env, index_id, 0, u64::MAX); assert_eq!(ledger_blocks.chain_length, NUM_BLOCKS); - assert_eq!(index_blocks.chain_length, 1); - - assert_eq!(icrc1_balance_of(env, index_id, TA1), 1_000u64); - assert_eq!(icrc1_balance_of(env, index_id, TA2), 0u64); + assert_eq!(index_blocks.chain_length, bad_block_index); - let status = sync_status(env, index_id); - assert!(!status.sync_active); assert_eq!( - status.sync_error, - Some("Unknown block at index 1.".to_string()) + icrc1_balance_of(env, index_id, TEST_ACCOUNT), + bad_block_index ); + + let status = sync_status(env, index_id); + if bad_block_index < NUM_BLOCKS { + assert!(!status.sync_active); + assert_eq!( + status.sync_error, + Some(format!("Unknown block at index {}.", bad_block_index)) + ); + } else { + assert!(status.sync_active); + assert_eq!(status.sync_error, None); + } } #[test] fn test_ledger_unknown_block_icrc3() { - let env = &StateMachine::new(); - let ledger_id = install_icrc3_test_ledger(env); - let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); + for bad_block_index in 0..NUM_BLOCKS { + let env = &StateMachine::new(); + let ledger_id = install_icrc3_test_ledger(env); + let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); - verify_unknown_block_handling(env, ledger_id, index_id); + verify_unknown_block_handling(env, ledger_id, index_id, bad_block_index); + } } #[test] fn test_ledger_unknown_block_legacy() { - let env = &StateMachine::new(); - let ledger_id = install_icrc3_test_ledger(env); - let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); + for bad_block_index in 0..NUM_BLOCKS { + let env = &StateMachine::new(); + let ledger_id = install_icrc3_test_ledger(env); + let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); - set_icrc3_enabled(env, ledger_id, false); + set_icrc3_enabled(env, ledger_id, false); - verify_unknown_block_handling(env, ledger_id, index_id); + verify_unknown_block_handling(env, ledger_id, index_id, bad_block_index); + } } #[test] From 5c87f33336cb1fd30d79b377b046161b508ca20f Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 13:24:28 +0000 Subject: [PATCH 14/67] add fee to mint and burn operations --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 21 +++++++++---- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 12 ++++---- rs/ledger_suite/icrc1/ledger/src/main.rs | 1 + rs/ledger_suite/icrc1/src/endpoints.rs | 5 +++- rs/ledger_suite/icrc1/src/lib.rs | 30 +++++++++++++++---- rs/ledger_suite/icrc1/test_utils/src/lib.rs | 13 +++++++- .../test_utils/in_memory_ledger/src/lib.rs | 11 +++++-- rs/ledger_suite/tests/sm-tests/src/lib.rs | 7 ++++- 8 files changed, 78 insertions(+), 22 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index dd61eb636352..1bc7a4206399 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -709,6 +709,7 @@ fn set_build_index_timer(after: Duration) { fn stop_timer_with_error(error: String) { log!(P0, "{}", error); + ic_cdk::eprintln!("{}", error); if let Some(timer_id) = TIMER_ID.with(|tid| *tid.borrow()) { ic_cdk_timers::clear_timer(timer_id); TIMER_ID.with(|tid| *tid.borrow_mut() = None); @@ -722,8 +723,8 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), () sync_active(), "Trying to append a block, even though the sync is stopped." ); + let original_block = block.clone(); - let original_hash = block.hash(); let block = match generic_block_to_encoded_block(block) { Ok(block) => block, Err(e) => { @@ -743,9 +744,12 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), () return Err(()); } }; - let decoded_hash = Block::::block_hash(&decoded_block.clone().encode()); - if original_hash != decoded_hash.as_slice() { - stop_timer_with_error(format!("Unknown block at index {block_index}.")); + let decoded_value = encoded_block_to_generic_block(&decoded_block.clone().encode()); + if original_block.hash() != decoded_value.hash() { + stop_timer_with_error(format!( + "Block at index {block_index} has unknown fields. Original block {}, decoded block: {}.", + original_block, decoded_value + )); return Err(()); } @@ -841,8 +845,13 @@ fn process_balance_changes(block_index: BlockIndex64, block: &Block) { &PROFILING_DATA, "append_blocks.process_balance_changes", move || match block.transaction.operation { - Operation::Burn { from, amount, .. } => debit(block_index, from, amount), - Operation::Mint { to, amount } => credit(block_index, to, amount), + Operation::Burn { + from, + amount, + fee: _, + .. + } => debit(block_index, from, amount), + Operation::Mint { to, amount, fee: _ } => credit(block_index, to, amount), Operation::Transfer { from, to, diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 23a70e08055f..bd03464e8924 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -548,10 +548,10 @@ fn verify_unknown_block_handling( let status = sync_status(env, index_id); if bad_block_index < NUM_BLOCKS { assert!(!status.sync_active); - assert_eq!( - status.sync_error, - Some(format!("Unknown block at index {}.", bad_block_index)) - ); + assert!(status.sync_error.unwrap().starts_with(&format!( + "Block at index {} has unknown fields.", + bad_block_index + ))); } else { assert!(status.sync_active); assert_eq!(status.sync_error, None); @@ -560,7 +560,7 @@ fn verify_unknown_block_handling( #[test] fn test_ledger_unknown_block_icrc3() { - for bad_block_index in 0..NUM_BLOCKS { + for bad_block_index in 0..NUM_BLOCKS + 1 { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); @@ -571,7 +571,7 @@ fn test_ledger_unknown_block_icrc3() { #[test] fn test_ledger_unknown_block_legacy() { - for bad_block_index in 0..NUM_BLOCKS { + for bad_block_index in 0..NUM_BLOCKS + 1 { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); diff --git a/rs/ledger_suite/icrc1/ledger/src/main.rs b/rs/ledger_suite/icrc1/ledger/src/main.rs index 0ee413b71dd8..87560044321c 100644 --- a/rs/ledger_suite/icrc1/ledger/src/main.rs +++ b/rs/ledger_suite/icrc1/ledger/src/main.rs @@ -744,6 +744,7 @@ fn execute_transfer_not_async( from: from_account, spender, amount, + fee: None, }, created_at_time: created_at_time.map(|t| t.as_nanos_since_unix_epoch()), memo, diff --git a/rs/ledger_suite/icrc1/src/endpoints.rs b/rs/ledger_suite/icrc1/src/endpoints.rs index 17cb893f0656..127ce0a26e11 100644 --- a/rs/ledger_suite/icrc1/src/endpoints.rs +++ b/rs/ledger_suite/icrc1/src/endpoints.rs @@ -165,7 +165,8 @@ impl From> for Transaction { let memo = b.transaction.memo; match b.transaction.operation { - Operation::Mint { to, amount } => { + Operation::Mint { to, amount, fee } => { + assert!(fee.is_none()); tx.kind = "mint".to_string(); tx.mint = Some(Mint { to, @@ -178,7 +179,9 @@ impl From> for Transaction { from, spender, amount, + fee, } => { + assert!(fee.is_none()); tx.kind = "burn".to_string(); tx.burn = Some(Burn { from, diff --git a/rs/ledger_suite/icrc1/src/lib.rs b/rs/ledger_suite/icrc1/src/lib.rs index 8655479026c1..c189b94721ff 100644 --- a/rs/ledger_suite/icrc1/src/lib.rs +++ b/rs/ledger_suite/icrc1/src/lib.rs @@ -30,6 +30,8 @@ pub enum Operation { to: Account, #[serde(rename = "amt")] amount: Tokens, + #[serde(skip_serializing_if = "Option::is_none")] + fee: Option, }, #[serde(rename = "xfer")] Transfer { @@ -60,6 +62,8 @@ pub enum Operation { spender: Option, #[serde(rename = "amt")] amount: Tokens, + #[serde(skip_serializing_if = "Option::is_none")] + fee: Option, }, #[serde(rename = "approve")] Approve { @@ -140,10 +144,12 @@ impl TryFrom> for Transaction Operation::Mint { to: value.to.ok_or("`to` field required for `mint` operation")?, - amount: value.amount, + amount: value.amount.clone(), + fee: value.fee, }, "xfer" => Operation::Transfer { from: value @@ -210,8 +216,10 @@ impl From> for FlattenedTransaction amount.clone(), }, fee: match &t.operation { - Transfer { fee, .. } | Approve { fee, .. } => fee.to_owned(), - _ => None, + Transfer { fee, .. } + | Approve { fee, .. } + | Mint { fee, .. } + | Burn { fee, .. } => fee.to_owned(), }, expected_allowance: match &t.operation { Approve { @@ -253,6 +261,7 @@ impl LedgerTransaction for Transaction { from, spender, amount, + fee: None, }, created_at_time: created_at_time.map(|t| t.as_nanos_since_unix_epoch()), memo: memo.map(Memo::from), @@ -355,7 +364,9 @@ impl LedgerTransaction for Transaction { from, spender, amount, + fee, } => { + assert!(fee.is_none()); if spender.is_some() && from != &spender.unwrap() { let allowance = context.approvals().allowance(from, &spender.unwrap(), now); if allowance.amount < *amount { @@ -372,7 +383,10 @@ impl LedgerTransaction for Transaction { .expect("bug: cannot use allowance"); } } - Operation::Mint { to, amount } => context.balances_mut().mint(to, amount.clone())?, + Operation::Mint { to, amount, fee } => { + assert!(fee.is_none()); + context.balances_mut().mint(to, amount.clone())?; + } Operation::Approve { from, spender, @@ -416,7 +430,11 @@ impl Transaction { memo: Option, ) -> Self { Self { - operation: Operation::Mint { to, amount }, + operation: Operation::Mint { + to, + amount, + fee: None, + }, created_at_time: created_at_time.map(|t| t.as_nanos_since_unix_epoch()), memo, } @@ -458,6 +476,7 @@ impl TryFrom TryFrom( let mint_strategy = account_strategy().prop_map(move |to| Operation::Mint { to, amount: mint_amount.clone(), + fee: None, }); let burn_amount = amount.clone(); let burn_strategy = account_strategy().prop_map(move |from| Operation::Burn { from, spender: None, amount: burn_amount.clone(), + fee: None, }); let transfer_amount = amount.clone(); let transfer_strategy = ( @@ -418,12 +420,14 @@ impl ArgWithCaller { Operation::Mint { amount: T::try_from(transfer_arg.amount.clone()).unwrap(), to: transfer_arg.to, + fee: None, } } else if burn_operation { Operation::Burn { amount: T::try_from(transfer_arg.amount.clone()).unwrap(), from: caller, spender: None, + fee: None, } } else { Operation::Transfer { @@ -770,6 +774,7 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( operation: Operation::Mint:: { amount: Tokens::from_e8s(amount), to, + fee: None, }, created_at_time, memo: memo.clone(), @@ -832,6 +837,7 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( amount: Tokens::from_e8s(amount), from, spender: None, + fee: None, }, created_at_time, memo: memo.clone(), @@ -1394,7 +1400,11 @@ where Tokens: TokensType, S: Strategy, { - (arb_account(), arb_tokens()).prop_map(|(to, amount)| Operation::Mint { to, amount }) + (arb_account(), arb_tokens()).prop_map(|(to, amount)| Operation::Mint { + to, + amount, + fee: None, + }) } pub fn arb_burn(arb_tokens: fn() -> S) -> impl Strategy> @@ -1411,6 +1421,7 @@ where from, spender, amount, + fee: None, }) } diff --git a/rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs b/rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs index 98e31824bef5..f7a18a09f146 100644 --- a/rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs +++ b/rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs @@ -496,7 +496,10 @@ where self.fee_collector = Some(fee_collector); } match &block.transaction.operation { - Operation::Mint { to, amount } => self.process_mint(to, amount), + Operation::Mint { to, amount, fee } => { + assert!(fee.is_none()); + self.process_mint(to, amount); + } Operation::Transfer { from, to, @@ -514,7 +517,11 @@ where from, spender, amount, - } => self.process_burn(from, spender, amount, index), + fee, + } => { + assert!(fee.is_none()); + self.process_burn(from, spender, amount, index); + } Operation::Approve { from, spender, diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 61e5bde8f621..90abca811581 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -286,7 +286,11 @@ fn arb_approve() -> impl Strategy> } fn arb_mint() -> impl Strategy> { - (arb_account(), arb_amount()).prop_map(|(to, amount)| Operation::Mint { to, amount }) + (arb_account(), arb_amount()).prop_map(|(to, amount)| Operation::Mint { + to, + amount, + fee: None, + }) } fn arb_burn() -> impl Strategy> { @@ -299,6 +303,7 @@ fn arb_burn() -> impl Strategy> { from, spender, amount, + fee: None, }) } From ae501817df0096f191fe925007bdb28a08bc706a Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 13:27:38 +0000 Subject: [PATCH 15/67] fix test ledger wasm constant --- rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs index b9c2f7feb696..eff7cde92e0f 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs @@ -98,7 +98,7 @@ pub fn install_ledger( } fn icrc3_test_ledger() -> Vec { - std::fs::read(std::env::var("ICRC3_TEST_LEDGER_CANISTER_WASM_PATH").unwrap()).unwrap() + std::fs::read(std::env::var("IC_ICRC3_TEST_LEDGER_WASM_PATH").unwrap()).unwrap() } #[allow(dead_code)] From 52912bfbec46ce1af2465a8d17aa880438fa4a14 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 13:33:49 +0000 Subject: [PATCH 16/67] build fix --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 10edbc4f559b..839df8f6d444 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -845,7 +845,12 @@ fn process_balance_changes(block_index: BlockIndex64, block: &Block) { &PROFILING_DATA, "append_blocks.process_balance_changes", move || match block.transaction.operation { - Operation::Burn { from, amount, .. } => { + Operation::Burn { + from, + amount, + fee: _, + .. + } => { let mut amount_with_fee = amount; if let Some(fee) = block.effective_fee { amount_with_fee = amount.checked_add(&fee).unwrap_or_else(|| { @@ -860,7 +865,7 @@ fn process_balance_changes(block_index: BlockIndex64, block: &Block) { } debit(block_index, from, amount_with_fee); } - Operation::Mint { to, amount } => { + Operation::Mint { to, amount, fee: _ } => { let mut amount_without_fee = amount; if let Some(fee) = block.effective_fee { amount_without_fee = amount.checked_sub(&fee).unwrap_or_else(|| { From c773910849d990de61bb0f8c42f98c850b624b7e Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 13:38:02 +0000 Subject: [PATCH 17/67] fix comment --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 839df8f6d444..9501bcdbcbf3 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -747,7 +747,7 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), () let decoded_value = encoded_block_to_generic_block(&decoded_block.clone().encode()); if original_block.hash() != decoded_value.hash() { stop_timer_with_error(format!( - "Block at index {block_index} has unknown fields. Original block {}, decoded block: {}.", + "Block at index {block_index} has unknown fields. Original block: {}, decoded block: {}.", original_block, decoded_value )); return Err(()); From d0d5270563278d2c374f7a54677f4dc04948760f Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 14:40:02 +0000 Subject: [PATCH 18/67] build fix --- .../icrc1/src/common/storage/types.rs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/rs/rosetta-api/icrc1/src/common/storage/types.rs b/rs/rosetta-api/icrc1/src/common/storage/types.rs index 89b8ab031633..79a7e0abd402 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/types.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/types.rs @@ -571,15 +571,22 @@ where from, spender, amount, - } => Self::Burn { - from, - spender, - amount: amount.into(), - }, - Op::Mint { to, amount } => Self::Mint { - to, - amount: amount.into(), - }, + fee, + } => { + assert!(fee.is_none()); + Self::Burn { + from, + spender, + amount: amount.into(), + } + } + Op::Mint { to, amount, fee } => { + assert!(fee.is_none()); + Self::Mint { + to, + amount: amount.into(), + } + } Op::Transfer { from, to, From 99d0cbc96e640fd2e42c548f5b6088e37b907b0b Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 14:48:41 +0000 Subject: [PATCH 19/67] clippy --- rs/rosetta-api/icrc1/src/common/storage/types.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/rosetta-api/icrc1/src/common/storage/types.rs b/rs/rosetta-api/icrc1/src/common/storage/types.rs index 79a7e0abd402..aa2462ca50cb 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/types.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/types.rs @@ -910,6 +910,7 @@ mod tests { from, spender, amount, + fee: _, }, IcrcOperation::Burn { from: rosetta_from, @@ -922,7 +923,7 @@ mod tests { assert_eq!(amount.into(), rosetta_amount, "amount"); } ( - ic_icrc1::Operation::Mint { to, amount }, + ic_icrc1::Operation::Mint { to, amount, fee: _ }, IcrcOperation::Mint { to: rosetta_to, amount: rosetta_amount, From 436fd3acd48425a0bb46f8e3315c97d48a40c58b Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 15:16:30 +0000 Subject: [PATCH 20/67] build fix --- rs/ledger_suite/icrc1/archive/tests/tests.rs | 2 ++ rs/ledger_suite/icrc1/ledger/src/tests.rs | 3 +++ rs/ledger_suite/icrc1/ledger/tests/tests.rs | 1 + 3 files changed, 6 insertions(+) diff --git a/rs/ledger_suite/icrc1/archive/tests/tests.rs b/rs/ledger_suite/icrc1/archive/tests/tests.rs index 5c7fb36b95a1..df26c0d640fd 100644 --- a/rs/ledger_suite/icrc1/archive/tests/tests.rs +++ b/rs/ledger_suite/icrc1/archive/tests/tests.rs @@ -128,6 +128,7 @@ fn test_icrc3_get_blocks() { Operation::Mint { to: Account::from(Principal::anonymous()), amount: Tokens::from(1_000_000_000u64), + fee: None, }, ); let blockid0 = block_with_id(0, block0.clone()); @@ -259,6 +260,7 @@ fn test_icrc3_get_blocks_number_of_blocks_limit() { operation: Operation::Mint { to: Account::from(Principal::anonymous()), amount: Tokens::from(amount as u64), + fee: None, }, created_at_time: None, memo: None, diff --git a/rs/ledger_suite/icrc1/ledger/src/tests.rs b/rs/ledger_suite/icrc1/ledger/src/tests.rs index e51756af46b4..3362a6733c86 100644 --- a/rs/ledger_suite/icrc1/ledger/src/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/src/tests.rs @@ -550,6 +550,7 @@ fn test_approval_burn_from() { from, spender: Some(spender), amount: tokens(100_000), + fee: None, }, created_at_time: None, memo: None, @@ -585,6 +586,7 @@ fn test_approval_burn_from() { from, spender: Some(spender), amount: tokens(100_000), + fee: None, }, created_at_time: None, memo: None, @@ -609,6 +611,7 @@ fn test_approval_burn_from() { from, spender: Some(spender), amount: tokens(100_000), + fee: None, }, created_at_time: None, memo: None, diff --git a/rs/ledger_suite/icrc1/ledger/tests/tests.rs b/rs/ledger_suite/icrc1/ledger/tests/tests.rs index 1aeccb1b3fd9..4f75c13acff7 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -1159,6 +1159,7 @@ fn test_icrc3_get_archives() { operation: Operation::Mint { to: minting_account, amount: Tokens::from(1_000_000u64), + fee: None, }, created_at_time: None, memo: None, From 08f0ea65563b521a1c80ed297400cc7936af9242 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 15:18:29 +0000 Subject: [PATCH 21/67] build fix --- rs/ledger_suite/icrc1/ledger/src/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/ledger_suite/icrc1/ledger/src/tests.rs b/rs/ledger_suite/icrc1/ledger/src/tests.rs index 3362a6733c86..0ab4f71a8eae 100644 --- a/rs/ledger_suite/icrc1/ledger/src/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/src/tests.rs @@ -521,6 +521,7 @@ fn test_burn_smoke() { from, spender: None, amount: tokens(100_000), + fee: None, }, created_at_time: None, memo: None, From f870f331ae59ffdf1fe835483fff8374de745276 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 16:11:02 +0000 Subject: [PATCH 22/67] add archiving --- Cargo.lock | 1 + rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 8 +++++++ .../icrc3_test_ledger/tests/tests.rs | 22 +------------------ .../state_machine_helpers/BUILD.bazel | 1 + .../state_machine_helpers/Cargo.toml | 1 + .../state_machine_helpers/src/lib.rs | 21 ++++++++++++++++++ 6 files changed, 33 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abf742a69ae7..d6b65c9005f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10621,6 +10621,7 @@ dependencies = [ "ic-base-types", "ic-http-types", "ic-icrc1", + "ic-icrc3-test-ledger", "ic-ledger-core", "ic-management-canister-types-private", "ic-state-machine-tests", diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index cb498c784fbd..9d466f8756a7 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -18,6 +18,7 @@ use ic_icrc1_test_utils::{ ArgWithCaller, LedgerEndpointArg, icrc3::BlockBuilder, minter_identity, valid_transactions_strategy, }; +use ic_ledger_suite_state_machine_helpers::archive_blocks; use ic_ledger_suite_state_machine_tests::test_http_request_decoding_quota; use ic_state_machine_tests::StateMachine; use icrc_ledger_types::icrc::generic_value::ICRC3Value; @@ -532,6 +533,13 @@ fn verify_unknown_block_handling( add_block(env, ledger_id, &block); } + let archive1 = install_icrc3_test_ledger(env); + let archive2 = install_icrc3_test_ledger(env); + let archived_count = archive_blocks(&env, ledger_id, archive1, 2); + assert_eq!(archived_count, 2); + let archived_count = archive_blocks(&env, ledger_id, archive2, 2); + assert_eq!(archived_count, 2); + env.advance_time(Duration::from_secs(60)); env.tick(); diff --git a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs index e897b2ef17ce..f0afab942a47 100644 --- a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs @@ -11,7 +11,7 @@ use ic_icrc1_ledger::Tokens; use ic_icrc1_test_utils::icrc3::BlockBuilder; use ic_icrc3_test_ledger::{AddBlockResult, ArchiveBlocksArgs}; use ic_ledger_suite_state_machine_helpers::{ - balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, + archive_blocks, balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, }; use ic_state_machine_tests::StateMachine; use ic_test_utilities_load_wasm::load_wasm; @@ -106,26 +106,6 @@ fn get_blocks( .expect("failed to decode icrc3_get_blocks response") } -fn archive_blocks( - env: &StateMachine, - ledger_id: CanisterId, - archive_id: CanisterId, - num_blocks: u64, -) -> u64 { - let archive_args = ArchiveBlocksArgs { - archive_id: archive_id.into(), - num_blocks, - }; - Decode!( - &env.execute_ingress(ledger_id, "archive_blocks", Encode!(&archive_args).unwrap()) - .expect("failed to archive blocks") - .bytes(), - Result - ) - .expect("failed to decode archive_blocks response") - .expect("archiving blocks operation failed") -} - fn check_legacy_get_blocks( env: &StateMachine, canister_id: CanisterId, diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel b/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel index 879c4ccd7846..90b7a70f07fc 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel +++ b/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel @@ -7,6 +7,7 @@ DEPENDENCIES = [ "//packages/ic-http-types", "//packages/icrc-ledger-types:icrc_ledger_types", "//rs/ledger_suite/common/ledger_core", + "//rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger", "//rs/ledger_suite/icp:icp_ledger", "//rs/ledger_suite/icrc1", "//rs/state_machine_tests", diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/Cargo.toml b/rs/ledger_suite/test_utils/state_machine_helpers/Cargo.toml index 827c5d9c5855..49db6096c594 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/Cargo.toml +++ b/rs/ledger_suite/test_utils/state_machine_helpers/Cargo.toml @@ -11,6 +11,7 @@ candid = { workspace = true } ic-base-types = { path = "../../../types/base_types" } ic-http-types = { path = "../../../../packages/ic-http-types" } ic-icrc1 = { path = "../../icrc1" } +ic-icrc3-test-ledger = { path = "../../icrc1/test_utils/icrc3_test_ledger" } ic-ledger-core = { path = "../../common/ledger_core" } ic-management-canister-types-private = { path = "../../../types/management_canister_types" } ic-state-machine-tests = { path = "../../../state_machine_tests" } diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs index a9b76ee52b2c..34d04f7c9d22 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs +++ b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs @@ -3,6 +3,7 @@ use ic_base_types::CanisterId; use ic_base_types::PrincipalId; use ic_http_types::{HttpRequest, HttpResponse}; use ic_icrc1::{Block, endpoints::StandardRecord}; +use ic_icrc3_test_ledger::ArchiveBlocksArgs; use ic_ledger_core::Tokens; use ic_ledger_core::block::BlockIndex; use ic_ledger_core::tokens::TokensType; @@ -824,3 +825,23 @@ fn universal_canister_payload( ) .build() } + +pub fn archive_blocks( + env: &StateMachine, + ledger_id: CanisterId, + archive_id: CanisterId, + num_blocks: u64, +) -> u64 { + let archive_args = ArchiveBlocksArgs { + archive_id: archive_id.into(), + num_blocks, + }; + Decode!( + &env.execute_ingress(ledger_id, "archive_blocks", Encode!(&archive_args).unwrap()) + .expect("failed to archive blocks") + .bytes(), + Result + ) + .expect("failed to decode archive_blocks response") + .expect("archiving blocks operation failed") +} From 9f46c51159e191a1f59846109ec6ca778c57f738 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 16:21:36 +0000 Subject: [PATCH 23/67] clippy --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 4 ++-- .../icrc1/test_utils/icrc3_test_ledger/tests/tests.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 9d466f8756a7..2b12841d1b4d 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -535,9 +535,9 @@ fn verify_unknown_block_handling( let archive1 = install_icrc3_test_ledger(env); let archive2 = install_icrc3_test_ledger(env); - let archived_count = archive_blocks(&env, ledger_id, archive1, 2); + let archived_count = archive_blocks(env, ledger_id, archive1, 2); assert_eq!(archived_count, 2); - let archived_count = archive_blocks(&env, ledger_id, archive2, 2); + let archived_count = archive_blocks(env, ledger_id, archive2, 2); assert_eq!(archived_count, 2); env.advance_time(Duration::from_secs(60)); diff --git a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs index f0afab942a47..993a7232b1e9 100644 --- a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs @@ -9,7 +9,7 @@ use ic_icrc1::endpoints::StandardRecord; use ic_icrc1_index_ng::{IndexArg, InitArg}; use ic_icrc1_ledger::Tokens; use ic_icrc1_test_utils::icrc3::BlockBuilder; -use ic_icrc3_test_ledger::{AddBlockResult, ArchiveBlocksArgs}; +use ic_icrc3_test_ledger::AddBlockResult; use ic_ledger_suite_state_machine_helpers::{ archive_blocks, balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, }; From 1381cd3295096d80abc405fab0a4d202d5ca9e5e Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 16:26:52 +0000 Subject: [PATCH 24/67] clippy --- rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel b/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel index 90b7a70f07fc..a7d0fb2d3cb4 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel +++ b/rs/ledger_suite/test_utils/state_machine_helpers/BUILD.bazel @@ -7,9 +7,9 @@ DEPENDENCIES = [ "//packages/ic-http-types", "//packages/icrc-ledger-types:icrc_ledger_types", "//rs/ledger_suite/common/ledger_core", - "//rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger", "//rs/ledger_suite/icp:icp_ledger", "//rs/ledger_suite/icrc1", + "//rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger", "//rs/state_machine_tests", "//rs/types/base_types", "//rs/types/management_canister_types", From acdeb6ab2e3e334aaa83900bcad45492f7091539 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 16:49:08 +0000 Subject: [PATCH 25/67] extract common code --- .../icrc1/index-ng/tests/common/mod.rs | 10 ++- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 67 ++----------------- .../icrc3_test_ledger/tests/tests.rs | 17 +---- .../state_machine_helpers/src/lib.rs | 17 ++++- 4 files changed, 30 insertions(+), 81 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs index eff7cde92e0f..ab4427b11f90 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs @@ -98,7 +98,15 @@ pub fn install_ledger( } fn icrc3_test_ledger() -> Vec { - std::fs::read(std::env::var("IC_ICRC3_TEST_LEDGER_WASM_PATH").unwrap()).unwrap() + let ledger_wasm_path = std::env::var("IC_ICRC3_TEST_LEDGER_WASM_PATH").expect( + "The Ledger wasm path must be set using the env variable IC_ICRC3_TEST_LEDGER_WASM_PATH", + ); + std::fs::read(&ledger_wasm_path).unwrap_or_else(|e| { + panic!( + "failed to load Wasm file from path {} (env var IC_ICRC3_TEST_LEDGER_WASM_PATH): {}", + ledger_wasm_path, e + ) + }) } #[allow(dead_code)] diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 2b12841d1b4d..d3c72adde7f8 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -18,7 +18,7 @@ use ic_icrc1_test_utils::{ ArgWithCaller, LedgerEndpointArg, icrc3::BlockBuilder, minter_identity, valid_transactions_strategy, }; -use ic_ledger_suite_state_machine_helpers::archive_blocks; +use ic_ledger_suite_state_machine_helpers::{add_block, archive_blocks}; use ic_ledger_suite_state_machine_tests::test_http_request_decoding_quota; use ic_state_machine_tests::StateMachine; use icrc_ledger_types::icrc::generic_value::ICRC3Value; @@ -235,22 +235,6 @@ fn transfer( icrc1_transfer(env, ledger_id, owner.into(), req) } -fn add_block(env: &StateMachine, ledger_id: CanisterId, block: &ICRC3Value) -> BlockIndex { - let req = Encode!(block).expect("Failed to encode block"); - let res = env - .execute_ingress_as( - ic_base_types::PrincipalId(Principal::anonymous()), - ledger_id, - "add_block", - req, - ) - .unwrap_or_else(|e| panic!("Failed to add block. arg:{:?} error:{}", block, e)) - .bytes(); - Decode!(&res, Result) - .expect("Failed to decode Result") - .unwrap_or_else(|e| panic!("Failed to add block. arg:{:?} error:{}", block, e)) -} - fn icrc2_approve( env: &StateMachine, ledger_id: CanisterId, @@ -530,7 +514,7 @@ fn verify_unknown_block_handling( block }; prev_hash = Some(block.clone().hash().to_vec()); - add_block(env, ledger_id, &block); + add_block(env, ledger_id, &block).expect("failed adding block to the ledger"); } let archive1 = install_icrc3_test_ledger(env); @@ -1412,12 +1396,9 @@ mod metrics { #[cfg(not(feature = "icrc3_disabled"))] mod fees_in_burn_and_mint_blocks { use super::*; - use crate::common::STARTING_CYCLES_PER_CANISTER; use ic_icrc1_ledger::Tokens; use ic_icrc1_test_utils::icrc3::BlockBuilder; - use ic_icrc3_test_ledger::AddBlockResult; use ic_types::time::GENESIS; - use icrc_ledger_types::icrc::generic_value::ICRC3Value; #[test] fn should_take_mint_block_fee_into_account() { @@ -1437,14 +1418,7 @@ mod fees_in_burn_and_mint_blocks { let env = StateMachine::new(); - let ledger_id = env - .install_canister_with_cycles( - test_ledger_wasm(), - Encode!(&()).unwrap(), - None, - ic_types::Cycles::new(STARTING_CYCLES_PER_CANISTER), - ) - .unwrap(); + let ledger_id = install_icrc3_test_ledger(&env); let mint = BlockBuilder::new(0, GENESIS.as_nanos_since_unix_epoch()) .with_fee(Tokens::from(MINT_FEE)) @@ -1507,14 +1481,7 @@ mod fees_in_burn_and_mint_blocks { let env = StateMachine::new(); - let ledger_id = env - .install_canister_with_cycles( - test_ledger_wasm(), - Encode!(&()).unwrap(), - None, - ic_types::Cycles::new(STARTING_CYCLES_PER_CANISTER), - ) - .unwrap(); + let ledger_id = install_icrc3_test_ledger(&env); let mint = BlockBuilder::new(0, GENESIS.as_nanos_since_unix_epoch()) .with_fee_collector(FEE_COLLECTOR_ACCOUNT) @@ -1570,30 +1537,4 @@ mod fees_in_burn_and_mint_blocks { actual_fee_collector_balance, BURN_FEE ); } - - fn add_block( - env: &StateMachine, - canister_id: CanisterId, - block: &ICRC3Value, - ) -> Result { - Decode!( - &env.execute_ingress(canister_id, "add_block", Encode!(block).unwrap()) - .expect("failed to add block") - .bytes(), - AddBlockResult - ) - .expect("failed to decode add_block response") - } - - fn test_ledger_wasm() -> Vec { - let ledger_wasm_path = std::env::var("IC_ICRC3_TEST_LEDGER_WASM_PATH").expect( - "The Ledger wasm path must be set using the env variable IC_ICRC3_TEST_LEDGER_WASM_PATH", - ); - std::fs::read(&ledger_wasm_path).unwrap_or_else(|e| { - panic!( - "failed to load Wasm file from path {} (env var IC_ICRC3_TEST_LEDGER_WASM_PATH): {}", - ledger_wasm_path, e - ) - }) - } } diff --git a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs index 993a7232b1e9..5ff52e90cb4b 100644 --- a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs @@ -9,9 +9,8 @@ use ic_icrc1::endpoints::StandardRecord; use ic_icrc1_index_ng::{IndexArg, InitArg}; use ic_icrc1_ledger::Tokens; use ic_icrc1_test_utils::icrc3::BlockBuilder; -use ic_icrc3_test_ledger::AddBlockResult; use ic_ledger_suite_state_machine_helpers::{ - archive_blocks, balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, + add_block, archive_blocks, balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, }; use ic_state_machine_tests::StateMachine; use ic_test_utilities_load_wasm::load_wasm; @@ -60,20 +59,6 @@ fn setup_icrc3_test_ledger() -> (StateMachine, CanisterId) { (env, canister_id) } -fn add_block( - env: &StateMachine, - canister_id: CanisterId, - block: &ICRC3Value, -) -> Result { - Decode!( - &env.execute_ingress(canister_id, "add_block", Encode!(block).unwrap()) - .expect("failed to add block") - .bytes(), - AddBlockResult - ) - .expect("failed to decode add_block response") -} - fn icrc3_get_blocks( env: &StateMachine, canister_id: CanisterId, diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs index 34d04f7c9d22..904709ce1239 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs +++ b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs @@ -3,7 +3,7 @@ use ic_base_types::CanisterId; use ic_base_types::PrincipalId; use ic_http_types::{HttpRequest, HttpResponse}; use ic_icrc1::{Block, endpoints::StandardRecord}; -use ic_icrc3_test_ledger::ArchiveBlocksArgs; +use ic_icrc3_test_ledger::{AddBlockResult, ArchiveBlocksArgs}; use ic_ledger_core::Tokens; use ic_ledger_core::block::BlockIndex; use ic_ledger_core::tokens::TokensType; @@ -15,6 +15,7 @@ use ic_types::Cycles; use ic_universal_canister::{call_args, wasm}; use icp_ledger::{AccountIdentifier, BinaryAccountBalanceArgs, IcpAllowanceArgs}; use icrc_ledger_types::icrc::generic_metadata_value::MetadataValue as Value; +use icrc_ledger_types::icrc::generic_value::ICRC3Value; use icrc_ledger_types::icrc1::account::Account; use icrc_ledger_types::icrc1::transfer::{Memo, TransferArg, TransferError}; use icrc_ledger_types::icrc2::allowance::{Allowance, AllowanceArgs}; @@ -845,3 +846,17 @@ pub fn archive_blocks( .expect("failed to decode archive_blocks response") .expect("archiving blocks operation failed") } + +pub fn add_block( + env: &StateMachine, + canister_id: CanisterId, + block: &ICRC3Value, +) -> Result { + Decode!( + &env.execute_ingress(canister_id, "add_block", Encode!(block).unwrap()) + .expect("failed to add block") + .bytes(), + AddBlockResult + ) + .expect("failed to decode add_block response") +} From fe7daff46b05cadd115150c3a34602232daaf08c Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 17:23:02 +0000 Subject: [PATCH 26/67] extract test fn --- .../test_utils/state_machine_helpers/src/lib.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs index 904709ce1239..2693f3c4c5fc 100644 --- a/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs +++ b/rs/ledger_suite/test_utils/state_machine_helpers/src/lib.rs @@ -860,3 +860,13 @@ pub fn add_block( ) .expect("failed to decode add_block response") } + +pub fn set_icrc3_enabled(env: &StateMachine, canister_id: CanisterId, enabled: bool) { + Decode!( + &env.execute_ingress(canister_id, "set_icrc3_enabled", Encode!(&enabled).unwrap()) + .expect("failed to set_icrc3_enabled") + .bytes(), + () + ) + .expect("failed to decode set_icrc3_enabled response") +} From 18bdd28e50cb1e87192b60d5f6a047eac1d1631d Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 8 Oct 2025 17:23:30 +0000 Subject: [PATCH 27/67] extract test fn --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 12 +----------- .../test_utils/icrc3_test_ledger/tests/tests.rs | 11 +---------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index d3c72adde7f8..79563cc92aab 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -18,7 +18,7 @@ use ic_icrc1_test_utils::{ ArgWithCaller, LedgerEndpointArg, icrc3::BlockBuilder, minter_identity, valid_transactions_strategy, }; -use ic_ledger_suite_state_machine_helpers::{add_block, archive_blocks}; +use ic_ledger_suite_state_machine_helpers::{add_block, archive_blocks, set_icrc3_enabled}; use ic_ledger_suite_state_machine_tests::test_http_request_decoding_quota; use ic_state_machine_tests::StateMachine; use icrc_ledger_types::icrc::generic_value::ICRC3Value; @@ -340,16 +340,6 @@ fn get_fee_collectors_ranges(env: &StateMachine, index: CanisterId) -> FeeCollec .expect("failed to decode get_fee_collectors_ranges response") } -fn set_icrc3_enabled(env: &StateMachine, canister_id: CanisterId, enabled: bool) { - Decode!( - &env.execute_ingress(canister_id, "set_icrc3_enabled", Encode!(&enabled).unwrap()) - .expect("failed to set_icrc3_enabled") - .bytes(), - () - ) - .expect("failed to decode set_icrc3_enabled response") -} - // Assert that the index canister contains the same blocks as the ledger. #[track_caller] fn assert_ledger_index_parity(env: &StateMachine, ledger_id: CanisterId, index_id: CanisterId) { diff --git a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs index 5ff52e90cb4b..1c98f7640959 100644 --- a/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/test_utils/icrc3_test_ledger/tests/tests.rs @@ -11,6 +11,7 @@ use ic_icrc1_ledger::Tokens; use ic_icrc1_test_utils::icrc3::BlockBuilder; use ic_ledger_suite_state_machine_helpers::{ add_block, archive_blocks, balance_of, icrc3_get_blocks as icrc3_get_blocks_helper, + set_icrc3_enabled, }; use ic_state_machine_tests::StateMachine; use ic_test_utilities_load_wasm::load_wasm; @@ -539,16 +540,6 @@ fn get_supported_standards(env: &StateMachine, canister_id: CanisterId) -> Vec Date: Thu, 9 Oct 2025 12:20:10 +0000 Subject: [PATCH 28/67] use tx.fee when calculating effective fee --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 9501bcdbcbf3..6d6341fb20ff 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -846,13 +846,11 @@ fn process_balance_changes(block_index: BlockIndex64, block: &Block) { "append_blocks.process_balance_changes", move || match block.transaction.operation { Operation::Burn { - from, - amount, - fee: _, - .. + from, amount, fee, .. } => { let mut amount_with_fee = amount; - if let Some(fee) = block.effective_fee { + let effective_fee = block.effective_fee.or(fee); + if let Some(fee) = effective_fee { amount_with_fee = amount.checked_add(&fee).unwrap_or_else(|| { trap(format!( "token amount overflow while indexing block {block_index}" @@ -865,9 +863,10 @@ fn process_balance_changes(block_index: BlockIndex64, block: &Block) { } debit(block_index, from, amount_with_fee); } - Operation::Mint { to, amount, fee: _ } => { + Operation::Mint { to, amount, fee } => { let mut amount_without_fee = amount; - if let Some(fee) = block.effective_fee { + let effective_fee = block.effective_fee.or(fee); + if let Some(fee) = effective_fee { amount_without_fee = amount.checked_sub(&fee).unwrap_or_else(|| { trap(format!( "token amount underflow while indexing block {block_index}" From d97a06ac075e0d4e6afdc1b131788d1e51c5b8e4 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 12:42:32 +0000 Subject: [PATCH 29/67] add fee to icrc3 transaction --- .../src/icrc3/transactions.rs | 2 ++ rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 3 +++ rs/ledger_suite/icrc1/src/endpoints.rs | 2 ++ rs/ledger_suite/icrc1/src/lib.rs | 18 ++++++++++++++++-- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc3/transactions.rs b/packages/icrc-ledger-types/src/icrc3/transactions.rs index e6f9f06141bb..f71945bce7aa 100644 --- a/packages/icrc-ledger-types/src/icrc3/transactions.rs +++ b/packages/icrc-ledger-types/src/icrc3/transactions.rs @@ -22,6 +22,7 @@ pub struct Mint { pub to: Account, pub memo: Option, pub created_at_time: Option, + pub fee: Option, } #[derive(CandidType, Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] @@ -31,6 +32,7 @@ pub struct Burn { pub spender: Option, pub memo: Option, pub created_at_time: Option, + pub fee: Option, } #[derive(CandidType, Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 79563cc92aab..65049cd7b86c 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -651,6 +651,7 @@ fn test_get_account_transactions() { amount: 1_000_000_000_000_u64.into(), created_at_time: None, memo: None, + fee: None, }, 0, ), @@ -784,6 +785,7 @@ fn test_get_account_transactions_start_length() { amount: (i * 10_000).into(), created_at_time: None, memo: None, + fee: None, }, 0, ), @@ -878,6 +880,7 @@ fn test_get_account_transactions_pagination() { amount: (id * 10_000).into(), created_at_time: None, memo: None, + fee: None, }), transfer: None, approve: None, diff --git a/rs/ledger_suite/icrc1/src/endpoints.rs b/rs/ledger_suite/icrc1/src/endpoints.rs index 127ce0a26e11..71aaa2cb671e 100644 --- a/rs/ledger_suite/icrc1/src/endpoints.rs +++ b/rs/ledger_suite/icrc1/src/endpoints.rs @@ -173,6 +173,7 @@ impl From> for Transaction { amount: amount.into(), created_at_time, memo, + fee: fee.map(Into::into), }); } Operation::Burn { @@ -189,6 +190,7 @@ impl From> for Transaction { amount: amount.into(), created_at_time, memo, + fee: fee.map(Into::into), }); } Operation::Transfer { diff --git a/rs/ledger_suite/icrc1/src/lib.rs b/rs/ledger_suite/icrc1/src/lib.rs index c189b94721ff..a55725337f36 100644 --- a/rs/ledger_suite/icrc1/src/lib.rs +++ b/rs/ledger_suite/icrc1/src/lib.rs @@ -473,10 +473,17 @@ impl TryFrom Some( + Tokens::try_from(fee) + .map_err(|_| "Could not convert Nat to Tokens".to_string())?, + ), + None => None, + }; let operation = Operation::Mint { to: mint.to, amount, - fee: None, + fee, }; return Ok(Self { operation, @@ -487,11 +494,18 @@ impl TryFrom Some( + Tokens::try_from(fee) + .map_err(|_| "Could not convert Nat to Tokens".to_string())?, + ), + None => None, + }; let operation = Operation::Burn { from: burn.from, spender: burn.spender, amount, - fee: None, + fee, }; return Ok(Self { operation, From 21556e220ddfa0216472c6c0123f5a4edccb00c8 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 12:53:07 +0000 Subject: [PATCH 30/67] build fix --- rs/nervous_system/initial_supply/src/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/nervous_system/initial_supply/src/tests.rs b/rs/nervous_system/initial_supply/src/tests.rs index 4bb2f4946c13..baa9a761bc00 100644 --- a/rs/nervous_system/initial_supply/src/tests.rs +++ b/rs/nervous_system/initial_supply/src/tests.rs @@ -68,6 +68,7 @@ async fn test_initial_supply() { subaccount: None, }, memo: None, + fee: None, }), kind: "mint".to_string(), From d9cfb0fa2bc9e42dbd2f0adcdda9c9cf2928f806 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 13:01:52 +0000 Subject: [PATCH 31/67] add fee to icrc3 schema --- packages/icrc-ledger-types/src/icrc3/schema.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/icrc-ledger-types/src/icrc3/schema.rs b/packages/icrc-ledger-types/src/icrc3/schema.rs index 4d22db2fe23c..3268b3f862c0 100644 --- a/packages/icrc-ledger-types/src/icrc3/schema.rs +++ b/packages/icrc-ledger-types/src/icrc3/schema.rs @@ -43,11 +43,13 @@ pub fn validate(block: &Value) -> Result<(), ValuePredicateFailures> { icrc1_common.clone(), item("op", Required, is(Value::text("burn"))), item("from", Required, is_account.clone()), + item("fee", Optional, is_amount.clone()), ]); let is_icrc1_mint = and(vec![ icrc1_common.clone(), item("op", Required, is(Value::text("mint"))), item("to", Required, is_account.clone()), + item("fee", Optional, is_amount.clone()), ]); let is_icrc2_approve = and(vec![ icrc1_common.clone(), From f2629e6a18318d5ed09e8c880d46d1e789e189bb Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 13:07:55 +0000 Subject: [PATCH 32/67] revert to trap in case of impossible state --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 6d6341fb20ff..7fc482af0220 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -686,14 +686,10 @@ async fn fetch_blocks_via_icrc3() -> Result { append_icrc3_blocks(res.blocks)?; let num_blocks = with_blocks(|blocks| blocks.len()); match num_blocks.checked_sub(previous_num_blocks) { - None => { - let error = format!( - "The number of blocks {} is smaller than the number of blocks before indexing {}. This is impossible. I'm trapping to reset the state", - num_blocks, previous_num_blocks - ); - stop_timer_with_error(error); - Err(()) - } + None => panic!( + "The number of blocks {} is smaller than the number of blocks before indexing {}. This is impossible. I'm trapping to reset the state", + num_blocks, previous_num_blocks + ), Some(new_blocks_indexed) => Ok(new_blocks_indexed), } } From 2c57ea9785ce645de6dd102e80308ee133be4987 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 13:24:46 +0000 Subject: [PATCH 33/67] remove asserts --- rs/ledger_suite/icrc1/src/endpoints.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rs/ledger_suite/icrc1/src/endpoints.rs b/rs/ledger_suite/icrc1/src/endpoints.rs index 71aaa2cb671e..4930df6f1902 100644 --- a/rs/ledger_suite/icrc1/src/endpoints.rs +++ b/rs/ledger_suite/icrc1/src/endpoints.rs @@ -166,7 +166,6 @@ impl From> for Transaction { match b.transaction.operation { Operation::Mint { to, amount, fee } => { - assert!(fee.is_none()); tx.kind = "mint".to_string(); tx.mint = Some(Mint { to, @@ -182,7 +181,6 @@ impl From> for Transaction { amount, fee, } => { - assert!(fee.is_none()); tx.kind = "burn".to_string(); tx.burn = Some(Burn { from, From e5f024df178045dca2030d31674f0dfc01bd4a88 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 14:17:26 +0000 Subject: [PATCH 34/67] return error if burn or mint fee while applying tx --- rs/ledger_suite/common/ledger_canister_core/src/ledger.rs | 6 ++++++ rs/ledger_suite/icp/ledger/src/lib.rs | 4 ++++ rs/ledger_suite/icrc1/ledger/src/lib.rs | 4 ++++ rs/ledger_suite/icrc1/src/lib.rs | 8 ++++++-- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs index a19eda8c9d9d..4276d0fb2af5 100644 --- a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs +++ b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs @@ -34,6 +34,7 @@ pub enum TxApplyError { ExpiredApproval { now: TimeStamp }, AllowanceChanged { current_allowance: Tokens }, SelfApproval, + BadFee, } impl From> for TxApplyError { @@ -186,6 +187,8 @@ pub trait LedgerData: LedgerContext { fn on_purged_transaction(&mut self, height: BlockIndex); fn fee_collector_mut(&mut self) -> Option<&mut FeeCollector>; + + fn expected_fee(&self) -> Self::Tokens; } #[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)] @@ -264,6 +267,9 @@ where TransferError::AllowanceChanged { current_allowance } } TxApplyError::SelfApproval => TransferError::SelfApproval, + TxApplyError::BadFee => TransferError::BadFee { + expected_fee: ledger.expected_fee(), + }, })?; let fee_collector = ledger.fee_collector().cloned(); diff --git a/rs/ledger_suite/icp/ledger/src/lib.rs b/rs/ledger_suite/icp/ledger/src/lib.rs index b5dc39f9b668..0961cec6570c 100644 --- a/rs/ledger_suite/icp/ledger/src/lib.rs +++ b/rs/ledger_suite/icp/ledger/src/lib.rs @@ -322,6 +322,10 @@ impl LedgerData for Ledger { ) -> Option<&mut ic_ledger_core::block::FeeCollector> { None } + + fn expected_fee(&self) -> Self::Tokens { + self.transfer_fee + } } impl Default for Ledger { diff --git a/rs/ledger_suite/icrc1/ledger/src/lib.rs b/rs/ledger_suite/icrc1/ledger/src/lib.rs index cd6ac9791cea..e4954ce8de99 100644 --- a/rs/ledger_suite/icrc1/ledger/src/lib.rs +++ b/rs/ledger_suite/icrc1/ledger/src/lib.rs @@ -873,6 +873,10 @@ impl LedgerData for Ledger { fn fee_collector_mut(&mut self) -> Option<&mut FeeCollector> { self.fee_collector.as_mut() } + + fn expected_fee(&self) -> Self::Tokens { + self.transfer_fee + } } impl Ledger { diff --git a/rs/ledger_suite/icrc1/src/lib.rs b/rs/ledger_suite/icrc1/src/lib.rs index a55725337f36..753b57f56731 100644 --- a/rs/ledger_suite/icrc1/src/lib.rs +++ b/rs/ledger_suite/icrc1/src/lib.rs @@ -366,7 +366,9 @@ impl LedgerTransaction for Transaction { amount, fee, } => { - assert!(fee.is_none()); + if fee.is_some() { + return Err(TxApplyError::BadFee); + } if spender.is_some() && from != &spender.unwrap() { let allowance = context.approvals().allowance(from, &spender.unwrap(), now); if allowance.amount < *amount { @@ -384,7 +386,9 @@ impl LedgerTransaction for Transaction { } } Operation::Mint { to, amount, fee } => { - assert!(fee.is_none()); + if fee.is_some() { + return Err(TxApplyError::BadFee); + } context.balances_mut().mint(to, amount.clone())?; } Operation::Approve { From 7e0c8f027dbd2ba8fce3e09ba53dac5be1ebf5ef Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 14:19:51 +0000 Subject: [PATCH 35/67] Revert "return error if burn or mint fee while applying tx" This reverts commit e5f024df178045dca2030d31674f0dfc01bd4a88. --- rs/ledger_suite/common/ledger_canister_core/src/ledger.rs | 6 ------ rs/ledger_suite/icp/ledger/src/lib.rs | 4 ---- rs/ledger_suite/icrc1/ledger/src/lib.rs | 4 ---- rs/ledger_suite/icrc1/src/lib.rs | 8 ++------ 4 files changed, 2 insertions(+), 20 deletions(-) diff --git a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs index 4276d0fb2af5..a19eda8c9d9d 100644 --- a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs +++ b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs @@ -34,7 +34,6 @@ pub enum TxApplyError { ExpiredApproval { now: TimeStamp }, AllowanceChanged { current_allowance: Tokens }, SelfApproval, - BadFee, } impl From> for TxApplyError { @@ -187,8 +186,6 @@ pub trait LedgerData: LedgerContext { fn on_purged_transaction(&mut self, height: BlockIndex); fn fee_collector_mut(&mut self) -> Option<&mut FeeCollector>; - - fn expected_fee(&self) -> Self::Tokens; } #[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)] @@ -267,9 +264,6 @@ where TransferError::AllowanceChanged { current_allowance } } TxApplyError::SelfApproval => TransferError::SelfApproval, - TxApplyError::BadFee => TransferError::BadFee { - expected_fee: ledger.expected_fee(), - }, })?; let fee_collector = ledger.fee_collector().cloned(); diff --git a/rs/ledger_suite/icp/ledger/src/lib.rs b/rs/ledger_suite/icp/ledger/src/lib.rs index 0961cec6570c..b5dc39f9b668 100644 --- a/rs/ledger_suite/icp/ledger/src/lib.rs +++ b/rs/ledger_suite/icp/ledger/src/lib.rs @@ -322,10 +322,6 @@ impl LedgerData for Ledger { ) -> Option<&mut ic_ledger_core::block::FeeCollector> { None } - - fn expected_fee(&self) -> Self::Tokens { - self.transfer_fee - } } impl Default for Ledger { diff --git a/rs/ledger_suite/icrc1/ledger/src/lib.rs b/rs/ledger_suite/icrc1/ledger/src/lib.rs index e4954ce8de99..cd6ac9791cea 100644 --- a/rs/ledger_suite/icrc1/ledger/src/lib.rs +++ b/rs/ledger_suite/icrc1/ledger/src/lib.rs @@ -873,10 +873,6 @@ impl LedgerData for Ledger { fn fee_collector_mut(&mut self) -> Option<&mut FeeCollector> { self.fee_collector.as_mut() } - - fn expected_fee(&self) -> Self::Tokens { - self.transfer_fee - } } impl Ledger { diff --git a/rs/ledger_suite/icrc1/src/lib.rs b/rs/ledger_suite/icrc1/src/lib.rs index 753b57f56731..a55725337f36 100644 --- a/rs/ledger_suite/icrc1/src/lib.rs +++ b/rs/ledger_suite/icrc1/src/lib.rs @@ -366,9 +366,7 @@ impl LedgerTransaction for Transaction { amount, fee, } => { - if fee.is_some() { - return Err(TxApplyError::BadFee); - } + assert!(fee.is_none()); if spender.is_some() && from != &spender.unwrap() { let allowance = context.approvals().allowance(from, &spender.unwrap(), now); if allowance.amount < *amount { @@ -386,9 +384,7 @@ impl LedgerTransaction for Transaction { } } Operation::Mint { to, amount, fee } => { - if fee.is_some() { - return Err(TxApplyError::BadFee); - } + assert!(fee.is_none()); context.balances_mut().mint(to, amount.clone())?; } Operation::Approve { From 0a18a5f438e3a2462bd9390df831771b54df3566 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 14:29:40 +0000 Subject: [PATCH 36/67] burn or mint fee error --- rs/ledger_suite/common/ledger_canister_core/src/ledger.rs | 6 +++++- rs/ledger_suite/icrc1/src/lib.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs index a19eda8c9d9d..d6c1403b2c85 100644 --- a/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs +++ b/rs/ledger_suite/common/ledger_canister_core/src/ledger.rs @@ -18,7 +18,7 @@ use crate::archive::{ArchivingGuardError, FailedToArchiveBlocks, LedgerArchiving use ic_ledger_core::balances::{BalanceError, Balances, BalancesStore}; use ic_ledger_core::block::{BlockIndex, BlockType, EncodedBlock, FeeCollector}; use ic_ledger_core::timestamp::TimeStamp; -use ic_ledger_core::tokens::TokensType; +use ic_ledger_core::tokens::{TokensType, Zero}; use ic_ledger_hash_of::HashOf; #[derive(Debug, Deserialize, Serialize)] @@ -34,6 +34,7 @@ pub enum TxApplyError { ExpiredApproval { now: TimeStamp }, AllowanceChanged { current_allowance: Tokens }, SelfApproval, + BurnOrMintFee, } impl From> for TxApplyError { @@ -264,6 +265,9 @@ where TransferError::AllowanceChanged { current_allowance } } TxApplyError::SelfApproval => TransferError::SelfApproval, + TxApplyError::BurnOrMintFee => TransferError::BadFee { + expected_fee: L::Tokens::zero(), + }, })?; let fee_collector = ledger.fee_collector().cloned(); diff --git a/rs/ledger_suite/icrc1/src/lib.rs b/rs/ledger_suite/icrc1/src/lib.rs index a55725337f36..7389b917586e 100644 --- a/rs/ledger_suite/icrc1/src/lib.rs +++ b/rs/ledger_suite/icrc1/src/lib.rs @@ -366,7 +366,9 @@ impl LedgerTransaction for Transaction { amount, fee, } => { - assert!(fee.is_none()); + if fee.is_some() { + return Err(TxApplyError::BurnOrMintFee); + } if spender.is_some() && from != &spender.unwrap() { let allowance = context.approvals().allowance(from, &spender.unwrap(), now); if allowance.amount < *amount { @@ -384,7 +386,9 @@ impl LedgerTransaction for Transaction { } } Operation::Mint { to, amount, fee } => { - assert!(fee.is_none()); + if fee.is_some() { + return Err(TxApplyError::BurnOrMintFee); + } context.balances_mut().mint(to, amount.clone())?; } Operation::Approve { From 1065e30bf2972afcfef5208f5bc999d3daca4063 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 15:29:38 +0000 Subject: [PATCH 37/67] build fix --- rs/ledger_suite/icrc1/ledger/tests/tests.rs | 2 ++ rs/sns/governance/token_valuation/src/tests.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/rs/ledger_suite/icrc1/ledger/tests/tests.rs b/rs/ledger_suite/icrc1/ledger/tests/tests.rs index 4f75c13acff7..6254d67cdb00 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -1906,6 +1906,7 @@ mod verify_written_blocks { to: mint_args.to, memo: mint_args.memo, created_at_time: mint_args.created_at_time, + fee: None, }); } @@ -2020,6 +2021,7 @@ mod verify_written_blocks { spender: Some(spender_account), memo: burn_args.memo, created_at_time: burn_args.created_at_time, + fee: None, }); } diff --git a/rs/sns/governance/token_valuation/src/tests.rs b/rs/sns/governance/token_valuation/src/tests.rs index f6a7728a6363..5b4e5875cce4 100644 --- a/rs/sns/governance/token_valuation/src/tests.rs +++ b/rs/sns/governance/token_valuation/src/tests.rs @@ -155,6 +155,7 @@ async fn test_icps_per_sns_token_client() { }, created_at_time: Some(GENESIS_TIMESTAMP_NANOSECONDS), memo: None, + fee: None, }), timestamp: GENESIS_TIMESTAMP_NANOSECONDS, kind: "mint".to_string(), From 507ed64f232e8a799321d53892d5a062dba799cc Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Thu, 9 Oct 2025 15:38:25 +0000 Subject: [PATCH 38/67] add mint and burn fees to rosetta --- .../src/common/storage/storage_operations.rs | 12 ++-- .../common/storage/storage_operations_test.rs | 2 + .../icrc1/src/common/storage/types.rs | 63 ++++++++++------- .../icrc1/src/common/utils/utils.rs | 70 +++++++++++++++---- rs/rosetta-api/icrc1/src/data_api/services.rs | 7 ++ 5 files changed, 112 insertions(+), 42 deletions(-) diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs index 32ab8e1b78c9..7ca28ddf32c8 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs @@ -295,7 +295,7 @@ pub fn update_account_balances(connection: &mut Connection) -> anyhow::Result<() )?; } } - crate::common::storage::types::IcrcOperation::Mint { to, amount } => { + crate::common::storage::types::IcrcOperation::Mint { to, amount, .. } => { let fee = rosetta_block .get_fee_paid()? .unwrap_or(Nat(BigUint::zero())); @@ -423,7 +423,7 @@ pub fn store_blocks( fee, approval_expires_at, ) = match transaction.operation { - crate::common::storage::types::IcrcOperation::Mint { to, amount } => ( + crate::common::storage::types::IcrcOperation::Mint { to, amount, fee } => ( "mint", None, None, @@ -433,7 +433,7 @@ pub fn store_blocks( None, amount, None, - None, + fee, None, ), crate::common::storage::types::IcrcOperation::Transfer { @@ -455,7 +455,9 @@ pub fn store_blocks( fee, None, ), - crate::common::storage::types::IcrcOperation::Burn { from, amount, .. } => ( + crate::common::storage::types::IcrcOperation::Burn { + from, amount, fee, .. + } => ( "burn", Some(from.owner), Some(*from.effective_subaccount()), @@ -465,7 +467,7 @@ pub fn store_blocks( None, amount, None, - None, + fee, None, ), crate::common::storage::types::IcrcOperation::Approve { diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs index 5d752921bdb5..fea9e5b58ff7 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs @@ -377,6 +377,7 @@ fn test_fee_collector_resolution_and_repair() -> anyhow::Result<()> { mint_block.block.transaction.operation = IcrcOperation::Mint { to: from_account, amount: Nat::from(1000000000u64), + fee: None, }; let mut block1 = create_test_rosetta_block(1, 1000000000, &principal1, 100); @@ -512,6 +513,7 @@ fn test_repair_fee_collector_edge_cases() -> anyhow::Result<()> { mint_block.block.transaction.operation = IcrcOperation::Mint { to: from_account, amount: Nat::from(1000000000u64), + fee: None, }; store_blocks(&mut connection, vec![mint_block])?; diff --git a/rs/rosetta-api/icrc1/src/common/storage/types.rs b/rs/rosetta-api/icrc1/src/common/storage/types.rs index aa2462ca50cb..b0ac9bc66d32 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/types.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/types.rs @@ -134,10 +134,10 @@ impl RosettaBlock { Ok(self .get_effective_fee() .or(match self.get_transaction().operation { - IcrcOperation::Mint { .. } => None, + IcrcOperation::Mint { fee, .. } => fee, IcrcOperation::Transfer { fee, .. } => fee, IcrcOperation::Approve { fee, .. } => fee, - IcrcOperation::Burn { .. } => None, + IcrcOperation::Burn { fee, .. } => fee, })) } @@ -338,6 +338,7 @@ pub enum IcrcOperation { Mint { to: Account, amount: Nat, + fee: Option, }, Transfer { from: Account, @@ -350,6 +351,7 @@ pub enum IcrcOperation { from: Account, spender: Option, amount: Nat, + fee: Option, }, Approve { from: Account, @@ -376,11 +378,12 @@ impl TryFrom> for IcrcOperation { from, spender, amount, + fee, }) } "mint" => { let to: Account = get_field(&map, FIELD_PREFIX, "to")?; - Ok(Self::Mint { to, amount }) + Ok(Self::Mint { to, amount, fee }) } "xfer" => { let from: Account = get_field(&map, FIELD_PREFIX, "from")?; @@ -452,6 +455,7 @@ impl From for BTreeMap { from, spender, amount, + fee, } => { map.insert("op".to_string(), Value::text("burn")); map.insert("from".to_string(), Value::from(from)); @@ -459,11 +463,17 @@ impl From for BTreeMap { map.insert("spender".to_string(), Value::from(spender)); } map.insert("amt".to_string(), Value::Nat(amount)); + if let Some(fee) = fee { + map.insert("fee".to_string(), Value::Nat(fee)); + } } - Op::Mint { to, amount } => { + Op::Mint { to, amount, fee } => { map.insert("op".to_string(), Value::text("mint")); map.insert("to".to_string(), Value::from(to)); map.insert("amt".to_string(), Value::Nat(amount)); + if let Some(fee) = fee { + map.insert("fee".to_string(), Value::Nat(fee)); + } } Op::Transfer { from, @@ -572,21 +582,17 @@ where spender, amount, fee, - } => { - assert!(fee.is_none()); - Self::Burn { - from, - spender, - amount: amount.into(), - } - } - Op::Mint { to, amount, fee } => { - assert!(fee.is_none()); - Self::Mint { - to, - amount: amount.into(), - } - } + } => Self::Burn { + from, + spender, + amount: amount.into(), + fee: fee.map(Into::into), + }, + Op::Mint { to, amount, fee } => Self::Mint { + to, + amount: amount.into(), + fee: fee.map(Into::into), + }, Op::Transfer { from, to, @@ -684,20 +690,23 @@ mod tests { arb_account(), // from option::of(arb_account()), // spender arb_nat(), // amount + option::of(arb_nat()), // fee ) - .prop_map(|(from, spender, amount)| IcrcOperation::Burn { + .prop_map(|(from, spender, amount, fee)| IcrcOperation::Burn { from, spender, amount, + fee, }) } fn arb_mint() -> impl Strategy { ( - arb_account(), // to - arb_nat(), // amount + arb_account(), // to + arb_nat(), // amount + option::of(arb_nat()), // fee ) - .prop_map(|(to, amount)| IcrcOperation::Mint { to, amount }) + .prop_map(|(to, amount, fee)| IcrcOperation::Mint { to, amount, fee }) } fn arb_transfer() -> impl Strategy { @@ -910,27 +919,31 @@ mod tests { from, spender, amount, - fee: _, + fee, }, IcrcOperation::Burn { from: rosetta_from, spender: rosetta_spender, amount: rosetta_amount, + fee: rosetta_fee, }, ) => { assert_eq!(from, rosetta_from, "from"); assert_eq!(spender, rosetta_spender, "spender"); assert_eq!(amount.into(), rosetta_amount, "amount"); + assert_eq!(fee.map(|t| t.into()), rosetta_fee, "fee"); } ( - ic_icrc1::Operation::Mint { to, amount, fee: _ }, + ic_icrc1::Operation::Mint { to, amount, fee }, IcrcOperation::Mint { to: rosetta_to, amount: rosetta_amount, + fee: rosetta_fee, }, ) => { assert_eq!(to, rosetta_to, "to"); assert_eq!(amount.into(), rosetta_amount, "amount"); + assert_eq!(fee.map(|t| t.into()), rosetta_fee, "fee"); } ( ic_icrc1::Operation::Transfer { diff --git a/rs/rosetta-api/icrc1/src/common/utils/utils.rs b/rs/rosetta-api/icrc1/src/common/utils/utils.rs index 6419d07d3241..99868e6bd11f 100644 --- a/rs/rosetta-api/icrc1/src/common/utils/utils.rs +++ b/rs/rosetta-api/icrc1/src/common/utils/utils.rs @@ -233,24 +233,20 @@ pub fn rosetta_core_operations_to_icrc1_operation( if self.spender.is_some() { bail!("Spender AccountIdentifier field is not allowed for Mint operation") } - if self.fee.is_some() { - bail!("Fee field is not allowed for Mint operation") - } crate::common::storage::types::IcrcOperation::Mint{ to: self.to.context("Account field needs to be populated for Mint operation")?.try_into()?, amount: self.amount.context("Amount field needs to be populated for Mint operation")?, + fee: self.fee, }}, IcrcOperation::Burn => { if self.to.is_some() { bail!("To AccountIdentifier field is not allowed for Burn operation") } - if self.fee.is_some() { - bail!("Fee field is not allowed for Burn operation") - } crate::common::storage::types::IcrcOperation::Burn{ from: self.from.context("From AccountIdentifier field needs to be populated for Burn operation")?.try_into()?, amount: self.amount.context("Amount field needs to be populated for Burn operation")?, spender: self.spender.map(|spender| spender.try_into()).transpose()?, + fee: self.fee, }}, IcrcOperation::Transfer => crate::common::storage::types::IcrcOperation::Transfer{ from: self.from.context("From AccountIdentifier field needs to be populated for Transfer operation")?.try_into()?, @@ -392,24 +388,48 @@ pub fn icrc1_operation_to_rosetta_core_operations( ) -> anyhow::Result> { let mut operations = vec![]; match operation { - crate::common::storage::types::IcrcOperation::Mint { to, amount } => { + crate::common::storage::types::IcrcOperation::Mint { to, amount, fee } => { operations.push(rosetta_core::objects::Operation::new( 0, OperationType::Mint.to_string(), Some(to.into()), Some(rosetta_core::objects::Amount::new( BigInt::from(amount.0), - currency, + currency.clone(), )), None, None, - )) + )); + + if let Some(fee_paid) = fee_payed { + operations.push(rosetta_core::objects::Operation::new( + 1, + OperationType::Fee.to_string(), + Some(to.into()), // Mint fees are payed by the receiving account. + Some(Amount::new( + BigInt::from_biguint(num_bigint::Sign::Minus, fee_paid.0), + currency, + )), + None, + // If the fee inside the operation is set that means the User set the fee and the Ledger did nothing + Some( + FeeMetadata { + fee_set_by: match fee { + Some(_) => FeeSetter::User, + None => FeeSetter::Ledger, + }, + } + .try_into()?, + ), + )); + } } crate::common::storage::types::IcrcOperation::Burn { from, spender, amount, + fee, } => { operations.push(rosetta_core::objects::Operation::new( 0, @@ -417,21 +437,47 @@ pub fn icrc1_operation_to_rosetta_core_operations( Some(from.into()), Some(rosetta_core::objects::Amount::new( BigInt::from_biguint(num_bigint::Sign::Minus, amount.0), - currency, + currency.clone(), )), None, None, )); + let mut idx = 2; + if let Some(spender) = spender { operations.push(rosetta_core::objects::Operation::new( - 1, + idx, OperationType::Spender.to_string(), Some(spender.into()), None, None, None, )); + idx += 1; + } + + if let Some(fee_paid) = fee_payed { + operations.push(rosetta_core::objects::Operation::new( + idx, + OperationType::Fee.to_string(), + Some(from.into()), + Some(Amount::new( + BigInt::from_biguint(num_bigint::Sign::Minus, fee_paid.0), + currency, + )), + None, + // If the fee inside the operation is set that means the User set the fee and the Ledger did nothing + Some( + FeeMetadata { + fee_set_by: match fee { + Some(_) => FeeSetter::User, + None => FeeSetter::Ledger, + }, + } + .try_into()?, + ), + )); } } @@ -487,7 +533,7 @@ pub fn icrc1_operation_to_rosetta_core_operations( Some(from.into()), Some(Amount::new( BigInt::from_biguint(num_bigint::Sign::Minus, fee_paid.0), - currency.clone(), + currency, )), None, // If the fee inside the operation is set that means the User set the fee and the Ledger did nothing diff --git a/rs/rosetta-api/icrc1/src/data_api/services.rs b/rs/rosetta-api/icrc1/src/data_api/services.rs index 6f688d2a2055..6f00059ec273 100644 --- a/rs/rosetta-api/icrc1/src/data_api/services.rs +++ b/rs/rosetta-api/icrc1/src/data_api/services.rs @@ -1536,6 +1536,7 @@ mod test { operation: IcrcOperation::Mint { to: main_account, amount: Nat::from(1000u64), + fee: None, }, created_at_time: Some(1000), memo: None, @@ -1603,6 +1604,7 @@ mod test { operation: IcrcOperation::Mint { to: main_account, amount: Nat::from(500u64), + fee: None, }, created_at_time: Some(1000), memo: None, @@ -1621,6 +1623,7 @@ mod test { operation: IcrcOperation::Mint { to: account1, amount: Nat::from(1000u64), + fee: None, }, created_at_time: Some(2000), memo: None, @@ -1770,6 +1773,7 @@ mod test { operation: IcrcOperation::Mint { to: main_account, amount: Nat::from(1000u64), + fee: None, }, created_at_time: Some(1000), memo: None, @@ -2125,6 +2129,7 @@ mod test { operation: IcrcOperation::Mint { to: main_account, amount: Nat::from(6000000u64), // 0.06 tokens + fee: None, }, created_at_time: Some(1), memo: None, @@ -2144,6 +2149,7 @@ mod test { operation: IcrcOperation::Mint { to: explicit_zero_account, amount: Nat::from(1000000u64), // 0.01 tokens + fee: None, }, created_at_time: Some(2), memo: None, @@ -2163,6 +2169,7 @@ mod test { operation: IcrcOperation::Mint { to: account1, amount: Nat::from(1000000u64), // 0.01 tokens + fee: None, }, created_at_time: Some(3), memo: None, From 6e65bdf9a6b87d2a4c9ab1df451a6c91576d1f07 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 10 Oct 2025 06:36:44 +0000 Subject: [PATCH 39/67] clippy --- rs/ethereum/cketh/minter/tests/ckerc20.rs | 8 ++++++++ rs/ethereum/cketh/minter/tests/cketh.rs | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/rs/ethereum/cketh/minter/tests/ckerc20.rs b/rs/ethereum/cketh/minter/tests/ckerc20.rs index 39f3a0e761de..2f1a1f217a85 100644 --- a/rs/ethereum/cketh/minter/tests/ckerc20.rs +++ b/rs/ethereum/cketh/minter/tests/ckerc20.rs @@ -440,6 +440,7 @@ mod withdraw_erc20 { .unwrap(), })), created_at_time: None, + fee: None, }); let balance_after_withdrawal = ckerc20.cketh.balance_of(caller); @@ -485,6 +486,7 @@ mod withdraw_erc20 { withdrawal_id: cketh_burn_index.into(), })), created_at_time: None, + fee: None, }); } @@ -725,6 +727,7 @@ mod withdraw_erc20 { .unwrap(), })), created_at_time: None, + fee: None, }) .call_ckerc20_ledger_get_transaction( deposit_params.token().ledger_canister_id, @@ -744,6 +747,7 @@ mod withdraw_erc20 { .unwrap(), })), created_at_time: None, + fee: None, }); let expected_cketh_balance_after_withdrawal = @@ -873,6 +877,7 @@ mod withdraw_erc20 { tx_hash: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH.parse().unwrap(), })), created_at_time: None, + fee: None, }); } } @@ -1361,6 +1366,7 @@ fn should_deposit_ckerc20() { log_index: params.transaction_data().log_index.into(), })), created_at_time: None, + fee: None, }); } } @@ -1454,6 +1460,7 @@ fn should_deposit_cketh_and_ckerc20() { log_index: cketh_params.transaction_data().log_index.into(), })), created_at_time: None, + fee: None, }) .call_ckerc20_ledger_get_transaction(params.token().ledger_canister_id, 0_u8) .expect_mint(Mint { @@ -1465,6 +1472,7 @@ fn should_deposit_cketh_and_ckerc20() { log_index: params.transaction_data().log_index.into(), })), created_at_time: None, + fee: None, }); } } diff --git a/rs/ethereum/cketh/minter/tests/cketh.rs b/rs/ethereum/cketh/minter/tests/cketh.rs index ea0e0e0f3a6b..540ac0b673aa 100644 --- a/rs/ethereum/cketh/minter/tests/cketh.rs +++ b/rs/ethereum/cketh/minter/tests/cketh.rs @@ -93,6 +93,7 @@ fn should_deposit_and_withdraw() { log_index: DEFAULT_DEPOSIT_LOG_INDEX.into(), })), created_at_time: None, + fee: None, }) .call_ledger_approve_minter(account.owner, EXPECTED_BALANCE, account.subaccount) .expect_ok(1) @@ -124,6 +125,7 @@ fn should_deposit_and_withdraw() { to_address: destination.parse().unwrap(), })), created_at_time: None, + fee: None, }); assert_eq!(cketh.balance_of(account), Nat::from(0_u8)); @@ -454,6 +456,7 @@ fn should_reimburse() { log_index: DEFAULT_DEPOSIT_LOG_INDEX.into(), })), created_at_time: None, + fee: None, }) .call_ledger_approve_minter(caller, EXPECTED_BALANCE, None) .expect_ok(1); @@ -493,6 +496,7 @@ fn should_reimburse() { to_address: destination.parse().unwrap(), })), created_at_time: None, + fee: None, }); assert_eq!(cketh.balance_of(caller), Nat::from(0_u8)); @@ -543,6 +547,7 @@ fn should_reimburse() { tx_hash: failed_tx_hash.parse().unwrap(), })), created_at_time: None, + fee: None, }) .assert_has_unique_events_in_order(&vec![ EventPayload::AcceptedEthWithdrawalRequest { From a2f3b2e96467c6448d77cb6c790672cbf5d8abc1 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 10 Oct 2025 09:10:23 +0000 Subject: [PATCH 40/67] fix did files --- rs/ledger_suite/icrc1/archive/archive.did | 6 ++++-- rs/ledger_suite/icrc1/index-ng/index-ng.did | 6 ++++-- rs/ledger_suite/icrc1/ledger/ledger.did | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rs/ledger_suite/icrc1/archive/archive.did b/rs/ledger_suite/icrc1/archive/archive.did index 3aad2cb11fcd..4eb4323600db 100644 --- a/rs/ledger_suite/icrc1/archive/archive.did +++ b/rs/ledger_suite/icrc1/archive/archive.did @@ -26,14 +26,16 @@ type Burn = record { memo : opt vec nat8; created_at_time : opt nat64; amount : nat; - spender : opt Account + spender : opt Account; + fee : opt nat }; type Mint = record { to : Account; memo : opt vec nat8; created_at_time : opt nat64; - amount : nat + amount : nat; + fee : opt nat }; type Transfer = record { diff --git a/rs/ledger_suite/icrc1/index-ng/index-ng.did b/rs/ledger_suite/icrc1/index-ng/index-ng.did index 0f54f6bcc642..d884ae9641af 100644 --- a/rs/ledger_suite/icrc1/index-ng/index-ng.did +++ b/rs/ledger_suite/icrc1/index-ng/index-ng.did @@ -76,14 +76,16 @@ type Burn = record { memo : opt vec nat8; created_at_time : opt nat64; amount : Tokens; - spender : opt Account + spender : opt Account; + fee : opt nat }; type Mint = record { to : Account; memo : opt vec nat8; created_at_time : opt nat64; - amount : Tokens + amount : Tokens; + fee : opt nat }; type Transfer = record { diff --git a/rs/ledger_suite/icrc1/ledger/ledger.did b/rs/ledger_suite/icrc1/ledger/ledger.did index b811c633d549..b093ec9bfada 100644 --- a/rs/ledger_suite/icrc1/ledger/ledger.did +++ b/rs/ledger_suite/icrc1/ledger/ledger.did @@ -233,14 +233,16 @@ type Burn = record { memo : opt blob; created_at_time : opt Timestamp; amount : nat; - spender : opt Account + spender : opt Account; + fee : opt nat }; type Mint = record { to : Account; memo : opt blob; created_at_time : opt Timestamp; - amount : nat + amount : nat; + fee : opt nat }; type Transfer = record { From 88720cd45a50061859e1b45077354c6694b7fef5 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 10 Oct 2025 09:56:45 +0000 Subject: [PATCH 41/67] fix operation index --- rs/rosetta-api/icrc1/src/common/utils/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/rosetta-api/icrc1/src/common/utils/utils.rs b/rs/rosetta-api/icrc1/src/common/utils/utils.rs index 99868e6bd11f..e522537b919e 100644 --- a/rs/rosetta-api/icrc1/src/common/utils/utils.rs +++ b/rs/rosetta-api/icrc1/src/common/utils/utils.rs @@ -443,7 +443,7 @@ pub fn icrc1_operation_to_rosetta_core_operations( None, )); - let mut idx = 2; + let mut idx = 1; if let Some(spender) = spender { operations.push(rosetta_core::objects::Operation::new( From fc4097f0a933dff6f781170df05007f11cb23e94 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 10 Oct 2025 14:31:37 +0000 Subject: [PATCH 42/67] test rosetta fee handling --- rs/rosetta-api/icrc1/src/data_api/services.rs | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/rs/rosetta-api/icrc1/src/data_api/services.rs b/rs/rosetta-api/icrc1/src/data_api/services.rs index 6f00059ec273..d622f755177e 100644 --- a/rs/rosetta-api/icrc1/src/data_api/services.rs +++ b/rs/rosetta-api/icrc1/src/data_api/services.rs @@ -2249,4 +2249,122 @@ mod test { ); } } + + #[test] + fn test_mint_and_burn_fees() { + use crate::common::storage::types::{ + IcrcBlock, IcrcOperation, IcrcTransaction, RosettaBlock, + }; + use candid::{Nat, Principal}; + use icrc_ledger_types::icrc1::account::Account; + use rosetta_core::identifiers::AccountIdentifier; + + let storage_client = StorageClient::new_in_memory().unwrap(); + let symbol = "ICP"; + let decimals = 8; + + let principal = Principal::anonymous(); + + // First, add some blocks to the database so we can test the validation logic + let main_account = Account { + owner: principal, + subaccount: None, + }; + let main_account_id = AccountIdentifier::from(main_account); + + let add_mint_block = + |block_id: u64, amount: u64, fee: Option, effective_fee: Option| { + let blocks = vec![RosettaBlock::from_icrc_ledger_block( + IcrcBlock { + parent_hash: None, + transaction: IcrcTransaction { + operation: IcrcOperation::Mint { + to: main_account, + amount: Nat::from(amount), + fee: fee.map(Into::into), + }, + created_at_time: None, + memo: None, + }, + effective_fee: effective_fee.map(Into::into), + timestamp: 1, + fee_collector: None, + fee_collector_block_index: None, + }, + block_id, + )]; + + storage_client.store_blocks(blocks).unwrap(); + storage_client.update_account_balances().unwrap(); + }; + + let add_burn_block = + |block_id: u64, amount: u64, fee: Option, effective_fee: Option| { + let blocks = vec![RosettaBlock::from_icrc_ledger_block( + IcrcBlock { + parent_hash: None, + transaction: IcrcTransaction { + operation: IcrcOperation::Burn { + from: main_account, + amount: Nat::from(amount), + fee: fee.map(Into::into), + spender: None, + }, + created_at_time: None, + memo: None, + }, + effective_fee: effective_fee.map(Into::into), + timestamp: 1, + fee_collector: None, + fee_collector_block_index: None, + }, + block_id, + )]; + + storage_client.store_blocks(blocks).unwrap(); + storage_client.update_account_balances().unwrap(); + }; + + let check_account_balance = |expected_balance: &str| { + let result = account_balance( + &storage_client, + &main_account_id, + &None, + decimals, + symbol.to_string(), + ); + + assert!(result.is_ok()); + let balance_response = result.unwrap(); + assert_eq!(balance_response.balances.len(), 1); + assert_eq!( + balance_response.balances[0].value.to_string(), + expected_balance + ); + }; + + // The operation fee of 100 is applied + add_mint_block(0, 1000, Some(100), None); + check_account_balance("900"); + add_burn_block(1, 100, Some(100), None); + check_account_balance("700"); + + // The block effective_fee of 100 is applied + add_mint_block(2, 200, Some(200), Some(100)); + check_account_balance("800"); + add_burn_block(3, 200, Some(200), Some(100)); + check_account_balance("500"); + + // The block effective_fee of 100 is applied + add_mint_block(4, 200, None, Some(100)); + check_account_balance("600"); + add_burn_block(5, 200, None, Some(100)); + check_account_balance("300"); + + // No fee + add_mint_block(6, 200, None, None); + check_account_balance("500"); + add_burn_block(7, 200, None, None); + check_account_balance("300"); + } } From a43043a8440864fd9871280be206a565c9af44cb Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 07:10:34 +0000 Subject: [PATCH 43/67] add fee to mint and burn in some strategies --- rs/ledger_suite/icrc1/test_utils/src/lib.rs | 54 ++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/rs/ledger_suite/icrc1/test_utils/src/lib.rs b/rs/ledger_suite/icrc1/test_utils/src/lib.rs index b3fef16dabb1..681a0dee4450 100644 --- a/rs/ledger_suite/icrc1/test_utils/src/lib.rs +++ b/rs/ledger_suite/icrc1/test_utils/src/lib.rs @@ -91,18 +91,26 @@ fn operation_strategy( amount_strategy.prop_flat_map(|amount| { // Clone amount due to move let mint_amount = amount.clone(); - let mint_strategy = account_strategy().prop_map(move |to| Operation::Mint { - to, - amount: mint_amount.clone(), - fee: None, - }); + let mint_strategy = ( + account_strategy(), + prop::option::of(Just(token_amount(DEFAULT_TRANSFER_FEE))), + ) + .prop_map(move |(to, fee)| Operation::Mint { + to, + amount: mint_amount.clone(), + fee, + }); let burn_amount = amount.clone(); - let burn_strategy = account_strategy().prop_map(move |from| Operation::Burn { - from, - spender: None, - amount: burn_amount.clone(), - fee: None, - }); + let burn_strategy = ( + account_strategy(), + prop::option::of(Just(token_amount(DEFAULT_TRANSFER_FEE))), + ) + .prop_map(move |(from, fee)| Operation::Burn { + from, + spender: None, + amount: burn_amount.clone(), + fee, + }); let transfer_amount = amount.clone(); let transfer_strategy = ( account_strategy(), @@ -213,8 +221,8 @@ pub fn blocks_strategy( let effective_fee = match transaction.operation { Operation::Transfer { ref fee, .. } => fee.clone().is_none().then_some(arb_fee), Operation::Approve { ref fee, .. } => fee.clone().is_none().then_some(arb_fee), - Operation::Burn { .. } => None, - Operation::Mint { .. } => None, + Operation::Burn { ref fee, .. } => fee.clone().is_none().then_some(arb_fee), + Operation::Mint { ref fee, .. } => fee.clone().is_none().then_some(arb_fee), }; Block { @@ -420,14 +428,14 @@ impl ArgWithCaller { Operation::Mint { amount: T::try_from(transfer_arg.amount.clone()).unwrap(), to: transfer_arg.to, - fee: None, + fee: transfer_arg.fee.clone().map(|f| T::try_from(f).unwrap()), } } else if burn_operation { Operation::Burn { amount: T::try_from(transfer_arg.amount.clone()).unwrap(), from: caller, spender: None, - fee: None, + fee: transfer_arg.fee.clone().map(|f| T::try_from(f).unwrap()), } } else { Operation::Transfer { @@ -1400,11 +1408,12 @@ where Tokens: TokensType, S: Strategy, { - (arb_account(), arb_tokens()).prop_map(|(to, amount)| Operation::Mint { - to, - amount, - fee: None, - }) + ( + arb_account(), + arb_tokens(), + proptest::option::of(arb_tokens()), + ) + .prop_map(|(to, amount, fee)| Operation::Mint { to, amount, fee }) } pub fn arb_burn(arb_tokens: fn() -> S) -> impl Strategy> @@ -1416,12 +1425,13 @@ where arb_account(), proptest::option::of(arb_account()), arb_tokens(), + proptest::option::of(arb_tokens()), ) - .prop_map(|(from, spender, amount)| Operation::Burn { + .prop_map(|(from, spender, amount, fee)| Operation::Burn { from, spender, amount, - fee: None, + fee, }) } From d40113e467b3787ff16ea6eaa4be39a7a124eab7 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 08:33:36 +0000 Subject: [PATCH 44/67] add fee to other strategies --- rs/ledger_suite/icrc1/test_utils/src/lib.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/rs/ledger_suite/icrc1/test_utils/src/lib.rs b/rs/ledger_suite/icrc1/test_utils/src/lib.rs index 681a0dee4450..7dbd587f4508 100644 --- a/rs/ledger_suite/icrc1/test_utils/src/lib.rs +++ b/rs/ledger_suite/icrc1/test_utils/src/lib.rs @@ -764,6 +764,7 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( /// - Avoid duplicate transactions and minting to the minter. fn mint_strategy( minter_identity: Arc, + default_fee: u64, now: SystemTime, tx_hash_set_pointer: Arc>>, ) -> impl Strategy { @@ -773,16 +774,17 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( amount_strategy(), valid_created_at_time_strategy(now), arb_memo(), + prop::option::of(Just(default_fee)), ) .prop_filter_map( "The minting account is set as to account or tx is a duplicate", - move |(to_signer, amount, created_at_time, memo)| { + move |(to_signer, amount, created_at_time, memo, fee)| { let to = to_signer.account(); let tx = Transaction { operation: Operation::Mint:: { amount: Tokens::from_e8s(amount), to, - fee: None, + fee: fee.map(Tokens::from_e8s), }, created_at_time, memo: memo.clone(), @@ -836,16 +838,17 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( default_fee..=balance, valid_created_at_time_strategy(now), arb_memo(), + prop::option::of(Just(default_fee)), ) .prop_filter_map( "Tx hash already exists", - move |(amount, created_at_time, memo)| { + move |(amount, created_at_time, memo, fee)| { let tx = Transaction { operation: Operation::Burn:: { amount: Tokens::from_e8s(amount), from, spender: None, - fee: None, + fee: fee.map(Tokens::from_e8s), }, created_at_time, memo: memo.clone(), @@ -1221,8 +1224,13 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( let tx_hashes_pointer = Arc::new(state.txs.clone()); let account_to_basic_identity_pointer = Arc::new(state.principal_to_basic_identity.clone()); let allowance_map_pointer = Arc::new(state.allowances.clone()); - let mint_strategy = - mint_strategy(minter_identity.clone(), now, tx_hashes_pointer.clone()).boxed(); + let mint_strategy = mint_strategy( + minter_identity.clone(), + default_fee, + now, + tx_hashes_pointer.clone(), + ) + .boxed(); let arb_tx = if balances.is_empty() { mint_strategy } else { From 861ee317fbc2e840d3b2c0c366a4f2bef1c9bf53 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 09:01:58 +0000 Subject: [PATCH 45/67] add mint and burn fees to schema --- rs/ledger_suite/icrc1/ledger/block.cddl | 2 ++ rs/ledger_suite/tests/sm-tests/src/lib.rs | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/rs/ledger_suite/icrc1/ledger/block.cddl b/rs/ledger_suite/icrc1/ledger/block.cddl index 6123c8f09c40..2a1df8a1b56d 100644 --- a/rs/ledger_suite/icrc1/ledger/block.cddl +++ b/rs/ledger_suite/icrc1/ledger/block.cddl @@ -26,6 +26,7 @@ BlockContent = { MintTx = ( op: "mint", to: Account, + ? fee: Amount, TxCommon ) @@ -33,6 +34,7 @@ BurnTx = ( op: "burn", from: Account, ? spender: Account, + ? fee: Amount, TxCommon ) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 90abca811581..59efc663cc96 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -286,11 +286,12 @@ fn arb_approve() -> impl Strategy> } fn arb_mint() -> impl Strategy> { - (arb_account(), arb_amount()).prop_map(|(to, amount)| Operation::Mint { - to, - amount, - fee: None, - }) + ( + arb_account(), + arb_amount(), + proptest::option::of(arb_amount()), + ) + .prop_map(|(to, amount, fee)| Operation::Mint { to, amount, fee }) } fn arb_burn() -> impl Strategy> { @@ -298,12 +299,13 @@ fn arb_burn() -> impl Strategy> { arb_account(), proptest::option::of(arb_account()), arb_amount(), + proptest::option::of(arb_amount()), ) - .prop_map(|(from, spender, amount)| Operation::Burn { + .prop_map(|(from, spender, amount, fee)| Operation::Burn { from, spender, amount, - fee: None, + fee, }) } From 7403aa0f44e3daa9f1af4d9b06e31399d6fb6c07 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 12:44:24 +0000 Subject: [PATCH 46/67] test mint burn fee rejected by ledgers --- 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 | 106 ++++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/rs/ledger_suite/icp/ledger/tests/tests.rs b/rs/ledger_suite/icp/ledger/tests/tests.rs index df49528876d7..0967374de272 100644 --- a/rs/ledger_suite/icp/ledger/tests/tests.rs +++ b/rs/ledger_suite/icp/ledger/tests/tests.rs @@ -462,6 +462,14 @@ fn test_mint_burn() { ic_ledger_suite_state_machine_tests::test_mint_burn(ledger_wasm(), encode_init_args); } +#[test] +fn test_mint_burn_fee_rejected() { + ic_ledger_suite_state_machine_tests::test_mint_burn_fee_rejected( + ledger_wasm(), + encode_init_args, + ); +} + #[test] fn test_anonymous_transfers() { ic_ledger_suite_state_machine_tests::test_anonymous_transfers(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 6254d67cdb00..65b3bf5d9c91 100644 --- a/rs/ledger_suite/icrc1/ledger/tests/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/tests/tests.rs @@ -314,6 +314,14 @@ fn test_mint_burn() { ic_ledger_suite_state_machine_tests::test_mint_burn(ledger_wasm(), encode_init_args); } +#[test] +fn test_mint_burn_fee_rejected() { + ic_ledger_suite_state_machine_tests::test_mint_burn_fee_rejected( + ledger_wasm(), + encode_init_args, + ); +} + #[test] fn test_anonymous_transfers() { ic_ledger_suite_state_machine_tests::test_anonymous_transfers(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 59efc663cc96..60ce7343d6d0 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -970,6 +970,112 @@ where ); } +pub fn test_mint_burn_fee_rejected(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T) +where + T: CandidType, +{ + let (env, canister_id) = setup(ledger_wasm, encode_init_args, vec![]); + let p1 = PrincipalId::new_user_test_id(1); + let p2 = PrincipalId::new_user_test_id(2); + + assert_eq!(0, total_supply(&env, canister_id)); + assert_eq!(0, balance_of(&env, canister_id, p1.0)); + assert_eq!(0, balance_of(&env, canister_id, MINTER)); + + const INITIAL_BALANCE: u64 = 10_000_000; + const TX_AMOUNT: u64 = 1_000_000; + + let mint_error = send_transfer( + &env, + canister_id, + MINTER.owner, + &TransferArg { + from_subaccount: None, + to: p1.0.into(), + fee: Some(FEE.into()), + created_at_time: None, + amount: Nat::from(INITIAL_BALANCE), + memo: None, + }, + ) + .unwrap_err(); + assert_eq!( + mint_error, + TransferError::BadFee { + expected_fee: Nat::from(0u64) + } + ); + + transfer(&env, canister_id, MINTER, p1.0, INITIAL_BALANCE).expect("mint failed"); + + let mut expected_balance = INITIAL_BALANCE; + + assert_eq!(expected_balance, total_supply(&env, canister_id)); + assert_eq!(expected_balance, balance_of(&env, canister_id, p1.0)); + assert_eq!(0, balance_of(&env, canister_id, MINTER)); + + let burn_error = send_transfer( + &env, + canister_id, + p1.0, + &TransferArg { + from_subaccount: None, + to: MINTER, + fee: Some(FEE.into()), + created_at_time: None, + amount: Nat::from(TX_AMOUNT), + memo: None, + }, + ) + .unwrap_err(); + assert_eq!( + burn_error, + TransferError::BadFee { + expected_fee: Nat::from(0u64) + } + ); + + transfer(&env, canister_id, p1.0, MINTER, TX_AMOUNT).expect("burn failed"); + + expected_balance -= TX_AMOUNT; + + assert_eq!(expected_balance, total_supply(&env, canister_id)); + assert_eq!(expected_balance, balance_of(&env, canister_id, p1.0)); + assert_eq!(0, balance_of(&env, canister_id, MINTER)); + + let approve_args = default_approve_args(p2.0, u64::MAX); + send_approval(&env, canister_id, p1.into(), &approve_args).expect("approval failed"); + + expected_balance -= FEE; + + let mut transfer_from_args = TransferFromArgs { + from: p1.0.into(), + to: MINTER, + fee: Some(FEE.into()), + created_at_time: None, + amount: Nat::from(TX_AMOUNT), + memo: None, + spender_subaccount: None, + }; + let burn_from_error = + send_transfer_from(&env, canister_id, p2.0, &transfer_from_args).unwrap_err(); + assert_eq!( + burn_from_error, + TransferFromError::BadFee { + expected_fee: Nat::from(0u64) + } + ); + + transfer_from_args.fee = None; + send_transfer_from(&env, canister_id, p2.0, &transfer_from_args).expect("transfer from failed"); + + expected_balance -= TX_AMOUNT; + + assert_eq!(expected_balance, total_supply(&env, canister_id)); + assert_eq!(expected_balance, balance_of(&env, canister_id, p1.0)); + assert_eq!(0, balance_of(&env, canister_id, MINTER)); +} + pub fn test_account_canonicalization(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T) where T: CandidType, From 5987385f531f5476f39f7efb1f01935e0e933647 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 14:07:09 +0000 Subject: [PATCH 47/67] rename tests --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 65049cd7b86c..0e6431eb2bba 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -541,7 +541,7 @@ fn verify_unknown_block_handling( } #[test] -fn test_ledger_unknown_block_icrc3() { +fn test_unknown_block_icrc3() { for bad_block_index in 0..NUM_BLOCKS + 1 { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); @@ -552,7 +552,7 @@ fn test_ledger_unknown_block_icrc3() { } #[test] -fn test_ledger_unknown_block_legacy() { +fn test_unknown_block_legacy() { for bad_block_index in 0..NUM_BLOCKS + 1 { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); From 7369c7f71409bb134eb200aa176bb3fa5987f1b5 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Mon, 13 Oct 2025 14:21:31 +0000 Subject: [PATCH 48/67] burn and mint fee unit tests --- rs/ledger_suite/icrc1/ledger/src/tests.rs | 51 +++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/rs/ledger_suite/icrc1/ledger/src/tests.rs b/rs/ledger_suite/icrc1/ledger/src/tests.rs index 0ab4f71a8eae..28bd3d8469e4 100644 --- a/rs/ledger_suite/icrc1/ledger/src/tests.rs +++ b/rs/ledger_suite/icrc1/ledger/src/tests.rs @@ -675,3 +675,54 @@ fn arb_allowance() -> impl Strategy> { }, ) } + +#[test] +fn test_burn_fee_error() { + let now = ts(1); + + let mut ctx = Ledger::from_init_args(DummyLogger, default_init_args(), now); + + let from = test_account_id(1); + + ctx.balances_mut().mint(&from, tokens(200_000)).unwrap(); + + assert_eq!(tokens_to_u64(ctx.balances().total_supply()), 200_000); + + let tr = Transaction { + operation: Operation::Burn { + from, + spender: None, + amount: tokens(1_000), + fee: Some(tokens(10_000)), + }, + created_at_time: None, + memo: None, + }; + assert_eq!( + tr.apply(&mut ctx, now, Tokens::ZERO).unwrap_err(), + TxApplyError::BurnOrMintFee + ); +} + +#[test] +fn test_mint_fee_error() { + let now = ts(1); + + let mut ctx = Ledger::from_init_args(DummyLogger, default_init_args(), now); + + let to = test_account_id(1); + + let tr = Transaction { + operation: Operation::Mint { + to, + amount: tokens(1_000), + fee: Some(tokens(10_000)), + }, + created_at_time: None, + memo: None, + }; + assert_eq!( + tr.apply(&mut ctx, now, Tokens::ZERO).unwrap_err(), + TxApplyError::BurnOrMintFee + ); +} From 8b1b57b3f611074aecaa759fc84875fdfa20ac71 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 14 Oct 2025 10:39:09 +0000 Subject: [PATCH 49/67] move sync status to status --- rs/ledger_suite/icrc1/index-ng/index-ng.did | 12 ++++--- rs/ledger_suite/icrc1/index-ng/src/lib.rs | 12 +++++-- rs/ledger_suite/icrc1/index-ng/src/main.rs | 24 ++++++++------ rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 32 +++++++++---------- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/index-ng.did b/rs/ledger_suite/icrc1/index-ng/index-ng.did index d884ae9641af..80fde4e3f900 100644 --- a/rs/ledger_suite/icrc1/index-ng/index-ng.did +++ b/rs/ledger_suite/icrc1/index-ng/index-ng.did @@ -136,12 +136,15 @@ type ListSubaccountsArgs = record { }; type Status = record { - num_blocks_synced : BlockIndex + num_blocks_synced : BlockIndex; + sync_status : SyncStatus }; -type SyncStatus = record { - sync_active : bool; - sync_error : opt text +type SyncStatus = variant { + Syncing; + NotSyncing : record { + error_msg : text + } }; type FeeCollectorRanges = record { @@ -156,5 +159,4 @@ service : (index_arg : opt IndexArg) -> { ledger_id : () -> (principal) query; list_subaccounts : (ListSubaccountsArgs) -> (vec SubAccount) query; status : () -> (Status) query; - sync_status : () -> (SyncStatus) query } diff --git a/rs/ledger_suite/icrc1/index-ng/src/lib.rs b/rs/ledger_suite/icrc1/index-ng/src/lib.rs index 5f5d669e9b9f..85097257cbd4 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/lib.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/lib.rs @@ -81,12 +81,18 @@ pub struct ListSubaccountsArgs { #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] pub struct Status { pub num_blocks_synced: BlockIndex, + pub sync_status: SyncStatus, } #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] -pub struct SyncStatus { - pub sync_active: bool, - pub sync_error: Option, +pub struct SyncError { + pub error_msg: String, +} + +#[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] +pub enum SyncStatus { + Syncing, + NotSyncing(SyncError), } #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 7fc482af0220..71e80d2ea0bd 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -13,7 +13,8 @@ use ic_icrc1::{Block, Operation}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksMethod, IndexArg, - InitArg, ListSubaccountsArgs, Log, LogEntry, Status, SyncStatus, TransactionWithId, UpgradeArg, + InitArg, ListSubaccountsArgs, Log, LogEntry, Status, SyncError, SyncStatus, TransactionWithId, + UpgradeArg, }; use ic_ledger_canister_core::runtime::heap_memory_size_bytes; use ic_ledger_core::block::{BlockIndex as BlockIndex64, BlockType, EncodedBlock}; @@ -1120,21 +1121,24 @@ fn icrc1_balance_of(account: Account) -> Nat { #[query] fn status() -> Status { let num_blocks_synced = with_blocks(|blocks| blocks.len().into()); - Status { num_blocks_synced } + let sync_status = if sync_active() { + SyncStatus::Syncing + } else { + let error_msg = SYNC_ERROR + .with(|error| error.borrow().clone()) + .unwrap_or_default(); + SyncStatus::NotSyncing(SyncError { error_msg }) + }; + Status { + num_blocks_synced, + sync_status, + } } fn sync_active() -> bool { TIMER_ID.with(|tid| *tid.borrow()).is_some() } -#[query] -fn sync_status() -> SyncStatus { - SyncStatus { - sync_active: sync_active(), - sync_error: SYNC_ERROR.with(|error| error.borrow().clone()), - } -} - #[query] fn list_subaccounts(args: ListSubaccountsArgs) -> Vec { let start_key = balance_key(Account { diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 0e6431eb2bba..75d3e6fbe8aa 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -9,7 +9,7 @@ use ic_base_types::{CanisterId, PrincipalId}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksResponse, IndexArg, - InitArg as IndexInitArg, ListSubaccountsArgs, SyncStatus, TransactionWithId, + InitArg as IndexInitArg, ListSubaccountsArgs, Status, SyncStatus, TransactionWithId, }; use ic_icrc1_ledger::{ ChangeFeeCollector, LedgerArgument, Tokens, UpgradeArgs as LedgerUpgradeArgs, @@ -87,12 +87,12 @@ fn icrc1_balance_of(env: &StateMachine, canister_id: CanisterId, account: Accoun .expect("Balance must be a u64!") } -fn sync_status(env: &StateMachine, canister_id: CanisterId) -> SyncStatus { +fn status(env: &StateMachine, canister_id: CanisterId) -> Status { let res = env - .execute_ingress(canister_id, "sync_status", Encode!(&()).unwrap()) - .expect("Failed to query sync_status") + .execute_ingress(canister_id, "status", Encode!(&()).unwrap()) + .expect("Failed to query index status") .bytes(); - Decode!(&res, SyncStatus).expect("Failed to decode sync_status response") + Decode!(&res, Status).expect("Failed to decode index status response") } fn index_get_blocks( @@ -527,17 +527,17 @@ fn verify_unknown_block_handling( bad_block_index ); - let status = sync_status(env, index_id); - if bad_block_index < NUM_BLOCKS { - assert!(!status.sync_active); - assert!(status.sync_error.unwrap().starts_with(&format!( - "Block at index {} has unknown fields.", - bad_block_index - ))); - } else { - assert!(status.sync_active); - assert_eq!(status.sync_error, None); - } + let status = status(env, index_id); + match status.sync_status { + SyncStatus::Syncing => assert!(bad_block_index >= NUM_BLOCKS), + SyncStatus::NotSyncing(sync_error) => { + assert!(bad_block_index < NUM_BLOCKS); + assert!(sync_error.error_msg.starts_with(&format!( + "Block at index {} has unknown fields.", + bad_block_index + ))); + } + }; } #[test] From cf49e9c543d584618194dca80f9886dbd5889971 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 14 Oct 2025 10:43:40 +0000 Subject: [PATCH 50/67] test comment --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 75d3e6fbe8aa..d09209865f01 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -467,6 +467,11 @@ fn test_ledger_growing() { ); } +// With 6 blocks we can store 2 blocks in 2 archives and the ledger each. +// This way we can test all possible locations for the unknown block: +// - ledger/archive +// - last/not-last archive +// - last block/not last block in the archive/ledger const NUM_BLOCKS: u64 = 6; fn verify_unknown_block_handling( From a9d5828a2af3d2a17e20834dc2d427a311d314d6 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 14 Oct 2025 11:11:00 +0000 Subject: [PATCH 51/67] buildifier fix --- .../integration_tests/src/pocket_ic_helpers.rs | 12 ++++++++++++ .../ic_xc_ledger_suite_orchestrator_test.rs | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index eebbf45796b3..a9ea8db55a7d 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -2088,6 +2088,18 @@ pub mod sns { #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] pub struct Status { pub num_blocks_synced: BlockIndex, + pub sync_status: SyncStatus, + } + + #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] + pub struct SyncError { + pub error_msg: String, + } + + #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] + pub enum SyncStatus { + Syncing, + NotSyncing(SyncError), } pub async fn ledger_id(pocket_ic: &PocketIc, canister_id: PrincipalId) -> PrincipalId { diff --git a/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs b/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs index 8cfd017d4d71..64bda7aea741 100644 --- a/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs +++ b/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs @@ -163,7 +163,8 @@ fn ic_xc_ledger_suite_orchestrator_test(env: TestEnv) { assert_eq!( index_status, ic_icrc1_index_ng::Status { - num_blocks_synced: Nat::from(0_u8) + num_blocks_synced: Nat::from(0_u8), + sync_status: ic_icrc1_index_ng::SyncStatus::Syncing, } ); }); From 716937aaf10610c85651f41f9b31589c9aa071ac Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 10:55:17 +0000 Subject: [PATCH 52/67] remove sync_status, check /logs for error --- rs/ledger_suite/icrc1/index-ng/src/lib.rs | 12 ------- rs/ledger_suite/icrc1/index-ng/src/main.rs | 16 ++------- .../icrc1/index-ng/tests/common/mod.rs | 2 +- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 33 ++++++++----------- 4 files changed, 16 insertions(+), 47 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/lib.rs b/rs/ledger_suite/icrc1/index-ng/src/lib.rs index 85097257cbd4..829790f55535 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/lib.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/lib.rs @@ -81,18 +81,6 @@ pub struct ListSubaccountsArgs { #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] pub struct Status { pub num_blocks_synced: BlockIndex, - pub sync_status: SyncStatus, -} - -#[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] -pub struct SyncError { - pub error_msg: String, -} - -#[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] -pub enum SyncStatus { - Syncing, - NotSyncing(SyncError), } #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index 71e80d2ea0bd..fbd5a6e85801 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -13,8 +13,7 @@ use ic_icrc1::{Block, Operation}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksMethod, IndexArg, - InitArg, ListSubaccountsArgs, Log, LogEntry, Status, SyncError, SyncStatus, TransactionWithId, - UpgradeArg, + InitArg, ListSubaccountsArgs, Log, LogEntry, Status, TransactionWithId, UpgradeArg, }; use ic_ledger_canister_core::runtime::heap_memory_size_bytes; use ic_ledger_core::block::{BlockIndex as BlockIndex64, BlockType, EncodedBlock}; @@ -1121,18 +1120,7 @@ fn icrc1_balance_of(account: Account) -> Nat { #[query] fn status() -> Status { let num_blocks_synced = with_blocks(|blocks| blocks.len().into()); - let sync_status = if sync_active() { - SyncStatus::Syncing - } else { - let error_msg = SYNC_ERROR - .with(|error| error.borrow().clone()) - .unwrap_or_default(); - SyncStatus::NotSyncing(SyncError { error_msg }) - }; - Status { - num_blocks_synced, - sync_status, - } + Status { num_blocks_synced } } fn sync_active() -> bool { diff --git a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs index ab4427b11f90..97c9843a62f2 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs @@ -299,7 +299,7 @@ fn assert_reply(result: WasmResult) -> Vec { } } -fn get_logs(env: &StateMachine, index_id: CanisterId) -> Log { +pub fn get_logs(env: &StateMachine, index_id: CanisterId) -> Log { let request = HttpRequest { method: "".to_string(), url: "/logs".to_string(), diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index d09209865f01..e236c2c688ce 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -1,6 +1,6 @@ use crate::common::{ ARCHIVE_TRIGGER_THRESHOLD, FEE, MAX_BLOCKS_FROM_ARCHIVE, account, default_archive_options, - index_ng_wasm, install_icrc3_test_ledger, install_index_ng, install_ledger, + get_logs, index_ng_wasm, install_icrc3_test_ledger, install_index_ng, install_ledger, ledger_get_all_blocks, ledger_wasm, wait_until_sync_is_completed, }; use candid::{Decode, Encode, Nat, Principal}; @@ -9,7 +9,7 @@ use ic_base_types::{CanisterId, PrincipalId}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksResponse, IndexArg, - InitArg as IndexInitArg, ListSubaccountsArgs, Status, SyncStatus, TransactionWithId, + InitArg as IndexInitArg, ListSubaccountsArgs, Status, TransactionWithId, }; use ic_icrc1_ledger::{ ChangeFeeCollector, LedgerArgument, Tokens, UpgradeArgs as LedgerUpgradeArgs, @@ -87,14 +87,6 @@ fn icrc1_balance_of(env: &StateMachine, canister_id: CanisterId, account: Accoun .expect("Balance must be a u64!") } -fn status(env: &StateMachine, canister_id: CanisterId) -> Status { - let res = env - .execute_ingress(canister_id, "status", Encode!(&()).unwrap()) - .expect("Failed to query index status") - .bytes(); - Decode!(&res, Status).expect("Failed to decode index status response") -} - fn index_get_blocks( env: &StateMachine, index_id: CanisterId, @@ -532,17 +524,18 @@ fn verify_unknown_block_handling( bad_block_index ); - let status = status(env, index_id); - match status.sync_status { - SyncStatus::Syncing => assert!(bad_block_index >= NUM_BLOCKS), - SyncStatus::NotSyncing(sync_error) => { - assert!(bad_block_index < NUM_BLOCKS); - assert!(sync_error.error_msg.starts_with(&format!( - "Block at index {} has unknown fields.", - bad_block_index - ))); + let logs = get_logs(env, index_id); + let mut found_error = false; + for entry in logs.entries { + found_error = entry.message.contains(&format!( + "Block at index {} has unknown fields.", + bad_block_index + )); + if found_error { + break; } - }; + } + assert_eq!(found_error, bad_block_index < NUM_BLOCKS); } #[test] From 26ceb2d299f32d309f5883749ebfaf817777df87 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 11:07:09 +0000 Subject: [PATCH 53/67] build fix --- .../integration_tests/src/pocket_ic_helpers.rs | 12 ------------ .../ic_xc_ledger_suite_orchestrator_test.rs | 3 +-- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index a9ea8db55a7d..eebbf45796b3 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -2088,18 +2088,6 @@ pub mod sns { #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] pub struct Status { pub num_blocks_synced: BlockIndex, - pub sync_status: SyncStatus, - } - - #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] - pub struct SyncError { - pub error_msg: String, - } - - #[derive(Eq, PartialEq, Debug, CandidType, Deserialize)] - pub enum SyncStatus { - Syncing, - NotSyncing(SyncError), } pub async fn ledger_id(pocket_ic: &PocketIc, canister_id: PrincipalId) -> PrincipalId { diff --git a/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs b/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs index 64bda7aea741..8cfd017d4d71 100644 --- a/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs +++ b/rs/tests/cross_chain/ic_xc_ledger_suite_orchestrator_test.rs @@ -163,8 +163,7 @@ fn ic_xc_ledger_suite_orchestrator_test(env: TestEnv) { assert_eq!( index_status, ic_icrc1_index_ng::Status { - num_blocks_synced: Nat::from(0_u8), - sync_status: ic_icrc1_index_ng::SyncStatus::Syncing, + num_blocks_synced: Nat::from(0_u8) } ); }); From 736f8979114e582299ba81e00138aef815e0211d Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 11:21:30 +0000 Subject: [PATCH 54/67] clippy --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index e236c2c688ce..7f5a157a2df1 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -9,7 +9,7 @@ use ic_base_types::{CanisterId, PrincipalId}; use ic_icrc1_index_ng::{ DEFAULT_MAX_BLOCKS_PER_RESPONSE, FeeCollectorRanges, GetAccountTransactionsArgs, GetAccountTransactionsResponse, GetAccountTransactionsResult, GetBlocksResponse, IndexArg, - InitArg as IndexInitArg, ListSubaccountsArgs, Status, TransactionWithId, + InitArg as IndexInitArg, ListSubaccountsArgs, TransactionWithId, }; use ic_icrc1_ledger::{ ChangeFeeCollector, LedgerArgument, Tokens, UpgradeArgs as LedgerUpgradeArgs, From 1ec96cd62b69e8f91882821aa5b2f007d039315b Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 11:45:00 +0000 Subject: [PATCH 55/67] return error, stop sync in one place --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 169 ++++++++++++--------- 1 file changed, 94 insertions(+), 75 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index fbd5a6e85801..b27066eed4de 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -114,9 +114,6 @@ thread_local! { /// The ID of the block sync timer. `None` means the sync is stopped due to an error. static TIMER_ID: RefCell> = const { RefCell::new(None) }; - - /// The description of the error encountered during block sync. - static SYNC_ERROR: RefCell> = const { RefCell::new(None) }; } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -221,6 +218,11 @@ struct Cache { pub get_blocks_method: Option, } +struct SyncError { + message: String, + retriable: bool, +} + #[test] fn test_account_data_type_storable() { assert_eq!( @@ -450,7 +452,7 @@ where .map_err(|err| format!("failed to candid decode the output: {err}")) } -async fn get_blocks_from_ledger(start: u64) -> Result { +async fn get_blocks_from_ledger(start: u64) -> Result { let (ledger_id, length) = with_state(|state| (state.ledger_id, state.max_blocks_per_response)); let req = GetBlocksRequest { start: Nat::from(start), @@ -468,15 +470,18 @@ async fn get_blocks_from_ledger(start: u64) -> Result { match res { Ok(res) => Ok(res), Err(err) => { - log!(P0, "[get_blocks_from_ledger] failed to get blocks: {}", err); - Err(()) + let message = format!("[get_blocks_from_ledger] failed to get blocks: {}", err); + Err(SyncError { + message, + retriable: true, + }) } } } async fn get_blocks_from_archive( archived: &ArchivedRange, -) -> Result { +) -> Result { let req = GetBlocksRequest { start: archived.start.clone(), length: archived.length.clone(), @@ -492,17 +497,16 @@ async fn get_blocks_from_archive( match res { Ok(res) => Ok(res), Err(err) => { - log!( - P0, - "[get_blocks_from_archive] failed to get blocks: {}", - err - ); - Err(()) + let message = format!("[get_blocks_from_archive] failed to get blocks: {}", err); + Err(SyncError { + message, + retriable: true, + }) } } } -async fn icrc3_get_blocks_from_ledger(start: u64) -> Result { +async fn icrc3_get_blocks_from_ledger(start: u64) -> Result { let (ledger_id, length) = with_state(|state| (state.ledger_id, state.max_blocks_per_response)); let req = vec![GetBlocksRequest { start: Nat::from(start), @@ -520,17 +524,21 @@ async fn icrc3_get_blocks_from_ledger(start: u64) -> Result match res { Ok(res) => Ok(res), Err(err) => { - log!( - P0, + let message = format!( "[icrc3_get_blocks_from_ledger] failed to get blocks: {}", err ); - Err(()) + Err(SyncError { + message, + retriable: true, + }) } } } -async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Result { +async fn icrc3_get_blocks_from_archive( + archived: &ArchivedBlocks, +) -> Result { let res = measured_call( "build_index.icrc3_get_blocks_from_archive.encode", "build_index.icrc3_get_blocks_from_archive.decode", @@ -542,12 +550,14 @@ async fn icrc3_get_blocks_from_archive(archived: &ArchivedBlocks) -> Result Ok(res), Err(err) => { - log!( - P0, + let message = format!( "[icrc3_get_blocks_from_archive] failed to get blocks: {}", err ); - Err(()) + Err(SyncError { + message, + retriable: true, + }) } } } @@ -582,24 +592,34 @@ pub async fn build_index() -> Option<()> { GetBlocksMethod::GetBlocks => fetch_blocks_via_get_blocks().await, GetBlocksMethod::ICRC3GetBlocks => fetch_blocks_via_icrc3().await, }; - if let Ok(num_indexed) = num_indexed { - assert!( - sync_active(), - "Indexing succeeded but the sync timer is stopped." - ); - let retrieve_blocks_from_ledger_interval = - with_state(|state| state.retrieve_blocks_from_ledger_interval()); - log!( - P1, - "Indexed: {} waiting : {:?}", - num_indexed, - retrieve_blocks_from_ledger_interval - ); - } + match num_indexed { + Ok(num_indexed) => { + let retrieve_blocks_from_ledger_interval = + with_state(|state| state.retrieve_blocks_from_ledger_interval()); + log!( + P1, + "Indexed: {} waiting : {:?}", + num_indexed, + retrieve_blocks_from_ledger_interval + ); + } + Err(error) => { + log!(P0, "{}", error.message); + ic_cdk::eprintln!("{}", error.message); + if !error.retriable { + let timer_id = TIMER_ID + .with(|tid| *tid.borrow()) + .expect("Sync is running even though timer id is None"); + ic_cdk_timers::clear_timer(timer_id); + TIMER_ID.with(|tid| *tid.borrow_mut() = None); + } + } + }; + Some(()) } -async fn fetch_blocks_via_get_blocks() -> Result { +async fn fetch_blocks_via_get_blocks() -> Result { let mut num_indexed = 0; let next_id = with_blocks(|blocks| blocks.len()); let res = get_blocks_from_ledger(next_id).await?; @@ -624,7 +644,7 @@ async fn fetch_blocks_via_get_blocks() -> Result { Ok(num_indexed as u64) } -async fn fetch_blocks_via_icrc3() -> Result { +async fn fetch_blocks_via_icrc3() -> Result { // The current number of blocks is also the id of the next // block to query from the Ledger. let previous_num_blocks = with_blocks(|blocks| blocks.len()); @@ -653,8 +673,10 @@ async fn fetch_blocks_via_icrc3() -> Result { "[fetch_blocks_via_icrc3]: wrong start index in archive args. Expected: {} actual: {}", expected_id, arg.start, ); - stop_timer_with_error(error); - return Err(()); + return Err(SyncError { + message: error, + retriable: false, + }); } let archived = ArchivedBlocks { @@ -671,8 +693,10 @@ async fn fetch_blocks_via_icrc3() -> Result { arg.clone(), res.archived_blocks, ); - stop_timer_with_error(error); - return Err(()); + return Err(SyncError { + message: error, + retriable: false, + }); } // change `arg` for the next iteration @@ -703,50 +727,47 @@ fn set_build_index_timer(after: Duration) { TIMER_ID.with(|tid| *tid.borrow_mut() = Some(timer_id)); } -fn stop_timer_with_error(error: String) { - log!(P0, "{}", error); - ic_cdk::eprintln!("{}", error); - if let Some(timer_id) = TIMER_ID.with(|tid| *tid.borrow()) { - ic_cdk_timers::clear_timer(timer_id); - TIMER_ID.with(|tid| *tid.borrow_mut() = None); - } - SYNC_ERROR.with(|sync_error| *sync_error.borrow_mut() = Some(error)); -} - -fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), ()> { +fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), SyncError> { measure_span(&PROFILING_DATA, "append_blocks", move || { - assert!( - sync_active(), - "Trying to append a block, even though the sync is stopped." - ); let original_block = block.clone(); let block = match generic_block_to_encoded_block(block) { Ok(block) => block, Err(e) => { - stop_timer_with_error(format!( - "Unable to decode generic block at index {block_index}. Error: {e}" - )); - return Err(()); + let message = format!( + "Unable to decode generic block at index {block_index}: {}. Error: {e}", + original_block + ); + return Err(SyncError { + message, + retriable: false, + }); } }; let decoded_block = match Block::::decode(block.clone()) { Ok(block) => block, Err(e) => { - stop_timer_with_error(format!( - "Unable to decode encoded block at index {block_index}. Error: {e}" - )); - return Err(()); + let message = format!( + "Unable to decode encoded block at index {block_index}: {}. Error: {e}", + original_block + ); + return Err(SyncError { + message, + retriable: false, + }); } }; let decoded_value = encoded_block_to_generic_block(&decoded_block.clone().encode()); if original_block.hash() != decoded_value.hash() { - stop_timer_with_error(format!( + let message = format!( "Block at index {block_index} has unknown fields. Original block: {}, decoded block: {}.", original_block, decoded_value - )); - return Err(()); + ); + return Err(SyncError { + message, + retriable: false, + }); } // append the encoded block to the block log @@ -773,7 +794,7 @@ fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), () }) } -fn append_blocks(new_blocks: Vec) -> Result<(), ()> { +fn append_blocks(new_blocks: Vec) -> Result<(), SyncError> { // the index of the next block that we // are going to append let mut block_index = with_blocks(|blocks| blocks.len()); @@ -784,7 +805,7 @@ fn append_blocks(new_blocks: Vec) -> Result<(), ()> { Ok(()) } -fn append_icrc3_blocks(new_blocks: Vec) -> Result<(), ()> { +fn append_icrc3_blocks(new_blocks: Vec) -> Result<(), SyncError> { let mut blocks = vec![]; let start_id = with_blocks(|blocks| blocks.len()); for BlockWithId { id, block } in new_blocks { @@ -795,8 +816,10 @@ fn append_icrc3_blocks(new_blocks: Vec) -> Result<(), ()> { "[fetch_blocks_via_icrc3]: wrong block index returned by ledger. Expected: {} actual: {}", expected_id, id ); - stop_timer_with_error(error); - return Err(()); + return Err(SyncError { + message: error, + retriable: false, + }); } // This conversion is safe as `Value` // can represent any `ICRC3Value`. @@ -1123,10 +1146,6 @@ fn status() -> Status { Status { num_blocks_synced } } -fn sync_active() -> bool { - TIMER_ID.with(|tid| *tid.borrow()).is_some() -} - #[query] fn list_subaccounts(args: ListSubaccountsArgs) -> Vec { let start_key = balance_key(Account { From bc1f6f2f64404e3a26c6102c9b41b48753f222c8 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 12:15:04 +0000 Subject: [PATCH 56/67] fix did file --- rs/ledger_suite/icrc1/index-ng/index-ng.did | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/index-ng.did b/rs/ledger_suite/icrc1/index-ng/index-ng.did index 80fde4e3f900..191a0b27cdfd 100644 --- a/rs/ledger_suite/icrc1/index-ng/index-ng.did +++ b/rs/ledger_suite/icrc1/index-ng/index-ng.did @@ -136,15 +136,7 @@ type ListSubaccountsArgs = record { }; type Status = record { - num_blocks_synced : BlockIndex; - sync_status : SyncStatus -}; - -type SyncStatus = variant { - Syncing; - NotSyncing : record { - error_msg : text - } + num_blocks_synced : BlockIndex }; type FeeCollectorRanges = record { From 6c22dcd83d040babe1606d5d20b01a6e241d861f Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 12:16:12 +0000 Subject: [PATCH 57/67] formatting --- rs/ledger_suite/icrc1/index-ng/index-ng.did | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/index-ng.did b/rs/ledger_suite/icrc1/index-ng/index-ng.did index 191a0b27cdfd..0f2351be1f92 100644 --- a/rs/ledger_suite/icrc1/index-ng/index-ng.did +++ b/rs/ledger_suite/icrc1/index-ng/index-ng.did @@ -150,5 +150,5 @@ service : (index_arg : opt IndexArg) -> { icrc1_balance_of : (Account) -> (Tokens) query; ledger_id : () -> (principal) query; list_subaccounts : (ListSubaccountsArgs) -> (vec SubAccount) query; - status : () -> (Status) query; + status : () -> (Status) query } From 890bf68b46225e89dcc5bb87ffb1b2ee17c77f61 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 13:16:53 +0000 Subject: [PATCH 58/67] make timer id non optional --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index b27066eed4de..a829f79f9099 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -112,8 +112,8 @@ thread_local! { /// persistent between upgrades static CACHE: RefCell = RefCell::new(Cache::default()); - /// The ID of the block sync timer. `None` means the sync is stopped due to an error. - static TIMER_ID: RefCell> = const { RefCell::new(None) }; + /// The ID of the block sync timer. + static TIMER_ID: RefCell = RefCell::new(TimerId::default()); } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -607,11 +607,8 @@ pub async fn build_index() -> Option<()> { log!(P0, "{}", error.message); ic_cdk::eprintln!("{}", error.message); if !error.retriable { - let timer_id = TIMER_ID - .with(|tid| *tid.borrow()) - .expect("Sync is running even though timer id is None"); + let timer_id = TIMER_ID.with(|tid| *tid.borrow()); ic_cdk_timers::clear_timer(timer_id); - TIMER_ID.with(|tid| *tid.borrow_mut() = None); } } }; @@ -724,7 +721,7 @@ fn set_build_index_timer(after: Duration) { let _ = build_index().await; }) }); - TIMER_ID.with(|tid| *tid.borrow_mut() = Some(timer_id)); + TIMER_ID.with(|tid| *tid.borrow_mut() = timer_id); } fn append_block(block_index: BlockIndex64, block: GenericBlock) -> Result<(), SyncError> { From a947d717d3c2e0edbcaa8db283704a866dfe3a43 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 13:58:04 +0000 Subject: [PATCH 59/67] test if indexing was stopped --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 7f5a157a2df1..da426a536075 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -511,8 +511,11 @@ fn verify_unknown_block_handling( let archived_count = archive_blocks(env, ledger_id, archive2, 2); assert_eq!(archived_count, 2); - env.advance_time(Duration::from_secs(60)); - env.tick(); + // Advance more than once to make sure the indexing was stopped. + for _ in 0..3 { + env.advance_time(Duration::from_secs(60)); + env.tick(); + } let ledger_blocks = ledger_get_all_blocks(env, ledger_id, 0, u64::MAX); let index_blocks = index_get_all_blocks(env, index_id, 0, u64::MAX); @@ -525,17 +528,21 @@ fn verify_unknown_block_handling( ); let logs = get_logs(env, index_id); - let mut found_error = false; + let mut error_count = 0; for entry in logs.entries { - found_error = entry.message.contains(&format!( + if entry.message.contains(&format!( "Block at index {} has unknown fields.", bad_block_index - )); - if found_error { - break; + )) { + error_count += 1; } } - assert_eq!(found_error, bad_block_index < NUM_BLOCKS); + if bad_block_index < NUM_BLOCKS { + // This additionally checks whether the indexing was stopped. + assert_eq!(error_count, 1); + } else { + assert_eq!(error_count, 0); + } } #[test] From 1bdb8b17f1c09abb384f3024bacba28d4d065794 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 14:05:18 +0000 Subject: [PATCH 60/67] remove prev hash --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index da426a536075..661edf1d3955 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -477,19 +477,10 @@ fn verify_unknown_block_handling( subaccount: None, }; - let mut prev_hash = None; - for i in 0..NUM_BLOCKS { - let block = if let Some(prev_hash) = prev_hash { - BlockBuilder::new(i, i) - .with_parent_hash(prev_hash) - .mint(TEST_ACCOUNT, Tokens::from(1u64)) - .build() - } else { - BlockBuilder::new(i, i) - .mint(TEST_ACCOUNT, Tokens::from(1u64)) - .build() - }; + let block = BlockBuilder::new(i, i) + .mint(TEST_ACCOUNT, Tokens::from(1u64)) + .build(); let block = if i == bad_block_index { let mut bad_block = match block { ICRC3Value::Map(btree_map) => btree_map, @@ -500,7 +491,6 @@ fn verify_unknown_block_handling( } else { block }; - prev_hash = Some(block.clone().hash().to_vec()); add_block(env, ledger_id, &block).expect("failed adding block to the ledger"); } From eccae33ced3c39bf07baf26c3a23e98751bc8e03 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 14:07:44 +0000 Subject: [PATCH 61/67] remove redundant test --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 661edf1d3955..555d906fadd8 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -527,17 +527,13 @@ fn verify_unknown_block_handling( error_count += 1; } } - if bad_block_index < NUM_BLOCKS { - // This additionally checks whether the indexing was stopped. - assert_eq!(error_count, 1); - } else { - assert_eq!(error_count, 0); - } + // This additionally checks whether the indexing was stopped. + assert_eq!(error_count, 1); } #[test] fn test_unknown_block_icrc3() { - for bad_block_index in 0..NUM_BLOCKS + 1 { + for bad_block_index in 0..NUM_BLOCKS { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); @@ -548,7 +544,7 @@ fn test_unknown_block_icrc3() { #[test] fn test_unknown_block_legacy() { - for bad_block_index in 0..NUM_BLOCKS + 1 { + for bad_block_index in 0..NUM_BLOCKS { let env = &StateMachine::new(); let ledger_id = install_icrc3_test_ledger(env); let index_id = install_index_ng(env, index_init_arg_without_interval(ledger_id)); From 21f6e64cf9950554f448f379f7d84aa3ac0e34dd Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 14:21:24 +0000 Subject: [PATCH 62/67] verify no errors in logs --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index 555d906fadd8..c2ef33854bf4 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -355,6 +355,8 @@ fn assert_ledger_index_parity(env: &StateMachine, ledger_id: CanisterId, index_i ); } } + // Veify there are no errors in the index log. + assert!(get_logs(env, index_id).entries.is_empty()); } #[cfg(any(feature = "get_blocks_disabled", feature = "icrc3_disabled"))] From b25a745c5c6db790e0eb59fdf49465fe1baef105 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 14:22:03 +0000 Subject: [PATCH 63/67] typo --- rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index c2ef33854bf4..edcc647fc88b 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -355,7 +355,7 @@ fn assert_ledger_index_parity(env: &StateMachine, ledger_id: CanisterId, index_i ); } } - // Veify there are no errors in the index log. + // Verify there are no errors in the index log. assert!(get_logs(env, index_id).entries.is_empty()); } From 69b77b80d31cad91e1c4ed018e0b98afbb4a3259 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 15:02:23 +0000 Subject: [PATCH 64/67] update icrc-ledger-types changelog --- packages/icrc-ledger-types/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/icrc-ledger-types/CHANGELOG.md b/packages/icrc-ledger-types/CHANGELOG.md index c35cf02fc772..e29b3c450f2c 100644 --- a/packages/icrc-ledger-types/CHANGELOG.md +++ b/packages/icrc-ledger-types/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `try_from_subaccount_to_principal` that returns an error rather than panicking if the subaccount is not a valid Principal. +- add optional fee to `Mint` and `Burn` icrc3 operations. ## 0.1.11 From 3aaaff4ca1472f6fccd5a618c8bd287bb04212fb Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Wed, 15 Oct 2025 16:41:51 +0000 Subject: [PATCH 65/67] log info about stopping the timer --- rs/ledger_suite/icrc1/index-ng/src/main.rs | 2 ++ rs/ledger_suite/icrc1/index-ng/tests/tests.rs | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/rs/ledger_suite/icrc1/index-ng/src/main.rs b/rs/ledger_suite/icrc1/index-ng/src/main.rs index a829f79f9099..6984dbcece0d 100644 --- a/rs/ledger_suite/icrc1/index-ng/src/main.rs +++ b/rs/ledger_suite/icrc1/index-ng/src/main.rs @@ -607,6 +607,8 @@ pub async fn build_index() -> Option<()> { log!(P0, "{}", error.message); ic_cdk::eprintln!("{}", error.message); if !error.retriable { + log!(P0, "Stopping the indexing timer."); + ic_cdk::eprintln!("Stopping the indexing timer."); let timer_id = TIMER_ID.with(|tid| *tid.borrow()); ic_cdk_timers::clear_timer(timer_id); } diff --git a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs index edcc647fc88b..de781b9f64b3 100644 --- a/rs/ledger_suite/icrc1/index-ng/tests/tests.rs +++ b/rs/ledger_suite/icrc1/index-ng/tests/tests.rs @@ -521,6 +521,7 @@ fn verify_unknown_block_handling( let logs = get_logs(env, index_id); let mut error_count = 0; + let mut stopping_message = false; for entry in logs.entries { if entry.message.contains(&format!( "Block at index {} has unknown fields.", @@ -528,9 +529,13 @@ fn verify_unknown_block_handling( )) { error_count += 1; } + if entry.message.contains("Stopping the indexing timer.") { + stopping_message = true; + } } // This additionally checks whether the indexing was stopped. assert_eq!(error_count, 1); + assert!(stopping_message); } #[test] From 85aca11e9fc463d4ac5fc2ad87be4b0ec12b0be5 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 17 Oct 2025 09:58:07 +0000 Subject: [PATCH 66/67] remove fee from mint and burn tx strategies --- rs/ledger_suite/icrc1/test_utils/src/lib.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/rs/ledger_suite/icrc1/test_utils/src/lib.rs b/rs/ledger_suite/icrc1/test_utils/src/lib.rs index 7dbd587f4508..681a0dee4450 100644 --- a/rs/ledger_suite/icrc1/test_utils/src/lib.rs +++ b/rs/ledger_suite/icrc1/test_utils/src/lib.rs @@ -764,7 +764,6 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( /// - Avoid duplicate transactions and minting to the minter. fn mint_strategy( minter_identity: Arc, - default_fee: u64, now: SystemTime, tx_hash_set_pointer: Arc>>, ) -> impl Strategy { @@ -774,17 +773,16 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( amount_strategy(), valid_created_at_time_strategy(now), arb_memo(), - prop::option::of(Just(default_fee)), ) .prop_filter_map( "The minting account is set as to account or tx is a duplicate", - move |(to_signer, amount, created_at_time, memo, fee)| { + move |(to_signer, amount, created_at_time, memo)| { let to = to_signer.account(); let tx = Transaction { operation: Operation::Mint:: { amount: Tokens::from_e8s(amount), to, - fee: fee.map(Tokens::from_e8s), + fee: None, }, created_at_time, memo: memo.clone(), @@ -838,17 +836,16 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( default_fee..=balance, valid_created_at_time_strategy(now), arb_memo(), - prop::option::of(Just(default_fee)), ) .prop_filter_map( "Tx hash already exists", - move |(amount, created_at_time, memo, fee)| { + move |(amount, created_at_time, memo)| { let tx = Transaction { operation: Operation::Burn:: { amount: Tokens::from_e8s(amount), from, spender: None, - fee: fee.map(Tokens::from_e8s), + fee: None, }, created_at_time, memo: memo.clone(), @@ -1224,13 +1221,8 @@ pub fn valid_transactions_strategy_with_excluded_transaction_types( let tx_hashes_pointer = Arc::new(state.txs.clone()); let account_to_basic_identity_pointer = Arc::new(state.principal_to_basic_identity.clone()); let allowance_map_pointer = Arc::new(state.allowances.clone()); - let mint_strategy = mint_strategy( - minter_identity.clone(), - default_fee, - now, - tx_hashes_pointer.clone(), - ) - .boxed(); + let mint_strategy = + mint_strategy(minter_identity.clone(), now, tx_hashes_pointer.clone()).boxed(); let arb_tx = if balances.is_empty() { mint_strategy } else { From e6b5fe1327d1247659aabb826fb930d5a126db2d Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Fri, 17 Oct 2025 11:58:11 +0000 Subject: [PATCH 67/67] list all operation fields explicitly --- .../src/common/storage/storage_operations.rs | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs index 7ca28ddf32c8..71018261c4d6 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs @@ -268,7 +268,12 @@ pub fn update_account_balances(connection: &mut Connection) -> anyhow::Result<() while !rosetta_blocks.is_empty() { for rosetta_block in rosetta_blocks { match rosetta_block.get_transaction().operation { - crate::common::storage::types::IcrcOperation::Burn { from, amount, .. } => { + crate::common::storage::types::IcrcOperation::Burn { + from, + amount, + fee: _, + spender: _, + } => { let fee = rosetta_block .get_fee_paid()? .unwrap_or(Nat(BigUint::zero())); @@ -295,7 +300,7 @@ pub fn update_account_balances(connection: &mut Connection) -> anyhow::Result<() )?; } } - crate::common::storage::types::IcrcOperation::Mint { to, amount, .. } => { + crate::common::storage::types::IcrcOperation::Mint { to, amount, fee: _ } => { let fee = rosetta_block .get_fee_paid()? .unwrap_or(Nat(BigUint::zero())); @@ -322,7 +327,14 @@ pub fn update_account_balances(connection: &mut Connection) -> anyhow::Result<() )?; } } - crate::common::storage::types::IcrcOperation::Approve { from, .. } => { + crate::common::storage::types::IcrcOperation::Approve { + from, + spender: _, + amount: _, + expected_allowance: _, + expires_at: _, + fee: _, + } => { let fee = rosetta_block .get_fee_paid()? .unwrap_or(Nat(BigUint::zero())); @@ -335,7 +347,11 @@ pub fn update_account_balances(connection: &mut Connection) -> anyhow::Result<() )?; } crate::common::storage::types::IcrcOperation::Transfer { - from, to, amount, .. + from, + to, + amount, + spender: _, + fee: _, } => { let fee = rosetta_block .get_fee_paid()?