Skip to content

feat(icrc-ledger-types): add ICRC-123 block schema validators#9981

Draft
bogwar wants to merge 1 commit intomasterfrom
icrc-123-1-types
Draft

feat(icrc-ledger-types): add ICRC-123 block schema validators#9981
bogwar wants to merge 1 commit intomasterfrom
icrc-123-1-types

Conversation

@bogwar
Copy link
Copy Markdown
Contributor

@bogwar bogwar commented Apr 22, 2026

Phase 1 of ICRC-123/153 implementation. Adds ICRC-123 block schema validators for 4 block types with permissive and strict validation modes.

Copy link
Copy Markdown
Contributor

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bogwar I mostly have some understanding questions but otherwise code LGTM

Comment thread packages/icrc-ledger-types/src/icrc123/schema.rs
Comment thread packages/icrc-ledger-types/src/icrc123/schema.rs Outdated
/// * `btype` – block type identifier
/// * `strict` – when `true`, `caller`, `mthd`, and `ts` (in tx) are required (ICRC-153);
/// when `false` they are optional (ICRC-123).
fn account_block_validator(btype: &'static str, strict: bool) -> ValuePredicate {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this seems to be almost the same as principal_block_validator. If you want to keep the expressivity (2 different method names) I would refactor so that they delegate to a common internal method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two functions differ only in the required tx field (account with is_account() vs principal with is_principal()). I could extract a common helper that takes the field name and predicate as parameters, but it would add indirection for very little deduplication — the outer block structure (phash, btype, ts, tx) is the same but the tx field predicates are different. Happy to refactor if you feel strongly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On reflection, I'd rather keep them separate. The deduplication gain is minimal (one line differs) and a shared helper with field name + predicate parameters would be harder to read. Keeping them explicit makes it clear at a glance what each block type expects.

/// tx: { account: Account, caller?: Principal, mthd?: Text, ts?: Nat,
/// reason?: Text, policy_ref?: Text } }
/// ```
pub fn validate_freeze_account(block: &Value) -> Result<(), ValuePredicateFailures> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understanding question: the ICRC number appears in certain methods, like validate_153_freeze_account but not all, like this one (validate_freeze_account). Is there a reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point — renamed all permissive validators to include the standard number: validate_123_freeze_account, validate_123_unfreeze_account, etc. The convention is now: validate_123_* = permissive (ICRC-123 block standard), validate_153_* = strict (ICRC-153 endpoint standard). Created DEFI-2791 to apply the same rename to the ICRC-122 module (validate_mintvalidate_122_mint).

Comment thread packages/icrc-ledger-types/src/icrc123/schema.rs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants