mirror of
https://github.com/XRPLF/rippled.git
synced 2025-11-13 07:35:57 +00:00
Compare commits
2 Commits
develop
...
pratik/Rem
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
060eff0cf4 | ||
|
|
0a41305a75 |
@@ -211,18 +211,13 @@ constexpr std::uint32_t const tfMPTokenIssuanceDestroyMask = ~tfUniversal;
|
||||
// issuer's reserve without bound.
|
||||
//
|
||||
// The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
|
||||
// tfTrustLine flag as a way to prevent the attack. But until the
|
||||
// amendment passes we still need to keep the old behavior available.
|
||||
// tfTrustLine flag as a way to prevent the attack. Since the amendment
|
||||
// has passed, we don't include tfTrustLine flag in tfNFTokenMintMask anymore.
|
||||
|
||||
constexpr std::uint32_t const tfNFTokenMintMask =
|
||||
~(tfUniversal | tfBurnable | tfOnlyXRP | tfTransferable);
|
||||
|
||||
constexpr std::uint32_t const tfNFTokenMintOldMask =
|
||||
~( ~tfNFTokenMintMask | tfTrustLine);
|
||||
|
||||
// if featureDynamicNFT enabled then new flag allowing mutable URI available.
|
||||
constexpr std::uint32_t const tfNFTokenMintOldMaskWithMutable =
|
||||
~( ~tfNFTokenMintOldMask | tfMutable);
|
||||
|
||||
// if featureDynamicNFT enabled then flag allowing mutable URI available.
|
||||
constexpr std::uint32_t const tfNFTokenMintMaskWithMutable =
|
||||
~( ~tfNFTokenMintMask | tfMutable);
|
||||
|
||||
|
||||
@@ -86,7 +86,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo
|
||||
XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(ImmediateOfferKilled, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
|
||||
XRPL_FIX (TrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(NonFungibleTokensV1_1, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo)
|
||||
@@ -155,3 +154,4 @@ XRPL_RETIRE(FlowCross)
|
||||
XRPL_RETIRE(fix1513)
|
||||
XRPL_RETIRE(fix1515)
|
||||
XRPL_RETIRE(fix1543)
|
||||
XRPL_RETIRE(fixRemoveNFTokenAutoTrustLine)
|
||||
|
||||
@@ -1621,180 +1621,78 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
|
||||
IOU const gwCAD(gw["CAD"]);
|
||||
IOU const gwEUR(gw["EUR"]);
|
||||
|
||||
// The behavior of this test changes dramatically based on the
|
||||
// presence (or absence) of the fixRemoveNFTokenAutoTrustLine
|
||||
// amendment. So we test both cases here.
|
||||
for (auto const& tweakedFeatures :
|
||||
{features - fixRemoveNFTokenAutoTrustLine,
|
||||
features | fixRemoveNFTokenAutoTrustLine})
|
||||
Env env{*this, features};
|
||||
env.fund(XRP(1000), alice, 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(trust(becky, gwCAD(1000)));
|
||||
env(trust(cheri, gwCAD(1000)));
|
||||
env(trust(becky, gwEUR(1000)));
|
||||
env(trust(cheri, gwEUR(1000)));
|
||||
env.close();
|
||||
env(pay(gw, becky, gwAUD(500)));
|
||||
env(pay(gw, becky, gwCAD(500)));
|
||||
env(pay(gw, becky, gwEUR(500)));
|
||||
env(pay(gw, cheri, gwAUD(500)));
|
||||
env(pay(gw, cheri, gwCAD(500)));
|
||||
env.close();
|
||||
|
||||
// An nft without flagCreateTrustLines but with a non-zero transfer
|
||||
// fee will not allow creating offers that use IOUs for payment.
|
||||
for (std::uint32_t xferFee : {0, 1})
|
||||
{
|
||||
Env env{*this, tweakedFeatures};
|
||||
env.fund(XRP(1000), alice, becky, cheri, gw);
|
||||
uint256 const nftNoAutoTrustID{
|
||||
token::getNextID(env, alice, 0u, tfTransferable, xferFee)};
|
||||
env(token::mint(alice, 0u),
|
||||
token::xferFee(xferFee),
|
||||
txflags(tfTransferable));
|
||||
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(trust(becky, gwCAD(1000)));
|
||||
env(trust(cheri, gwCAD(1000)));
|
||||
env(trust(becky, gwEUR(1000)));
|
||||
env(trust(cheri, gwEUR(1000)));
|
||||
// becky buys the nft for 1 drop.
|
||||
uint256 const beckyBuyOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
|
||||
token::owner(alice));
|
||||
env.close();
|
||||
env(pay(gw, becky, gwAUD(500)));
|
||||
env(pay(gw, becky, gwCAD(500)));
|
||||
env(pay(gw, becky, gwEUR(500)));
|
||||
env(pay(gw, cheri, gwAUD(500)));
|
||||
env(pay(gw, cheri, gwCAD(500)));
|
||||
env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
|
||||
env.close();
|
||||
|
||||
// An nft without flagCreateTrustLines but with a non-zero transfer
|
||||
// fee will not allow creating offers that use IOUs for payment.
|
||||
for (std::uint32_t xferFee : {0, 1})
|
||||
{
|
||||
uint256 const nftNoAutoTrustID{
|
||||
token::getNextID(env, alice, 0u, tfTransferable, xferFee)};
|
||||
env(token::mint(alice, 0u),
|
||||
token::xferFee(xferFee),
|
||||
txflags(tfTransferable));
|
||||
env.close();
|
||||
// becky attempts to sell the nft for AUD.
|
||||
TER const createOfferTER =
|
||||
xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS);
|
||||
uint256 const beckyOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
|
||||
txflags(tfSellNFToken),
|
||||
ter(createOfferTER));
|
||||
env.close();
|
||||
|
||||
// becky buys the nft for 1 drop.
|
||||
uint256 const beckyBuyOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
|
||||
token::owner(alice));
|
||||
env.close();
|
||||
env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
|
||||
env.close();
|
||||
// cheri offers to buy the nft for CAD.
|
||||
uint256 const cheriOfferIndex =
|
||||
keylet::nftoffer(cheri, env.seq(cheri)).key;
|
||||
env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
|
||||
token::owner(becky),
|
||||
ter(createOfferTER));
|
||||
env.close();
|
||||
|
||||
// becky attempts to sell the nft for AUD.
|
||||
TER const createOfferTER =
|
||||
xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS);
|
||||
uint256 const beckyOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
|
||||
txflags(tfSellNFToken),
|
||||
ter(createOfferTER));
|
||||
env.close();
|
||||
|
||||
// cheri offers to buy the nft for CAD.
|
||||
uint256 const cheriOfferIndex =
|
||||
keylet::nftoffer(cheri, env.seq(cheri)).key;
|
||||
env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
|
||||
token::owner(becky),
|
||||
ter(createOfferTER));
|
||||
env.close();
|
||||
|
||||
// To keep things tidy, cancel the offers.
|
||||
env(token::cancelOffer(becky, {beckyOfferIndex}));
|
||||
env(token::cancelOffer(cheri, {cheriOfferIndex}));
|
||||
env.close();
|
||||
}
|
||||
// An nft with flagCreateTrustLines but with a non-zero transfer
|
||||
// fee allows transfers using IOUs for payment.
|
||||
{
|
||||
std::uint16_t transferFee = 10000; // 10%
|
||||
|
||||
uint256 const nftAutoTrustID{token::getNextID(
|
||||
env, alice, 0u, tfTransferable | tfTrustLine, transferFee)};
|
||||
|
||||
// If the fixRemoveNFTokenAutoTrustLine amendment is active
|
||||
// then this transaction fails.
|
||||
{
|
||||
TER const mintTER =
|
||||
tweakedFeatures[fixRemoveNFTokenAutoTrustLine]
|
||||
? static_cast<TER>(temINVALID_FLAG)
|
||||
: static_cast<TER>(tesSUCCESS);
|
||||
|
||||
env(token::mint(alice, 0u),
|
||||
token::xferFee(transferFee),
|
||||
txflags(tfTransferable | tfTrustLine),
|
||||
ter(mintTER));
|
||||
env.close();
|
||||
|
||||
// If fixRemoveNFTokenAutoTrustLine is active the rest
|
||||
// of this test falls on its face.
|
||||
if (tweakedFeatures[fixRemoveNFTokenAutoTrustLine])
|
||||
break;
|
||||
}
|
||||
// becky buys the nft for 1 drop.
|
||||
uint256 const beckyBuyOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftAutoTrustID, drops(1)),
|
||||
token::owner(alice));
|
||||
env.close();
|
||||
env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
|
||||
env.close();
|
||||
|
||||
// becky sells the nft for AUD.
|
||||
uint256 const beckySellOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)),
|
||||
txflags(tfSellNFToken));
|
||||
env.close();
|
||||
env(token::acceptSellOffer(cheri, beckySellOfferIndex));
|
||||
env.close();
|
||||
|
||||
// alice should now have a trust line for gwAUD.
|
||||
BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10));
|
||||
|
||||
// becky buys the nft back for CAD.
|
||||
uint256 const beckyBuyBackOfferIndex =
|
||||
keylet::nftoffer(becky, env.seq(becky)).key;
|
||||
env(token::createOffer(becky, nftAutoTrustID, gwCAD(50)),
|
||||
token::owner(cheri));
|
||||
env.close();
|
||||
env(token::acceptBuyOffer(cheri, beckyBuyBackOfferIndex));
|
||||
env.close();
|
||||
|
||||
// alice should now have a trust line for gwAUD and gwCAD.
|
||||
BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10));
|
||||
BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(5));
|
||||
}
|
||||
// Now that alice has trust lines preestablished, an nft without
|
||||
// flagCreateTrustLines will work for preestablished trust lines.
|
||||
{
|
||||
std::uint16_t transferFee = 5000; // 5%
|
||||
uint256 const nftNoAutoTrustID{token::getNextID(
|
||||
env, alice, 0u, tfTransferable, transferFee)};
|
||||
env(token::mint(alice, 0u),
|
||||
token::xferFee(transferFee),
|
||||
txflags(tfTransferable));
|
||||
env.close();
|
||||
|
||||
// alice sells the nft using AUD.
|
||||
uint256 const aliceSellOfferIndex =
|
||||
keylet::nftoffer(alice, env.seq(alice)).key;
|
||||
env(token::createOffer(alice, nftNoAutoTrustID, gwAUD(200)),
|
||||
txflags(tfSellNFToken));
|
||||
env.close();
|
||||
env(token::acceptSellOffer(cheri, aliceSellOfferIndex));
|
||||
env.close();
|
||||
|
||||
// alice should now have AUD(210):
|
||||
// o 200 for this sale and
|
||||
// o 10 for the previous sale's fee.
|
||||
BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(210));
|
||||
|
||||
// cheri can't sell the NFT for EUR, but can for CAD.
|
||||
env(token::createOffer(cheri, nftNoAutoTrustID, gwEUR(50)),
|
||||
txflags(tfSellNFToken),
|
||||
ter(tecNO_LINE));
|
||||
env.close();
|
||||
uint256 const cheriSellOfferIndex =
|
||||
keylet::nftoffer(cheri, env.seq(cheri)).key;
|
||||
env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
|
||||
txflags(tfSellNFToken));
|
||||
env.close();
|
||||
env(token::acceptSellOffer(becky, cheriSellOfferIndex));
|
||||
env.close();
|
||||
|
||||
// alice should now have CAD(10):
|
||||
// o 5 from this sale's fee and
|
||||
// o 5 for the previous sale's fee.
|
||||
BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(10));
|
||||
}
|
||||
// To keep things tidy, cancel the offers.
|
||||
env(token::cancelOffer(becky, {beckyOfferIndex}));
|
||||
env(token::cancelOffer(cheri, {cheriOfferIndex}));
|
||||
env.close();
|
||||
}
|
||||
// An nft with flagCreateTrustLines but with a non-zero transfer
|
||||
// fee allows transfers using IOUs for payment.
|
||||
|
||||
std::uint16_t transferFee = 10000; // 10%
|
||||
|
||||
env(token::mint(alice, 0u),
|
||||
token::xferFee(transferFee),
|
||||
txflags(tfTransferable | tfTrustLine),
|
||||
ter(static_cast<TER>(temINVALID_FLAG)));
|
||||
env.close();
|
||||
}
|
||||
|
||||
void
|
||||
@@ -7433,12 +7331,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
|
||||
// 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
|
||||
// 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:
|
||||
// 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.
|
||||
@@ -7456,8 +7354,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
|
||||
//
|
||||
// In both cases we remove the fixRemoveNFTokenAutoTrustLine amendment.
|
||||
// Otherwise we can't create NFTokens with tfTrustLine enabled.
|
||||
FeatureBitset const localFeatures =
|
||||
features - fixRemoveNFTokenAutoTrustLine;
|
||||
FeatureBitset const localFeatures = features;
|
||||
for (FeatureBitset feats :
|
||||
{localFeatures - fixEnforceNFTokenTrustline,
|
||||
localFeatures | fixEnforceNFTokenTrustline})
|
||||
@@ -7627,8 +7524,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
|
||||
//
|
||||
// We remove the fixRemoveNFTokenAutoTrustLine amendment. Otherwise
|
||||
// we can't create NFTokens with tfTrustLine enabled.
|
||||
FeatureBitset const localFeatures =
|
||||
features - fixRemoveNFTokenAutoTrustLine;
|
||||
FeatureBitset const localFeatures = features;
|
||||
|
||||
Env env{*this, localFeatures};
|
||||
env.fund(XRP(1000), issuer, becky, cheri);
|
||||
|
||||
@@ -55,28 +55,11 @@ NFTokenMint::checkExtraFeatures(PreflightContext const& ctx)
|
||||
std::uint32_t
|
||||
NFTokenMint::getFlagsMask(PreflightContext const& ctx)
|
||||
{
|
||||
// Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between
|
||||
// accounts allowed a TrustLine to be added to the issuer of that token
|
||||
// without explicit permission from that issuer. This was enabled by
|
||||
// minting the NFToken with the tfTrustLine flag set.
|
||||
//
|
||||
// That capability could be used to attack the NFToken issuer. It
|
||||
// would be possible for two accounts to trade the NFToken back and forth
|
||||
// building up any number of TrustLines on the issuer, increasing the
|
||||
// issuer's reserve without bound.
|
||||
//
|
||||
// The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
|
||||
// tfTrustLine flag as a way to prevent the attack. But until the
|
||||
// amendment passes we still need to keep the old behavior available.
|
||||
std::uint32_t const nfTokenMintMask =
|
||||
ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine)
|
||||
// if featureDynamicNFT enabled then new flag allowing mutable URI
|
||||
// available
|
||||
? ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintMaskWithMutable
|
||||
: tfNFTokenMintMask
|
||||
: ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable
|
||||
: tfNFTokenMintOldMask;
|
||||
|
||||
ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintMaskWithMutable
|
||||
: tfNFTokenMintMask;
|
||||
return nfTokenMintMask;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user