XLS-85d: Token-Enabled Escrows by dangell7 · Pull Request #5185 · XRPLF/rippled

dangell7 · 2024-11-07T10:37:00Z

High Level Overview of Change

XLS: XRPLF/XRPL-Standards#248
XLS PR: XRPLF/XRPL-Standards#272

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)

src/test/app/AccountDelete_test.cpp

src/test/app/EscrowToken_test.cpp

src/test/ledger/Invariants_test.cpp

src/xrpld/app/tx/detail/InvariantCheck.cpp

include/xrpl/protocol/STAmount.h

src/xrpld/ledger/View.h

src/xrpld/ledger/detail/View.cpp

src/xrpld/app/tx/detail/Escrow.cpp

dangell7 · 2025-02-26T08:37:53Z

@oleks-rip Thank you for the initial review. Right now I'm just focusing on the actual core implementation and how it effects MPT and IOU. I will address the nit changes you requested once the core implementation is done.

src/xrpld/app/tx/detail/Escrow.cpp

codecov · 2025-02-26T17:20:46Z

Codecov Report

Attention: Patch coverage is 89.38193% with 67 lines in your changes missing coverage. Please review.

Project coverage is 79.0%. Comparing base (7e24adb) to head (f11f746).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/ledger/detail/View.cpp	67.2%	41 Missing ⚠️
src/xrpld/rpc/handlers/GatewayBalances.cpp	20.0%	12 Missing ⚠️
src/xrpld/app/tx/detail/Escrow.cpp	98.4%	6 Missing ⚠️
src/xrpld/app/tx/detail/InvariantCheck.cpp	85.7%	5 Missing ⚠️
src/libxrpl/protocol/STAmount.cpp	96.9%	2 Missing ⚠️
src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp	50.0%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5185     +/-   ##
=========================================
+ Coverage     78.9%   79.0%   +0.1%     
=========================================
  Files          817     817             
  Lines        71139   71737    +598     
  Branches      8262    8313     +51     
=========================================
+ Hits         56112   56657    +545     
- Misses       15027   15080     +53

Files with missing lines	Coverage Δ
include/xrpl/protocol/STAmount.h	`95.5% <ø> (ø)`
include/xrpl/protocol/TER.h	`100.0% <ø> (ø)`
include/xrpl/protocol/detail/ledger_entries.macro	`100.0% <ø> (ø)`
include/xrpl/protocol/detail/transactions.macro	`100.0% <ø> (ø)`
src/libxrpl/protocol/TER.cpp	`100.0% <ø> (ø)`
src/xrpld/app/tx/detail/Escrow.h	`100.0% <ø> (ø)`
src/xrpld/app/tx/detail/MPTokenAuthorize.cpp	`95.4% <100.0%> (+0.8%)`	⬆️
src/xrpld/app/tx/detail/MPTokenAuthorize.h	`100.0% <ø> (ø)`
src/xrpld/app/tx/detail/SetAccount.cpp	`98.7% <100.0%> (+<0.1%)`	⬆️
src/xrpld/ledger/View.h	`100.0% <ø> (ø)`
... and 6 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.

- makeFieldAbsent `sfLockedAmount` on mptoken - add logging to MPTLock and MPTUnlock - remove canAdd check for MPT - add max MPT amount tests - add overflow check to canSubtract - use MPTAmount:: value_type max/min

bthomee · 2025-06-03T14:35:05Z

@XRPLF/rpc-reviewers we would like to cut the release candidate for 2.5.0 today - any chance you can take a look at this PR for the RPC-related changes?

oleks-rip

LGTM

kuznetsss

LGTM

bthomee · 2025-06-03T16:08:23Z

@dangell7 I'm planning on merging this soon - is it ready to go?

dangell7 · 2025-06-03T16:26:27Z

@dangell7 I'm planning on merging this soon - is it ready to go?

Add support for XLS-85 Token Escrow

- Specification: https://github.com/XRPLF/XRPL-Standards/pull/272
- Amendment: `TokenEscrow`
- Enables escrowing of IOU and MPT tokens in addition to native XRP
- Allows accounts to lock issued tokens (IOU/MPT) in escrow objects, with
  support for freeze, authorization, and transfer rates.
- Adds new ledger fields (`sfLockedAmount`, `sfIssuerNode`, etc.) to
  track locked balances for IOU and MPT escrows
- Updates EscrowCreate, EscrowFinish, and EscrowCancel transaction logic
  to support IOU and MPT assets, including proper handling of trustline
  and MPT authorization, transfer rates, and locked balances
- Enforces invariant checks for escrowed IOU/MPT amounts
- Extends GatewayBalances RPC to report locked (escrowed) balances

mvadari · 2025-06-04T16:27:11Z

src/xrpld/app/tx/detail/Escrow.cpp

+    auto const k = keylet::escrow(ctx.tx[sfOwner], ctx.tx[sfOfferSequence]);
+    auto const slep = ctx.view.read(k);
+    if (!slep)
+        return tecNO_TARGET;


@dangell7 since this check is being moved up, doesn't it need to be amendment-guarded, due to the possibility of the transaction error changing since they're now checked in a different order?

this check was moved up from the the start of doApply. The order shouldn't change.

There are a number of checks in Transactor that occur between preclaim and doApply

Yes looks like it thank you

@mvadari #5473

* Update the `account_info` API so that the `allowTrustLineLocking` flag is included in the response. * The proposed `TokenEscrow` amendment added an `allowTrustLineLocking` flag in the `AccountRoot` object. * In the API response, under `account_flags`, there is now an `allowTrustLineLocking` field with a boolean (`true` or `false`) value. * For reference, the XLS-85 Token-Enabled Escrows implementation can be found in #5185

…draw Similarly to other transaction typed that can create a trust line or MPToken for the transaction submitter (e.g. CashCheck #5285, EscrowFinish #5185 ), VaultWithdraw should enforce reserve before creating a new object. Additionally, the lsfRequireDestTag account flag should be enforced for the transaction submitter.

…draw (#5857) Similarly to other transaction typed that can create a trust line or MPToken for the transaction submitter (e.g. CashCheck #5285, EscrowFinish #5185 ), VaultWithdraw should enforce reserve before creating a new object. Additionally, the lsfRequireDestTag account flag should be enforced for the transaction submitter. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com>

dangell7 force-pushed the iou-escrow branch from 1f66573 to 4a8fede Compare November 22, 2024 10:53

dangell7 force-pushed the iou-escrow branch from 79ef781 to d4c056a Compare December 2, 2024 20:25

dangell7 marked this pull request as ready for review December 2, 2024 20:28

featureTokenEscrow

67fa360

dangell7 force-pushed the iou-escrow branch from b8cc826 to 67fa360 Compare February 12, 2025 21:40

oleks-rip reviewed Feb 24, 2025

View reviewed changes

oleks-rip reviewed Feb 25, 2025

View reviewed changes

include/xrpl/protocol/STAmount.h Outdated Show resolved Hide resolved

oleks-rip reviewed Feb 25, 2025

View reviewed changes

src/xrpld/ledger/View.h Outdated Show resolved Hide resolved

oleks-rip reviewed Feb 25, 2025

View reviewed changes

src/xrpld/ledger/detail/View.cpp Outdated Show resolved Hide resolved

oleks-rip reviewed Feb 25, 2025

View reviewed changes