feat(ic-icrc1): add ICRC-122 AuthorizedMint/AuthorizedBurn candid types, index-ng, and .did support#9694
Conversation
…es, index-ng, and .did support - Add AuthorizedMint/AuthorizedBurn Candid structs and Transaction fields - Replace panic stubs with real implementations in index-ng (balance changes and account extraction), endpoints (Block-to-Transaction conversion), test_utils, in_memory_ledger, and Transaction::apply - Update ledger.did, archive.did, and index-ng.did with new types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for ICRC-122 AuthorizedMint and AuthorizedBurn transaction types to the ledger system. The changes include adding new Candid structs and transaction fields in icrc-ledger-types, replacing panic stubs with real implementations for balance operations across multiple modules (index-ng, endpoints, lib.rs, test_utils, in_memory_ledger), and updating the .did files with the new types.
Changes:
- Add AuthorizedMint and AuthorizedBurn Candid structs and fields to the Transaction type in icrc-ledger-types
- Implement balance credit/debit operations for authorized mints/burns in index-ng, lib.rs, and test utilities
- Add Block-to-Transaction conversion support for the new operation types in endpoints
- Update ledger.did, archive.did, and index-ng.did with AuthorizedMint and AuthorizedBurn type definitions
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/icrc-ledger-types/src/icrc3/transactions.rs | Adds AuthorizedMint/AuthorizedBurn structs, new constants, Transaction fields, and helper methods |
| rs/ledger_suite/icrc1/src/endpoints.rs | Implements Block-to-Transaction conversion for AuthorizedMint/AuthorizedBurn operations |
| rs/ledger_suite/icrc1/src/lib.rs | Implements balance operations (mint/burn) for authorized operations |
| rs/ledger_suite/icrc1/test_utils/src/lib.rs | Adds balance credit/debit and account validity tracking for authorized operations |
| rs/ledger_suite/test_utils/in_memory_ledger/src/lib.rs | Implements process_mint/process_burn for authorized operations |
| rs/ledger_suite/icrc1/index-ng/src/main.rs | Implements balance changes and account extraction for authorized operations |
| rs/ledger_suite/icrc1/ledger/ledger.did | Adds AuthorizedMint/AuthorizedBurn type definitions |
| rs/ledger_suite/icrc1/archive/archive.did | Adds AuthorizedMint/AuthorizedBurn type definitions |
| rs/ledger_suite/icrc1/index-ng/index-ng.did | Adds AuthorizedMint/AuthorizedBurn type definitions |
| rs/ledger_suite/icrc1/index-ng/tests/tests.rs | Updates test Transaction initialization with new fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…izedBurn Candid types
ICRC-122 block schema defines tx fields as {mthd, to/from, amt, caller,
reason} — memo and created_at_time are not part of the standard. Remove
them from the Candid structs, endpoint conversion, and .did files.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CBOR round-trip: encode/decode preserves all fields and btype - Hash stability: generic block hash matches encoded block hash - Candid conversion: Block→Transaction produces correct kind and fields - Schema validation: blocks with caller/mthd pass ICRC-152 validators, blocks without them pass ICRC-122 but fail ICRC-152 - Transaction::apply: AuthorizedMint credits, AuthorizedBurn debits, insufficient balance fails - Proptest: extend operation_strategy to generate AuthorizedMint/AuthorizedBurn - Fix btype in blocks_strategy for AuthorizedMint/AuthorizedBurn - Fix schema: use is_more_or_equal_to(0) for amt to accept Nat64 from CBOR decoder (same approach as ts field) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er ICRC-152 ICRC-152 canonical tx mapping includes created_at_time as a tx field (used for deduplication). Add it back to Candid structs, endpoint conversion, .did files, and schema validators. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…CBOR encoding The FlattenedTransaction serializes created_at_time as "ts" in CBOR. Update schema validators to check for "ts" inside tx (matching what the encoder produces), make it required in strict (ICRC-152) mode, and fix test helpers to pass created_at_time for ICRC-152 blocks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ls in sns and nervous_system tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… Rosetta support The blocks_strategy is used by Rosetta proptests which still have panic stubs for these variants. Comment out the new strategies until Rosetta PR implements real handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ilders Add BlockBuilder::authorized_mint() and BlockBuilder::authorized_burn() methods for constructing ICRC-122 blocks as ICRC3Value. These can be used with the icrc3_test_ledger to test index-ng and Rosetta integration with the new block types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rn via icrc3_test_ledger Test the full pipeline: load ICRC-122 blocks into the test ledger, sync index-ng, and verify: - Balances are correct (authorized mint credits, authorized burn debits) - get_account_transactions returns correct kind and operation fields - Minimal ICRC-122 blocks (no caller/mthd) are indexed correctly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mbjorkqvist
left a comment
There was a problem hiding this comment.
Thanks @bogwar, I have a few minor comments!
| Operation::AuthorizedBurn { from, amount, .. } => { | ||
| self.process_burn(from, &None, amount, index); |
There was a problem hiding this comment.
Here we pass &None as the spender to self.process_burn. In process_burn, the spender is used to deduct from an allowance (since this process_burn is also called if someone performs an icrc2_transfer_from with the minter as the destination account. We don't mention anything regarding allowances in the ICRC-122 or ICRC-152 specs. What should happen if the AuthorizedBurn is executed where the caller is a principal for which there exists an allowance (for caller with the default subaccount) to spend from the from account? The implementation in the in_memory_ledger here suggests that any existing allowance shouldn't be touched, which makes sense, but it may be worth it to explicitly mention this in (one of) the standard(s).
There was a problem hiding this comment.
Yes, this is intentional — passing &None as the spender means no allowance is touched. AuthorizedBurn is a privileged operation that doesn't go through the transfer API (which is what triggers allowance deduction), so there's no reason it should consume any allowance.
I'll raise this as a spec clarification on the ICRC-122/152 standards to make this explicit.
|
|
||
| assert_eq!(icrc1_balance_of(env, index_id, account_1), 1_000_000); | ||
| } | ||
|
|
There was a problem hiding this comment.
Related to the comment regarding allowances in the in_memory_ledger, could we add a test here that creates an allowance for the authorized burn principal (and default subaccount), then a block that performs an authorized burn that for a regular burn would (partially) use the aforementioned allowance, and afterward check that the allowance created was not affected by the authorized burn?
There was a problem hiding this comment.
Added two unit tests in the in_memory_ledger test module (rather than in the index-ng integration tests, since index-ng doesn't track allowances):
should_not_consume_allowance_on_authorized_burn— mints, approves, then does an authorized burn, and verifies the allowance is fully intact.should_consume_allowance_on_regular_burn_after_authorized_burn— same setup plus a regular burn via spender afterward, verifying only the regular burn deducts from the allowance.
| } | ||
| } | ||
|
|
||
| mod authorized_mint_burn_tests { |
There was a problem hiding this comment.
These tests only use the U64 token type. I created DEFI-2757 to facilitate testing with both token types, so maybe we can leave these tests as-is for now, and address the issue in the aforementioned ticket.
There was a problem hiding this comment.
Acknowledged — will leave as-is and let DEFI-2757 address both token types.
| item(account_field, Required, is_account()), | ||
| item("amt", Required, is_nat()), | ||
| item("caller", caller_mthd_req, is_principal()), | ||
| item("amt", Required, is_more_or_equal_to(0)), |
There was a problem hiding this comment.
| item("amt", Required, is_more_or_equal_to(0)), | |
| item("amt", Required, is_more_or_equal_to(1)), |
The ICRC-122 spec says that this shall be greater than 0 for both mint and burn
There was a problem hiding this comment.
Good catch, fixed — changed to is_more_or_equal_to(1) per spec.
- Change amt validation to is_more_or_equal_to(1) per ICRC-122 spec - Add in_memory_ledger tests verifying authorized burn does not consume allowances Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
Summary
AuthorizedMint/AuthorizedBurnCandid structs andTransactionfields inicrc-ledger-typesBlock-to-TransactionconversionTransaction::applybalance operationsledger.did,archive.did, andindex-ng.didwith new typesmemo, includecreated_at_timeis_more_or_equal_to(1)foramtto acceptNat64from CBOR decoder; addtx.ts(created_at_time) as required in strict (ICRC-152) modeBlockBuilder::authorized_mint()andBlockBuilder::authorized_burn()for constructing ICRC-122 test blocksbtypeinblocks_strategyforAuthorizedMint/AuthorizedBurnTest plan
Unit tests (
rs/ledger_suite/icrc1/tests/tests.rs)btypeset to"122mint"/"122burn"Block→Transactionproduces correctkind, populates the right struct with correct fields (includingcreated_at_time), all other operation fields areNonecaller,mthd, andts(created_at_time) passvalidate_152_mint/validate_152_burncaller/mthd/tspass permissivevalidate_mint/validate_burnbut fail strict ICRC-152 validatorsUnit tests (
rs/ledger_suite/icrc1/ledger/src/tests.rs)Integration tests (
rs/ledger_suite/icrc1/index-ng/tests/tests.rs)kind("122mint"/"122burn") and correct operation fields for authorized operationsProptest
operation_strategy(currently disabled pending Rosetta support — Rosetta proptests useblocks_strategyand would hit panic stubs)🤖 Generated with Claude Code