diff --git a/src/data/BackendInterface.cpp b/src/data/BackendInterface.cpp index 6cc45252b..2406e5ea0 100644 --- a/src/data/BackendInterface.cpp +++ b/src/data/BackendInterface.cpp @@ -110,7 +110,7 @@ BackendInterface::fetchLedgerObjectSeq( ) const { auto seq = doFetchLedgerObjectSeq(key, sequence, yield); - if (!seq) + if (!seq) LOG(gLog.trace()) << "Missed in db"; return seq; } diff --git a/src/data/BackendInterface.hpp b/src/data/BackendInterface.hpp index ec3f6c08c..3a0dba87a 100644 --- a/src/data/BackendInterface.hpp +++ b/src/data/BackendInterface.hpp @@ -429,7 +429,8 @@ class BackendInterface { * @return The sequence in unit32_t on success; nullopt otherwise */ virtual std::optional - doFetchLedgerObjectSeq(ripple::uint256 const& key, std::uint32_t sequence, boost::asio::yield_context yield) const = 0; + doFetchLedgerObjectSeq(ripple::uint256 const& key, std::uint32_t sequence, boost::asio::yield_context yield) + const = 0; /** * @brief The database-specific implementation for fetching ledger objects. diff --git a/src/data/CassandraBackend.hpp b/src/data/CassandraBackend.hpp index f18ac5950..9c1bf2f61 100644 --- a/src/data/CassandraBackend.hpp +++ b/src/data/CassandraBackend.hpp @@ -573,11 +573,12 @@ class BasicCassandraBackend : public BackendInterface { { LOG(log_.debug()) << "Fetching ledger object for seq " << sequence << ", key = " << ripple::to_string(key); if (auto const res = executor_.read(yield, schema_->selectObject, key, sequence); res) { - if (auto const result = res->template get(); result) { - auto [_ ,seq] = result.value(); + if (auto const result = res->template get(); result) { + auto [_, seq] = result.value(); return seq; - } LOG(log_.debug()) << "Could not fetch ledger object sequence - no rows"; - + } + LOG(log_.debug()) << "Could not fetch ledger object sequence - no rows"; + } else { LOG(log_.error()) << "Could not fetch ledger object sequence: " << res.error(); } diff --git a/src/etl/NFTHelpers.cpp b/src/etl/NFTHelpers.cpp index f15915c9e..9b70086cd 100644 --- a/src/etl/NFTHelpers.cpp +++ b/src/etl/NFTHelpers.cpp @@ -116,8 +116,8 @@ getNFTokenMintData(ripple::TxMeta const& txMeta, ripple::STTx const& sttx) } } - std::sort(finalIDs.begin(), finalIDs.end()); - std::sort(prevIDs.begin(), prevIDs.end()); + std::ranges::sort(finalIDs); + std::ranges::sort(prevIDs); // Find the first NFT ID that doesn't match. We're looking for an // added NFT, so the one we want will be the mismatch in finalIDs. @@ -295,7 +295,7 @@ getNFTokenCancelOfferData(ripple::TxMeta const& txMeta, ripple::STTx const& sttx } // Deduplicate any transactions based on tokenID - std::sort(txs.begin(), txs.end(), [](NFTTransactionsData const& a, NFTTransactionsData const& b) { + std::ranges::sort(txs, [](NFTTransactionsData const& a, NFTTransactionsData const& b) { return a.tokenID < b.tokenID; }); auto last = std::unique(txs.begin(), txs.end(), [](NFTTransactionsData const& a, NFTTransactionsData const& b) { @@ -356,4 +356,21 @@ getNFTDataFromObj(std::uint32_t const seq, std::string const& key, std::string c return nfts; } + +std::vector +getUniqueNFTsDatas(std::vector const& nfts) +{ + std::vector results = nfts; + + std::ranges::sort(results, [](NFTsData const& a, NFTsData const& b) { + return a.tokenID == b.tokenID ? a.transactionIndex > b.transactionIndex : a.tokenID > b.tokenID; + }); + + auto const last = std::unique(results.begin(), results.end(), [](NFTsData const& a, NFTsData const& b) { + return a.tokenID == b.tokenID; + }); + results.erase(last, results.end()); + return results; +} + } // namespace etl diff --git a/src/etl/NFTHelpers.hpp b/src/etl/NFTHelpers.hpp index e89084c4d..c60eb9bf8 100644 --- a/src/etl/NFTHelpers.hpp +++ b/src/etl/NFTHelpers.hpp @@ -104,4 +104,14 @@ getNFTDataFromTx(ripple::TxMeta const& txMeta, ripple::STTx const& sttx); std::vector getNFTDataFromObj(std::uint32_t seq, std::string const& key, std::string const& blob); +/** + * @brief Get the unique NFTs data from a vector of NFTsData happening in the same ledger. For example, if a NFT has + * both accept offer and burn happening in the same ledger,we only keep the final state of the NFT. + + * @param nfts The NFTs data to filter, happening in the same ledger + * @return The unique NFTs data + */ +std::vector +getUniqueNFTsDatas(std::vector const& nfts); + } // namespace etl diff --git a/src/etl/impl/LedgerLoader.hpp b/src/etl/impl/LedgerLoader.hpp index ea3e6a3e0..7fe716f4a 100644 --- a/src/etl/impl/LedgerLoader.hpp +++ b/src/etl/impl/LedgerLoader.hpp @@ -136,20 +136,7 @@ class LedgerLoader { ); } - // Remove all but the last NFTsData for each id. unique removes all but the first of a group, so we want to - // reverse sort by transaction index - std::sort(result.nfTokensData.begin(), result.nfTokensData.end(), [](NFTsData const& a, NFTsData const& b) { - return a.tokenID > b.tokenID && a.transactionIndex > b.transactionIndex; - }); - - // Now we can unique the NFTs by tokenID. - auto last = std::unique( - result.nfTokensData.begin(), - result.nfTokensData.end(), - [](NFTsData const& a, NFTsData const& b) { return a.tokenID == b.tokenID; } - ); - result.nfTokensData.erase(last, result.nfTokensData.end()); - + result.nfTokensData = getUniqueNFTsDatas(result.nfTokensData); return result; } diff --git a/src/rpc/handlers/LedgerEntry.cpp b/src/rpc/handlers/LedgerEntry.cpp index 780962624..14be58305 100644 --- a/src/rpc/handlers/LedgerEntry.cpp +++ b/src/rpc/handlers/LedgerEntry.cpp @@ -165,13 +165,13 @@ LedgerEntryHandler::process(LedgerEntryHandler::Input input, Context const& ctx) auto ledgerObject = sharedPtrBackend_->fetchLedgerObject(key, lgrInfo.seq, ctx.yield); if (!ledgerObject || ledgerObject->empty()) { - if (not input.includeDeleted) + if (not input.includeDeleted) return Error{Status{"entryNotFound"}}; auto const deletedSeq = sharedPtrBackend_->fetchLedgerObjectSeq(key, lgrInfo.seq, ctx.yield); - if (!deletedSeq) + if (!deletedSeq) return Error{Status{"entryNotFound"}}; ledgerObject = sharedPtrBackend_->fetchLedgerObject(key, deletedSeq.value() - 1, ctx.yield); - if (!ledgerObject || ledgerObject->empty()) + if (!ledgerObject || ledgerObject->empty()) return Error{Status{"entryNotFound"}}; output.deletedLedgerIndex = deletedSeq; } diff --git a/tests/unit/etl/NFTHelpersTests.cpp b/tests/unit/etl/NFTHelpersTests.cpp index 845f56cce..e31177971 100644 --- a/tests/unit/etl/NFTHelpersTests.cpp +++ b/tests/unit/etl/NFTHelpersTests.cpp @@ -17,22 +17,28 @@ */ //============================================================================== +#include "data/DBHelpers.hpp" #include "etl/NFTHelpers.hpp" #include "util/LoggerFixtures.hpp" #include "util/TestObject.hpp" #include +#include #include +#include +#include #include #include #include +#include #include #include constexpr static auto ACCOUNT = "rM2AGCCCRb373FRuD8wHyUwUsh2dV4BW5Q"; constexpr static auto NFTID = "0008013AE1CD8B79A8BCB52335CD40DE97401B2D60A828720000099B00000000"; constexpr static auto NFTID2 = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"; +constexpr static auto OFFER1 = "23F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D68499076441C4837282BF8"; constexpr static auto TX = "13F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D68499076441C4837282BF8"; struct NFTHelpersTests : public NoLoggerFixture {}; @@ -59,3 +65,37 @@ TEST_F(NFTHelpersTests, ConvertDataFromNFTCancelOfferTxContainingDuplicateNFT) EXPECT_EQ(nftTxs.size(), 2); EXPECT_FALSE(nftDatas); } + +TEST_F(NFTHelpersTests, UniqueNFTDatas) +{ + std::vector nftDatas; + + auto const generateNFTsData = [](char const* nftID, std::uint32_t txIndex) { + auto const tx = CreateCreateNFTOfferTxWithMetadata(ACCOUNT, 1, 50, nftID, 123, OFFER1); + ripple::SerialIter s{tx.metadata.data(), tx.metadata.size()}; + ripple::STObject meta{s, ripple::sfMetadata}; + meta.setFieldU32(ripple::sfTransactionIndex, txIndex); + ripple::TxMeta const txMeta(ripple::uint256(TX), 1, meta.getSerializer().peekData()); + + auto const account = GetAccountIDWithString(ACCOUNT); + return NFTsData{ripple::uint256(nftID), account, ripple::Blob{}, txMeta}; + }; + + nftDatas.push_back(generateNFTsData(NFTID, 3)); + nftDatas.push_back(generateNFTsData(NFTID, 1)); + nftDatas.push_back(generateNFTsData(NFTID, 2)); + + nftDatas.push_back(generateNFTsData(NFTID2, 4)); + nftDatas.push_back(generateNFTsData(NFTID2, 1)); + nftDatas.push_back(generateNFTsData(NFTID2, 5)); + + auto const uniqueNFTDatas = etl::getUniqueNFTsDatas(nftDatas); + + EXPECT_EQ(uniqueNFTDatas.size(), 2); + EXPECT_EQ(uniqueNFTDatas[0].ledgerSequence, 1); + EXPECT_EQ(uniqueNFTDatas[1].ledgerSequence, 1); + EXPECT_EQ(uniqueNFTDatas[0].transactionIndex, 5); + EXPECT_EQ(uniqueNFTDatas[1].transactionIndex, 3); + EXPECT_EQ(uniqueNFTDatas[0].tokenID, ripple::uint256(NFTID2)); + EXPECT_EQ(uniqueNFTDatas[1].tokenID, ripple::uint256(NFTID)); +} diff --git a/tests/unit/rpc/handlers/LedgerEntryTests.cpp b/tests/unit/rpc/handlers/LedgerEntryTests.cpp index 44a6e5e23..1b43c0d17 100644 --- a/tests/unit/rpc/handlers/LedgerEntryTests.cpp +++ b/tests/unit/rpc/handlers/LedgerEntryTests.cpp @@ -2847,7 +2847,7 @@ TEST_F(RPCLedgerEntryTest, ObjectUpdateIncludeDelete) .WillRepeatedly(Return(line1.getSerializer().peekData())); EXPECT_CALL(*backend, doFetchLedgerObject(ripple::uint256{INDEX1}, RANGEMAX - 1, _)) .WillRepeatedly(Return(line2.getSerializer().peekData())); - + runSpawn([&, this](auto yield) { auto const handler = AnyHandler{LedgerEntryHandler{backend}}; auto const req = json::parse(fmt::format( @@ -2920,8 +2920,7 @@ TEST_F(RPCLedgerEntryTest, ObjectSeqNotExist) EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _)).WillRepeatedly(Return(ledgerinfo)); EXPECT_CALL(*backend, doFetchLedgerObject(ripple::uint256{INDEX1}, RANGEMAX, _)) .WillOnce(Return(std::optional{})); - EXPECT_CALL(*backend, doFetchLedgerObjectSeq(ripple::uint256{INDEX1}, RANGEMAX, _)) - .WillOnce(Return(std::nullopt)); + EXPECT_CALL(*backend, doFetchLedgerObjectSeq(ripple::uint256{INDEX1}, RANGEMAX, _)).WillOnce(Return(std::nullopt)); runSpawn([&, this](auto yield) { auto const handler = AnyHandler{LedgerEntryHandler{backend}}; @@ -2939,4 +2938,3 @@ TEST_F(RPCLedgerEntryTest, ObjectSeqNotExist) EXPECT_EQ(myerr, "entryNotFound"); }); } -