spec(XLS-65): Update VaultClawback failure conditions and state changes

[Tapanito]

Syncs the VaultClawback spec section with the current implementation in src/libxrpl/tx/transactors/vault/VaultClawback.cpp.

Data Verification (3.7.2.1) — was _None._, now has three items:

• VaultID = 0 (temMALFORMED)
• Amount < 0 (temBAD_AMOUNT)
• Amount specifies XRP (temMALFORMED)

Protocol-Level Failures (3.7.2.2) — significantly reworked:

• Bug fix: item 4.3 said lsfMPTCanLock — the code checks lsfMPTCanClawback. Corrected.
• Added: vault owner share-burn case (burn stranded shares when vault has no assets) as a distinct top-level case with its own sub-conditions
• Added: ambiguous target condition — when issuer == vault owner and no Amount is specified (tecWRONG_ASSET)
• Added: holder == submitter check for asset clawback (tecNO_PERMISSION)
• Added: tecPRECISION_LOSS when computed amount rounds to zero
• Added: tecPATH_DRY for arithmetic overflow
• Added error codes to all items

State Changes (3.7.3) — rewritten:

• Split into share-burn case and asset-clawback case
• Fixed terminology: "depositor" → "Holder"
• Added: share MPToken deletion when Holder balance reaches zero (for non-owners)
• Clarified that asset clawback sends recovered assets to the Issuer

[tyalymov]

3.7.3 State Changes branches on who submits ("if the submitter is the vault owner" / "if the submitter is the asset issuer"), but 3.7.2.2 Failure Conditions and the code (VaultClawback.cpp) branch on the asset of Amount (shares vs Vault.Asset). The two agree in the normal case, but when the vault owner is also the asset issuer, both submitter conditions are true and 3.7.3 no longer says which branch applies. The code resolves this by amount.asset(), and it rejects the owner-is-issuer case when Amount is omitted. Could we reword 3.7.3 to branch on the asset of Amount, matching 3.7.2.2 and the implementation?

[Tapanito]

Agreed — checked VaultClawback.cpp: both preclaim and doApply branch on amount.asset() (shares vs Vault.Asset), not on who the submitter is. The owner-is-issuer edge case is exactly why: Amount disambiguates it, and if Amount is omitted while owner == issuer, preclaim already rejects it as tecWRONG_ASSET (3.7.2.2 item 4) before 3.7.3 is ever reached. Reworded 3.7.3's branch conditions from "if the submitter is the vault owner" / "if the submitter is the asset Issuer" to "if the Amount asset is the vault share" / "if the Amount asset is Vault.Asset", matching 3.7.2.2's phrasing.

[xrplf-ai-reviewer]

Duplicate PR reference in merge buckets — see inline.

[xrplf-ai-reviewer]

This MR is a documentation-only change to the XLS-65 spec (VaultClawback failure conditions/state changes) plus a new XLS-65.2 sub-spec. I read through the reworked 3.7.2.1/3.7.2.2/3.7.3 sections and the new 65.2/README.md for internal consistency (numbering cross-references, error-code alignment with the described conditions, and consistency between failure conditions and the corresponding state-change/invariant sections). Everything cross-checks cleanly: the 'item 4'/'item 5' back-references in 3.7.3 point to the correct share-burn and asset-clawback conditions in 3.7.2.2, the described lsfMPTCanLock→lsfMPTCanClawback correction is reflected correctly, the pseudo-account/precision/overflow additions in 65.2 map onto the corresponding items in the main spec, and the invariants match the new state-change list. No correctness, security, or consistency problems worth flagging were found in the added lines.

[xrplf-ai-reviewer]

This is a documentation-only spec change (XLS-65 VaultClawback) that syncs the spec with the current implementation and adds a new 65.2 amendment sub-spec. The content is well-structured and internally consistent; no code is changed. No correctness, security, or logic issues found in the added prose — normal review categories (bugs, security, resource leaks, etc.) largely don't apply to this markdown-only diff.

[xrplf-ai-reviewer]

This is a documentation-only sync of the VaultClawback spec (main README + new XLS-65.2 patch doc) with the current/planned implementation. The content is internally consistent between the two files (error codes, condition ordering, and state-change descriptions match), and no code was changed, so there are no security/correctness code issues to flag. The one confirmed defect is a section-numbering gap in the newly added patch document.

[xrplf-ai-reviewer]

Documentation-only change syncing the VaultClawback spec (main README) and adding a new XLS-65.2 amendment addendum with the fixCleanup3_4_0 changes. Cross-checked the reworked Data Verification, Protocol-Level Failures, State Changes, and Invariants sections against each other and against the new 65.2 sub-spec for internal consistency (item numbering, cross-references like "see 3.7.2.2 item 4/5", error codes, and relative markdown links). Everything is internally consistent and the previously-reported lsfMPTCanLock/lsfMPTCanClawback bug is correctly fixed. No implementation source file was included in this diff, so the accuracy of the spec against the actual C++ code could not be independently verified — that check would need to be done against src/libxrpl/tx/transactors/vault/VaultClawback.cpp directly. No correctness, security, or consistency issues found in the diff itself.

[tyalymov]

5.3.2 says "Increase the MPToken.MPTAmount of the Issuer MPToken for Vault.Asset". The issuer has no MPToken for its own issuance. Sending an MPT to the issuer burns it: the credit path decreases MPTokenIssuance.OutstandingAmount instead of touching a holder object. Suggested wording:

5.3.2 Decrease MPTokenIssuance(Vault.Asset).OutstandingAmount by Δ_asset.

The IOU branch in 5.2.1 already describes it this way (only the RippleState balance moves).

[Tapanito]

Fixed. doApply sends the recovered assets to the submitter, who this branch requires to be the asset issuer, and directSendNoFeeMPT decrements MPTokenIssuance.OutstandingAmount when the receiver is the issuer rather than touching a holder object. 5.3.2 now reads that the issuance OutstandingAmount decreases by Δ_asset.

[tyalymov]

Same as VaultCreate in #549: the VaultClawback transaction type is 70, not 63.

[Tapanito]

Fixed. transactions.macro has ttVAULT_CLAWBACK as 70. Updated the table.

[xrplf-ai-reviewer]

This is a documentation-only spec update syncing the VaultClawback section with the current implementation. The added text is largely internally consistent (numbering cross-references between 3.7.2.2 and 3.7.3 line up, and the new failure/state-change items match the description). One thing stands out as worth confirming before merge: the TransactionType default value for VaultClawback was changed from 63 to 70, which is not mentioned anywhere in the MR description (which only discusses failure conditions and state changes).

[mvadari]

@ai-review --aggressive

[Copilot]

🟡 Changes recommended

The nested specification is unsupported by publication tooling and does not conform to the required Amendment template structure.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates XLS-65’s VaultClawback specification to align with current amendment behavior.

Changes:

• Documents validation, failure conditions, state changes, and invariants.
• Adds an XLS-65.2 amendment record and changelog references.

File summaries

File	Review
XLS-0065-single-asset-vault/README.md	Nit: Rename Appendix C to Appendix B to avoid a numbering gap.
XLS-0065-single-asset-vault/65.2/README.md	Critical: Restructure the transaction section to conform to the Amendment template. Moderate: Use a supported top-level document or extend publication and validation tooling for nested specifications. Nit: Correct the amendment-gating description; nodes do not disagree before activation.

Review details

Suppressed comments (2)

XLS-0065-single-asset-vault/65.2/README.md:119

• should-fix: Amendment gating prevents this disagreement: before activation, amendment-aware nodes retain the legacy behavior, and activation changes the rules at the activation ledger. Saying nodes disagree until activation incorrectly describes a pre-activation consensus split.

The pseudo-account rejection, conversion changes, scale clamp, and non-zero-dust evaluation are gated on the amendment, so nodes disagree about affected transactions until it activates; the general arithmetic-overflow and zero-share failures remain ungated.

XLS-0065-single-asset-vault/README.md:1198

• nit: The document has Appendix A but no Appendix B, so introducing this as Appendix C creates a numbering gap. Rename the changelog to Appendix B unless a missing Appendix B is also being added.

## Appendix C: Changelog

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Tapanito]

This is a post-Final nested patch on XLS-65, not a top-level Amendment XLS. Nested 65.2 stays. The Amendment template (## 3. Transaction: … at H2) is for top-level Amendment XLSes; promoting Transaction: VaultClawback here would fight the agreed 65.2 patch layout. Parser/site tooling that only indexes root-level XLS-*/README.md is a known gap, not a reason to flatten this document.

[xrplf-ai-reviewer]

This is a documentation-only spec sync (XLS-65.2 addendum plus updates to the main XLS-65 README) that mirrors the current VaultClawback implementation. I cross-checked the failure-condition ordering, error codes, and state-change descriptions between the new 65.2 document and the modified sections of the main README, and they are internally consistent (same enumerated order, same tec/tem codes, same gating notes for fixCleanup3_4_0). The lsfMPTCanLock→lsfMPTCanClawback correction, the TransactionType value change, and the new share-burn vs asset-clawback split all read as coherent, deliberate corrections rather than introduced errors. No factual contradictions, broken cross-references, or logic inconsistencies were found in the added text.