ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-04-29 15:37:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add the fixEnforceNFTokenTrustline amendment: (#4946)

Browse Source 
Fix interactions between NFTokenOffers and trust lines.

Since the NFTokenAcceptOffer does not check the trust line that
the issuer receives as a transfer fee in the NFTokenAcceptOffer,
if the issuer deletes the trust line after NFTokenCreateOffer,
the trust line is created for the issuer by the
NFTokenAcceptOffer.  That's fixed.

Resolves #4925.
This commit is contained in:
Scott Schurr
2024-06-17 23:47:54 -07:00
committed by GitHub
parent 825864032a
commit 223e6c7590
5 changed files with 202 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
View File

@@ -270,6 +270,28 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
}
}
// Fix a bug where the transfer of an NFToken with a transfer fee could
// give the NFToken issuer an undesired trust line.
if (ctx.view.rules().enabled(fixEnforceNFTokenTrustline))
{
std::shared_ptr<SLE const> const& offer = bo ? bo : so;
if (!offer)
// Should be caught in preflight.
return tecINTERNAL;
uint256 const& tokenID = offer->at(sfNFTokenID);
STAmount const& amount = offer->at(sfAmount);
if (nft::getTransferFee(tokenID) != 0 &&
(nft::getFlags(tokenID) & nft::flagCreateTrustLines) == 0 &&
!amount.native())
{
auto const issuer = nft::getIssuer(tokenID);
// Issuer doesn't need a trust line to accept their own currency.
if (issuer != amount.getIssuer() &&
!ctx.view.read(keylet::line(issuer, amount.issue())))
return tecNO_LINE;
}
}
return tesSUCCESS;
}

3
src/ripple/protocol/Feature.h
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 = 75;
static constexpr std::size_t numFeatures = 76;
/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
@@ -368,6 +368,7 @@ extern uint256 const fixPreviousTxnID;
extern uint256 const fixAMMv1_1;
extern uint256 const featureNFTokenMintOffer;
extern uint256 const fixReducedOffersV2;
extern uint256 const fixEnforceNFTokenTrustline;
} // namespace ripple

1
src/ripple/protocol/impl/Feature.cpp
View File

@@ -495,6 +495,7 @@ REGISTER_FIX (fixPreviousTxnID, Supported::yes, VoteBehavior::De
REGISTER_FIX (fixAMMv1_1, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixReducedOffersV2, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixEnforceNFTokenTrustline, Supported::yes, VoteBehavior::DefaultNo);
// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.

171
src/test/app/NFToken_test.cpp
View File

@@ -7416,6 +7416,176 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
}
}
void
testUnaskedForAutoTrustline(FeatureBitset features)
{
testcase("Test fix unasked for auto-trustline.");
using namespace test::jtx;
Account const issuer{"issuer"};
Account const becky{"becky"};
Account const cheri{"cheri"};
Account const gw("gw");
IOU const gwAUD(gw["AUD"]);
// This test case covers issue...
// https://github.com/XRPLF/rippled/issues/4925
//
// For an NFToken with a transfer fee, the issuer must be able to
// accept the transfer fee or else a transfer should fail. If the
// NFToken is transferred for a non-XRP asset, then the issuer must
// have a trustline to that asset to receive the fee.
//
// This test looks at a situation where issuer would get a trustline
// for the fee without the issuer's consent. Here are the steps:
// 1. Issuer has a trustline (i.e., USD)
// 2. Issuer mints NFToken with transfer fee.
// 3. Becky acquires the NFToken, paying with XRP.
// 4. Becky creates offer to sell NFToken for USD(100).
// 5. Issuer deletes trustline for USD.
// 6. Carol buys NFToken from Becky for USD(100).
// 7. The transfer fee from Carol's purchase re-establishes issuer's
// USD trustline.
//
// The fixEnforceNFTokenTrustline amendment addresses this oversight.
//
// We run this test case both with and without
// fixEnforceNFTokenTrustline enabled so we can see the change
// in behavior.
//
// In both cases we remove the fixRemoveNFTokenAutoTrustLine amendment.
// Otherwise we can't create NFTokens with tfTrustLine enabled.
FeatureBitset const localFeatures =
features - fixRemoveNFTokenAutoTrustLine;
for (FeatureBitset feats :
{localFeatures - fixEnforceNFTokenTrustline,
localFeatures | fixEnforceNFTokenTrustline})
{
Env env{*this, feats};
env.fund(XRP(1000), issuer, becky, cheri, gw);
env.close();
// Set trust lines so becky and cheri can use gw's currency.
env(trust(becky, gwAUD(1000)));
env(trust(cheri, gwAUD(1000)));
env.close();
env(pay(gw, cheri, gwAUD(500)));
env.close();
// issuer creates two NFTs: one with and one without AutoTrustLine.
std::uint16_t xferFee = 5000; // 5%
uint256 const nftAutoTrustID{token::getNextID(
env, issuer, 0u, tfTransferable | tfTrustLine, xferFee)};
env(token::mint(issuer, 0u),
token::xferFee(xferFee),
txflags(tfTransferable | tfTrustLine));
env.close();
uint256 const nftNoAutoTrustID{
token::getNextID(env, issuer, 0u, tfTransferable, xferFee)};
env(token::mint(issuer, 0u),
token::xferFee(xferFee),
txflags(tfTransferable));
env.close();
// becky buys the nfts for 1 drop each.
{
uint256 const beckyBuyOfferIndex1 =
keylet::nftoffer(becky, env.seq(becky)).key;
env(token::createOffer(becky, nftAutoTrustID, drops(1)),
token::owner(issuer));
uint256 const beckyBuyOfferIndex2 =
keylet::nftoffer(becky, env.seq(becky)).key;
env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
token::owner(issuer));
env.close();
env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex1));
env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex2));
env.close();
}
// becky creates offers to sell the nfts for AUD.
uint256 const beckyAutoTrustOfferIndex =
keylet::nftoffer(becky, env.seq(becky)).key;
env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)),
txflags(tfSellNFToken));
env.close();
// Creating an offer for the NFToken without tfTrustLine fails
// because issuer does not have a trust line for AUD.
env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
txflags(tfSellNFToken),
ter(tecNO_LINE));
env.close();
// issuer creates a trust line. Now the offer create for the
// NFToken without tfTrustLine succeeds.
BEAST_EXPECT(ownerCount(env, issuer) == 0);
env(trust(issuer, gwAUD(1000)));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 1);
uint256 const beckyNoAutoTrustOfferIndex =
keylet::nftoffer(becky, env.seq(becky)).key;
env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
txflags(tfSellNFToken));
env.close();
// Now that the offers are in place, issuer removes the trustline.
BEAST_EXPECT(ownerCount(env, issuer) == 1);
env(trust(issuer, gwAUD(0)));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
// cheri attempts to accept becky's offers. Behavior with the
// AutoTrustline NFT is uniform: issuer gets a new trust line.
env(token::acceptSellOffer(cheri, beckyAutoTrustOfferIndex));
env.close();
// Here's evidence that issuer got the new AUD trust line.
BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(env.balance(issuer, gwAUD) == gwAUD(5));
// issuer once again removes the trust line for AUD.
env(pay(issuer, gw, gwAUD(5)));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
// cheri attempts to accept the NoAutoTrustLine NFT. Behavior
// depends on whether fixEnforceNFTokenTrustline is enabled.
if (feats[fixEnforceNFTokenTrustline])
{
// With fixEnforceNFTokenTrustline cheri can't accept the
// offer because issuer could not get their transfer fee
// without the appropriate trustline.
env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex),
ter(tecNO_LINE));
env.close();
// But if issuer re-establishes the trustline then the offer
// can be accepted.
env(trust(issuer, gwAUD(1000)));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 1);
env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex));
env.close();
}
else
{
// Without fixEnforceNFTokenTrustline the offer just works
// and issuer gets a trustline that they did not request.
env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex));
env.close();
}
BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(env.balance(issuer, gwAUD) == gwAUD(5));
} // for feats
}
void
testNFTIssuerIsIOUIssuer(FeatureBitset features)
{
@@ -7609,6 +7779,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
testFeatMintWithOffer(features);
testTxJsonMetaFields(features);
testFixNFTokenBuyerReserve(features);
testUnaskedForAutoTrustline(features);
testNFTIssuerIsIOUIssuer(features);
}

12
src/test/app/Offer_test.cpp
View File

@@ -5467,12 +5467,12 @@ class Offer_manual_test : public OfferBaseUtil_test
}
};
BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFlowCross, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWTakerDryOffer, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferAllFeatures, tx, ripple, 4);
BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFlowCross, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWTakerDryOffer, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferAllFeatures, tx, ripple, 2);
BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Offer_manual, tx, ripple, 20);
} // namespace test