From 0aa7ed49198cd9c8d858a96191c223e69553183b Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Mon, 15 Jun 2026 06:46:55 -0400 Subject: [PATCH] feat: Update MPToken ETL to support all transactions types (#3102) ## Summary Fixes incomplete MPT holder indexing in ETL. Previously, Clio only indexed MPT holders when an `ltMPTOKEN` ledger node was created by an `MPTokenAuthorize` transaction. However, with future amendments, XRPL can also create `ltMPTOKEN` nodes as part of other successful transactions, such as `Payment`, when a holder receives an MPT. Those holders were silently skipped and therefore missing from the `mpt_holders` RPC results. This change makes MPT holder indexing metadata-driven instead of transaction-type-driven. ETL now scans successful transaction metadata for newly created `ltMPTOKEN` nodes and writes every holder it finds. ## What Changed - MPT holder extraction now works for any successful transaction that creates an `ltMPTOKEN`. - Multiple MPT holders created in a single transaction are all indexed. - The `mpt_holders` table is kept consistent with ledger metadata instead of relying on assumptions about which transaction types can create holder nodes. - Added regression coverage for MPT holders created by a `Payment` transaction. ## Testing - `build/clio_tests '--gtest_filter=MPTExtTests.*'` - `clang-tidy -p build src/etl/MPTHelpers.cpp src/etl/MPTHelpers.hpp src/etl/impl/ext/MPT.cpp tests/unit/etl/ext/MPTTests.cpp` --- src/etl/MPTHelpers.cpp | 38 +++----- src/etl/MPTHelpers.hpp | 9 +- src/etl/impl/ext/MPT.cpp | 4 +- tests/unit/etl/ext/MPTTests.cpp | 165 +++++++++++++++++++++++++++++++- 4 files changed, 184 insertions(+), 32 deletions(-) diff --git a/src/etl/MPTHelpers.cpp b/src/etl/MPTHelpers.cpp index b56daf457..1ed6aef95 100644 --- a/src/etl/MPTHelpers.cpp +++ b/src/etl/MPTHelpers.cpp @@ -11,45 +11,37 @@ #include #include #include -#include #include #include +#include namespace etl { -/** - * @brief Get the MPToken created from a transaction - * - * @param txMeta Transaction metadata - * @return MPT and holder account pair - */ -std::optional -getMPTokenAuthorize(ripple::TxMeta const& txMeta) +std::vector +getMPTHolderFromTx(ripple::TxMeta const& txMeta, ripple::STTx const&) { + if (txMeta.getResultTER() != ripple::tesSUCCESS) + return {}; + + std::vector holders; + for (ripple::STObject const& node : txMeta.getNodes()) { if (node.getFieldU16(ripple::sfLedgerEntryType) != ripple::ltMPTOKEN) continue; if (node.getFName() == ripple::sfCreatedNode) { auto const& newMPT = node.peekAtField(ripple::sfNewFields).downcast(); - return MPTHolderData{ - .mptID = newMPT[ripple::sfMPTokenIssuanceID], - .holder = newMPT.getAccountID(ripple::sfAccount) - }; + holders.push_back( + MPTHolderData{ + .mptID = newMPT[ripple::sfMPTokenIssuanceID], + .holder = newMPT.getAccountID(ripple::sfAccount) + } + ); } } - return {}; -} -std::optional -getMPTHolderFromTx(ripple::TxMeta const& txMeta, ripple::STTx const& sttx) -{ - if (txMeta.getResultTER() != ripple::tesSUCCESS || - sttx.getTxnType() != ripple::TxType::ttMPTOKEN_AUTHORIZE) - return {}; - - return getMPTokenAuthorize(txMeta); + return holders; } std::optional diff --git a/src/etl/MPTHelpers.hpp b/src/etl/MPTHelpers.hpp index dc9e30a0a..e4b2d3878 100644 --- a/src/etl/MPTHelpers.hpp +++ b/src/etl/MPTHelpers.hpp @@ -6,6 +6,10 @@ #include #include +#include +#include +#include + namespace etl { /** @@ -13,9 +17,10 @@ namespace etl { * * @param txMeta Transaction metadata * @param sttx The transaction - * @return The MPTIssuanceID and holder pair as a optional + * @return The MPTIssuanceID and holder pairs created by the transaction; empty if the transaction + * failed or created no MPToken. */ -std::optional +std::vector getMPTHolderFromTx(ripple::TxMeta const& txMeta, ripple::STTx const& sttx); /** diff --git a/src/etl/impl/ext/MPT.cpp b/src/etl/impl/ext/MPT.cpp index 343afca9f..87c7e333a 100644 --- a/src/etl/impl/ext/MPT.cpp +++ b/src/etl/impl/ext/MPT.cpp @@ -49,8 +49,8 @@ MPTExt::writeMPTHoldersFromTransactions(model::LedgerData const& data) std::vector holders; for (auto const& tx : data.transactions) { - if (auto const mptHolder = getMPTHolderFromTx(tx.meta, tx.sttx); mptHolder.has_value()) - holders.push_back(*mptHolder); + auto const mptHolders = getMPTHolderFromTx(tx.meta, tx.sttx); + holders.append_range(mptHolders); } if (not holders.empty()) diff --git a/tests/unit/etl/ext/MPTTests.cpp b/tests/unit/etl/ext/MPTTests.cpp index 294f75d72..64664d695 100644 --- a/tests/unit/etl/ext/MPTTests.cpp +++ b/tests/unit/etl/ext/MPTTests.cpp @@ -1,3 +1,4 @@ +#include "data/DBHelpers.hpp" #include "etl/Models.hpp" #include "etl/impl/ext/MPT.hpp" #include "rpc/RPCHelpers.hpp" @@ -8,9 +9,20 @@ #include #include +#include +#include +#include +#include +#include +#include +#include +#include #include +#include #include +#include +#include #include #include @@ -25,6 +37,8 @@ constinit auto const kSeq = 123u; constinit auto const kLedgerHash = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652"; constinit auto const kHolderAccount = "rK1EX542EgA9m948JrJRaEzwLVEhqWvnr9"; +constinit auto const kHolderAccount2 = "rnd1nHuzceyQDqnLH8urWNr4QBKt4v7WVk"; +constinit auto const kMptIssuanceID = "000004C463C52827307480341125DA0577DEFC38405B0E3E"; constinit auto const kTxnHex = "120039220000000024002DBD1A201B002DBDA36840000000000000017321EDECF25C029811CAD07AFD616EB75E3803" @@ -54,18 +68,115 @@ constinit auto const kHash = "6005B465CBBF7FA8E41AC0C0CD38491026D9411FCB7BA46E2A constinit auto const kHash2 = "6005B465CBBF7FA8E41AC0C0CD38491026D9411FCB7BA46E2AEBB3AF7654261C"; constinit auto const kHash3 = "6005B465CBBF7FA8E41AC0C0CD38491026D9411FCB7BA46E2AEBB3AF7654261D"; +auto +createTransactionFromObjects( + ripple::STObject const& txObj, + ripple::STObject const& metaObj, + ripple::TxType type +) +{ + auto const txBlob = txObj.getSerializer().peekData(); + auto const metaBlob = metaObj.getSerializer().peekData(); + + ripple::SerialIter txIter{txBlob.data(), txBlob.size()}; + ripple::uint256 const id{kHash}; + + return etl::model::Transaction{ + .raw = "", + .metaRaw = "", + .sttx = ripple::STTx{txIter}, + .meta = ripple::TxMeta{id, kSeq, metaBlob}, + .id = id, + .key = std::string{kHash}, + .type = type + }; +} + +ripple::STObject +createNewMPTokenNode(std::string_view holder) +{ + ripple::STObject newFields(ripple::sfNewFields); + newFields.setFieldU16(ripple::sfLedgerEntryType, ripple::ltMPTOKEN); + newFields[ripple::sfMPTokenIssuanceID] = ripple::uint192{kMptIssuanceID}; + newFields.setAccountID(ripple::sfAccount, getAccountIdWithString(holder)); + + ripple::STObject createdNode(ripple::sfCreatedNode); + createdNode.setFieldU16(ripple::sfLedgerEntryType, ripple::ltMPTOKEN); + createdNode.setFieldH256(ripple::sfLedgerIndex, ripple::uint256{}); + createdNode.emplace_back(std::move(newFields)); + return createdNode; +} + +ripple::STObject +createPaymentMetaWithNewMPTokens(ripple::TER result = ripple::tesSUCCESS) +{ + ripple::STObject metaObj(ripple::sfTransactionMetaData); + metaObj.setFieldU8(ripple::sfTransactionResult, TERtoInt(result)); + metaObj.setFieldU32(ripple::sfTransactionIndex, 0); + + ripple::STArray affectedNodes(ripple::sfAffectedNodes); + affectedNodes.push_back(createNewMPTokenNode(kHolderAccount)); + affectedNodes.push_back(createNewMPTokenNode(kHolderAccount2)); + metaObj.setFieldArray(ripple::sfAffectedNodes, affectedNodes); + + return metaObj; +} + +auto +createPaymentWithMultipleHoldersTestData(ripple::TER result = ripple::tesSUCCESS) +{ + auto transactions = std::vector{createTransactionFromObjects( + createPaymentTransactionObject(kHolderAccount, kHolderAccount2, 1, 1, 1), + createPaymentMetaWithNewMPTokens(result), + ripple::TxType::ttPAYMENT + )}; + + auto const header = createLedgerHeader(kLedgerHash, kSeq); + return etl::model::LedgerData{ + .transactions = std::move(transactions), + .objects = {}, + .successors = {}, + .edgeKeys = {}, + .header = header, + .rawHeader = {}, + .seq = kSeq + }; +} + +auto +createTestDataWithoutMPToken() +{ + auto transactions = std::vector{ + util::createTransaction( + ripple::TxType::ttMPTOKEN_ISSUANCE_CREATE + ), // metadata does not create an MPT holder + util::createTransaction(ripple::TxType::ttAMM_CREATE), // metadata is not MPT + }; + + auto const header = createLedgerHeader(kLedgerHash, kSeq); + return etl::model::LedgerData{ + .transactions = std::move(transactions), + .objects = {}, + .successors = {}, + .edgeKeys = {}, + .header = header, + .rawHeader = {}, + .seq = kSeq + }; +} + auto createTestData() { auto transactions = std::vector{ util::createTransaction( ripple::TxType::ttMPTOKEN_ISSUANCE_CREATE - ), // not AUTHORIZE so will not be written + ), // metadata does not create an MPT holder util::createTransaction(ripple::TxType::ttMPTOKEN_AUTHORIZE, kHash, kTxnMeta, kTxnHex), - util::createTransaction(ripple::TxType::ttAMM_CREATE), // not MPT - will be filtered + util::createTransaction(ripple::TxType::ttAMM_CREATE), // metadata is not MPT util::createTransaction( ripple::TxType::ttMPTOKEN_ISSUANCE_CREATE - ), // not unique - will be filtered + ), // metadata does not create an MPT holder }; auto const header = createLedgerHeader(kLedgerHash, kSeq); @@ -113,7 +224,7 @@ TEST_F(MPTExtTests, OnLedgerDataFiltersAndWritesMPTs) auto const data = createTestData(); EXPECT_CALL(*backend_, writeMPTHolders).WillOnce([](auto const& holders) { - EXPECT_EQ(holders.size(), 1); // Only AUTHORIZE is written in the end + EXPECT_EQ(holders.size(), 1); // Only metadata creating an MPToken is written }); ext_.onLedgerData(data); @@ -124,7 +235,7 @@ TEST_F(MPTExtTests, OnInitialDataFiltersAndWritesMPTs) auto const data = createTestData(); EXPECT_CALL(*backend_, writeMPTHolders).WillOnce([](auto const& holders) { - EXPECT_EQ(holders.size(), 1); // Only AUTHORIZE is written in the end + EXPECT_EQ(holders.size(), 1); // Only metadata creating an MPToken is written }); ext_.onInitialData(data); @@ -157,3 +268,47 @@ TEST_F(MPTExtTests, OnInitialDataWithMultipleHolders) ext_.onInitialData(data); } + +TEST_F(MPTExtTests, OnInitialDataDoesNotWriteFailedMPTokenCreations) +{ + auto const data = createPaymentWithMultipleHoldersTestData(ripple::tecINCOMPLETE); + + EXPECT_CALL(*backend_, writeMPTHolders).Times(0); + + ext_.onInitialData(data); +} + +TEST_F(MPTExtTests, OnInitialDataDoesNotWriteWithoutCreatedMPToken) +{ + auto const data = createTestDataWithoutMPToken(); + + EXPECT_CALL(*backend_, writeMPTHolders).Times(0); + + ext_.onInitialData(data); +} + +TEST_F(MPTExtTests, OnInitialDataWritesAllMPTsCreatedByPayment) +{ + auto const data = createPaymentWithMultipleHoldersTestData(); + auto const expectedMptID = ripple::uint192{kMptIssuanceID}; + auto const expectedAccount = getAccountIdWithString(kHolderAccount); + auto const expectedAccount2 = getAccountIdWithString(kHolderAccount2); + + EXPECT_CALL(*backend_, writeMPTHolders).WillOnce([&](auto const& holders) { + EXPECT_THAT( + holders, + UnorderedElementsAre( + AllOf( + Field(&MPTHolderData::mptID, expectedMptID), + Field(&MPTHolderData::holder, expectedAccount) + ), + AllOf( + Field(&MPTHolderData::mptID, expectedMptID), + Field(&MPTHolderData::holder, expectedAccount2) + ) + ) + ); + }); + + ext_.onInitialData(data); +}