feat(ic-icrc1-ledger): implement ICRC-153 freeze/unfreeze endpoints with freeze guard#9976
feat(ic-icrc1-ledger): implement ICRC-153 freeze/unfreeze endpoints with freeze guard#9976bogwar wants to merge 5 commits intoicrc-123-4-rosettafrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Add 4 new Operation variants: FreezeAccount, UnfreezeAccount, FreezePrincipal, UnfreezePrincipal (ICRC-123). These are no-op operations that record freeze/unfreeze events on the ledger without affecting balances. - Add variants with account/principal, caller, mthd, reason fields - Add FlattenedTransaction fields (account, principal) for CBOR serde - Set btype correctly using BTYPE_123_* constants from icrc-ledger-types - Wire btype-based deserialization in Block::decode path - No-op apply() — no balance changes - Fix all exhaustive matches across ic-icrc1, index-ng, rosetta, test utils - Add freeze_operation_tests: CBOR round-trip, btype, generic block tests Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
….did updates Add ICRC-153 Candid types (freeze/unfreeze args, errors, query types), ICRC-123 transaction structs (FreezeAccount, UnfreezeAccount, FreezePrincipal, UnfreezePrincipal) to the Transaction type, implement From<Block> conversion in endpoints.rs, update all .did files, add BlockBuilder methods for freeze/unfreeze blocks, and add index-ng integration tests verifying sync, balance invariance, and account transaction retrieval. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Replace panic stubs with real implementations for FreezeAccount, UnfreezeAccount, FreezePrincipal, and UnfreezePrincipal operations across the Rosetta API stack: storage types, storage operations, utils, construction API, data API, and system tests. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…ith freeze guard Add ICRC-153 support for account/principal freezing: - Feature flag `icrc153` in FeatureFlags (default false) - Stable memory maps for frozen accounts and principals (MemoryId 5, 6) - StorablePrincipal newtype wrapper for StableBTreeMap compatibility - 4 update endpoints: freeze_account, unfreeze_account, freeze_principal, unfreeze_principal - 4 query endpoints: is_frozen_account, is_frozen_principal, list_frozen_accounts, list_frozen_principals - Freeze guard in execute_transfer_not_async and icrc2_approve_not_async (traps on frozen from/spender) - Unfreeze_principal also clears account-level freezes for that principal - Conditional ICRC-153 in supported_standards and 4 block types in icrc3_supported_block_types - Updated ledger.did with all new types and endpoints - 15 integration tests covering: feature flag, authorization, validation, happy paths, freeze guard blocking transfers/approvals, allowing transfers TO frozen, privileged ops, deduplication, pagination, and supported standards - Updated FeatureFlags construction in 14 files across the repo - Regenerated canbench baselines Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Implements ICRC-123/153 freeze/unfreeze functionality across the ICRC ledger suite, including ledger endpoints, stable-state persistence, indexing, and Rosetta ingestion/representation.
Changes:
- Adds new freeze/unfreeze block schemas and operation variants (ICRC-123/153) plus Candid/.did surface area.
- Implements ledger freeze state (stable maps) and exposes ICRC-153 update/query endpoints with transfer/approve guards.
- Extends index-ng and Rosetta to ingest, store, and expose the new operation types; updates tests/bench baselines and mechanical FeatureFlags/Transaction literals.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/tests/financial_integrations/icrc1_agent_test.rs | Updates FeatureFlags literals to include icrc153. |
| rs/tests/cross_chain/ic_xc_cketh_test.rs | Updates FeatureFlags literals to include icrc153. |
| rs/sns/governance/token_valuation/src/tests.rs | Updates Transaction test literals with new optional freeze fields. |
| rs/rosetta-api/icrc1/tests/system_tests.rs | Adds system test coverage for freeze/unfreeze ops and updates construction-submit filtering. |
| rs/rosetta-api/icrc1/tests/multitoken_system_tests.rs | Updates construction-submit filtering to ignore new ops. |
| rs/rosetta-api/icrc1/tests/integration_test_components/storage/storing_blockchain_data_test.rs | Updates FeatureFlags literals to include icrc153. |
| rs/rosetta-api/icrc1/tests/common/local_replica.rs | Updates default init args FeatureFlags to include icrc153. |
| rs/rosetta-api/icrc1/src/data_api/services.rs | Adjusts test logic around account association for new IcrcOperation variants. |
| rs/rosetta-api/icrc1/src/construction_api/utils.rs | Rejects freeze/unfreeze ops in Rosetta construction API. |
| rs/rosetta-api/icrc1/src/construction_api/services.rs | Updates construction API tests to skip new operations. |
| rs/rosetta-api/icrc1/src/common/utils/utils.rs | Adds bidirectional mapping between Rosetta ops and new IcrcOperation variants. |
| rs/rosetta-api/icrc1/src/common/types.rs | Extends OperationType enum with freeze/unfreeze variants. |
| rs/rosetta-api/icrc1/src/common/storage/types.rs | Adds storage-layer IcrcOperation variants and (de)serialization/btype mapping + proptests. |
| rs/rosetta-api/icrc1/src/common/storage/storage_operations/tests.rs | Adds SQLite storage/read and balance-sync tests for freeze/unfreeze account ops. |
| rs/rosetta-api/icrc1/src/common/storage/storage_operations/mod.rs | Ensures balance sync ignores freeze/unfreeze; stores these ops in DB. |
| rs/nervous_system/initial_supply/src/tests.rs | Updates Transaction test literals with new optional freeze fields. |
| rs/ledger_suite/tests/sm-tests/src/lib.rs | Adds ICRC-153 integration helpers + extensive state-machine tests; updates FeatureFlags literals. |
| rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs | Treats freeze/unfreeze operations as no-ops for balance processing. |
| rs/ledger_suite/icrc1/tests/upgrade_downgrade.rs | Updates FeatureFlags literals to include icrc153. |
| rs/ledger_suite/icrc1/tests/tests.rs | Adds unit tests for freeze/unfreeze CBOR/btype/hash behavior. |
| rs/ledger_suite/icrc1/test_utils/src/lib.rs | Extends proptest strategies and balance models for new operations. |
| rs/ledger_suite/icrc1/test_utils/src/icrc3.rs | Adds BlockBuilder helpers to create freeze/unfreeze blocks for tests. |
| rs/ledger_suite/icrc1/src/lib.rs | Adds new Operation variants and extends tx flattening/unflattening and btype mapping. |
| rs/ledger_suite/icrc1/src/endpoints.rs | Extends ICRC-3 Transaction conversion to include freeze/unfreeze kinds/fields. |
| rs/ledger_suite/icrc1/ledger/tests/tests.rs | Wires new ICRC-153 state-machine tests into ledger test suite; updates FeatureFlags literals. |
| rs/ledger_suite/icrc1/ledger/src/main.rs | Implements ICRC-153 endpoints, freeze guard checks, supported standards/block types, and list/query APIs. |
| rs/ledger_suite/icrc1/ledger/src/lib.rs | Adds stable maps for freeze state, StorablePrincipal, icrc153 FeatureFlag, and helper is_frozen_account. |
| rs/ledger_suite/icrc1/ledger/src/benches/benches_u64.rs | Updates bench init FeatureFlags to include icrc153. |
| rs/ledger_suite/icrc1/ledger/src/benches/benches_u256.rs | Updates bench init FeatureFlags to include icrc153. |
| rs/ledger_suite/icrc1/ledger/ledger.did | Extends public Candid interface for ICRC-153 and new transaction fields/types. |
| rs/ledger_suite/icrc1/ledger/canbench_results/canbench_u64.yml | Updates canbench baselines after changes. |
| rs/ledger_suite/icrc1/ledger/canbench_results/canbench_u256.yml | Updates canbench baselines after changes. |
| rs/ledger_suite/icrc1/index-ng/tests/tests.rs | Adds index-ng integration tests for freeze/unfreeze indexing; updates Transaction literals. |
| rs/ledger_suite/icrc1/index-ng/tests/common/mod.rs | Updates FeatureFlags literals to include icrc153. |
| rs/ledger_suite/icrc1/index-ng/src/main.rs | Ensures index-ng ignores freeze/unfreeze for balances; associates account-level ops with accounts. |
| rs/ledger_suite/icrc1/index-ng/index-ng.did | Extends index-ng Candid Transaction type with freeze/unfreeze fields/types. |
| rs/ledger_suite/icrc1/archive/archive.did | Extends archive Candid Transaction type with freeze/unfreeze fields/types. |
| rs/ethereum/ledger-suite-orchestrator/src/scheduler/mod.rs | Updates ledger init feature flags to include icrc153. |
| rs/ethereum/cketh/test_utils/src/lib.rs | Updates FeatureFlags literals to include icrc153. |
| rs/bitcoin/ckbtc/minter/tests/tests.rs | Updates FeatureFlags literals to include icrc153. |
| packages/icrc-ledger-types/src/lib.rs | Exposes new icrc123 and icrc153 modules. |
| packages/icrc-ledger-types/src/icrc3/transactions.rs | Adds transaction structs/kinds for freeze/unfreeze operations in ICRC-3 transaction model. |
| packages/icrc-ledger-types/src/icrc153/mod.rs | Adds Candid argument/error/query types for ICRC-153 endpoints. |
| packages/icrc-ledger-types/src/icrc123/schema.rs | Adds ICRC-123 block schema validators and ICRC-153 strict validation modes. |
| packages/icrc-ledger-types/src/icrc123/mod.rs | Declares the icrc123 module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| crate::common::storage::types::IcrcOperation::FreezePrincipal { principal, .. } | ||
| | crate::common::storage::types::IcrcOperation::UnfreezePrincipal { | ||
| principal, .. | ||
| } => ( | ||
| "freeze_principal", | ||
| Some(principal), | ||
| Some(*Account::from(principal).effective_subaccount()), | ||
| Some(principal), | ||
| Some(*Account::from(principal).effective_subaccount()), | ||
| None, |
There was a problem hiding this comment.
store_blocks uses the same operation_type string ("freeze_principal") for both FreezePrincipal and UnfreezePrincipal operations. This makes it impossible to distinguish freeze vs unfreeze in DB queries; store distinct values (e.g., "freeze_principal" vs "unfreeze_principal").
| if ledger.feature_flags().icrc153 | ||
| && let Some(ref spender_acc) = spender | ||
| && is_frozen_account(spender_acc) | ||
| { | ||
| ic_cdk::trap("spender account is frozen"); | ||
| } |
There was a problem hiding this comment.
The freeze guard in execute_transfer_not_async also traps when the spender account is frozen. The PR description says the guard blocks transfers/approves from frozen accounts; this extra restriction changes semantics for icrc2_transfer_from (a frozen spender could be prevented from moving non-frozen funds). Either update the description/standard rationale or remove the spender-side freeze check if it's not intended.
| let max_results = req.max_results.0.to_u64().unwrap_or(u64::MAX) as usize; | ||
| let accounts: Vec<Account> = FROZEN_ACCOUNTS.with(|fa| { | ||
| let map = fa.borrow(); | ||
| let iter: Box<dyn Iterator<Item = (Account, ())>> = match req.start_after { | ||
| Some(ref start) => Box::new(map.range(*start..)), | ||
| None => Box::new(map.iter()), | ||
| }; | ||
| let mut iter = iter.peekable(); | ||
| // Skip the start_after key itself if present | ||
| if req.start_after.is_some() | ||
| && let Some((k, _)) = iter.peek() | ||
| && Some(k) == req.start_after.as_ref() | ||
| { | ||
| iter.next(); | ||
| } | ||
| iter.take(max_results + 1) | ||
| .map(|(account, _)| account) | ||
| .collect() |
There was a problem hiding this comment.
max_results is derived from Nat via to_u64() as usize, and the code then does max_results + 1. On wasm32, large Nat values can cast to usize::MAX, making max_results + 1 overflow; even without overflow, truncation from u64 to usize can silently change requested limits. Clamp max_results to a safe upper bound and use saturating_add(1) (or equivalent) when computing the lookahead limit.
| let max_results = req.max_results.0.to_u64().unwrap_or(u64::MAX) as usize; | ||
| let principals: Vec<Principal> = FROZEN_PRINCIPALS.with(|fp| { | ||
| let map = fp.borrow(); | ||
| let start_key = req.start_after.map(StorablePrincipal); | ||
| let iter: Box<dyn Iterator<Item = (StorablePrincipal, ())>> = match start_key { | ||
| Some(ref start) => Box::new(map.range(start.clone()..)), | ||
| None => Box::new(map.iter()), | ||
| }; | ||
| let mut iter = iter.peekable(); | ||
| // Skip the start_after key itself if present | ||
| if start_key.is_some() | ||
| && let Some((k, _)) = iter.peek() | ||
| && Some(k) == start_key.as_ref() | ||
| { | ||
| iter.next(); | ||
| } | ||
| iter.take(max_results + 1).map(|(sp, _)| sp.0).collect() | ||
| }); |
There was a problem hiding this comment.
Same max_results conversion/overflow risk as icrc153_list_frozen_accounts: Nat -> u64 -> usize can truncate on wasm32 and max_results + 1 can overflow for large values. Clamp to a bounded maximum and use saturating arithmetic when adding the lookahead element.
| // account_1 should appear in block 0 (mint), 1 (freeze), 2 (freeze), 3 (unfreeze), 4 (freeze principal), 5 (unfreeze principal) | ||
| assert!( | ||
| block_indices.contains(&0), | ||
| "account_1 should appear in block 0" | ||
| ); | ||
| assert!( | ||
| block_indices.contains(&1), | ||
| "account_1 should appear in block 1" | ||
| ); | ||
| assert!( | ||
| block_indices.contains(&2), | ||
| "account_1 should appear in block 2" | ||
| ); | ||
| assert!( | ||
| block_indices.contains(&3), | ||
| "account_1 should appear in block 3" | ||
| ); |
There was a problem hiding this comment.
The comment says account_1 should appear in blocks 0-5 (including principal freeze/unfreeze), but the assertions only check for blocks 0-3. Either add assertions for blocks 4 and 5 or update the comment so the test matches the stated expectation.
| fee_collector : opt FeeCollector; | ||
| authorized_mint : opt AuthorizedMint; | ||
| authorized_burn : opt AuthorizedBurn; | ||
| freeze_account : opt FreezeAccountTx; |
There was a problem hiding this comment.
Transaction.freeze_account references FreezeAccountTx, but no such type is defined in this .did (the defined type is FreezeAccount). This will break candid interface generation/compat checks; rename the field to use the existing FreezeAccount type or add the missing FreezeAccountTx definition.
| freeze_account : opt FreezeAccountTx; | |
| freeze_account : opt FreezeAccount; |
| crate::common::storage::types::IcrcOperation::FreezeAccount { account, .. } | ||
| | crate::common::storage::types::IcrcOperation::UnfreezeAccount { account, .. } => ( | ||
| "freeze_account", | ||
| Some(account.owner), | ||
| Some(*account.effective_subaccount()), | ||
| Some(account.owner), | ||
| Some(*account.effective_subaccount()), |
There was a problem hiding this comment.
store_blocks uses the same operation_type string ("freeze_account") for both FreezeAccount and UnfreezeAccount operations. This loses information and can make operation-type filtering/reporting incorrect; use distinct values (e.g., "freeze_account" vs "unfreeze_account").
274921f to
618ef70
Compare
618ef70 to
6323bc4
Compare
Summary
Implements the full ICRC-123/153 standard for freeze/unfreeze operations on the ICRC ledger suite. Built in 5 phases as incremental commits:
123freezeaccount,123unfreezeaccount,123freezeprincipal,123unfreezeprincipal) with permissive (ICRC-123) and strict (ICRC-153) validation modesOperationenum variants with no-opapply()(no balance changes), CBOR encoding, exhaustive match fixups across 14 filesicrc-ledger-types, Transaction struct fields, endpoint conversions, index-ng support, BlockBuilder extensions, .did updatesIcrcOperationvariants, storage, data API, construction API rejection, proptest strategies, system testicrc153feature flag,StorablePrincipalnewtype, twoStableBTreeMaps for freeze state, 4 update endpoints, 4 query endpoints (including paginated listing), freeze guard onicrc1_transfer/icrc2_approve/icrc2_transfer_from, supported standards/block typesKey design decisions
StableBTreeMaps in stable memory (frozen_accounts,frozen_principals). Survives upgrades automatically.unfreeze_principalclears the principal AND all account-level freezes for that principal's accounts.icrc152_mint/icrc152_burn).freeze_account/freeze_principalreturnAlreadyFrozenif target is already frozen. Unfreeze always succeeds.is_controller().Cross-team changes (mechanical)
FeatureFlagsgainedicrc153: bool(defaultfalse,#[serde(default)]).Transactionstruct gained 4 newOption<...>fields (set toNone). Updated struct literals in ckbtc, cketh, ledger-suite-orchestrator, nervous_system, sns, rosetta, and test files.Governance team review needed
rs/nervous_system/initial_supply/src/tests.rs— addsfreeze_account: Noneetc. to Transaction literalrs/sns/governance/token_valuation/src/tests.rs— same mechanical changeTest plan
Schema tests
Operation unit tests
Index-ng integration tests
Rosetta tests
Ledger integration tests (15 tests)
is_frozenverificationCanbench
🤖 Generated with Claude Code