Skip to content

Commit

Permalink
Fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@cindyyan317
cindyyan317 committed Jul 25, 2024
1 parent 9fef261 commit 85e33fd
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 76 deletions.
1 change: 0 additions & 1 deletion src/data/CassandraBackend.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,6 @@ class BasicCassandraBackend : public BackendInterface {
return seq;
}
LOG(log_.debug()) << "Could not fetch ledger object sequence - no rows";

} else {
LOG(log_.error()) << "Could not fetch ledger object sequence: " << res.error();
}
Expand Down
7 changes: 4 additions & 3 deletions src/etl/NFTHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ namespace etl {
std::pair<std::vector<NFTTransactionsData>, std::optional<NFTsData>>
getNFTokenMofidyData(ripple::TxMeta const& txMeta, ripple::STTx const& sttx)
{
auto tx = NFTTransactionsData(sttx.getFieldH256(ripple::sfNFTokenID), txMeta, sttx.getTransactionID());

auto const tokenID = sttx.getFieldH256(ripple::sfNFTokenID);
// note: sfURI is optional, if it is absent, we will update the uri as empty string
return {{tx}, NFTsData(tokenID, txMeta, sttx.getFieldVL(ripple::sfURI))};
return {
{NFTTransactionsData(sttx.getFieldH256(ripple::sfNFTokenID), txMeta, sttx.getTransactionID())},
NFTsData(tokenID, txMeta, sttx.getFieldVL(ripple::sfURI))
};
}

std::pair<std::vector<NFTTransactionsData>, std::optional<NFTsData>>
Expand Down
3 changes: 3 additions & 0 deletions src/etl/impl/LedgerLoader.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class LedgerLoader {

auto const [nftTxs, maybeNFT] = getNFTDataFromTx(txMeta, sttx);
result.nfTokenTxData.insert(result.nfTokenTxData.end(), nftTxs.begin(), nftTxs.end());

// We need to unique the URI changes separately, in case the URI changes are discarded
if (maybeNFT) {
if (maybeNFT->onlyUriChanged) {
Expand All @@ -145,6 +146,8 @@ class LedgerLoader {
}

auto const uniqueNftData = [](std::vector<NFTsData>& nfTokensData) {
if (nfTokensData.empty())
return;
// 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(nfTokensData.begin(), nfTokensData.end(), [](NFTsData const& a, NFTsData const& b) {
Expand Down
145 changes: 73 additions & 72 deletions tests/unit/etl/NFTHelpersTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,56 +60,57 @@ constexpr static auto TX = "13F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D6849907644
constexpr static auto PAGE_INDEX = "E1CD8B79A8BCB52335CD40DE97401B2D60A82872FFFFFFFFFFFFFFFFFFFFFFFF";
constexpr static auto OFFER_ID = "AA86CBF29770F72FA3FF4A5D9A9FA54D6F399A8E038F72393EF782224865E27F";

struct NFTHelpersTest : NoLoggerFixture {};

void
VerifyNFTTransactionsData(
NFTTransactionsData const& data,
ripple::STTx const& sttx,
ripple::TxMeta const& txMeta,
std::string_view nftId
)
{
EXPECT_EQ(data.tokenID, ripple::uint256(nftId));
EXPECT_EQ(data.ledgerSequence, txMeta.getLgrSeq());
EXPECT_EQ(data.transactionIndex, txMeta.getIndex());
EXPECT_EQ(data.txHash, sttx.getTransactionID());
}

void
VerifyNFTsData(
NFTsData const& data,
ripple::STTx const& sttx,
ripple::TxMeta const& txMeta,
std::string_view nftId,
std::optional<std::string> const& owner
)
{
EXPECT_EQ(data.tokenID, ripple::uint256(nftId));
EXPECT_EQ(data.ledgerSequence, txMeta.getLgrSeq());
EXPECT_EQ(data.transactionIndex, txMeta.getIndex());
if (owner)
EXPECT_EQ(data.owner, GetAccountIDWithString(*owner));

if (sttx.getTxnType() == ripple::ttNFTOKEN_MINT || sttx.getTxnType() == ripple::ttNFTOKEN_MODIFY) {
EXPECT_TRUE(data.uri.has_value());
EXPECT_EQ(*data.uri, sttx.getFieldVL(ripple::sfURI));
} else {
EXPECT_FALSE(data.uri.has_value());
struct NFTHelpersTest : NoLoggerFixture {
protected:
static void
verifyNFTTransactionsData(
NFTTransactionsData const& data,
ripple::STTx const& sttx,
ripple::TxMeta const& txMeta,
std::string_view nftId
)
{
EXPECT_EQ(data.tokenID, ripple::uint256(nftId));
EXPECT_EQ(data.ledgerSequence, txMeta.getLgrSeq());
EXPECT_EQ(data.transactionIndex, txMeta.getIndex());
EXPECT_EQ(data.txHash, sttx.getTransactionID());
}

if (sttx.getTxnType() == ripple::ttNFTOKEN_BURN) {
EXPECT_TRUE(data.isBurned);
} else {
EXPECT_FALSE(data.isBurned);
static void
verifyNFTsData(
NFTsData const& data,
ripple::STTx const& sttx,
ripple::TxMeta const& txMeta,
std::string_view nftId,
std::optional<std::string> const& owner
)
{
EXPECT_EQ(data.tokenID, ripple::uint256(nftId));
EXPECT_EQ(data.ledgerSequence, txMeta.getLgrSeq());
EXPECT_EQ(data.transactionIndex, txMeta.getIndex());
if (owner)
EXPECT_EQ(data.owner, GetAccountIDWithString(*owner));

if (sttx.getTxnType() == ripple::ttNFTOKEN_MINT || sttx.getTxnType() == ripple::ttNFTOKEN_MODIFY) {
EXPECT_TRUE(data.uri.has_value());
EXPECT_EQ(*data.uri, sttx.getFieldVL(ripple::sfURI));
} else {
EXPECT_FALSE(data.uri.has_value());
}

if (sttx.getTxnType() == ripple::ttNFTOKEN_BURN) {
EXPECT_TRUE(data.isBurned);
} else {
EXPECT_FALSE(data.isBurned);
}

if (sttx.getTxnType() == ripple::ttNFTOKEN_MODIFY) {
EXPECT_TRUE(data.onlyUriChanged);
} else {
EXPECT_FALSE(data.onlyUriChanged);
}
}

if (sttx.getTxnType() == ripple::ttNFTOKEN_MODIFY) {
EXPECT_TRUE(data.onlyUriChanged);
} else {
EXPECT_FALSE(data.onlyUriChanged);
}
}
};

TEST_F(NFTHelpersTest, NFTDataFromFailedTx)
{
Expand Down Expand Up @@ -164,8 +165,8 @@ TEST_F(NFTHelpersTest, NFTModifyWithURI)
etl::getNFTDataFromTx(txMeta, ripple::STTx(ripple::SerialIter{tx.transaction.data(), tx.transaction.size()}));

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, std::nullopt);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, std::nullopt);
}

TEST_F(NFTHelpersTest, NFTModifyWithoutURI)
Expand All @@ -176,8 +177,8 @@ TEST_F(NFTHelpersTest, NFTModifyWithoutURI)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, std::nullopt);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, std::nullopt);
}

TEST_F(NFTHelpersTest, NFTMintFromModifedNode)
Expand All @@ -189,8 +190,8 @@ TEST_F(NFTHelpersTest, NFTMintFromModifedNode)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTMintCantFindNewNFT)
Expand All @@ -216,8 +217,8 @@ TEST_F(NFTHelpersTest, NFTMintFromCreatedNode)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTMintWithoutUriField)
Expand All @@ -229,8 +230,8 @@ TEST_F(NFTHelpersTest, NFTMintWithoutUriField)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTMintZeroMetaNode)
Expand All @@ -254,8 +255,8 @@ TEST_F(NFTHelpersTest, NFTBurnFromDeletedNode)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTBurnZeroMetaNode)
Expand All @@ -280,8 +281,8 @@ TEST_F(NFTHelpersTest, NFTBurnFromModifiedNode)
auto const [nftTxs, nftDatas] = etl::getNFTDataFromTx(txMeta, sttx);

EXPECT_EQ(nftTxs.size(), 1);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTCancelOffer)
Expand All @@ -294,8 +295,8 @@ TEST_F(NFTHelpersTest, NFTCancelOffer)

EXPECT_EQ(nftTxs.size(), 2);
EXPECT_FALSE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTTransactionsData(nftTxs[1], sttx, txMeta, NFTID2);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTTransactionsData(nftTxs[1], sttx, txMeta, NFTID2);
}

TEST_F(NFTHelpersTest, NFTCancelOfferContainsDuplicateNFTs)
Expand All @@ -308,8 +309,8 @@ TEST_F(NFTHelpersTest, NFTCancelOfferContainsDuplicateNFTs)

EXPECT_EQ(nftTxs.size(), 2);
EXPECT_FALSE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTTransactionsData(nftTxs[1], sttx, txMeta, NFTID2);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTTransactionsData(nftTxs[1], sttx, txMeta, NFTID2);
}

TEST_F(NFTHelpersTest, NFTAcceptBuyerOffer)
Expand All @@ -321,8 +322,8 @@ TEST_F(NFTHelpersTest, NFTAcceptBuyerOffer)

EXPECT_EQ(nftTxs.size(), 1);
EXPECT_TRUE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

// The offer id in tx is different from the offer id in deleted node in metadata
Expand All @@ -348,8 +349,8 @@ TEST_F(NFTHelpersTest, NFTAcceptSellerOfferFromCreatedNode)

EXPECT_EQ(nftTxs.size(), 1);
EXPECT_TRUE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTAcceptSellerOfferFromModifiedNode)
Expand All @@ -361,8 +362,8 @@ TEST_F(NFTHelpersTest, NFTAcceptSellerOfferFromModifiedNode)

EXPECT_EQ(nftTxs.size(), 1);
EXPECT_TRUE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
VerifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTsData(*nftDatas, sttx, txMeta, NFTID, ACCOUNT);
}

TEST_F(NFTHelpersTest, NFTAcceptSellerOfferCheckFail)
Expand Down Expand Up @@ -427,7 +428,7 @@ TEST_F(NFTHelpersTest, NFTCreateOffer)

EXPECT_EQ(nftTxs.size(), 1);
EXPECT_FALSE(nftDatas);
VerifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, NFTID);
}

TEST_F(NFTHelpersTest, NFTDataFromLedgerObject)
Expand Down

0 comments on commit 85e33fd

Please sign in to comment.