spec(XLS-65): Update VaultClawback failure conditions and state changes - #554
spec(XLS-65): Update VaultClawback failure conditions and state changes#554Tapanito wants to merge 36 commits into
Conversation
…tion - Add Example JSON sections for Vault ledger entry and all transactions (VaultCreate, VaultSet, VaultDelete, VaultDeposit, VaultWithdraw, VaultClawback, Payment) with real transaction data - Add invariants for the Vault ledger entry (universal checks) and all transaction types derived from the ValidVault invariant checker - Restructure section 10 from "API" to "RPC: vault_info" matching the amendment template format with Request Fields, Response Fields, Failure Conditions, Example Request, and Example Response subsections - Update response fields table with missing fields (Data, Asset.mpt_issuance_id, shares.DomainID, shares.MPTokenMetadata) and correct Always Present values - Update response examples to use proper JSON format with response envelope - Add section 9.1 Fields for Payment transaction - Remove Index section and all Return to Index links Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reorganize top-level sections: Abstract (1), Introduction (2), Specification (3), Rationale (4), Security Considerations (5), Appendix - Move all ledger entry, transaction, and RPC sections under "3. Specification" as subsections (3.1-3.9) - Remove "1.1 Overview" heading, merge content into Introduction body - Renumber Introduction subsections: Terminology (2.1), Actors (2.2), Connecting to the Vault (2.3) - Demote all specification headings by one level with new numbering - Add Rationale section explaining decoupled vault design - Rename FAQ section to "Appendix A: FAQ" with A.x numbering - Fix heading levels for Key Variables and Vault State Update Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove functional additions (invariants, example JSONs, error codes) added in this branch and retain only structural changes that bring the spec into conformance with AMENDMENT_TEMPLATE.md and XLS_TEMPLATE.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Convert failure conditions and state changes from numbered lists back to master's original nested bullet-point format. Keep the Data Verification / Protocol-Level Failures subsection headers as template compliance, but use master's original content and structure inside them. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Convert bullet points in Failure Conditions and State Changes sections to numbered lists with nested sub-numbering, per template requirements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Mayukha Vadari <mvadari@gmail.com>
…-clawback # Conflicts: # XLS-0065-single-asset-vault/README.md
tyalymov
left a comment
tyalymov
left a comment
There was a problem hiding this comment.
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?
3.7.3 now branches on the asset of Amount (vault share vs Vault.Asset), matching 3.7.2.2 and the implementation (VaultClawback.cpp branches on amount.asset()), instead of "who submitted the transaction" — which is ambiguous when the vault owner is also the asset issuer. Addresses review feedback on PR #554.
|
Agreed — checked |
| 1. The `Issuer` account is not the submitter of the transaction. | ||
| 2. If the `AccountRoot(Issuer)` object does not have `lsfAllowTrustLineClawback` flag set (the asset does not support clawback). | ||
| 3. If the `AccountRoot(Issuer)` has the `lsfNoFreeze` flag set (the asset cannot be frozen). | ||
| 4. `Vault.Asset` is not `XRP`, the issuer of `Vault.Asset` is the vault owner, and no `Amount` is specified (ambiguous clawback target). (`tecWRONG_ASSET`) |
There was a problem hiding this comment.
This check runs first in the code, not fourth. VaultClawback.cpp:101-104 rejects the ambiguous case before either branch is evaluated, and the branches start at :113 and :158. 3.7.3 in this PR already describes it that way ("3.7.2.2 item 4 rejects the ambiguous case ... before 3.7.3 is ever reached"), so moving it to position 2 would make the list agree with the code and with your own text.
| 5. The computed asset or share amount to clawback is zero. (`tecPRECISION_LOSS`) | ||
|
|
||
| 5. The `MPToken` object for the `Vault.ShareMPTID` of the `Holder` `AccountRoot` does not exist OR `MPToken.MPTAmount == 0`. | ||
| 6. Arithmetic overflow during share/asset calculation. (`tecPATH_DRY`) |
There was a problem hiding this comment.
One more failure isn't covered: if Amount is neither the vault share nor Vault.Asset, preclaim falls through to tecWRONG_ASSET at VaultClawback.cpp:218. #553 spells out the equivalent case as its item 2, so adding it here would keep the two sections symmetric.
2 participants
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:temMALFORMED)temBAD_AMOUNT)temMALFORMED)Protocol-Level Failures (3.7.2.2) — significantly reworked:
lsfMPTCanLock— the code checkslsfMPTCanClawback. Corrected.tecWRONG_ASSET)tecNO_PERMISSION)tecPRECISION_LOSSwhen computed amount rounds to zerotecPATH_DRYfor arithmetic overflowState Changes (3.7.3) — rewritten: