ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-28 06:55:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: retire/remove NFT amendments (#5971)

Browse Source 
Amendments activated for more than 2 years can be retired, and obsolete retirements that were never activated can also be removed after 2 years. This change retires the NonFungibleTokensV1_1, fixNonFungibleTokensV1_2, and fixNFTokenRemint amendments, and removes the NonFungibleTokensV1, fixNFTokenNegOffer, and fixNFTokenDirV1 amendments.
This commit is contained in:
Jingchen
2025-11-03 18:43:57 +00:00
committed by GitHub
parent 8ac8a47c99
commit 63a08560ca
18 changed files with 397 additions and 1307 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
include/xrpl/protocol/detail/features.macro
View File

@@ -77,14 +77,11 @@ XRPL_FIX (DisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo
XRPL_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (NFTokenRemint, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (NonFungibleTokensV1_2, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(DisallowIncoming, 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)
XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes)
@@ -111,9 +108,6 @@ XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYe
//
// If a feature remains obsolete for long enough that no clients are able
// to vote for it, the feature can be removed (entirely?) from the code.
XRPL_FIX (NFTokenNegOffer, Supported::yes, VoteBehavior::Obsolete)
XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete)
XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete)
XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete)
// The following amendments have been active for at least two years. Their
@@ -137,6 +131,8 @@ XRPL_RETIRE(fix1623)
XRPL_RETIRE(fix1781)
XRPL_RETIRE(fixAmendmentMajorityCalc)
XRPL_RETIRE(fixCheckThreading)
XRPL_RETIRE(fixNonFungibleTokensV1_2)
XRPL_RETIRE(fixNFTokenRemint)
XRPL_RETIRE(fixMasterKeyAsRegularKey)
XRPL_RETIRE(fixQualityUpperBound)
XRPL_RETIRE(fixReducedOffersV1)
@@ -150,6 +146,7 @@ XRPL_RETIRE(FeeEscalation)
XRPL_RETIRE(FlowCross)
XRPL_RETIRE(ImmediateOfferKilled)
XRPL_RETIRE(MultiSign)
XRPL_RETIRE(NonFungibleTokensV1_1)
XRPL_RETIRE(PayChan)
XRPL_RETIRE(SortedDirectories)
XRPL_RETIRE(TickSize)

10
include/xrpl/protocol/detail/transactions.macro
View File

@@ -332,7 +332,7 @@ TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete,
#endif
TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint,
Delegation::delegatable,
featureNonFungibleTokensV1,
uint256{},
changeNFTCounts,
({
{sfNFTokenTaxon, soeREQUIRED},
@@ -350,7 +350,7 @@ TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint,
#endif
TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn,
Delegation::delegatable,
featureNonFungibleTokensV1,
uint256{},
changeNFTCounts,
({
{sfNFTokenID, soeREQUIRED},
@@ -363,7 +363,7 @@ TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn,
#endif
TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer,
Delegation::delegatable,
featureNonFungibleTokensV1,
uint256{},
noPriv,
({
{sfNFTokenID, soeREQUIRED},
@@ -379,7 +379,7 @@ TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer,
#endif
TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer,
Delegation::delegatable,
featureNonFungibleTokensV1,
uint256{},
noPriv,
({
{sfNFTokenOffers, soeREQUIRED},
@@ -391,7 +391,7 @@ TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer,
#endif
TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer,
Delegation::delegatable,
featureNonFungibleTokensV1,
uint256{},
noPriv,
({
{sfNFTokenBuyOffer, soeOPTIONAL},

11
src/libxrpl/protocol/Rules.cpp
View File

@@ -131,17 +131,6 @@ Rules::enabled(uint256 const& feature) const
{
XRPL_ASSERT(impl_, "ripple::Rules::enabled : initialized");
// The functionality of the "NonFungibleTokensV1_1" amendment is
// precisely the functionality of the following three amendments
// so if their status is ever queried individually, we inject an
// extra check here to simplify the checking elsewhere.
if (feature == featureNonFungibleTokensV1 ||
feature == fixNFTokenNegOffer || feature == fixNFTokenDirV1)
{
if (impl_->enabled(featureNonFungibleTokensV1_1))
return true;
}
return impl_->enabled(feature);
}

52
src/test/app/Delegate_test.cpp
View File

@@ -1800,58 +1800,6 @@ class Delegate_test : public beast::unit_test::suite
for (auto const& tx : txRequiredFeatures)
txAmendmentEnabled(tx.first);
}
// NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and
// NFTokenAcceptOffer are tested separately. Since
// featureNonFungibleTokensV1_1 includes the functionality of
// featureNonFungibleTokensV1, fixNFTokenNegOffer, and fixNFTokenDirV1,
// both featureNonFungibleTokensV1_1 and featureNonFungibleTokensV1 need
// to be disabled to block these transactions from being delegated.
{
Env env(
*this,
features - featureNonFungibleTokensV1 -
featureNonFungibleTokensV1_1);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
for (auto const tx :
{"NFTokenMint",
"NFTokenBurn",
"NFTokenCreateOffer",
"NFTokenCancelOffer",
"NFTokenAcceptOffer"})
{
env(delegate::set(alice, bob, {tx}), ter(temMALFORMED));
}
}
// NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and
// NFTokenAcceptOffer are allowed to be delegated if either
// featureNonFungibleTokensV1 or featureNonFungibleTokensV1_1 is
// enabled.
{
for (auto const feature :
{featureNonFungibleTokensV1, featureNonFungibleTokensV1_1})
{
Env env(*this, features - feature);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
for (auto const tx :
{"NFTokenMint",
"NFTokenBurn",
"NFTokenCreateOffer",
"NFTokenCancelOffer",
"NFTokenAcceptOffer"})
env(delegate::set(alice, bob, {tx}));
}
}
}
void

10
src/test/app/FixNFTokenPageLinks_test.cpp
View File

@@ -69,12 +69,10 @@ class FixNFTokenPageLinks_test : public beast::unit_test::suite
return 0u;
}();
// If fixNFTokenRemint amendment is on, we must
// add FirstNFTokenSequence.
if (env.current()->rules().enabled(fixNFTokenRemint))
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
// We must add FirstNFTokenSequence.
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
return toUInt32(nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon)));
};

210
src/test/app/NFTokenBurn_test.cpp
View File

@@ -28,7 +28,7 @@
namespace ripple {
class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
class NFTokenBurn_test : public beast::unit_test::suite
{
// Helper function that returns the number of nfts owned by an account.
static std::uint32_t
@@ -375,12 +375,10 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
std::uint32_t tokenSeq =
env.le(acct)->at(~sfMintedNFTokens).value_or(0);
// If fixNFTokenRemint amendment is on, we must
// add FirstNFTokenSequence.
if (env.current()->rules().enabled(fixNFTokenRemint))
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
// We must add FirstNFTokenSequence.
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
return toUInt32(
nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon)));
@@ -893,107 +891,9 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
using namespace test::jtx;
// Test what happens if a NFT is unburnable when there are
// more than 500 offers, before fixNonFungibleTokensV1_2 goes live
if (!features[fixNonFungibleTokensV1_2])
{
Env env{*this, features};
Account const alice("alice");
Account const becky("becky");
env.fund(XRP(1000), alice, becky);
env.close();
// We structure the test to try and maximize the metadata produced.
// This verifies that we don't create too much metadata during a
// maximal burn operation.
//
// 1. alice mints an nft with a full-sized URI.
// 2. We create 500 new accounts, each of which creates an offer
// for alice's nft.
// 3. becky creates one more offer for alice's NFT
// 4. Attempt to burn the nft which fails because there are too
// many offers.
// 5. Cancel becky's offer and the nft should become burnable.
uint256 const nftokenID =
token::getNextID(env, alice, 0, tfTransferable);
env(token::mint(alice, 0),
token::uri(std::string(maxTokenURILength, 'u')),
txflags(tfTransferable));
env.close();
std::vector<uint256> offerIndexes;
offerIndexes.reserve(maxTokenOfferCancelCount);
for (std::uint32_t i = 0; i < maxTokenOfferCancelCount; ++i)
{
Account const acct(std::string("acct") + std::to_string(i));
env.fund(XRP(1000), acct);
env.close();
offerIndexes.push_back(
keylet::nftoffer(acct, env.seq(acct)).key);
env(token::createOffer(acct, nftokenID, drops(1)),
token::owner(alice));
env.close();
}
// Verify all offers are present in the ledger.
for (uint256 const& offerIndex : offerIndexes)
{
BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex)));
}
// Create one too many offers.
uint256 const beckyOfferIndex =
keylet::nftoffer(becky, env.seq(becky)).key;
env(token::createOffer(becky, nftokenID, drops(1)),
token::owner(alice));
// Attempt to burn the nft which should fail.
env(token::burn(alice, nftokenID), ter(tefTOO_BIG));
// Close enough ledgers that the burn transaction is no longer
// retried.
for (int i = 0; i < 10; ++i)
env.close();
// Cancel becky's offer, but alice adds a sell offer. The token
// should still not be burnable.
env(token::cancelOffer(becky, {beckyOfferIndex}));
env.close();
uint256 const aliceOfferIndex =
keylet::nftoffer(alice, env.seq(alice)).key;
env(token::createOffer(alice, nftokenID, drops(1)),
txflags(tfSellNFToken));
env.close();
env(token::burn(alice, nftokenID), ter(tefTOO_BIG));
env.close();
// Cancel alice's sell offer. Now the token should be burnable.
env(token::cancelOffer(alice, {aliceOfferIndex}));
env.close();
env(token::burn(alice, nftokenID));
env.close();
// Burning the token should remove all the offers from the ledger.
for (uint256 const& offerIndex : offerIndexes)
{
BEAST_EXPECT(!env.le(keylet::nftoffer(offerIndex)));
}
// Both alice and becky should have ownerCounts of zero.
BEAST_EXPECT(ownerCount(env, alice) == 0);
BEAST_EXPECT(ownerCount(env, becky) == 0);
}
// Test that up to 499 buy/sell offers will be removed when NFT is
// burned after fixNonFungibleTokensV1_2 is enabled. This is to test
// that we can successfully remove all offers if the number of offers is
// less than 500.
if (features[fixNonFungibleTokensV1_2])
// burned. This is to test that we can successfully remove all offers
// if the number of offers is less than 500.
{
Env env{*this, features};
@@ -1042,9 +942,7 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, becky) == 0);
}
// Test that up to 500 buy offers are removed when NFT is burned
// after fixNonFungibleTokensV1_2 is enabled
if (features[fixNonFungibleTokensV1_2])
// Test that up to 500 buy offers are removed when NFT is burned.
{
Env env{*this, features};
@@ -1087,9 +985,7 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, alice) == 1);
}
// Test that up to 500 buy/sell offers are removed when NFT is burned
// after fixNonFungibleTokensV1_2 is enabled
if (features[fixNonFungibleTokensV1_2])
// Test that up to 500 buy/sell offers are removed when NFT is burned.
{
Env env{*this, features};
@@ -1181,12 +1077,10 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
std::uint32_t tokenSeq =
env.le(acct)->at(~sfMintedNFTokens).value_or(0);
// If fixNFTokenRemint amendment is on, we must
// add FirstNFTokenSequence.
if (env.current()->rules().enabled(fixNFTokenRemint))
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
// We must add FirstNFTokenSequence.
tokenSeq += env.le(acct)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(acct));
return toUInt32(
nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon)));
@@ -1363,6 +1257,9 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
}
}
protected:
FeatureBitset const allFeatures{test::jtx::testable_amendments()};
void
testWithFeats(FeatureBitset features)
{
@@ -1372,84 +1269,15 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite
exerciseBrokenLinks(features);
}
protected:
void
run(std::uint32_t instance, bool last = false)
{
using namespace test::jtx;
static FeatureBitset const all{testable_amendments()};
static FeatureBitset const fixNFTV1_2{fixNonFungibleTokensV1_2};
static FeatureBitset const fixNFTDir{fixNFTokenDirV1};
static FeatureBitset const fixNFTRemint{fixNFTokenRemint};
static FeatureBitset const fixNFTPageLinks{fixNFTokenPageLinks};
static std::array<FeatureBitset, 5> const feats{
all - fixNFTV1_2 - fixNFTDir - fixNFTRemint - fixNFTPageLinks,
all - fixNFTV1_2 - fixNFTRemint - fixNFTPageLinks,
all - fixNFTRemint - fixNFTPageLinks,
all - fixNFTPageLinks,
all,
};
if (BEAST_EXPECT(instance < feats.size()))
{
testWithFeats(feats[instance]);
}
BEAST_EXPECT(!last || instance == feats.size() - 1);
}
public:
void
run() override
{
run(0);
testWithFeats(allFeatures - fixNFTokenPageLinks);
testWithFeats(allFeatures);
}
};
class NFTokenBurnWOfixFungTokens_test : public NFTokenBurnBaseUtil_test
{
public:
void
run() override
{
NFTokenBurnBaseUtil_test::run(1);
}
};
class NFTokenBurnWOFixTokenRemint_test : public NFTokenBurnBaseUtil_test
{
public:
void
run() override
{
NFTokenBurnBaseUtil_test::run(2);
}
};
class NFTokenBurnWOFixNFTPageLinks_test : public NFTokenBurnBaseUtil_test
{
public:
void
run() override
{
NFTokenBurnBaseUtil_test::run(3);
}
};
class NFTokenBurnAllFeatures_test : public NFTokenBurnBaseUtil_test
{
public:
void
run() override
{
NFTokenBurnBaseUtil_test::run(4, true);
}
};
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnBaseUtil, app, ripple, 3);
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOfixFungTokens, app, ripple, 3);
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOFixTokenRemint, app, ripple, 3);
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOFixNFTPageLinks, app, ripple, 3);
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnAllFeatures, app, ripple, 3);
BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurn, app, ripple, 3);
} // namespace ripple

71
src/test/app/NFTokenDir_test.cpp
View File

@@ -191,11 +191,10 @@ class NFTokenDir_test : public beast::unit_test::suite
Account::base58Seed, std::string(seed));
env.fund(XRP(10000), account);
// Do not close the ledger inside the loop. If
// fixNFTokenRemint is enabled and accounts are initialized
// at different ledgers, they will have different account
// sequences. That would cause the accounts to have
// different NFTokenID sequence numbers.
// Do not close the ledger inside the loop. If accounts are
// initialized at different ledgers, they will have
// different account sequences. That would cause the
// accounts to have different NFTokenID sequence numbers.
}
env.close();
@@ -377,11 +376,9 @@ class NFTokenDir_test : public beast::unit_test::suite
}
void
testFixNFTokenDirV1(FeatureBitset features)
testNFTokenDir(FeatureBitset features)
{
// Exercise a fix for an off-by-one in the creation of an NFTokenPage
// index.
testcase("fixNFTokenDirV1");
testcase("NFTokenDir");
using namespace test::jtx;
@@ -391,7 +388,7 @@ class NFTokenDir_test : public beast::unit_test::suite
// the index for the new page. This test recreates the problem.
// Lambda that exercises the split.
auto exerciseFixNFTokenDirV1 =
auto exercise =
[this,
&features](std::initializer_list<std::string_view const> seeds) {
Env env{
@@ -415,11 +412,10 @@ class NFTokenDir_test : public beast::unit_test::suite
Account::base58Seed, std::string(seed));
env.fund(XRP(10000), account);
// Do not close the ledger inside the loop. If
// fixNFTokenRemint is enabled and accounts are initialized
// at different ledgers, they will have different account
// sequences. That would cause the accounts to have
// different NFTokenID sequence numbers.
// Do not close the ledger inside the loop. If accounts are
// initialized at different ledgers, they will have
// different account sequences. That would cause the
// accounts to have different NFTokenID sequence numbers.
}
env.close();
@@ -453,16 +449,6 @@ class NFTokenDir_test : public beast::unit_test::suite
env.close();
}
// Here is the last offer. Without the fix accepting this
// offer causes tecINVARIANT_FAILED. With the fix the offer
// accept succeeds.
if (!features[fixNFTokenDirV1])
{
env(token::acceptSellOffer(buyer, offers.back()),
ter(tecINVARIANT_FAILED));
env.close();
return;
}
env(token::acceptSellOffer(buyer, offers.back()));
env.close();
@@ -598,8 +584,8 @@ class NFTokenDir_test : public beast::unit_test::suite
};
// Run the test cases.
exerciseFixNFTokenDirV1(seventeenHi);
exerciseFixNFTokenDirV1(seventeenLo);
exercise(seventeenHi);
exercise(seventeenLo);
}
void
@@ -665,10 +651,9 @@ class NFTokenDir_test : public beast::unit_test::suite
accounts.emplace_back(Account::base58Seed, std::string(seed));
env.fund(XRP(10000), account);
// Do not close the ledger inside the loop. If
// fixNFTokenRemint is enabled and accounts are initialized
// at different ledgers, they will have different account
// sequences. That would cause the accounts to have
// Do not close the ledger inside the loop. If accounts are
// initialized at different ledgers, they will have different
// account sequences. That would cause the accounts to have
// different NFTokenID sequence numbers.
}
env.close();
@@ -760,11 +745,7 @@ class NFTokenDir_test : public beast::unit_test::suite
// All NFTs should now be accounted for, so nftIDs should be empty.
BEAST_EXPECT(nftIDs.empty());
// Show that Without fixNFTokenDirV1 no more NFTs can be added to
// buyer. Also show that fixNFTokenDirV1 fixes the problem.
TER const expect = features[fixNFTokenDirV1]
? static_cast<TER>(tesSUCCESS)
: static_cast<TER>(tecNO_SUITABLE_NFTOKEN_PAGE);
TER const expect = tesSUCCESS;
env(token::mint(buyer, 0), txflags(tfTransferable), ter(expect));
env.close();
}
@@ -783,11 +764,6 @@ class NFTokenDir_test : public beast::unit_test::suite
// Lastly, none of the remaining NFTs should be acquirable by the
// buyer. They would cause page overflow.
// This test collapses in a heap if fixNFTokenDirV1 is not enabled.
// If it is enabled just return so we skip the test.
if (!features[fixNFTokenDirV1])
return;
testcase("NFToken consecutive packing");
using namespace test::jtx;
@@ -846,10 +822,9 @@ class NFTokenDir_test : public beast::unit_test::suite
accounts.emplace_back(Account::base58Seed, std::string(seed));
env.fund(XRP(10000), account);
// Do not close the ledger inside the loop. If
// fixNFTokenRemint is enabled and accounts are initialized
// at different ledgers, they will have different account
// sequences. That would cause the accounts to have
// Do not close the ledger inside the loop. If accounts are
// initialized at different ledgers, they will have different
// account sequences. That would cause the accounts to have
// different NFTokenID sequence numbers.
}
env.close();
@@ -1088,7 +1063,7 @@ class NFTokenDir_test : public beast::unit_test::suite
{
testConsecutiveNFTs(features);
testLopsidedSplits(features);
testFixNFTokenDirV1(features);
testNFTokenDir(features);
testTooManyEquivalent(features);
testConsecutivePacking(features);
}
@@ -1099,11 +1074,7 @@ public:
{
using namespace test::jtx;
FeatureBitset const all{testable_amendments()};
FeatureBitset const fixNFTDir{
fixNFTokenDirV1, featureNonFungibleTokensV1_1};
testWithFeats(all - fixNFTDir - fixNFTokenRemint);
testWithFeats(all - fixNFTokenRemint);
testWithFeats(all);
}
};

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

File diff suppressed because it is too large Load Diff

12
src/test/jtx/impl/token.cpp
View File

@@ -87,14 +87,10 @@ getID(
std::uint16_t flags,
std::uint16_t xferFee)
{
if (env.current()->rules().enabled(fixNFTokenRemint))
{
// If fixNFTokenRemint is enabled, we must add issuer's
// FirstNFTokenSequence to offset the starting NFT sequence number.
nftSeq += env.le(issuer)
->at(~sfFirstNFTokenSequence)
.value_or(env.seq(issuer));
}
// We must add issuer's FirstNFTokenSequence to offset the starting NFT
// sequence number.
nftSeq +=
env.le(issuer)->at(~sfFirstNFTokenSequence).value_or(env.seq(issuer));
return ripple::NFTokenMint::createNFTokenID(
flags, xferFee, issuer, nft::toTaxon(nfTokenTaxon), nftSeq);
}

2
src/test/rpc/Feature_test.cpp
View File

@@ -547,7 +547,7 @@ class Feature_test : public beast::unit_test::suite
using namespace test::jtx;
Env env{*this};
constexpr char const* featureName = "NonFungibleTokensV1";
constexpr char const* featureName = "CryptoConditionsSuite";
auto jrr = env.rpc("feature", featureName)[jss::result];
if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))

39
src/xrpld/app/tx/detail/DeleteAccount.cpp
View File

@@ -265,24 +265,20 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
if (!sleAccount)
return terNO_ACCOUNT;
if (ctx.view.rules().enabled(featureNonFungibleTokensV1))
{
// If an issuer has any issued NFTs resident in the ledger then it
// cannot be deleted.
if ((*sleAccount)[~sfMintedNFTokens] !=
(*sleAccount)[~sfBurnedNFTokens])
return tecHAS_OBLIGATIONS;
// If an issuer has any issued NFTs resident in the ledger then it
// cannot be deleted.
if ((*sleAccount)[~sfMintedNFTokens] != (*sleAccount)[~sfBurnedNFTokens])
return tecHAS_OBLIGATIONS;
// If the account owns any NFTs it cannot be deleted.
Keylet const first = keylet::nftpage_min(account);
Keylet const last = keylet::nftpage_max(account);
// If the account owns any NFTs it cannot be deleted.
Keylet const first = keylet::nftpage_min(account);
Keylet const last = keylet::nftpage_max(account);
auto const cp = ctx.view.read(Keylet(
ltNFTOKEN_PAGE,
ctx.view.succ(first.key, last.key.next()).value_or(last.key)));
if (cp)
return tecHAS_OBLIGATIONS;
}
auto const cp = ctx.view.read(Keylet(
ltNFTOKEN_PAGE,
ctx.view.succ(first.key, last.key.next()).value_or(last.key)));
if (cp)
return tecHAS_OBLIGATIONS;
// We don't allow an account to be deleted if its sequence number
// is within 256 of the current ledger. This prevents replay of old
@@ -294,8 +290,8 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
if ((*sleAccount)[sfSequence] + seqDelta > ctx.view.seq())
return tecTOO_SOON;
// When fixNFTokenRemint is enabled, we don't allow an account to be
// deleted if <FirstNFTokenSequence + MintedNFTokens> is within 256 of the
// We don't allow an account to be deleted if
// <FirstNFTokenSequence + MintedNFTokens> is within 256 of the
// current ledger. This is to prevent having duplicate NFTokenIDs after
// account re-creation.
//
@@ -305,10 +301,9 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
// their account and mints a NFToken, it is possible that the
// NFTokenSequence of this NFToken is the same as the one that the
// authorized minter minted in a previous ledger.
if (ctx.view.rules().enabled(fixNFTokenRemint) &&
((*sleAccount)[~sfFirstNFTokenSequence].value_or(0) +
(*sleAccount)[~sfMintedNFTokens].value_or(0) + seqDelta >
ctx.view.seq()))
if ((*sleAccount)[~sfFirstNFTokenSequence].value_or(0) +
(*sleAccount)[~sfMintedNFTokens].value_or(0) + seqDelta >
ctx.view.seq())
return tecTOO_SOON;
// Verify that the account does not own any objects that would prevent

84
src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp
View File

@@ -75,10 +75,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
return {nullptr, tecEXPIRED};
// The initial implementation had a bug that allowed a negative
// amount. The fixNFTokenNegOffer amendment fixes that.
if ((*offerSLE)[sfAmount].negative() &&
ctx.view.rules().enabled(fixNFTokenNegOffer))
if ((*offerSLE)[sfAmount].negative())
return {nullptr, temBAD_OFFER};
return {std::move(offerSLE), tesSUCCESS};
@@ -106,8 +103,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
// The two offers may not form a loop. A broker may not sell the
// token to the current owner of the token.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2) &&
((*bo)[sfOwner] == (*so)[sfOwner]))
if (((*bo)[sfOwner] == (*so)[sfOwner]))
return tecCANT_ACCEPT_OWN_NFTOKEN_OFFER;
// Ensure that the buyer is willing to pay at least as much as the
@@ -115,32 +111,20 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
if ((*so)[sfAmount] > (*bo)[sfAmount])
return tecINSUFFICIENT_PAYMENT;
// If the buyer specified a destination
if (auto const dest = bo->at(~sfDestination))
// The destination must be whoever is submitting the tx if the buyer
// specified it
if (auto const dest = bo->at(~sfDestination);
dest && *dest != ctx.tx[sfAccount])
{
// Before this fix the destination could be either the seller or
// a broker. After, it must be whoever is submitting the tx.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (*dest != ctx.tx[sfAccount])
return tecNO_PERMISSION;
}
else if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
return tecNO_PERMISSION;
}
// If the seller specified a destination
if (auto const dest = so->at(~sfDestination))
// The destination must be whoever is submitting the tx if the seller
// specified it
if (auto const dest = so->at(~sfDestination);
dest && *dest != ctx.tx[sfAccount])
{
// Before this fix the destination could be either the seller or
// a broker. After, it must be whoever is submitting the tx.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (*dest != ctx.tx[sfAccount])
return tecNO_PERMISSION;
}
else if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
return tecNO_PERMISSION;
}
// The broker can specify an amount that represents their cut; if they
@@ -210,21 +194,10 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
// After this amendment, we allow an IOU issuer to buy an NFT with their
// own currency
auto const needed = bo->at(sfAmount);
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountFunds(
ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) <
needed)
return tecINSUFFICIENT_FUNDS;
}
else if (
accountHolds(
ctx.view,
(*bo)[sfOwner],
needed.getCurrency(),
needed.getIssuer(),
fhZERO_IF_FROZEN,
ctx.j) < needed)
if (accountFunds(
ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) <
needed)
return tecINSUFFICIENT_FUNDS;
// Check that the account accepting the buy offer (he's selling the NFT)
@@ -285,18 +258,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
// The account offering to buy must have funds:
auto const needed = so->at(sfAmount);
if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountHolds(
ctx.view,
ctx.tx[sfAccount],
needed.getCurrency(),
needed.getIssuer(),
fhZERO_IF_FROZEN,
ctx.j) < needed)
return tecINSUFFICIENT_FUNDS;
}
else if (!bo)
if (!bo)
{
// After this amendment, we allow buyers to buy with their own
// issued currency.
@@ -403,13 +365,11 @@ NFTokenAcceptOffer::pay(
auto const result = accountSend(view(), from, to, amount, j_);
// After this amendment, if any payment would cause a non-IOU-issuer to
// have a negative balance, or an IOU-issuer to have a positive balance in
// their own currency, we know that something went wrong. This was
// originally found in the context of IOU transfer fees. Since there are
// several payouts in this tx, just confirm that the end state is OK.
if (!view().rules().enabled(fixNonFungibleTokensV1_2))
return result;
// If any payment causes a non-IOU-issuer to have a negative balance,
// or an IOU-issuer to have a positive balance in their own currency,
// we know that something went wrong. This was originally found in the
// context of IOU transfer fees. Since there are several payouts in this tx,
// just confirm that the end state is OK.
if (result != tesSUCCESS)
return result;
if (accountFunds(view(), from, amount, fhZERO_IF_FROZEN, j_).signum() < 0)

43
src/xrpld/app/tx/detail/NFTokenBurn.cpp
View File

@@ -64,13 +64,6 @@ NFTokenBurn::preclaim(PreclaimContext const& ctx)
}
}
if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
// If there are too many offers, then burning the token would produce
// too much metadata. Disallow burning a token with too many offers.
return nft::notTooManyOffers(ctx.view, ctx.tx[sfNFTokenID]);
}
return tesSUCCESS;
}
@@ -96,37 +89,21 @@ NFTokenBurn::doApply()
view().update(issuer);
}
if (ctx_.view().rules().enabled(fixNonFungibleTokensV1_2))
{
// Delete up to 500 offers in total.
// Because the number of sell offers is likely to be less than
// the number of buy offers, we prioritize the deletion of sell
// offers in order to clean up sell offer directory
std::size_t const deletedSellOffers = nft::removeTokenOffersWithLimit(
view(),
keylet::nft_sells(ctx_.tx[sfNFTokenID]),
maxDeletableTokenOfferEntries);
// Delete up to 500 offers in total.
// Because the number of sell offers is likely to be less than
// the number of buy offers, we prioritize the deletion of sell
// offers in order to clean up sell offer directory
std::size_t const deletedSellOffers = nft::removeTokenOffersWithLimit(
view(),
keylet::nft_sells(ctx_.tx[sfNFTokenID]),
maxDeletableTokenOfferEntries);
if (maxDeletableTokenOfferEntries > deletedSellOffers)
{
nft::removeTokenOffersWithLimit(
view(),
keylet::nft_buys(ctx_.tx[sfNFTokenID]),
maxDeletableTokenOfferEntries - deletedSellOffers);
}
}
else
if (maxDeletableTokenOfferEntries > deletedSellOffers)
{
// Deletion of all offers.
nft::removeTokenOffersWithLimit(
view(),
keylet::nft_sells(ctx_.tx[sfNFTokenID]),
std::numeric_limits<int>::max());
nft::removeTokenOffersWithLimit(
view(),
keylet::nft_buys(ctx_.tx[sfNFTokenID]),
std::numeric_limits<int>::max());
maxDeletableTokenOfferEntries - deletedSellOffers);
}
return tesSUCCESS;

18
src/xrpld/app/tx/detail/NFTokenMint.cpp
View File

@@ -230,24 +230,6 @@ NFTokenMint::doApply()
// Should not happen. Checked in preclaim.
return Unexpected(tecNO_ISSUER);
if (!ctx_.view().rules().enabled(fixNFTokenRemint))
{
// Get the unique sequence number for this token:
std::uint32_t const tokenSeq =
(*root)[~sfMintedNFTokens].value_or(0);
{
std::uint32_t const nextTokenSeq = tokenSeq + 1;
if (nextTokenSeq < tokenSeq)
return Unexpected(tecMAX_SEQUENCE_REACHED);
(*root)[sfMintedNFTokens] = nextTokenSeq;
}
ctx_.view().update(root);
return tokenSeq;
}
// With fixNFTokenRemint amendment enabled:
//
// If the issuer hasn't minted an NFToken before we must add a
// FirstNFTokenSequence field to the issuer's AccountRoot. The
// value of the FirstNFTokenSequence must equal the issuer's

6
src/xrpld/app/tx/detail/NFTokenModify.cpp
View File

@@ -25,12 +25,6 @@
namespace ripple {
bool
NFTokenModify::checkExtraFeatures(PreflightContext const& ctx)
{
return ctx.rules.enabled(featureNonFungibleTokensV1_1);
}
NotTEC
NFTokenModify::preflight(PreflightContext const& ctx)
{

3
src/xrpld/app/tx/detail/NFTokenModify.h
View File

@@ -33,9 +33,6 @@ public:
{
}
static bool
checkExtraFeatures(PreflightContext const& ctx);
static NotTEC
preflight(PreflightContext const& ctx);

91
src/xrpld/app/tx/detail/NFTokenUtils.cpp
View File

@@ -142,32 +142,26 @@ getPageForToken(
// equivalent tokens. This requires special handling.
if (splitIter == narr.begin())
{
// Prior to fixNFTokenDirV1 we simply stopped.
if (!view.rules().enabled(fixNFTokenDirV1))
return nullptr;
else
auto const relation{(id & nft::pageMask) <=> cmp};
if (relation == 0)
{
auto const relation{(id & nft::pageMask) <=> cmp};
if (relation == 0)
{
// If the passed in id belongs exactly on this (full) page
// this account simply cannot store the NFT.
return nullptr;
}
if (relation > 0)
{
// We need to leave the entire contents of this page in
// narr so carr stays empty. The new NFT will be
// inserted in carr. This keeps the NFTs that must be
// together all on their own page.
splitIter = narr.end();
}
// If neither of those conditions apply then put all of
// narr into carr and produce an empty narr where the new NFT
// will be inserted. Leave the split at narr.begin().
// If the passed in id belongs exactly on this (full) page
// this account simply cannot store the NFT.
return nullptr;
}
if (relation > 0)
{
// We need to leave the entire contents of this page in
// narr so carr stays empty. The new NFT will be
// inserted in carr. This keeps the NFTs that must be
// together all on their own page.
splitIter = narr.end();
}
// If neither of those conditions apply then put all of
// narr into carr and produce an empty narr where the new NFT
// will be inserted. Leave the split at narr.begin().
}
// Split narr at splitIter.
@@ -178,9 +172,7 @@ getPageForToken(
std::swap(carr, newCarr);
}
// Determine the ID for the page index. This decision is conditional on
// fixNFTokenDirV1 being enabled. But the condition for the decision
// is not possible unless fixNFTokenDirV1 is enabled.
// Determine the ID for the page index.
//
// Note that we use uint256::next() because there's a subtlety in the way
// NFT pages are structured. The low 96-bits of NFT ID must be strictly
@@ -217,13 +209,6 @@ getPageForToken(
createCallback(view, owner);
// fixNFTokenDirV1 corrects a bug in the initial implementation that
// would put an NFT in the wrong page. The problem was caused by an
// off-by-one subtlety that the NFT can only be stored in the first page
// with a key that's strictly greater than `first`
if (!view.rules().enabled(fixNFTokenDirV1))
return (first.key <= np->key()) ? np : cp;
return (first.key < np->key()) ? np : cp;
}
@@ -844,7 +829,7 @@ tokenOfferCreatePreflight(
std::optional<AccountID> const& owner,
std::uint32_t txFlags)
{
if (amount.negative() && rules.enabled(fixNFTokenNegOffer))
if (amount.negative())
// An offer for a negative amount makes no sense.
return temBAD_AMOUNT;
@@ -874,21 +859,10 @@ tokenOfferCreatePreflight(
if (owner && owner == acctID)
return temMALFORMED;
if (dest)
// The destination can't be the account executing the transaction.
if (dest && dest == acctID)
{
// Some folks think it makes sense for a buy offer to specify a
// specific broker using the Destination field. This change doesn't
// deserve it's own amendment, so we're piggy-backing on
// fixNFTokenNegOffer.
//
// Prior to fixNFTokenNegOffer any use of the Destination field on
// a buy offer was malformed.
if (!isSellOffer && !rules.enabled(fixNFTokenNegOffer))
return temMALFORMED;
// The destination can't be the account executing the transaction.
if (dest == acctID)
return temMALFORMED;
return temMALFORMED;
}
return tesSUCCESS;
}
@@ -946,23 +920,10 @@ tokenOfferCreatePreclaim(
// offer may later become unfunded.
if ((txFlags & tfSellNFToken) == 0)
{
// After this amendment, we allow an IOU issuer to make a buy offer
// We allow an IOU issuer to make a buy offer
// using their own currency.
if (view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountFunds(
view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j)
.signum() <= 0)
return tecUNFUNDED_OFFER;
}
else if (
accountHolds(
view,
acctID,
amount.getCurrency(),
amount.getIssuer(),
FreezeHandling::fhZERO_IF_FROZEN,
j)
if (accountFunds(
view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j)
.signum() <= 0)
return tecUNFUNDED_OFFER;
}

30
src/xrpld/app/tx/detail/SetAccount.cpp
View File

@@ -171,17 +171,14 @@ SetAccount::preflight(PreflightContext const& ctx)
return telBAD_DOMAIN;
}
if (ctx.rules.enabled(featureNonFungibleTokensV1))
{
// Configure authorized minting account:
if (uSetFlag == asfAuthorizedNFTokenMinter &&
!tx.isFieldPresent(sfNFTokenMinter))
return temMALFORMED;
// Configure authorized minting account:
if (uSetFlag == asfAuthorizedNFTokenMinter &&
!tx.isFieldPresent(sfNFTokenMinter))
return temMALFORMED;
if (uClearFlag == asfAuthorizedNFTokenMinter &&
tx.isFieldPresent(sfNFTokenMinter))
return temMALFORMED;
}
if (uClearFlag == asfAuthorizedNFTokenMinter &&
tx.isFieldPresent(sfNFTokenMinter))
return temMALFORMED;
return tesSUCCESS;
}
@@ -613,15 +610,12 @@ SetAccount::doApply()
}
// Configure authorized minting account:
if (ctx_.view().rules().enabled(featureNonFungibleTokensV1))
{
if (uSetFlag == asfAuthorizedNFTokenMinter)
sle->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]);
if (uSetFlag == asfAuthorizedNFTokenMinter)
sle->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]);
if (uClearFlag == asfAuthorizedNFTokenMinter &&
sle->isFieldPresent(sfNFTokenMinter))
sle->makeFieldAbsent(sfNFTokenMinter);
}
if (uClearFlag == asfAuthorizedNFTokenMinter &&
sle->isFieldPresent(sfNFTokenMinter))
sle->makeFieldAbsent(sfNFTokenMinter);
// Set or clear flags for disallowing various incoming instruments
if (ctx_.view().rules().enabled(featureDisallowIncoming))