spec(XLS-65): Update VaultCreate failure conditions and state changes - #549
spec(XLS-65): Update VaultCreate failure conditions and state changes#549Tapanito wants to merge 42 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>
…-create # Conflicts: # XLS-0065-single-asset-vault/README.md
tyalymov
left a comment
tyalymov
left a comment
There was a problem hiding this comment.
This PR syncs the spec with the current VaultCreate code, so I want to flag one thing. The order of the failure conditions in 3.2.5.2 differs from the evaluation order in VaultCreate.cpp, and that order is observable: when several conditions hold at once, the transactor returns the first one it hits.
Order in the code:
preclaim (checks run in this order):
- canAddHolding — can the vault's pseudo-account hold this asset at all:
IOU: terNO_ACCOUNT (issuer account missing),
then terNO_RIPPLE (issuer has no DefaultRipple)
MPT: tecOBJECT_NOT_FOUND (no MPTokenIssuance),
then tecNO_AUTH (token not transferable) - tecWRONG_ASSET (issuer is a pseudo-account)
- freeze/lock: tecFROZEN (IOU) / tecLOCKED (MPT)
- domain missing: tecOBJECT_NOT_FOUND
- terADDRESS_COLLISION
doApply (runs after all of preclaim):
6. tecINSUFFICIENT_RESERVE
Two places where the spec list disagrees:
tecWRONG_ASSET vs freeze/lock. The spec puts the freeze/lock checks (1.3 and 2.3) before tecWRONG_ASSET (item 3), but the code checks the pseudo-account issuer first. So for an asset whose issuer is a pseudo-account and is also frozen, the spec implies tecFROZEN/tecLOCKED while the code returns tecWRONG_ASSET.
tecINSUFFICIENT_RESERVE vs terADDRESS_COLLISION. The spec lists reserve (item 5) before address collision (item 6). In the code the collision check is in preclaim and the reserve check is in doApply, which runs later, so the order is reversed.
Is the numbered list meant to be the evaluation order? If so, can we move items 3 and 5/6 to match preclaim and doApply? If it is just a list of conditions with no order implied, a one-line note saying so would help, so nobody reads a precedence into it.
3.2.5.2 now follows the actual preclaim/doApply evaluation order: tecWRONG_ASSET is checked before freeze/lock, and terADDRESS_COLLISION (preclaim) before tecINSUFFICIENT_RESERVE (doApply, runs later). Addresses review feedback on PR #549.
|
Good catch, and yes — the list is meant to reflect evaluation order. Verified against |
Add 65.1/65.2 patch folders so this PR merges under LendingProtocolV1_1.
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
Spec-doc only change (XLS-65 VaultCreate section) syncing README with VaultCreate.cpp. The bulk of the failure-condition and state-change updates are internally consistent with the MR description. One naming inconsistency stands out: the new §3.2.6.1 subsection references an amendment fixCleanup3_2_0 that doesn't match the fixCleanup3_4_0 amendment introduced elsewhere in this same diff (65.2/README.md, Appendix C changelog), and is not documented anywhere else in the changeset.
There was a problem hiding this comment.
Documentation-only change syncing the VaultCreate spec (Data Verification, Protocol-Level Failures, State Changes, Invariants) with the current implementation, plus a new XLS-65.1 sub-spec for closed-ended vault lifecycle fields. Cross-checked the new/updated sections against each other and found them internally consistent (failure codes match, moved checks line up, state-change steps match the MR description) except for one gap: a newly introduced amendment gate is not registered in the new canonical amendments/changelog lists added in this same diff.
There was a problem hiding this comment.
This is a documentation-only sync of the VaultCreate spec (Data Verification, Protocol-Level Failures, State Changes) with the current implementation, plus a large, mostly unrelated addition for a new draft extension (XLS-65.1, LendingProtocolV1_1 closed-ended vault lifecycle) and a fixCleanup3_2_0 sub-section. The core VaultCreate failure-condition rewrite is internally consistent: items removed from 3.2.5.2 reappear correctly in 3.2.5.1, error codes are added throughout, and the closed-ended vault invariants (VaultKind/SubscriptionDate/RedemptionDate presence rules) are enforced consistently between the main README and the new 65.1/README.md. The one thing worth double-checking before merge is the fixCleanup3_2_0/sfReferenceHolding addition, which isn't mentioned anywhere in the MR title/description and isn't tied to a field definition elsewhere in this document — worth confirming it's intentionally in scope for this MR rather than accidentally bundled from another change.
| | `ShareMPTID` | No | Yes | `number` | `UINT192` | 0 | The identifier of the share MPTokenIssuance object. | | ||
| | `WithdrawalPolicy` | No | Yes | `string` | `UINT8` | `N/A` | Indicates the withdrawal strategy used by the Vault. | | ||
| | `Scale` | No | Yes | `number` | `UINT8` | 6 | The `Scale` specifies the power of 10 ($10^{\text{scale}}$) to multiply an asset's value by when converting it into an integer-based number of shares. | | ||
| | `LEVersion` | Yes | No | `number` | `UINT8` | absent/`0` | Protocol-written vault schema version. Immutable. Absent or `0` is legacy. Set to `1` (`CashBasis`) on create when `LendingProtocolV1_1` is enabled. Not a `VaultCreate` field. | |
There was a problem hiding this comment.
Immutable fields have Modifiable?=Yes. Change to No for LEVersion, VaultKind, SubscriptionDate, RedemptionDate to match their descriptions.
| | `LEVersion` | Yes | No | `number` | `UINT8` | absent/`0` | Protocol-written vault schema version. Immutable. Absent or `0` is legacy. Set to `1` (`CashBasis`) on create when `LendingProtocolV1_1` is enabled. Not a `VaultCreate` field. | | |
| | `LEVersion` | No | No | `number` | `UINT8` | absent/`0` | Protocol-written vault schema version. Immutable. Absent or `0` is legacy. Set to `1` (`CashBasis`) on create when `LendingProtocolV1_1` is enabled. Not a `VaultCreate` field. | |
2 participants
Syncs the VaultCreate spec section with the current implementation in
src/libxrpl/tx/transactors/vault/VaultCreate.cpp.Data Verification (3.2.5.1) — was
_TBD_, now populated:Protocol-Level Failures (3.2.5.2) — reorganised and extended:
terNO_ACCOUNT,terNO_RIPPLEfor IOU issuer checks (fromcanAddHolding)tecWRONG_ASSETfor pseudo-account issuerstecLOCKEDfor MPT lock (global or per-account, viaisFrozen)terADDRESS_COLLISIONfor pseudo-account address collisionState Changes (3.2.6) — extended:
tfVaultShareNonTransferableandtfVaultPrivate