rippled Add AMMClawback Transaction (XLS-0073d)

High Level Overview of Change

Context of Change

Spec link: https://github.com/XRPLF/XRPL-Standards/discussions/212

This PR includes:

Allow AMMCreate for clawback-enabled tokens changes are in AMMCreate and AMM_test
AMMDeposit prohibits depositing if either of the paired tokens is frozen changes are in AMMDeposit and AMM_test
Added AMMClawback transaction. mainly in AMMClawback and AMMClawback_test

Some refactor happens because I want to call some functions from AMMWithdraw. 1. Refactor of withdraw:

Added static withdraw function to return the amountWithdrawActual and amount2WithdrawActual as well.
private overload withdraw.
Previously in withdraw function, it checks if (!(ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll))) to decide if it needs to call adjustAmountsByLPTokens. Because these flags only belong to AMMWithdraw, to make this function also work for AMMClawback, I changed it by passing enum WithdrawAll.

2. Refactor of equalWithdrawTokens: Because I want to call this function in AMMClawback, I make a public static equalWithdrawTokens function. And also private overload equalWithdrawTokens

3. Added deleteAMMAccountIfEmpty Since I need to reuse the logic in AMMWithdraw that when all the tokens are withdrawn, the amm account is deleted, so I added function deleteAMMAccountIfEmpty.

Reason of adding flag tfClawTwoAssets: If the issuer issues both assets A and B in the amm pool, when clawing back A, tfClawTwoAssets is used to decide if the paired token B to be clawed back as well or goes back to the holder. If tfClawTwoAssets` is set true, the paired token will be clawed back as well. Otherwise, the paired token goes back to the holder.

Reason of adding both asset and amount in the AMMClawback request: amount is used to decide if the issuer wants to clawback all the tokens or not. (if amount is not provided, then clawback all) Imagine if an issuer issues both asset A and B in the amm pool, and the issuer wants to clawback all A and give back all the corresponding B to the holder. Then the issuer just needs to specify A in asset field and leave out amount field.

Reasons for setting tfee as 0: Because we are doing a two-asset withdrawal. tfee is not used.

Reasons for using asset1 and asset2 to navigate amm To keep the same with other amm transactions and make it simple. So we require providing asset1 and asset2 in the AMMClawback request.

Type of Change

[ ] Bug fix (non-breaking change which fixes an issue)
[X] 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)
[x] 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

[x] 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)
[x] 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)

Sep 19 '24 04:09 yinyiqian1

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.6%. Comparing base (d6dbf0e) to head (25f9ba6). Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5142     +/-   ##
=========================================
+ Coverage     77.5%   77.6%   +0.1%     
=========================================
  Files          777     779      +2     
  Lines        65833   65975    +142     
  Branches      8182    8173      -9     
=========================================
+ Hits         51031   51180    +149     
+ Misses       14802   14795      -7

Files with missing lines	Coverage Δ
include/xrpl/protocol/Feature.h	`100.0% <ø> (ø)`
include/xrpl/protocol/detail/transactions.macro	`100.0% <100.0%> (ø)`
src/libxrpl/protocol/TER.cpp	`100.0% <ø> (ø)`
src/xrpld/app/tx/detail/AMMClawback.cpp	`100.0% <100.0%> (ø)`
src/xrpld/app/tx/detail/AMMClawback.h	`100.0% <100.0%> (ø)`
src/xrpld/app/tx/detail/AMMCreate.cpp	`90.5% <100.0%> (+0.1%)`	:arrow_up:
src/xrpld/app/tx/detail/AMMDeposit.cpp	`95.3% <100.0%> (-0.1%)`	:arrow_down:
src/xrpld/app/tx/detail/AMMWithdraw.cpp	`95.9% <100.0%> (+4.5%)`	:arrow_up:
src/xrpld/app/tx/detail/AMMWithdraw.h	`100.0% <ø> (ø)`
src/xrpld/app/tx/detail/InvariantCheck.cpp	`86.5% <100.0%> (+<0.1%)`	:arrow_up:
... and 1 more

... and 5 files with indirect coverage changes

Sep 19 '24 05:09 codecov[bot]

Note: The PR title is wrong, it should be XLS-73d

Sep 19 '24 15:09 mvadari

Note: The PR title is wrong, it should be XLS-73d Just corrected it. Thank you.

Sep 19 '24 16:09 yinyiqian1

@yinyiqian1 Are we happy with the commit message being just

Add AMMClawback Transaction (XLS-0073d) (#5142)

?

If not, please suggest the details you'd like to include.

Nov 04 '24 18:11 ximinez

@yinyiqian1 Are we happy with the commit message being just
Add AMMClawback Transaction (XLS-0073d) (#5142)
?

If not, please suggest the details you'd like to include.

@ximinez I think it's fine that we keep the current message

Nov 04 '24 18:11 yinyiqian1