ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-23 15:10:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix cross-issuer MPToken authorization in AMMClawback

Browse Source 
Commit 21cfba66fd recreated a holder's missing MPToken as authorized
for both pool assets. On a cross-issuer MPT/MPT pool this let one
issuer's clawback re-authorize the holder for the *other* issuer's
asset, bypassing its lsfMPTRequireAuth. Only auto-authorize the
recreated MPToken for the clawback issuer's own asset; recreate the
paired asset unauthorized.
This commit is contained in:
Gregory Tsipenyuk
2026-07-07 14:29:48 -04:00
parent 54a934db68
commit b247d2db29
4 changed files with 116 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
include/xrpl/tx/transactors/dex/AMMWithdraw.h
View File

@@ -116,6 +116,7 @@ public:
Sandbox& view,
SLE const& ammSle,
AccountID const account,
std::optional<AccountID> const& clawbackIssuer,
AccountID const& ammAccount,
STAmount const& amountBalance,
STAmount const& amount2Balance,
@@ -135,6 +136,11 @@ public:
* @param view
* @param ammSle AMM ledger entry
* @param ammAccount AMM account
* @param clawbackIssuer when set (AMMClawback path), the issuer performing
* the clawback. A recreated MPToken is only auto-authorized when the
* asset's issuer matches this account, so a clawback cannot grant
* authorization on behalf of a different (paired-asset) issuer.
* @param account LP account
* @param amountBalance current LP asset1 balance
* @param amountWithdraw asset1 withdraw amount
* @param amount2Withdraw asset2 withdraw amount
@@ -150,6 +156,7 @@ public:
Sandbox& view,
SLE const& ammSle,
AccountID const& ammAccount,
std::optional<AccountID> const& clawbackIssuer,
AccountID const& account,
STAmount const& amountBalance,
STAmount const& amountWithdraw,

9
src/libxrpl/tx/transactors/dex/AMMClawback.cpp
View File

@@ -227,6 +227,7 @@ AMMClawback::applyGuts(Sandbox& sb)
sb,
*ammSle,
holder,
issuer,
ammAccount,
amountBalance,
amount2Balance,
@@ -311,6 +312,11 @@ AMMClawback::equalWithdrawMatchingOneAmount(
STAmount const& holdLPtokens,
STAmount const& amount)
{
// The clawback issuer signs for its own asset only. Threaded into the
// withdrawal so a recreated MPToken is auto-authorized only for the
// clawback issuer's asset, never for a paired asset from another issuer.
AccountID const issuer = ctx_.tx[sfAccount];
auto frac = Number{amount} / amountBalance;
auto amount2Withdraw = amount2Balance * frac;
@@ -324,6 +330,7 @@ AMMClawback::equalWithdrawMatchingOneAmount(
sb,
ammSle,
holder,
issuer,
ammAccount,
amountBalance,
amount2Balance,
@@ -357,6 +364,7 @@ AMMClawback::equalWithdrawMatchingOneAmount(
sb,
ammSle,
ammAccount,
issuer,
holder,
amountBalance,
amountRounded,
@@ -377,6 +385,7 @@ AMMClawback::equalWithdrawMatchingOneAmount(
sb,
ammSle,
ammAccount,
issuer,
holder,
amountBalance,
amount,

21
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp
View File

@@ -493,6 +493,7 @@ AMMWithdraw::withdraw(
view,
ammSle,
ammAccount,
std::nullopt,
accountID_,
amountBalance,
amountWithdraw,
@@ -513,6 +514,7 @@ AMMWithdraw::withdraw(
Sandbox& view,
SLE const& ammSle,
AccountID const& ammAccount,
std::optional<AccountID> const& clawbackIssuer,
AccountID const& account,
STAmount const& amountBalance,
STAmount const& amountWithdraw,
@@ -689,9 +691,18 @@ AMMWithdraw::withdraw(
// AMMClawback ignores authorization so the issuer can recover
// MPT locked in the pool even if the holder deleted their
// MPToken. Recreate that MPToken as authorized for the
// clawback withdrawal path only.
createFlags = lsfMPTAuthorized;
// MPToken. Only auto-authorize the recreated MPToken for the
// clawback issuer's own asset: authorization is granted by an
// asset's issuer, and the clawback transaction is signed by
// that issuer only for its own asset. For a paired asset issued
// by a different account, recreate the MPToken *unauthorized* so
// the clawback does not grant authorization on behalf of that
// issuer (which would bypass its lsfMPTRequireAuth). The holder
// still receives the paired asset (accountSend only requires the
// MPToken to exist, not to be authorized); the balance remains
// gated by its issuer until that issuer authorizes it.
if (clawbackIssuer && asset.getIssuer() == *clawbackIssuer)
createFlags = lsfMPTAuthorized;
}
if (auto const err = checkCreateMPT(view, mptIssue, account, createFlags, journal);
@@ -790,6 +801,7 @@ AMMWithdraw::equalWithdrawTokens(
view,
ammSle,
accountID_,
std::nullopt,
ammAccount,
amountBalance,
amount2Balance,
@@ -841,6 +853,7 @@ AMMWithdraw::equalWithdrawTokens(
Sandbox& view,
SLE const& ammSle,
AccountID const account,
std::optional<AccountID> const& clawbackIssuer,
AccountID const& ammAccount,
STAmount const& amountBalance,
STAmount const& amount2Balance,
@@ -863,6 +876,7 @@ AMMWithdraw::equalWithdrawTokens(
view,
ammSle,
ammAccount,
clawbackIssuer,
account,
amountBalance,
amountBalance,
@@ -898,6 +912,7 @@ AMMWithdraw::equalWithdrawTokens(
view,
ammSle,
ammAccount,
clawbackIssuer,
account,
amountBalance,
amountWithdraw,

82
src/test/app/AMMClawbackMPT_test.cpp
View File

@@ -17,6 +17,7 @@
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
@@ -1979,6 +1980,86 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
}
}
void
testClawbackCrossIssuerPairedAssetAuth(FeatureBitset features)
{
testcase("test AMMClawback recreates paired-issuer MPToken unauthorized");
using namespace jtx;
// Cross-issuer MPT/MPT pool: btc is issued by gw, eth by gw2, and both
// require authorization. Alice deposits her entire balance of both and
// deletes the resulting zero-balance MPTokens. When gw claws back its
// own asset (btc), the two-asset withdrawal must recreate both of
// Alice's MPTokens so the pool can pay her the paired asset. The
// recreated MPToken may only be auto-authorized for the clawback
// issuer's own asset (btc); the paired asset's issuer (gw2) never
// consented, so eth must be recreated *unauthorized*, leaving gw2 in
// control of its own token and preserving its RequireAuth guarantee.
Env env(*this, features);
Account const gw{"gateway"};
Account const gw2{"gateway2"};
Account const alice{"alice"};
env.fund(XRP(100'000), gw, gw2, alice);
env.close();
MPTTester btc(
{.env = env,
.issuer = gw,
.holders = {alice},
.pay = 10'000,
.flags = tfMPTCanClawback | tfMPTRequireAuth | kMptDexFlags,
.authHolder = true});
MPTTester eth(
{.env = env,
.issuer = gw2,
.holders = {alice},
.pay = 10'000,
.flags = tfMPTCanClawback | tfMPTRequireAuth | kMptDexFlags,
.authHolder = true});
// Alice deposits everything into the pool; her MPT balances drop to 0.
AMM const amm(env, alice, btc(10'000), eth(10'000));
env.close();
BEAST_EXPECT(amm.expectBalances(btc(10'000), eth(10'000), IOUAmount{10'000}));
BEAST_EXPECT(env.balance(alice, btc) == btc(0));
BEAST_EXPECT(env.balance(alice, eth) == eth(0));
// Alice deletes both zero-balance MPTokens to reclaim reserves.
btc.authorize({.account = alice, .flags = tfMPTUnauthorize});
eth.authorize({.account = alice, .flags = tfMPTUnauthorize});
BEAST_EXPECT(!env.le(keylet::mptoken(btc.issuanceID(), alice.id())));
BEAST_EXPECT(!env.le(keylet::mptoken(eth.issuanceID(), alice.id())));
// gw (issuer of btc) claws back part of Alice's btc. This is a
// cross-issuer pool, so tfClawTwoAssets is not permitted: only btc is
// clawed back, while the paired eth is returned to Alice.
env(amm::ammClawback(gw, alice, btc, eth, btc(1'000)));
env.close();
// Both MPTokens were recreated so the withdrawal could pay Alice.
auto const sleBtc = env.le(keylet::mptoken(btc.issuanceID(), alice.id()));
auto const sleEth = env.le(keylet::mptoken(eth.issuanceID(), alice.id()));
BEAST_EXPECT(sleBtc);
BEAST_EXPECT(sleEth);
// The clawback issuer's own asset (btc) may be recreated authorized:
// gw has authority over its own token.
BEAST_EXPECT(sleBtc && sleBtc->isFlag(lsfMPTAuthorized));
// The paired asset (eth) is issued by gw2, who did not sign this
// transaction. It must be recreated *unauthorized* so gw2's RequireAuth
// is not bypassed. This is the core assertion for the cross-issuer fix.
BEAST_EXPECT(sleEth && !sleEth->isFlag(lsfMPTAuthorized));
// The clawback still completed: btc was clawed back (Alice keeps a zero
// btc balance) and the paired eth was delivered into Alice's now
// unauthorized, gw2-gated MPToken (non-zero raw balance).
BEAST_EXPECT(sleBtc && sleBtc->getFieldU64(sfMPTAmount) == 0);
BEAST_EXPECT(sleEth && sleEth->getFieldU64(sfMPTAmount) > 0);
BEAST_EXPECT(amm.ammExists());
}
void
run() override
{
@@ -1994,6 +2075,7 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
testAssetFrozenOrLocked(all);
testClawbackCreatesMissingMPToken(all);
testClawbackAfterDeletingMPTokens(all);
testClawbackCrossIssuerPairedAssetAuth(all);
testSingleDepositAndClawback(all);
testLastHolderLPTokenBalance(all);
testLastHolderLPTokenBalance(all - fixAMMv1_3 - fixAMMClawbackRounding);