-
Notifications
You must be signed in to change notification settings - Fork 37
Add ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees #205
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?
Changes from all commits
16dd618
1abc199
ef883d5
ef74c58
14b5b18
0bae797
db81fa3
e9e87c7
575b6fc
68214e8
2855323
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # ICRC-1 Advisory | ||
|
|
||
| This document provides clarifications and implementation guidance for the | ||
| ICRC-1 standard. | ||
|
|
||
| It focuses on error handling and atomicity for `icrc1_transfer`. This | ||
| advisory is non-normative and does not change the ICRC-1 specification. | ||
|
|
||
|
|
||
| ## Error Semantics and Atomicity | ||
|
|
||
| ### Clarification | ||
|
|
||
| The ICRC-1 specification defines error variants for `icrc1_transfer`, but | ||
| does not explicitly state which ledger effects are guaranteed not to occur | ||
| when an error is returned. | ||
|
|
||
| This advisory clarifies the expected behavior. | ||
|
|
||
| ### Advisory Guidance | ||
|
|
||
| Ledger implementations SHOULD ensure that `icrc1_transfer` is atomic with | ||
| respect to **externally observable ledger effects**, including: | ||
|
|
||
| - account balances, and | ||
| - the transaction log. | ||
|
|
||
| In particular: | ||
|
|
||
| - A successful response (`Ok(nat)`) SHOULD imply that all balance updates and | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe make explicit that this is made up by the balance updates for the tx itself as well as the balance updates for the fees (and possibly other things). |
||
| the corresponding transaction log entry have been applied. | ||
|
|
||
| - An error response SHOULD imply that: | ||
| - no account balances have been modified, and | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd use a semicolon here following the more prevalent opinion on style. |
||
| - no transaction log entry corresponding to the call has been recorded. | ||
|
|
||
| This guidance does not restrict changes to internal or auxiliary ledger | ||
| state (e.g., caches, metrics, bookkeeping data). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe add ", or internal logs" |
||
|
|
||
| ### Client Considerations | ||
|
|
||
| - Clients MAY assume that an error response from `icrc1_transfer` implies no | ||
| externally observable ledger effects. | ||
| - Clients SHOULD NOT assume stronger guarantees unless explicitly documented | ||
| by the ledger. | ||
|
|
||
| ## Scope and Non-Goals | ||
|
|
||
| This advisory does not modify the ICRC-1 interface, define new error types, or | ||
| constrain internal ledger state. | ||
|
|
||
|
|
||
| ## Summary | ||
|
|
||
| ICRC-1 transfers are expected to be atomic with respect to balances and the | ||
| transaction log, and error responses should not result in partial externally | ||
| observable effects. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error responses should not only result in no "partial" but in no externally-observable effects at all, right? This is a bit ambiguous in my view currently. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| # ICRC-2 Advisory | ||
|
|
||
| This document provides clarifications and implementation guidance for the | ||
| ICRC-2 standard. | ||
|
|
||
| The intent of this advisory is to make explicit certain behaviors that are | ||
| underspecified in the ICRC-2 specification, in order to reduce ambiguity for | ||
| ledger implementers and client developers. This document does not change the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Less committing in the sense of not saying the previous version is underspec'd: |
||
| normative requirements of ICRC-2, but clarifies expectations and highlights | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "...expectations, the intended meaning, and highlights..."
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "but rather" |
||
| recommended practices. | ||
|
|
||
|
|
||
| ## 1. Transaction Deduplication | ||
|
|
||
| ### Clarification | ||
|
|
||
| ICRC-2 methods (`icrc2_approve` and `icrc2_transfer_from`) include arguments and error | ||
| variants (`created_at_time`, `memo`, `Duplicate`, `TooOld`) that imply support | ||
| for transaction deduplication and replay protection. | ||
|
|
||
| This advisory clarifies the expected deduplication behavior for ICRC-2 | ||
| operations. | ||
|
|
||
| ### Advisory Guidance | ||
|
|
||
| Ledger implementations SHOULD implement transaction deduplication for | ||
| `icrc2_approve` and `icrc2_transfer_from` according to the following rules: | ||
|
|
||
| #### Transaction Identity | ||
|
|
||
| - A transaction is identified by the combination of: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe make it a little clearer and relate to the below-used term "transaction identity": |
||
| - the caller, | ||
| - the method name (`icrc2_approve` or `icrc2_transfer_from`), | ||
| - the full set of method arguments, including `created_at_time` and `memo` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ";" as line separater, "; and" on the penultimate line to make it nicer to read |
||
| if provided. | ||
|
|
||
| - Two calls with identical transaction identity are considered duplicates. | ||
|
|
||
| #### Time Window | ||
|
|
||
| - If `created_at_time` is provided: | ||
| - The ledger SHOULD reject calls whose `created_at_time` is too far in the | ||
| past or too far in the future relative to the ledger’s current time, | ||
| returning a `TooOld`/`CreatedInFuture` error. | ||
| - The ledger SHOULD define and document the accepted time window. | ||
| - The ledger SHOULD NOT process duplicates. | ||
|
|
||
bogwar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - If `created_at_time` is not provided: | ||
| - The ledger MAY process duplicates, or | ||
| - MAY apply ledger-specific policies; such behavior SHOULD be documented. | ||
|
|
||
| #### Duplicate Handling | ||
|
|
||
| - If duplicate transactions are not processed: | ||
| - The ledger SHOULD reject the call and return a `Duplicate` error. | ||
| - The ledger MUST NOT apply the transaction effects again. | ||
|
|
||
|
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Editorial: Headings seem to be separated by 1, 2, or 3 newlines. Make it consistent as the current version may look a little sloppy to some. |
||
| #### State Changes and Ordering | ||
|
|
||
| - Deduplication checks SHOULD be performed before any externally observable | ||
| ledger effects are applied. | ||
| - `Duplicate`, `TooOld` or `CreatedInFuture` responses SHOULD imply that no balances, allowances, | ||
| or transaction log entries have been modified. | ||
|
|
||
| #### Interaction with `expected_allowance` | ||
|
|
||
| - For `icrc2_approve`, the `expected_allowance` field provides an additional | ||
| conditional update mechanism. | ||
| - `expected_allowance` does not replace transaction deduplication and SHOULD | ||
| be evaluated only after deduplication checks have passed. | ||
|
|
||
| ### Client Considerations | ||
|
|
||
| - When the ledger does not process duplicate transactions, | ||
| **retrying an ICRC-2 call with identical parameters is safe** and will not | ||
| result in duplicated ledger effects. | ||
| - Clients MAY rely on this behavior to safely retry requests in the presence | ||
| of timeouts, transient failures, or uncertain outcomes. | ||
| - Clients SHOULD still be prepared to handle `Duplicate`/`TooOld`/`CreatedInFuture` errors | ||
| and SHOULD avoid modifying parameters when retrying unless a new | ||
| transaction is intended. | ||
|
|
||
|
|
||
|
|
||
| ## 2. Error Semantics and Atomicity | ||
|
|
||
| ### Clarification | ||
|
|
||
| The ICRC-2 specification does not explicitly state that `icrc2_approve` and | ||
| `icrc2_transfer_from` are atomic operations, nor does it clearly define which ledger | ||
| effects are guaranteed not to occur when an error is returned. | ||
|
|
||
| With the exception of `AllowanceChanged`, which explicitly states that no | ||
| allowance update has occurred, the semantics of other error variants are not | ||
| specified. | ||
|
|
||
| ### Advisory Guidance | ||
|
|
||
| To align with common ledger expectations and reduce ambiguity: | ||
|
|
||
| - Ledger implementations SHOULD ensure that `icrc2_approve` and `icrc2_transfer_from` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly to the ICRC-1 advisory, can we give an exhaustive list? |
||
| operations are atomic with respect to **externally observable ledger | ||
| effects**, such as: | ||
| - account balances, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: I'd use the following style consistently for enumerations (you can also omit the "and")":
|
||
| - allowances, | ||
| - and the transaction log. | ||
|
|
||
| - If an ICRC-2 method successfully applies such effects, the ledger SHOULD | ||
| return a success response (`Ok(nat)`). | ||
|
|
||
| - If an error response is returned, the ledger SHOULD ensure that: | ||
| - no balances or allowances have been modified, and | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ";" |
||
| - no transaction log entry corresponding to the call has been recorded. | ||
|
|
||
| This guidance does not restrict ledgers from mutating internal or auxiliary | ||
| state (e.g., caches, metrics, rate limits, or bookkeeping data) when handling | ||
| a call that results in an error. | ||
|
|
||
| Additionally: | ||
|
|
||
| - Ledger implementations SHOULD document the meaning of each error variant. | ||
| - Client implementations SHOULD NOT assume stronger guarantees than those | ||
| described above unless explicitly documented by the ledger. | ||
|
|
||
| ## 3. Fees for `icrc2_approve` and `icrc2_transfer_from` | ||
|
|
||
| ### Clarification | ||
|
|
||
| The ICRC-2 specification does not explicitly state the fees charged for | ||
| `icrc2_approve` or `icrc2_transfer_from`. | ||
|
|
||
| This advisory clarifies that these operations are expected to be charged | ||
| the same fee returned by the `icrc1_fee` method. | ||
|
|
||
| ### Advisory Guidance | ||
|
|
||
| - Ledger implementations SHOULD charge the fee returned by `icrc1_fee` for | ||
| both `icrc2_approve` and `icrc2_transfer_from`. | ||
| - Clients MAY assume that the applicable fee for ICRC-2 operations is the | ||
| value returned by `icrc1_fee`. | ||
| - Fees SHOULD only be charged when the operation succeeds. | ||
| - The fee SHOULD be applied in a manner consistent with ICRC-1 transfers. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no summary here, even though there is one for the ICRC-1 advisory. |
||
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 paragraph suggests that the list are examples of externally-observable ledger effects. Can we give an exhaustive list so that it is clearer, essentially giving a clear definition of externally-observable ledger effects? Same for the ICRC-2 guidance.