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

[Tapanito]

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:

• Data > 256 bytes
• Invalid WithdrawalPolicy value
• DomainID = 0
• DomainID without tfVaultPrivate
• Negative AssetsMaximum
• Empty or oversized MPTokenMetadata
• Scale with XRP or MPT asset
• Scale > 18 for IOU asset

Protocol-Level Failures (3.2.5.2) — reorganised and extended:

• Removed preflight checks (moved to Data Verification above)
• Added terNO_ACCOUNT, terNO_RIPPLE for IOU issuer checks (from canAddHolding)
• Added tecWRONG_ASSET for pseudo-account issuers
• Added tecLOCKED for MPT lock (global or per-account, via isFrozen)
• Added terADDRESS_COLLISION for pseudo-account address collision
• Added error codes to all items

State Changes (3.2.6) — extended:

• MPTokenIssuance flags based on tfVaultShareNonTransferable and tfVaultPrivate
• Authorized MPToken for pseudo-account when vault is private
• XRP case: no holding object created
• OwnerCount incremented by 2

[xrplf-ai-reviewer]

This is a documentation-only sync of the VaultCreate spec with the current C++ implementation. The added Data Verification, Protocol-Level Failures, and State Changes content is largely internally consistent and matches the MR description. Two things worth a second look: an internal inconsistency between the MPTokenIssuance auth-flag condition and the holder MPToken auth-flag condition in the new State Changes text, and an unexplained renumbering of all Vault transaction type codes (58-63 → 65-70) that isn't mentioned anywhere in the MR description.

[Copilot]

🟡 Changes recommended

The updated reserve/OwnerCount statements for VaultCreate appear internally inconsistent with the spec’s own state changes and XLS-0033’s stated reserve costs for MPToken objects.

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

Review details

Suppressed comments (1)

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

• Step 1 says the Vault Owner’s OwnerCount increases by 2 “for the Vault object and the pseudo-account”, but step 2.4 also creates an MPToken owned by the Vault Owner. Per XLS-0033, each MPToken is an owner-directory object and costs an incremental reserve to its owner (XLS-0033-multi-purpose-tokens/README.md:338-340), so this OwnerCount statement should be updated (or the spec should explain a reserve exemption).

1. Create a new `Vault` ledger object, linked into the `Vault.Owner`'s `DirectoryNode`. Increment the Vault Owner's `OwnerCount` by 2 (for the `Vault` object and the _pseudo-account_).

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[xrplf-ai-reviewer]

This is a documentation-only sync of the VaultCreate spec section with its C++ implementation. I checked the Data Verification, Protocol-Level Failures, and State Changes sections against the MR description and against each other for internal consistency (numbering, error codes, OwnerCount arithmetic, TransactionType values across the file). Everything lines up: OwnerCount increments (2 in state changes + 1 for the owner's MPToken = 3, matching §3.2.4), the new error codes match the described conditions, and the TransactionType renumbering (65-70) is sequential and consistent across all Vault transactions in the file. I did not find any concrete, changed-line defects worth blocking on; no source code was included in this diff to cross-check the new failure-condition claims against, so I limited findings to internal doc consistency, which held up.

[Copilot]

🔵 Needs a closer look

The Vault ledger entry table documents Flags as a JSON string, which contradicts the numeric bitmask representation used elsewhere and should be corrected for implementer accuracy.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

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

• In the Vault ledger entry field table, Flags is listed as JSON type string, but flags are encoded as a numeric bitmask in JSON (consistent with this spec’s transaction tables and other ledger entry tables, e.g. XLS-0066-lending-protocol/README.md:141). This mismatch makes the JSON representation ambiguous for implementers.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[xrplf-ai-reviewer]

This is a documentation-only spec update (XLS-65 README) syncing text/tables with the VaultCreate implementation. No source code is changed, and the content is internally consistent (transaction type numbers renumbered consistently across VaultCreate/Set/Delete/Deposit/Withdraw/Clawback, new failure conditions and state changes align with the described amendment behavior). No actionable issues found.

[xrplf-ai-reviewer]

This is a documentation-only spec update (XLS-0065-single-asset-vault/README.md) syncing the VaultCreate section with the current VaultCreate.cpp implementation. I checked the new Data Verification, Protocol-Level Failures, and State Changes content for internal consistency against the rest of the document (field tables, flag tables, OwnerCount math, Scale/WithdrawalPolicy defaults, transaction type renumbering) and found it self-consistent: the OwnerCount breakdown (2 in step 1 + 1 for the owner's MPToken = 3, matching 3.2.4's 'three increments'), the Scale/temMALFORMED rules match the field table notes, and the renumbered TransactionType values (65-70) are sequential and match the XLS-65 designation. No code files are part of this diff, so there is nothing to review for security/correctness bugs in implementation logic — only prose/spec text changed.

[Copilot]

🟡 Changes recommended

The updated reserve/OwnerCount accounting for VaultCreate appears incomplete/unclear relative to reserves implied by pseudo-account-owned objects created during the transaction (e.g., share MPTokenIssuance and potential holdings).

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

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[xrplf-ai-reviewer]

Documentation-only sync of the VaultCreate spec section with the current implementation. The content update itself (Data Verification, Protocol-Level Failures, State Changes, Invariants) reads as internally consistent and well cross-referenced with the rest of the document. Two things stood out worth double-checking: an unexplained batch renumbering of TransactionType values across all six Vault transactions (only VaultCreate's failure/state sections were supposed to change per the MR description), and a flag-setting rule for the Vault Owner's share MPToken that conflates two seemingly unrelated conditions (private and non-transferable) under the same lsfMPTAuthorized flag.

[Copilot]

🔵 Needs a closer look

The Vault ledger-entry field table documents Flags as a JSON string even though it is a UINT32 and represented as a JSON number, which can mislead implementers.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

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

• Vault.Flags is defined as UINT32 but the ledger-entry table lists its JSON type as string. In XRPL JSON (and in this spec’s transaction tables), Flags is a numeric field; documenting it as a string can lead to incorrect client serialization/parsing.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[xrplf-ai-reviewer]

This is a documentation-only spec update syncing the VaultCreate section (and related transaction type numbers) with the current implementation. I checked cross-references between the field tables, failure-condition lists, state-change steps, and the new fixCleanup3_2_0 amendment section for internal consistency (e.g., OwnerCount increments in 3.2.4 vs 3.2.6 sum correctly to 3, the Scale field rules in 3.2.1/3.1.2/3.2.5.1 agree, the TransactionType renumbering is a consistent sequential shift, and the new failure codes match the described conditions). Everything checks out internally — I did not find any correctness issues, broken cross-references, or contradictions introduced by this diff, so I have no actionable findings to raise.

[Copilot]

🔵 Needs a closer look

The updated spec introduces (or exposes) internal normative ambiguities/contradictions around Scale semantics and MPToken creation timing that should be resolved to avoid divergent implementations.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

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

• Vault.Scale is described as configurable for IOU vaults with a range of 0�0�18 and a default of 6 (§3.1.6.1.1), but the updated Vault ledger-entry row says the field is "written when non-zero" and also implies a default-at-create behavior. This combination is ambiguous for the IOU Scale == 0 case (is it representable on-ledger, and if the field is absent does that mean 0 or the default 6?). The spec should state one unambiguous encoding rule for IOU scale (including how 0 is represented) and align the default semantics accordingly.

This issue also appears in the following locations of the same file:

• line 379
• line 410

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

• Data verification for IOU Scale only rejects values that exceed 18, but elsewhere (§3.1.6.1.1) Scale is defined as ranging from 0 to 18. As written, a negative or otherwise out-of-range Scale value is not explicitly rejected here, which can lead to divergent client expectations about which inputs are temMALFORMED.

6. The `MPTokenMetadata` field, if provided, is empty or exceeds 1024 bytes. (`temMALFORMED`)
7. The `Scale` field is provided when the `Asset` is `XRP` or `MPT`. (`temMALFORMED`)
8. The `Scale` field is provided, the `Asset` is an `IOU`, and `Scale` exceeds 18. (`temMALFORMED`)

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

• §3.1.6.3 states that share MPToken objects are created when an account deposits liquidity, but §3.2.6 now requires VaultCreate to create at least one zero-balance share MPToken (and a second one for the pseudo-account when private) before any deposit occurs. This is a normative contradiction about when MPToken ledger objects come into existence, and it affects reserve/OwnerCount expectations and off-ledger indexing logic. Align §3.1.6.3 with the VaultCreate behavior (e.g., explicitly list the VaultCreate zero-balance holdings as an exception and clarify the general rule for other holders).

   4. Create an `MPToken` object for the Vault Owner to hold Vault Shares, with `MPTAmount == 0` (no shares are issued at creation). Increase the Vault Owner's `OwnerCount` by 1 for that `MPToken`. Flags as in [3.1.6.3](#3163-mptoken): `lsfMPTAuthorized` when the vault is private or the shares are non-transferable.
   5. If `tfVaultPrivate` is set: create an authorized `MPToken` object for the _pseudo-account_ to hold Vault Shares, also with `MPTAmount == 0`. That holding is owned by the _pseudo-account_, not the Vault Owner.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mvadari]

@Tapanito some reasonable inconsistencies in the suppressed comments

[xrplf-ai-reviewer]

Documentation-only change syncing the VaultCreate spec (Data Verification, Protocol-Level Failures, State Changes, Invariants) with the current implementation. Most additions are internally consistent (OwnerCount math ties out across 3.2.4/3.2.6, ReferenceHolding description matches the new 3.2.6.1 section, error codes are consistently applied). One state-change bullet appears to conflate two unrelated conditions for when lsfMPTAuthorized is set on the owner's MPToken.