-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add AMMClawback Transaction (XLS-0073d) #5142
Conversation
58b7089
to
384a73c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
|
Note: The PR title is wrong, it should be XLS-73d |
|
ad78c7c
to
a8f9c1b
Compare
a8f9c1b
to
6bf51d1
Compare
da47fb2
to
6510b72
Compare
6510b72
to
0d88f0c
Compare
0d88f0c
to
9b30183
Compare
4879ffc
to
3be6878
Compare
8a8553d
to
01cbe02
Compare
5381c04
to
51df02e
Compare
bc50276
to
9f714bb
Compare
f493c03
to
b206991
Compare
8c2a8b4
to
369d626
Compare
369d626
to
a15db14
Compare
acbded4
to
6894afe
Compare
6894afe
to
fc577a5
Compare
@yinyiqian1 Are we happy with the commit message being just
? If not, please suggest the details you'd like to include. |
@ximinez I think it's fine that we keep the current message |
Amendment: - AMMClawback New Transactions: - AMMClawback Modified Transactions: - AMMCreate - AMMDeposit
High Level Overview of Change
Context of Change
Spec link: XRPLF/XRPL-Standards#212
This PR includes:
changes are in
AMMCreate
andAMM_test
changes are in
AMMDeposit
andAMM_test
mainly in
AMMClawback
andAMMClawback_test
Some refactor happens because I want to call some functions from AMMWithdraw.
1. Refactor of
withdraw
:withdraw
function to return theamountWithdrawActual
andamount2WithdrawActual
as well.withdraw
.withdraw
function, it checksif (!(ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll)))
to decide if it needs to calladjustAmountsByLPTokens
. Because these flags only belong toAMMWithdraw
, 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 staticequalWithdrawTokens
function. And also private overloadequalWithdrawTokens
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
andB
in the amm pool, when clawing backA
,tfClawTwoAssets
is used to decide if the paired tokenB
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
andamount
in theAMMClawback
request:amount
is used to decide if the issuer wants to clawback all the tokens or not. (ifamount
is not provided, then clawback all) Imagine if an issuer issues both assetA
andB
in the amm pool, and the issuer wants to clawback allA
and give back all the correspondingB
to the holder. Then the issuer just needs to specifyA
inasset
field and leave outamount
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
.gitignore
, formatting, dropping support for older tooling)API Impact
libxrpl
change (any change that may affectlibxrpl
or dependents oflibxrpl
)