Single Asset Vault by Bronek · Pull Request #5224 · XRPLF/rippled

Bronek · 2024-12-17T12:13:31Z

High Level Overview of Change

This is implementation of XLS-65 Single Asset Vault. First draft was implemented by @thejohnfreeman in #5147

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

Public API: New feature (new methods and/or new fields)
Public API: Breaking change (in general, breaking changes should only impact the next api_version)
libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
Peer protocol change (must be backward compatible or bump the peer protocol version)

gregtatcam

I got compile error on MAC M1 Sequoia 15.1.1, apple clang 16.0.0

xrpl/json/json_value.h:705:5: error: constexpr function's return type 'Value' is not a literal type
705 |     operator()(T&& t) const
xrpl/json/json_value.h:148:7: note: 'Value' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move constructors
148 | class Value

gregtatcam

There are a few warnings, of which the most common is:

xrpl/protocol/STIssue.h:49:5: warning: definition of implicit copy constructor for 'STIssue' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]
   49 |     operator=(STIssue const& rhs) = default;

And once the compile error is fixed (remove constexpr in to_json_fn::operator() return value), there are VaultDelete unit-tests failures.

Bronek · 2024-12-18T15:03:12Z

There are a few warnings, of which the most common is:
xrpl/protocol/STIssue.h:49:5: warning: definition of implicit copy constructor for 'STIssue' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]
   49 |     operator=(STIssue const& rhs) = default; 
And once the compile error is fixed (remove constexpr in to_json_fn::operator() return value), there are VaultDelete unit-tests failures.

This is fixed now. The VaultDelete still needs to be fixed, but for the time being I've commented the failing test out. Compilation errors in clang 16 fixed, also simplified json_value.h a little.

Also remove potential ODR violation from json_value.h

codecov · 2024-12-18T17:52:20Z

Codecov Report

Attention: Patch coverage is 94.24815% with 70 lines in your changes missing coverage. Please review.

Project coverage is 78.5%. Comparing base (dd62cfc) to head (990d04b).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/ledger/detail/View.cpp	89.8%	25 Missing ⚠️
src/xrpld/app/tx/detail/VaultClawback.cpp	92.8%	8 Missing ⚠️
src/xrpld/app/misc/CredentialHelpers.cpp	87.3%	7 Missing ⚠️
src/xrpld/app/tx/detail/VaultDelete.cpp	91.7%	5 Missing ⚠️
src/xrpld/app/tx/detail/VaultDeposit.cpp	95.5%	5 Missing ⚠️
src/xrpld/app/tx/detail/VaultWithdraw.cpp	95.4%	5 Missing ⚠️
src/libxrpl/protocol/STParsedJSON.cpp	50.0%	3 Missing ⚠️
src/libxrpl/protocol/STAmount.cpp	85.7%	2 Missing ⚠️
src/xrpld/app/tx/detail/InvariantCheck.cpp	93.5%	2 Missing ⚠️
src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp	89.5%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5224     +/-   ##
=========================================
+ Coverage     78.3%   78.5%   +0.3%     
=========================================
  Files          800     813     +13     
  Lines        69026   70085   +1059     
  Branches      8275    8282      +7     
=========================================
+ Hits         54019   55037   +1018     
- Misses       15007   15048     +41

Files with missing lines	Coverage Δ
include/xrpl/json/json_value.h	`98.5% <ø> (ø)`
include/xrpl/protocol/AMMCore.h	`100.0% <ø> (ø)`
include/xrpl/protocol/Asset.h	`95.5% <100.0%> (+0.7%)`	⬆️
include/xrpl/protocol/IOUAmount.h	`100.0% <ø> (ø)`
include/xrpl/protocol/Indexes.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/MPTAmount.h	`100.0% <ø> (ø)`
include/xrpl/protocol/MPTIssue.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/SField.h	`100.0% <ø> (ø)`
include/xrpl/protocol/STAmount.h	`95.5% <100.0%> (+0.3%)`	⬆️
include/xrpl/protocol/STBase.h	`94.7% <100.0%> (+0.6%)`	⬆️
... and 59 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vlntb

LGTM 👍

Bronek · 2025-05-20T17:17:38Z

I am marking this as Ready to merge however there are few follow-up items which, while not directly part of this PR, will improve the quality for this feature and ideally should be delivered as separate PRs:

Add DomainID support to MPTokenIssuanceCreate and MPTokenIssuanceSet to work similarly like VaultCreate and VaultSet:
- can only be set if token issuance flag lsfMPTRequireAuth is set
- can be updated at any time
- update to zero/empty DomainID will clear the field
Add support for objects owed by pseudo-accounts to adjustOwnerCount, ownership count passed to the actual owner, no recursion
Implement vault-specific invariants

Before #5224, pseudoaccount ID was calculated using prefix expressed in std::uint16_t. The refactoring to move the pseudoaccount ID calculation to View.cpp had accidentally changed the prefix type to int (derived from `auto i = 0`) which in turn changed the length of the input to `sha512Half` from 2 bytes to 4, altering the result. This resulted in a different ID of the pseudoaccount calculated from the function after the refactoring, breaking the ledger. This impacts AMMCreate, even when SingleAssetVault amendment is not active.

Before #5224, the pseudoaccount ID was calculated using prefix expressed in `std::uint16_t`. The refactoring to move the pseudoaccount ID calculation to View.cpp had accidentally changed the prefix type to `int` (derived from `auto i = 0`) which in turn changed the length of the input to `sha512Half` from 2 bytes to 4, altering the result. This resulted in a different ID of the pseudoaccount calculated from the function after the refactoring, breaking the ledger. This impacts AMMCreate, even when the `SingleAssetVault` amendment is not active. This change restores the prefix type to `std::uint16_t`.

ckeshava · 2025-06-03T20:54:57Z

include/xrpl/protocol/TxFlags.h

 constexpr std::uint32_t tfClearAccountCreateAmount     = 0x00010000;
 constexpr std::uint32_t tfBridgeModifyMask = ~(tfUniversal | tfClearAccountCreateAmount);
+
+// VaultCreate flags:


Hello, a previous version of the specification contained the following transaction flags. Can you confirm that these are no longer included in the feature implementation? (please ignore the case-sensitivity)

TF_FREEZE = 0x0001 TF_UNFREEZE = 0x0002

These are no longer in the XLS-65d specification

ckeshava · 2025-06-03T20:58:42Z

include/xrpl/protocol/detail/ledger_entries.macro


+/** A ledger object representing a single asset vault.
+
+    \sa keylet::mptoken


typo in the documentation: this should have been keylet::vault

thank you, this will be fixed in #5509

This change adds support for `DomainID` to existing transactions `MPTokenIssuanceCreate` and `MPTokenIssuanceSet`. In #5224 `DomainID` was added as an access control mechanism for `SingleAssetVault`. The actual implementation of this feature lies in `MPToken` and `MPTokenIssuance`, hence it makes sense to enable the use of `DomainID` also in `MPTokenIssuanceCreate` and `MPTokenIssuanceSet`, following same rules as in Vault: * `MPTokenIssuanceCreate` and `MPTokenIssuanceSet` can only set `DomainID` if flag `MPTRequireAuth` is set. * `MPTokenIssuanceCreate` requires that `DomainID` be a non-zero, uint256 number. * `MPTokenIssuanceSet` allows `DomainID` to be zero (or empty) in which case it will remove `DomainID` from the `MPTokenIssuance` object. The change is amendment-gated by `SingleAssetVault`. This is a non-breaking change because `SingleAssetVault` amendment is `Supported::no`, i.e. at this moment considered a work in progress, which cannot be enabled on the network.

#5224 added (among other things) a `VaultWithdraw` transaction that allows setting the recipient of the withdrawn funds in the `Destination` transaction field. This technically turns this transaction into a payment, and in some respect the implementation does follow payment rules, e.g. enforcement of `lsfRequireDestTag` or `lsfDepositAuth`, or that MPT transfer has destination `MPToken`. However for IOUs, it missed verification that the destination account has a trust line to the asset issuer. Since the default behavior of `accountSendIOU` is to create this trust line (if missing), this is what `VaultWithdraw` currently does. This is incorrect, since the `Destination` might not be interested in holding the asset in question; this basically enables spammy transfers. This change, therefore, removes automatic creation of a trust line to the `Destination` account in `VaultWithdraw`.

This change adds invariants for SingleAssetVault #5224 (XLS-065), which had been intentionally skipped earlier to keep the SAV PR size manageable.

bthomee · 2026-03-20T20:27:36Z

/ai-reviewer

thejohnfreeman added 16 commits December 17, 2024 11:10

Integrate STNumber with STParsedJSON

1a032f0

Start vault implementation

ff8c649

wip

a8ec8e7

format

d09e74e

progress

1680477

progress

ebc97ae

progress

286612c

progress

6046fa2

progress

1255216

AND_THEN

1ff1274

wip

ea6c040

fix

ea30f44

revise tests

12646cb

remove blob constants

54d511b

bless sfAsset to carry MPT

b5619fb

wip clawback

ad58166

gregtatcam reviewed Dec 17, 2024

View reviewed changes

Fix failing MPToken payment test

ecec6e5

Bronek force-pushed the vault branch from f2dd852 to ecec6e5 Compare December 17, 2024 18:05

gregtatcam reviewed Dec 17, 2024

View reviewed changes

Fix compilation errors

eae2cd3

Bronek force-pushed the vault branch 3 times, most recently from 2bcc6ad to 17af6e7 Compare December 18, 2024 17:23

Bronek added 3 commits December 18, 2024 17:30

Comment out failing test

526f715

Fix formatting

b9f1200

Simplify conversions to Json::Value

241429b

Also remove potential ODR violation from json_value.h

Bronek force-pushed the vault branch from 17af6e7 to 241429b Compare December 18, 2024 17:34

Fix logic error from review feedback

9a470ce

vlntb approved these changes May 20, 2025

View reviewed changes

Bronek added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 20, 2025

Merge branch 'develop' into vault

990d04b

bthomee merged commit e514de7 into XRPLF:develop May 20, 2025
2 checks passed

Bronek mentioned this pull request May 21, 2025

Enable SingleAssetVault feature #5444

Closed

9 tasks

Bronek mentioned this pull request May 23, 2025

Fix pseudo-account ID calculation #5447

Merged

13 tasks

Bronek mentioned this pull request May 27, 2025

Fix handling of expired credentials in VaultDeposit #5452

Merged

9 tasks

ckeshava reviewed Jun 3, 2025

View reviewed changes

khancode mentioned this pull request Jun 11, 2025

feat: add Single Asset Vault (XLS-65) XRPLF/xrpl.js#3008

Merged

9 tasks

This was referenced Jun 12, 2025

Proposed 2.5.0-rc2 #5491

Merged

Release 2.5.0 #5501

Closed

Set version to 2.5.0 #5502

Closed

legleux mentioned this pull request Jun 23, 2025

Release 2.5.0 #5504

Merged

This was referenced Jun 24, 2025

Add support for DomainID in MPTokenIssuance transactions #5509

Merged

Add vault invariants #5518

Merged

Bronek mentioned this pull request Jul 17, 2025

VaultWithdraw destination account bugfix #5572

Merged

9 tasks

khancode mentioned this pull request Jul 30, 2025

feat: support for Single Asset Vault (XLS-65) ripple/explorer#1213

Merged

11 tasks

bthomee pushed a commit that referenced this pull request Oct 8, 2025

Add vault invariants (#5518)

5ecde3c

This change adds invariants for SingleAssetVault #5224 (XLS-065), which had been intentionally skipped earlier to keep the SAV PR size manageable.

Bronek added a commit that referenced this pull request Oct 27, 2025

Add vault invariants (#5518)

e075fee

This change adds invariants for SingleAssetVault #5224 (XLS-065), which had been intentionally skipped earlier to keep the SAV PR size manageable.

Bronek added a commit that referenced this pull request Oct 31, 2025

Add vault invariants (#5518)

220ab26

This change adds invariants for SingleAssetVault #5224 (XLS-065), which had been intentionally skipped earlier to keep the SAV PR size manageable.

pdp2121 mentioned this pull request Mar 24, 2026

Support SingleAssetVault (XLS-65) XRPLF/xrpl-rust#137

Open

bthomee requested a review from pratikmankawde March 25, 2026 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single Asset Vault#5224

Single Asset Vault#5224
bthomee merged 212 commits intoXRPLF:developfrom
Bronek:vault

Bronek commented Dec 17, 2024

Uh oh!

gregtatcam left a comment

Uh oh!

gregtatcam left a comment

Uh oh!

Bronek commented Dec 18, 2024

Uh oh!

codecov bot commented Dec 18, 2024 •

edited

Loading

Uh oh!

vlntb left a comment

Uh oh!

Bronek commented May 20, 2025 •

edited

Loading

Uh oh!

Uh oh!

ckeshava Jun 3, 2025

Uh oh!

Bronek Jun 24, 2025

Uh oh!

ckeshava Jun 3, 2025

Uh oh!

Bronek Jun 24, 2025

Uh oh!

bthomee commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants


		/** A ledger object representing a single asset vault.

		\sa keylet::mptoken

Conversation

Bronek commented Dec 17, 2024

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

Bronek commented Dec 18, 2024

Uh oh!

codecov bot commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vlntb left a comment

Choose a reason for hiding this comment

Uh oh!

Bronek commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ckeshava Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Bronek Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

ckeshava Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Bronek Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

bthomee commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

codecov bot commented Dec 18, 2024 •

edited

Loading

Bronek commented May 20, 2025 •

edited

Loading