Amendment fixFrozenLPTokenTransfer (#5227)

Prohibits LPToken holders from sending LPToken to others if they have been frozen by one of the assets in AMM pool.
This commit is contained in:
Shawn Xie
2025-02-05 10:05:24 -08:00
committed by GitHub
parent f6d63082c0
commit fb3713bc25
9 changed files with 597 additions and 10 deletions

View File

@@ -16,6 +16,7 @@ runs:
conan export external/snappy snappy/1.1.10@
conan export external/rocksdb rocksdb/6.29.5@
conan export external/soci soci/4.0.3@
conan export external/nudb nudb/2.0.8@
- name: add Ripple Conan remote
shell: bash
run: |

View File

@@ -80,7 +80,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 86;
static constexpr std::size_t numFeatures = 87;
/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated

View File

@@ -29,6 +29,7 @@
// If you add an amendment here, then do not forget to increment `numFeatures`
// in include/xrpl/protocol/Feature.h.
XRPL_FIX (FrozenLPTokenTransfer, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(DeepFreeze, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(PermissionedDomains, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(DynamicNFT, Supported::yes, VoteBehavior::DefaultNo)

View File

@@ -0,0 +1,486 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2024 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <test/jtx.h>
#include <test/jtx/AMM.h>
#include <test/jtx/AMMTest.h>
namespace ripple {
namespace test {
class LPTokenTransfer_test : public jtx::AMMTest
{
void
testDirectStep(FeatureBitset features)
{
testcase("DirectStep");
using namespace jtx;
Env env{*this, features};
fund(env, gw, {alice}, {USD(20'000), BTC(0.5)}, Fund::All);
env.close();
AMM ammAlice(env, alice, USD(20'000), BTC(0.5));
BEAST_EXPECT(
ammAlice.expectBalances(USD(20'000), BTC(0.5), IOUAmount{100, 0}));
fund(env, gw, {carol}, {USD(4'000), BTC(1)}, Fund::Acct);
ammAlice.deposit(carol, 10);
BEAST_EXPECT(
ammAlice.expectBalances(USD(22'000), BTC(0.55), IOUAmount{110, 0}));
fund(env, gw, {bob}, {USD(4'000), BTC(1)}, Fund::Acct);
ammAlice.deposit(bob, 10);
BEAST_EXPECT(
ammAlice.expectBalances(USD(24'000), BTC(0.60), IOUAmount{120, 0}));
auto const lpIssue = ammAlice.lptIssue();
env.trust(STAmount{lpIssue, 500}, alice);
env.trust(STAmount{lpIssue, 500}, bob);
env.trust(STAmount{lpIssue, 500}, carol);
env.close();
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// bob can still send lptoken to carol even tho carol's USD is
// frozen, regardless of whether fixFrozenLPTokenTransfer is enabled or
// not
// Note: Deep freeze is not considered for LPToken transfer
env(pay(bob, carol, STAmount{lpIssue, 5}));
env.close();
// cannot transfer to an amm account
env(pay(carol, lpIssue.getIssuer(), STAmount{lpIssue, 5}),
ter(tecNO_PERMISSION));
env.close();
if (features[fixFrozenLPTokenTransfer])
{
// carol is frozen on USD and therefore can't send lptoken to bob
env(pay(carol, bob, STAmount{lpIssue, 5}), ter(tecPATH_DRY));
}
else
{
// carol can still send lptoken with frozen USD
env(pay(carol, bob, STAmount{lpIssue, 5}));
}
}
void
testBookStep(FeatureBitset features)
{
testcase("BookStep");
using namespace jtx;
Env env{*this, features};
fund(
env,
gw,
{alice, bob, carol},
{USD(10'000), EUR(10'000)},
Fund::All);
AMM ammAlice(env, alice, USD(10'000), EUR(10'000));
ammAlice.deposit(carol, 1'000);
ammAlice.deposit(bob, 1'000);
auto const lpIssue = ammAlice.lptIssue();
// carols creates an offer to sell lptoken
env(offer(carol, XRP(10), STAmount{lpIssue, 10}), txflags(tfPassive));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
env.trust(STAmount{lpIssue, 1'000'000'000}, alice);
env.trust(STAmount{lpIssue, 1'000'000'000}, bob);
env.trust(STAmount{lpIssue, 1'000'000'000}, carol);
env.close();
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// exercises alice's ability to consume carol's offer to sell lptoken
// when carol's USD is frozen pre/post fixFrozenLPTokenTransfer
// amendment
if (features[fixFrozenLPTokenTransfer])
{
// with fixFrozenLPTokenTransfer, alice fails to consume carol's
// offer since carol's USD is frozen
env(pay(alice, bob, STAmount{lpIssue, 10}),
txflags(tfPartialPayment),
sendmax(XRP(10)),
ter(tecPATH_DRY));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
// gateway unfreezes carol's USD
env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze));
env.close();
// alice successfully consumes carol's offer
env(pay(alice, bob, STAmount{lpIssue, 10}),
txflags(tfPartialPayment),
sendmax(XRP(10)));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 0));
}
else
{
// without fixFrozenLPTokenTransfer, alice can consume carol's offer
// even when carol's USD is frozen
env(pay(alice, bob, STAmount{lpIssue, 10}),
txflags(tfPartialPayment),
sendmax(XRP(10)));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 0));
}
// make sure carol's USD is not frozen
env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze));
env.close();
// ensure that carol's offer to buy lptoken can be consumed by alice
// even when carol's USD is frozen
{
// carol creates an offer to buy lptoken
env(offer(carol, STAmount{lpIssue, 10}, XRP(10)),
txflags(tfPassive));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// alice successfully consumes carol's offer
env(pay(alice, bob, XRP(10)),
txflags(tfPartialPayment),
sendmax(STAmount{lpIssue, 10}));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 0));
}
}
void
testOfferCreation(FeatureBitset features)
{
testcase("Create offer");
using namespace jtx;
Env env{*this, features};
fund(
env,
gw,
{alice, bob, carol},
{USD(10'000), EUR(10'000)},
Fund::All);
AMM ammAlice(env, alice, USD(10'000), EUR(10'000));
ammAlice.deposit(carol, 1'000);
ammAlice.deposit(bob, 1'000);
auto const lpIssue = ammAlice.lptIssue();
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// exercises carol's ability to create a new offer to sell lptoken with
// frozen USD, before and after fixFrozenLPTokenTransfer
if (features[fixFrozenLPTokenTransfer])
{
// with fixFrozenLPTokenTransfer, carol can't create an offer to
// sell lptoken when one of the assets is frozen
// carol can't create an offer to sell lptoken
env(offer(carol, XRP(10), STAmount{lpIssue, 10}),
txflags(tfPassive),
ter(tecUNFUNDED_OFFER));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 0));
// gateway unfreezes carol's USD
env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze));
env.close();
// carol can create an offer to sell lptoken after USD is unfrozen
env(offer(carol, XRP(10), STAmount{lpIssue, 10}),
txflags(tfPassive));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
}
else
{
// without fixFrozenLPTokenTransfer, carol can create an offer
env(offer(carol, XRP(10), STAmount{lpIssue, 10}),
txflags(tfPassive));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
}
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// carol can create offer to buy lptoken even if USD is frozen
env(offer(carol, STAmount{lpIssue, 10}, XRP(5)), txflags(tfPassive));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 2));
}
void
testOfferCrossing(FeatureBitset features)
{
testcase("Offer crossing");
using namespace jtx;
Env env{*this, features};
// Offer crossing with two AMM LPTokens.
fund(env, gw, {alice, carol}, {USD(10'000)}, Fund::All);
AMM ammAlice1(env, alice, XRP(10'000), USD(10'000));
ammAlice1.deposit(carol, 10'000'000);
fund(env, gw, {alice, carol}, {EUR(10'000)}, Fund::IOUOnly);
AMM ammAlice2(env, alice, XRP(10'000), EUR(10'000));
ammAlice2.deposit(carol, 10'000'000);
auto const token1 = ammAlice1.lptIssue();
auto const token2 = ammAlice2.lptIssue();
// carol creates offer
env(offer(carol, STAmount{token2, 100}, STAmount{token1, 100}));
env.close();
BEAST_EXPECT(expectOffers(env, carol, 1));
// gateway freezes carol's USD, carol's token1 should be frozen as well
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// alice creates an offer which exhibits different behavior on offer
// crossing depending on if fixFrozenLPTokenTransfer is enabled
env(offer(alice, STAmount{token1, 100}, STAmount{token2, 100}));
env.close();
// exercises carol's offer's ability to cross with alice's offer when
// carol's USD is frozen, before and after fixFrozenLPTokenTransfer
if (features[fixFrozenLPTokenTransfer])
{
// with fixFrozenLPTokenTransfer enabled, alice's offer can no
// longer cross with carol's offer
BEAST_EXPECT(
expectLine(env, alice, STAmount{token1, 10'000'000}) &&
expectLine(env, alice, STAmount{token2, 10'000'000}));
BEAST_EXPECT(
expectLine(env, carol, STAmount{token2, 10'000'000}) &&
expectLine(env, carol, STAmount{token1, 10'000'000}));
BEAST_EXPECT(
expectOffers(env, alice, 1) && expectOffers(env, carol, 0));
}
else
{
// alice's offer still crosses with carol's offer despite carol's
// token1 is frozen
BEAST_EXPECT(
expectLine(env, alice, STAmount{token1, 10'000'100}) &&
expectLine(env, alice, STAmount{token2, 9'999'900}));
BEAST_EXPECT(
expectLine(env, carol, STAmount{token2, 10'000'100}) &&
expectLine(env, carol, STAmount{token1, 9'999'900}));
BEAST_EXPECT(
expectOffers(env, alice, 0) && expectOffers(env, carol, 0));
}
}
void
testCheck(FeatureBitset features)
{
testcase("Check");
using namespace jtx;
Env env{*this, features};
fund(
env,
gw,
{alice, bob, carol},
{USD(10'000), EUR(10'000)},
Fund::All);
AMM ammAlice(env, alice, USD(10'000), EUR(10'000));
ammAlice.deposit(carol, 1'000);
ammAlice.deposit(bob, 1'000);
auto const lpIssue = ammAlice.lptIssue();
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// carol can always create a check with lptoken that has frozen
// token
uint256 const carolChkId{keylet::check(carol, env.seq(carol)).key};
env(check::create(carol, bob, STAmount{lpIssue, 10}));
env.close();
// with fixFrozenLPTokenTransfer enabled, bob fails to cash the check
if (features[fixFrozenLPTokenTransfer])
env(check::cash(bob, carolChkId, STAmount{lpIssue, 10}),
ter(tecPATH_PARTIAL));
else
env(check::cash(bob, carolChkId, STAmount{lpIssue, 10}));
env.close();
// bob creates a check
uint256 const bobChkId{keylet::check(bob, env.seq(bob)).key};
env(check::create(bob, carol, STAmount{lpIssue, 10}));
env.close();
// carol cashes the bob's check. Even though carol is frozen, she can
// still receive LPToken
env(check::cash(carol, bobChkId, STAmount{lpIssue, 10}));
env.close();
}
void
testNFTOffers(FeatureBitset features)
{
testcase("NFT Offers");
using namespace test::jtx;
Env env{*this, features};
// Setup AMM
fund(
env,
gw,
{alice, bob, carol},
{USD(10'000), EUR(10'000)},
Fund::All);
AMM ammAlice(env, alice, USD(10'000), EUR(10'000));
ammAlice.deposit(carol, 1'000);
ammAlice.deposit(bob, 1'000);
auto const lpIssue = ammAlice.lptIssue();
// bob mints a nft
uint256 const nftID{token::getNextID(env, bob, 0u, tfTransferable)};
env(token::mint(bob, 0), txflags(tfTransferable));
env.close();
// bob creates a sell offer for lptoken
uint256 const sellOfferIndex = keylet::nftoffer(bob, env.seq(bob)).key;
env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}),
txflags(tfSellNFToken));
env.close();
// gateway freezes carol's USD
env(trust(gw, carol["USD"](0), tfSetFreeze));
env.close();
// exercises one's ability to transfer NFT using lptoken when one of the
// assets is frozen
if (features[fixFrozenLPTokenTransfer])
{
// with fixFrozenLPTokenTransfer, freezing USD will prevent buy/sell
// offers with lptokens from being created/accepted
// carol fails to accept bob's offer with lptoken because carol's
// USD is frozen
env(token::acceptSellOffer(carol, sellOfferIndex),
ter(tecINSUFFICIENT_FUNDS));
env.close();
// gateway unfreezes carol's USD
env(trust(gw, carol["USD"](1'000'000), tfClearFreeze));
env.close();
// carol can now accept the offer and own the nft
env(token::acceptSellOffer(carol, sellOfferIndex));
env.close();
// gateway freezes bobs's USD
env(trust(gw, bob["USD"](0), tfSetFreeze));
env.close();
// bob fails to create a buy offer with lptoken for carol's nft
// since bob's USD is frozen
env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}),
token::owner(carol),
ter(tecUNFUNDED_OFFER));
env.close();
// gateway unfreezes bob's USD
env(trust(gw, bob["USD"](1'000'000), tfClearFreeze));
env.close();
// bob can now create a buy offer
env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}),
token::owner(carol));
env.close();
}
else
{
// without fixFrozenLPTokenTransfer, freezing USD will still allow
// buy/sell offers to be created/accepted with lptoken
// carol can still accept bob's offer despite carol's USD is frozen
env(token::acceptSellOffer(carol, sellOfferIndex));
env.close();
// gateway freezes bob's USD
env(trust(gw, bob["USD"](0), tfSetFreeze));
env.close();
// bob creates a buy offer with lptoken despite bob's USD is frozen
uint256 const buyOfferIndex =
keylet::nftoffer(bob, env.seq(bob)).key;
env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}),
token::owner(carol));
env.close();
// carol accepts bob's offer
env(token::acceptBuyOffer(carol, buyOfferIndex));
env.close();
}
}
public:
void
run() override
{
FeatureBitset const all{jtx::supported_amendments()};
for (auto const features : {all, all - fixFrozenLPTokenTransfer})
{
testDirectStep(features);
testBookStep(features);
testOfferCreation(features);
testOfferCrossing(features);
testCheck(features);
testNFTOffers(features);
}
}
};
BEAST_DEFINE_TESTSUITE(LPTokenTransfer, app, ripple);
} // namespace test
} // namespace ripple

View File

@@ -116,13 +116,45 @@ ammLPHolds(
AccountID const& lpAccount,
beast::Journal const j)
{
return accountHolds(
view,
lpAccount,
ammLPTCurrency(cur1, cur2),
ammAccount,
FreezeHandling::fhZERO_IF_FROZEN,
j);
// This function looks similar to `accountHolds`. However, it only checks if
// a LPToken holder has enough balance. On the other hand, `accountHolds`
// checks if the underlying assets of LPToken are frozen with the
// fixFrozenLPTokenTransfer amendment
auto const currency = ammLPTCurrency(cur1, cur2);
STAmount amount;
auto const sle = view.read(keylet::line(lpAccount, ammAccount, currency));
if (!sle)
{
amount.clear(Issue{currency, ammAccount});
JLOG(j.trace()) << "ammLPHolds: no SLE "
<< " lpAccount=" << to_string(lpAccount)
<< " amount=" << amount.getFullText();
}
else if (isFrozen(view, lpAccount, currency, ammAccount))
{
amount.clear(Issue{currency, ammAccount});
JLOG(j.trace()) << "ammLPHolds: frozen currency "
<< " lpAccount=" << to_string(lpAccount)
<< " amount=" << amount.getFullText();
}
else
{
amount = sle->getFieldAmount(sfBalance);
if (lpAccount > ammAccount)
{
// Put balance in account terms.
amount.negate();
}
amount.setIssuer(ammAccount);
JLOG(j.trace()) << "ammLPHolds:"
<< " lpAccount=" << to_string(lpAccount)
<< " amount=" << amount.getFullText();
}
return view.balanceHook(lpAccount, ammAccount, amount);
}
STAmount

View File

@@ -204,7 +204,8 @@ protected:
logStringImpl(char const* name) const
{
std::ostringstream ostr;
ostr << name << ": " << "\nSrc: " << src_ << "\nDst: " << dst_;
ostr << name << ": "
<< "\nSrc: " << src_ << "\nDst: " << dst_;
return ostr.str();
}

View File

@@ -21,6 +21,7 @@
#define RIPPLE_APP_PATHS_IMPL_STEP_CHECKS_H_INCLUDED
#include <xrpld/ledger/ReadView.h>
#include <xrpld/ledger/View.h>
#include <xrpl/basics/Log.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/protocol/AccountID.h>
@@ -60,6 +61,26 @@ checkFreeze(
}
}
if (view.rules().enabled(fixFrozenLPTokenTransfer))
{
if (auto const sleDst = view.read(keylet::account(dst));
sleDst && sleDst->isFieldPresent(sfAMMID))
{
auto const sleAmm = view.read(keylet::amm((*sleDst)[sfAMMID]));
if (!sleAmm)
return tecINTERNAL; // LCOV_EXCL_LINE
if (isLPTokenFrozen(
view,
src,
(*sleAmm)[sfAsset].get<Issue>(),
(*sleAmm)[sfAsset2].get<Issue>()))
{
return terNO_LINE;
}
}
}
return tesSUCCESS;
}

View File

@@ -160,6 +160,13 @@ isDeepFrozen(
Currency const& currency,
AccountID const& issuer);
[[nodiscard]] bool
isLPTokenFrozen(
ReadView const& view,
AccountID const& account,
Issue const& asset,
Issue const& asset2);
// Returns the amount an account can spend without going into debt.
//
// <-- saAmount: amount of currency held by account. May be negative.

View File

@@ -293,6 +293,17 @@ isDeepFrozen(
return sle->isFlag(lsfHighDeepFreeze) || sle->isFlag(lsfLowDeepFreeze);
}
bool
isLPTokenFrozen(
ReadView const& view,
AccountID const& account,
Issue const& asset,
Issue const& asset2)
{
return isFrozen(view, account, asset.currency, asset.account) ||
isFrozen(view, account, asset2.currency, asset2.account);
}
STAmount
accountHolds(
ReadView const& view,
@@ -323,6 +334,32 @@ accountHolds(
{
return false;
}
// when fixFrozenLPTokenTransfer is enabled, if currency is lptoken,
// we need to check if the associated assets have been frozen
if (view.rules().enabled(fixFrozenLPTokenTransfer))
{
auto const sleIssuer = view.read(keylet::account(issuer));
if (!sleIssuer)
{
return false; // LCOV_EXCL_LINE
}
else if (sleIssuer->isFieldPresent(sfAMMID))
{
auto const sleAmm =
view.read(keylet::amm((*sleIssuer)[sfAMMID]));
if (!sleAmm ||
isLPTokenFrozen(
view,
account,
(*sleAmm)[sfAsset].get<Issue>(),
(*sleAmm)[sfAsset2].get<Issue>()))
{
return false;
}
}
}
}
return true;
@@ -492,7 +529,8 @@ xrpLiquid(
STAmount const amount =
(balance < reserve) ? STAmount{0} : balance - reserve;
JLOG(j.trace()) << "accountHolds:" << " account=" << to_string(id)
JLOG(j.trace()) << "accountHolds:"
<< " account=" << to_string(id)
<< " amount=" << amount.getFullText()
<< " fullBalance=" << fullBalance.getFullText()
<< " balance=" << balance.getFullText()