-
Notifications
You must be signed in to change notification settings - Fork 7
ICRC-124: blocks for recording management actions -- stopping, unstopping and deactivating ledgers #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
ICRCs/ICRC-124/ICRC-124.md:69
- [nitpick] Consider using consistent metadata keys across the examples (e.g., 'reason' in the pause example versus 'note' in the unpause example) to improve clarity and ease of parsing.
record { "reason"; variant { Text = "DAO vote: pause due to upgrade prep" }}
ICRCs/ICRC-124/ICRC-124.md
Outdated
|
||
## Overview of Block Types | ||
|
||
- **Pause Ledger**: `124pause` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The identifier '124pause' is used here, but the PR metadata description references '123pause'. Please confirm the intended block type and update the identifier consistently.
Copilot uses AI. Check for mistakes.
ICRCs/ICRC-124/ICRC-124.md
Outdated
vec { | ||
// ... other supported types like ICRC-1, ICRC-3, ICRC-122, ICRC-123 ... | ||
variant { Record = vec { | ||
record { "btype"; variant { Text = "124pause" }}; | ||
record { "url"; variant { Text = "[https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md](https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md)" }}; // Placeholder URL | ||
}}; | ||
variant { Record = vec { | ||
record { "btype"; variant { Text = "124unpause" }}; | ||
record { "url"; variant { Text = "[https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md](https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md)" }}; // Placeholder URL | ||
}}; | ||
variant { Record = vec { | ||
record { "btype"; variant { Text = "124deactivate" }}; | ||
record { "url"; variant { Text = "[https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md](https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-124.md)" }}; // Placeholder URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bogwar placeholder url comment should be removed before merging or do you plan to keep this here? also the URLs seem to be wrong, right? (it is a markdown link)
you should also check that for ICRC-122.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than the comment about the URLs in icrc3_supported_block_types
, the PR seems fine to me/
@bogwar wouldn't we also want to add a section for the query interface to check the state of the ledger? in ICRC-123 we defined this. |
- While paused, the ledger MUST reject incoming requests for standard token operations such as `icrc1_transfer`, `icrc2_approve`, and other non-administrative state changes such as those defined in ICRC-122 (e.g., `122mint`, `122burn`). | ||
- However, while paused, the ledger MUST continue to accept specific administrative or management operations necessary for governance or recovery. This includes operations defined in ICRC-123 (e.g., `123freezeaccount`, `123unfreezeaccount`, `123freezeprincipal`, `123unfreezeprincipal`) and, critically, requests that result in recording a `124unpause` block. Ledger implementations MAY also permit requests that result in recording a `124deactivate` block while the ledger is paused, according to their defined governance policies. The exact set of allowed administrative operations during a pause SHOULD be defined by the specific ledger implementation's policy. | ||
- Query calls SHOULD generally remain operational while the ledger is paused. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- An `124pause` block has no effect if the ledger is already paused or if it is in a terminal state due to deactivation. | |
ICRCs/ICRC-124/ICRC-124.md
Outdated
|
||
### Pause Ledger (`124pause`) | ||
- When a `124pause` block is recorded, the ledger MUST enter a "paused" state. | ||
- While paused, the ledger MUST reject incoming requests for standard token operations such as `icrc1_transfer`, `icrc2_approve`, and other non-administrative state changes such as those defined in ICRC-122 (e.g., `122mint`, `122burn`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and other non-administrative state changes such as those defined in ICRC-122 (e.g.,
122mint
,122burn
).
Why are ICRC-122 methods non-administrative state changes? These are state changes that can only be performed by authorized individuals, i.e., admins, no?
Real World Asset ledgers require several management APIs that would allow authorised entities to carry out ledger changes. This standard introduces three new blocktypes:
123pause
that records that all transactions on the ledger were paused by an authorised party123unpause
that records that a paused ledger was unpause by an authorised party123deactivate
that records that a ledger was deactivated by an authorised party