XLS-64: Adds freeze semantics for pseudo-accounts - #568
Conversation
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
The new Freeze Handling section contains a couple of grammatical errors in table rows and an incorrect rule reference (“Rule 3” vs “Rule 5”) that could mislead implementers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the XLS-0064 Pseudo-Account specification to define explicit freeze/lock handling rules for protocols that move assets into/out of pseudo-accounts, and refreshes the document’s updated date.
Changes:
- Updates the preamble
updateddate to 2026-06-22. - Adds a new “Freeze Handling” subsection specifying deposit and withdrawal rejection rules (including MPT-specific considerations).
File summaries
| File | Description |
|---|---|
| XLS-0064-pseudo-account/README.md | Adds pseudo-account freeze/lock semantics guidance for deposits/withdrawals and updates the spec metadata date. |
Review details
Suppressed comments (1)
XLS-0064-pseudo-account/README.md:117
- The withdrawal freeze-condition row reads "...the underlying asset of the share if frozen / locked"; this is missing the verb and is grammatically unclear.
| If the asset is a Vault Share, the underlying asset of the share if frozen / locked | `tecLOCKED` |
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add XLS-64.1 so this PR merges under fixCleanup3_4_0, not a new patch number.
There was a problem hiding this comment.
This is a documentation-only change to the XLS-0064 pseudo-account spec, adding a new 'Freeze Handling' section and a 64.1 patch sub-document. The freeze rules for deposits and withdrawals are internally consistent (issuer bypass, self-withdrawal exemption, deep-freeze vs regular-freeze distinction, MPT lock equivalence to deep-freeze all check out against each other). No code is touched, so there are no functional/security bugs to flag. One minor documentation-structure item is worth a quick sanity check.
There was a problem hiding this comment.
This MR adds a new XLS-64.1 spec document detailing freeze/lock semantics for pseudo-account deposits and withdrawals, and updates the parent XLS-64 README with a matching summary, amendments entry, and changelog. The change is purely documentation/spec text — no executable code is touched. The two documents are internally consistent (deposit/withdrawal condition tables, ordering, issuer/self-withdrawal exemptions, and MPT-lock equivalence all match between the new 64.1 file and the README summary). The main issue found is a metadata inconsistency in the front-matter date.
There was a problem hiding this comment.
This is a documentation-only change adding a new XLS-64.1 patch spec and updating the base XLS-64 README to describe freeze/lock semantics for pseudo-account deposits and withdrawals. The two documents (the new 64.1/README.md and the amended top-level README.md) describe the same rules and are consistent with each other in terms of conditions, ordering, exemptions (issuer destination, self-withdrawal), and MPT lock/deep-freeze equivalence. No code, configuration, or infrastructure is touched, so the categories this review tooling targets (SQL/command injection, resource leaks, race conditions, IAM/K8s/Terraform misconfig, etc.) do not apply. I did not find any factual contradictions, missing exemption cases, or logic errors in the freeze/withdrawal rule tables between the two files.
There was a problem hiding this comment.
This is a documentation-only change adding an XLS-64.1 amendment record and corresponding freeze-semantics section to the XLS-64 pseudo-account spec. I cross-checked the new companion document (64.1/README.md) against the updated main README.md freeze-handling section for consistency in conditions, exemptions (issuer destination, self-withdrawal), deep-freeze vs regular-freeze treatment, and MPT lock equivalence — they align. No code, security-sensitive config, or broken relative links were found; the two new cross-references (./64.1/README.md and ../README.md) both resolve correctly given the files added/modified in this diff. No substantive issues to flag.
State the call-site amendment gate explicitly and add the missing test-plan and implementation sections to the patch.
There was a problem hiding this comment.
Documentation-only change adding a formal spec (XLS-64.1) and updates to XLS-64 for pseudo-account freeze semantics. The two documents (top-level README and new 64.1/README.md) are consistent with each other in the freeze rules described. One structural defect found: the new 64.1/README.md skips section number 5 in its heading numbering.
|
@ai-review --aggressive |
There was a problem hiding this comment.
🟡 Changes recommended
Transfer classification and evaluation ordering remain ambiguous for multi-asset and pseudo-account-to-pseudo-account operations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
XLS-0064-pseudo-account/64.1/README.md:34
- [blocking] The universal scope is ambiguous for pseudo-account-to-pseudo-account transfers.
LoanManagemovesDefaultCoveredfrom the LoanBroker pseudo-account to the Vault pseudo-account (XLS-0066-lending-protocol/README.md:1278-1287), so the same movement is both a withdrawal and a deposit. Applying only the withdrawal rules permits a regularly frozen destination, while also applying the deposit rules rejects it; the account-role and error-order mappings are likewise unclear. Specify whether both rule sets run and in which order, or explicitly narrow the scope to named transaction paths.
When the amendment is enabled, these rules take precedence over conflicting freeze or lock rules in a transaction specification that moves an asset into or out of a pseudo-account, including XLS-30, XLS-65, and XLS-66. Transaction-specific checks continue to apply and can reject a transaction before or after these checks pass; an “allowed” result in this section therefore means that these checks pass, not that the complete transaction must succeed. When the amendment is not enabled, the pre-amendment, transaction-specific checks recorded in the parent specification continue to apply.
XLS-0064-pseudo-account/README.md:101
- [blocking] “The asset” is under-specified for two-asset AMM operations. XLS-30 permits
AMMDepositandAMMWithdrawto move both pool assets (XLS-0030-automated-market-maker/README.md:81,330-333,544-547), but this section orders only the conditions, not evaluation across assets. When one asset is MPT-locked and the other IOU-frozen, implementations can return different codes. Define a canonical asset order or condition-first ordering across every transferred asset.
Throughout this section, an asset is _globally frozen_ when its issuance is frozen or locked, and an account is _locally frozen_ for an asset when it is individually frozen for that asset. A local freeze is a regular freeze; deep freeze is called out explicitly where it applies. For a Multi-Purpose Token, `lsfMPTLocked` on the `MPTokenIssuance` is a global freeze/lock of the issuance, and `lsfMPTLocked` on the holder's `MPToken` satisfies both a local-freeze check and a deep-freeze check for that holder, so the “locally frozen” rows include a locked MPT holder and return `tecLOCKED`. The checks operate on the asset transferred into or out of the pseudo-account; for a Vault deposit or withdrawal, this is the Vault's underlying asset, not its Vault Share.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
This is a documentation-only MR adding a new XLS-64.1 spec for pseudo-account freeze semantics and updating the parent XLS-64 README with matching content. No code is changed, and the content is well-specified, internally consistent, and cross-referenced (rules for deposit/withdrawal freeze checks, MPT lock equivalence, issuer/self-withdrawal exemptions). No correctness, security, or infrastructure issues found in this diff.
The bold patch-title prefixes were leftover formatting.
There was a problem hiding this comment.
This MR adds a new XLS-64.1 specification document and updates the parent XLS-64 README to describe freeze/lock semantics for pseudo-accounts. It is purely documentation/specification content (no executable code), so there are no correctness, security, or infrastructure issues to flag under the review criteria. The specification text is internally consistent between the new sub-spec and the parent README updates.
There was a problem hiding this comment.
🟡 Changes recommended
The pre-amendment AMM freeze behavior remains insufficiently specified for implementation or ledger replay.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
Tapanito
changed the title
That paragraph belongs on XLS-64.1, which already has its own Rationale section.
There was a problem hiding this comment.
This is a documentation-only change adding a new XLS-64.1 sub-specification (freeze/lock semantics for pseudo-account deposits and withdrawals) and updating the parent XLS-64 README to reference and summarize it. The rule tables, ordering, and exemptions (issuer destination, self-withdrawal) are internally consistent between the two documents and the prose explanations line up with the tables (e.g., MPT lock/deep-freeze equivalence, self-withdrawal exemption limited to regular freeze). The main concern is that the full rule tables and supporting prose are duplicated verbatim in both README.md and 64.1/README.md rather than the parent linking to/summarizing the sub-spec, which risks the two documents silently diverging on future edits.
There was a problem hiding this comment.
🟡 Changes recommended
The new Vault behavior permits deposits to credit locked share holdings, conflicting with established MPT lock semantics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
The template requires a parent Rationale; this one is about the pseudo-account itself. The freeze tables still apply to the transferred asset, and VaultDeposit/VaultWithdraw still apply their XLS-65 share checks.
There was a problem hiding this comment.
This is a documentation-only change (new XLS-64.1 spec plus updates to the parent XLS-64 README) describing freeze/lock semantics for pseudo-accounts. No code is included, so the usual bug/security categories don't apply. The spec text is internally consistent between the two documents; only minor documentation-consistency concerns are worth flagging.
There was a problem hiding this comment.
🟡 Changes recommended
The parent specification incorrectly attributes locked-MPT self-withdrawal rejection to Rule 3 instead of Rule 4.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
This PR is documentation-only (two XLS spec README files: a new XLS-64.1 draft and updates to the parent XLS-64 README). I cross-checked the freeze-condition tables and rationale text duplicated between the new 64.1/README.md and the parent README.md for consistency (a common source of copy-paste drift), and they match in ordering, conditions, error codes, and the issuer/self-withdrawal exemptions. The described freeze semantics (regular freeze blocks sending except to issuer; deep freeze blocks receiving; MPT lock conflating both) are internally consistent with real XRPL freeze mechanics. No security, correctness, or infrastructure issues apply since no code is changed — only a very minor, non-actionable wording inconsistency exists (the deposit table's 'evaluated in order' phrase is present in 64.1 but omitted in the parent README's deposit list), which doesn't affect correctness since the three deposit conditions aren't order-dependent. No issues worth flagging.
High Level Overview of Change
Context of Change
Type of Change