Skip to content

URITokenTransferFee Amendment [DO NOT MERGE]#721

Open
tequdev wants to merge 25 commits into
Xahau:devfrom
tequdev:uritoken-xfer-fee
Open

URITokenTransferFee Amendment [DO NOT MERGE]#721
tequdev wants to merge 25 commits into
Xahau:devfrom
tequdev:uritoken-xfer-fee

Conversation

@tequdev

@tequdev tequdev commented Mar 17, 2026

Copy link
Copy Markdown
Member

High Level Overview of Change

#720

Context of Change

Type of Change

  • New feature (non-breaking change which adds functionality)

RichardAH and others added 6 commits February 24, 2026 16:07
* Add AMM bid/create/deposit/swap/withdraw/vote invariants:
  - Deposit, Withdrawal invariants: `sqrt(asset1Balance * asset2Balance) >= LPTokens`.
  - Bid: `sqrt(asset1Balance * asset2Balance) > LPTokens` and the pool balances don't change.
  - Create: `sqrt(asset1Balance * assetBalance2) == LPTokens`.
  - Swap: `asset1BalanceAfter * asset2BalanceAfter >= asset1BalanceBefore * asset2BalanceBefore`
     and `LPTokens` don't change.
  - Vote: `LPTokens` and pool balances don't change.
  - All AMM and swap transactions: amounts and tokens are greater than zero, except on withdrawal if all tokens
    are withdrawn.
* Add AMM deposit and withdraw rounding to ensure AMM invariant:
  - On deposit, tokens out are rounded downward and deposit amount is rounded upward.
  - On withdrawal, tokens in are rounded upward and withdrawal amount is rounded downward.
* Add Order Book Offer invariant to verify consumed amounts. Consumed amounts are less than the offer.
* Fix Bid validation. `AuthAccount` can't have duplicate accounts or the submitter account.
Due to rounding, the LPTokenBalance of the last LP might not match the LP's trustline balance. This was fixed for `AMMWithdraw` in `fixAMMv1_1` by adjusting the LPTokenBalance to be the same as the trustline balance. Since `AMMClawback` is also performing a withdrawal, we need to adjust LPTokenBalance as well in `AMMClawback.`

This change includes:
1. Refactored `verifyAndAdjustLPTokenBalance` function in `AMMUtils`, which both`AMMWithdraw` and `AMMClawback` call to adjust LPTokenBalance.
2. Added the unit test `testLastHolderLPTokenBalance` to test the scenario.
3. Modify the existing unit tests for `fixAMMClawbackRounding`.
@tequdev tequdev force-pushed the uritoken-xfer-fee branch from 50e09d1 to a29e248 Compare March 17, 2026 04:10
@tequdev tequdev added the feature Feature Request label Apr 18, 2026
@codecov

codecov Bot commented May 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.26087% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.3%. Comparing base (bb244ef) to head (af071c0).

Files with missing lines Patch % Lines
src/xrpld/app/tx/detail/Remit.cpp 96.2% 1 Missing ⚠️
src/xrpld/app/tx/detail/URIToken.cpp 98.9% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##             dev    #721   +/-   ##
=====================================
  Coverage   77.3%   77.3%           
=====================================
  Files        837     837           
  Lines      78585   78675   +90     
  Branches   11559   11556    -3     
=====================================
+ Hits       60743   60841   +98     
+ Misses     17832   17824    -8     
  Partials      10      10           
Files with missing lines Coverage Δ
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <ø> (ø)
include/xrpl/protocol/detail/transactions.macro 100.0% <ø> (ø)
src/libxrpl/protocol/InnerObjectFormats.cpp 100.0% <ø> (ø)
src/xrpld/app/tx/detail/Remit.cpp 88.2% <96.2%> (+0.6%) ⬆️
src/xrpld/app/tx/detail/URIToken.cpp 89.2% <98.9%> (+5.6%) ⬆️

... and 9 files with indirect coverage changes

Impacted file tree graph

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Feature Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants