Add ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees #205

bogwar · 2026-01-03T14:19:31Z

Add ICRC-1, ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees

This PR adds an ADVISORY.md document next to standards/ICRC-2/README.md to clarify several aspects of the ICRC-2 standard that were previously underspecified, and raised within a security review.

The advisory is non-normative and does not change the ICRC-2 interface or compliance requirements. Its purpose is to reduce ambiguity for ledger implementers and client developers by making certain expectations explicit.

Specifically, the advisory clarifies:

Transaction deduplication for approve and transfer_from, including that retrying calls with identical parameters is safe when deduplication is implemented -- SECFIND-1088
Error semantics and atomicity, limited to externally observable ledger effects (balances, allowances, transaction log) -- SECFIND-1089
Fees for ICRC-2 operations, specifying that approve and transfer_from are expected to charge the fee returned by icrc1_fee -- SECFIND-1090

The advisory documents common expectations and best practices without retroactively invalidating existing implementations.

Copilot

Pull request overview

This PR adds a non-normative advisory document (ADVISORY.md) to clarify underspecified aspects of the ICRC-2 standard that were identified during a security review. The advisory provides explicit guidance to reduce ambiguity for ledger implementers and client developers without changing compliance requirements.

Clarifies transaction deduplication behavior for approve and transfer_from operations
Defines error semantics and atomicity expectations for externally observable ledger effects
Specifies fee charging expectations for ICRC-2 operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tmu0

LGTM overall 🚀

Just two minor remarks:

In (1) I would use more consistent wording for duplicates (see suggestions). In Transaction Identity we define what a duplicate is, independently of ledger behaviour. The ledger can then decide to either process a duplicate or not.

Nit: In the whole document I would include the ICRC prefix in the method names and call them icrc2_approve and icrc2_transfer_from.

standards/ICRC-2/ADVISORY.md

Co-authored-by: tmu0 <[email protected]>

bogwar · 2026-01-07T09:41:44Z

Thanks @tmu0! I've accepted your suggestions. For completeness, I've added an advisory for ICRC-1 that parallels the text on errors/atomicity expectations.

dietersommer

Good and nice-to-read content overall.

My comments mainly concern:

editorial aspects;
style improvements; and
making formulations more crisp to support their intention.

Please address at your discretion.

dietersommer · 2026-01-08T14:37:09Z

standards/ICRC-1/ADVISORY.md

+  the corresponding transaction log entry have been applied.
+
+- An error response SHOULD imply that:
+  - no account balances have been modified, and


I'd use a semicolon here following the more prevalent opinion on style.

dietersommer · 2026-01-08T14:39:11Z

standards/ICRC-2/ADVISORY.md

+  - The ledger MUST NOT apply the transaction effects again.
+
+
+


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.

dietersommer · 2026-01-08T14:40:57Z

standards/ICRC-2/ADVISORY.md

+- Ledger implementations SHOULD ensure that `icrc2_approve` and `icrc2_transfer_from`
+  operations are atomic with respect to **externally observable ledger
+  effects**, such as:
+  - account balances,


Style: I'd use the following style consistently for enumerations (you can also omit the "and")":

account balances;

allowances; and

the transaction log.

dietersommer · 2026-01-08T14:41:12Z

standards/ICRC-2/ADVISORY.md

+  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


dietersommer · 2026-01-08T14:43:54Z

standards/ICRC-2/ADVISORY.md

+
+#### Transaction Identity
+
+- A transaction is identified by the combination of:


Maybe make it a little clearer and relate to the below-used term "transaction identity":
"A transaction is identified by its transaction identity, which is the combination of:"

dietersommer · 2026-01-08T15:00:39Z

standards/ICRC-1/ADVISORY.md

+
+In particular:
+
+- A successful response (`Ok(nat)`) SHOULD imply that all balance updates and


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).

dietersommer · 2026-01-08T15:01:40Z

standards/ICRC-1/ADVISORY.md

+  - 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).


maybe add ", or internal logs"

dietersommer · 2026-01-08T15:04:54Z

standards/ICRC-1/ADVISORY.md

+
+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.


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.
Resulting in no effects would imply resulting in no partial effects.

dietersommer · 2026-01-08T15:06:23Z

standards/ICRC-2/ADVISORY.md

+  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.
+


There is no summary here, even though there is one for the ICRC-1 advisory.

dietersommer · 2026-01-08T15:09:55Z

standards/ICRC-2/ADVISORY.md

+
+To align with common ledger expectations and reduce ambiguity:
+
+- Ledger implementations SHOULD ensure that `icrc2_approve` and `icrc2_transfer_from`


Similarly to the ICRC-1 advisory, can we give an exhaustive list?

dietersommer

Update: Once my comments have been considered at your discretion, consider the PR approved.

bogwar added 6 commits January 3, 2026 13:22

initial empty advisory.md

16dd618

advisory on deduplication

1abc199

stage changes & client considerations

ef883d5

error ==> no ledger/balances changes; any change ==> Ok()

ef74c58

section on fees

14b5b18

removed security review references, as irrelevant

0bae797

bogwar requested a review from Copilot January 3, 2026 14:19

Copilot started reviewing on behalf of bogwar January 3, 2026 14:20 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes

gregorydemay requested a review from tmu0 January 5, 2026 07:07

tmu0 reviewed Jan 5, 2026

View reviewed changes

bogwar and others added 5 commits January 7, 2026 10:16

Update standards/ICRC-2/ADVISORY.md

db81fa3

Co-authored-by: tmu0 <[email protected]>

Apply suggestions from code review

e9e87c7

Co-authored-by: tmu0 <[email protected]>

added icrc2 prefix to methods

575b6fc

advisory for ICRC-1

68214e8

parallel text to the one for ICRC-2

2855323

bogwar assigned sea-snake and dietersommer Jan 7, 2026

dietersommer reviewed Jan 8, 2026

View reviewed changes

dietersommer approved these changes Jan 8, 2026

View reviewed changes


		#### Transaction Identity

		- A transaction is identified by the combination of:


		In particular:

		- A successful response (`Ok(nat)`) SHOULD imply that all balance updates and


		To align with common ledger expectations and reduce ambiguity:

		- Ledger implementations SHOULD ensure that `icrc2_approve` and `icrc2_transfer_from`

Add ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees #205

Are you sure you want to change the base?

Add ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees #205

Uh oh!

Conversation

bogwar commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add ICRC-1, ICRC-2 Advisory Clarifying Deduplication, Error Semantics, and Fees

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tmu0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bogwar commented Jan 7, 2026

Uh oh!

dietersommer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dietersommer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bogwar commented Jan 3, 2026 •

edited

Loading