From feae85782ceaea08a129459e1b50facbba4439e6 Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Thu, 9 Nov 2023 13:35:08 +0000 Subject: [PATCH] DeliverMax alias of Payment tx (#979) Fix #973 --- src/rpc/RPCHelpers.cpp | 22 +++++- src/rpc/RPCHelpers.h | 10 +++ src/rpc/handlers/AccountTx.cpp | 2 +- src/rpc/handlers/Ledger.cpp | 2 +- src/rpc/handlers/NFTHistory.cpp | 2 +- src/rpc/handlers/TransactionEntry.cpp | 2 +- src/rpc/handlers/Tx.h | 2 +- unittests/rpc/RPCHelpersTests.cpp | 81 +++++++++++++++++++++++ unittests/rpc/handlers/AccountTxTests.cpp | 3 +- unittests/rpc/handlers/LedgerTests.cpp | 2 + unittests/rpc/handlers/TxTests.cpp | 66 ++++++++++++++++++ 11 files changed, 186 insertions(+), 8 deletions(-) diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index 42d5c829..a995edcf 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -185,12 +185,18 @@ toJson(ripple::STBase const& obj) } std::pair -toExpandedJson(data::TransactionAndMetadata const& blobs, NFTokenjson nftEnabled, std::optional networkId) +toExpandedJson( + data::TransactionAndMetadata const& blobs, + std::uint32_t const apiVersion, + NFTokenjson nftEnabled, + std::optional networkId +) { auto [txn, meta] = deserializeTxPlusMeta(blobs, blobs.ledgerSequence); auto txnJson = toJson(*txn); auto metaJson = toJson(*meta); insertDeliveredAmount(metaJson, txn, meta, blobs.date); + insertDeliverMaxAlias(txnJson, apiVersion); if (nftEnabled == NFTokenjson::ENABLE) { Json::Value nftJson; @@ -246,6 +252,17 @@ insertDeliveredAmount( return false; } +void +insertDeliverMaxAlias(boost::json::object& txJson, std::uint32_t const apiVersion) +{ + if (txJson.contains(JS(TransactionType)) and txJson.at(JS(TransactionType)).is_string() and + txJson.at(JS(TransactionType)).as_string() == JS(Payment) and txJson.contains(JS(Amount))) { + txJson[JS(DeliverMax)] = txJson[JS(Amount)]; + if (apiVersion > 1) + txJson.erase(JS(Amount)); + } +} + boost::json::object toJson(ripple::TxMeta const& meta) { @@ -456,7 +473,8 @@ traverseNFTObjects( if (!page) { if (nextPage == beast::zero) { // no nft objects in lastNFTPage return AccountCursor{beast::zero, 0}; - } // marker is in the right range, but still invalid + } + // marker is in the right range, but still invalid return Status{RippledError::rpcINVALID_PARAMS, "Invalid marker."}; } diff --git a/src/rpc/RPCHelpers.h b/src/rpc/RPCHelpers.h index ac7a5151..22d2552f 100644 --- a/src/rpc/RPCHelpers.h +++ b/src/rpc/RPCHelpers.h @@ -73,10 +73,20 @@ deserializeTxPlusMeta(data::TransactionAndMetadata const& blobs, std::uint32_t s std::pair toExpandedJson( data::TransactionAndMetadata const& blobs, + std::uint32_t apiVersion, NFTokenjson nftEnabled = NFTokenjson::DISABLE, std::optional networkId = std::nullopt ); +/** + * @brief Add "DeliverMax" which is the alias of "Amount" for "Payment" transaction to transaction json. Remove the + * "Amount" field when version is greater than 1 + * @param txJson The transaction json object + * @param apiVersion The api version + */ +void +insertDeliverMaxAlias(boost::json::object& txJson, std::uint32_t apiVersion); + bool insertDeliveredAmount( boost::json::object& metaJson, diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index 5f3b612b..9d8053d6 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -165,7 +165,7 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con boost::json::object obj; if (!input.binary) { - auto [txn, meta] = toExpandedJson(txnPlusMeta, NFTokenjson::ENABLE); + auto [txn, meta] = toExpandedJson(txnPlusMeta, ctx.apiVersion, NFTokenjson::ENABLE); obj[JS(meta)] = std::move(meta); obj[JS(tx)] = std::move(txn); diff --git a/src/rpc/handlers/Ledger.cpp b/src/rpc/handlers/Ledger.cpp index 04e535a3..aa976465 100644 --- a/src/rpc/handlers/Ledger.cpp +++ b/src/rpc/handlers/Ledger.cpp @@ -67,7 +67,7 @@ LedgerHandler::process(LedgerHandler::Input input, Context const& ctx) const [&](auto obj) { boost::json::object entry; if (!input.binary) { - auto [txn, meta] = toExpandedJson(obj); + auto [txn, meta] = toExpandedJson(obj, ctx.apiVersion); entry = std::move(txn); entry[JS(metaData)] = std::move(meta); } else { diff --git a/src/rpc/handlers/NFTHistory.cpp b/src/rpc/handlers/NFTHistory.cpp index ff730ec7..6140f706 100644 --- a/src/rpc/handlers/NFTHistory.cpp +++ b/src/rpc/handlers/NFTHistory.cpp @@ -106,7 +106,7 @@ NFTHistoryHandler::process(NFTHistoryHandler::Input input, Context const& ctx) c boost::json::object obj; if (!input.binary) { - auto [txn, meta] = toExpandedJson(txnPlusMeta); + auto [txn, meta] = toExpandedJson(txnPlusMeta, ctx.apiVersion); obj[JS(meta)] = std::move(meta); obj[JS(tx)] = std::move(txn); obj[JS(tx)].as_object()[JS(ledger_index)] = txnPlusMeta.ledgerSequence; diff --git a/src/rpc/handlers/TransactionEntry.cpp b/src/rpc/handlers/TransactionEntry.cpp index 8ae0a0af..70b1163d 100644 --- a/src/rpc/handlers/TransactionEntry.cpp +++ b/src/rpc/handlers/TransactionEntry.cpp @@ -47,7 +47,7 @@ TransactionEntryHandler::process(TransactionEntryHandler::Input input, Context c return Error{Status{RippledError::rpcTXN_NOT_FOUND, "transactionNotFound", "Transaction not found."}}; auto output = TransactionEntryHandler::Output{}; - auto [txn, meta] = toExpandedJson(*dbRet); + auto [txn, meta] = toExpandedJson(*dbRet, ctx.apiVersion); output.tx = std::move(txn); output.metadata = std::move(meta); diff --git a/src/rpc/handlers/Tx.h b/src/rpc/handlers/Tx.h index 9c04db91..6c6bee15 100644 --- a/src/rpc/handlers/Tx.h +++ b/src/rpc/handlers/Tx.h @@ -142,7 +142,7 @@ public: return Error{Status{RippledError::rpcTXN_NOT_FOUND}}; } - auto const [txn, meta] = toExpandedJson(*dbResponse, NFTokenjson::ENABLE, currentNetId); + auto const [txn, meta] = toExpandedJson(*dbResponse, ctx.apiVersion, NFTokenjson::ENABLE, currentNetId); if (!input.binary) { output.tx = txn; diff --git a/unittests/rpc/RPCHelpersTests.cpp b/unittests/rpc/RPCHelpersTests.cpp index 07170129..1d4fc0f2 100644 --- a/unittests/rpc/RPCHelpersTests.cpp +++ b/unittests/rpc/RPCHelpersTests.cpp @@ -21,8 +21,11 @@ #include #include +#include #include +#include +#include #include using namespace rpc; @@ -337,3 +340,81 @@ TEST_F(RPCHelpersTest, DecodeInvalidCTID) EXPECT_FALSE(decodeCTID('c')); EXPECT_FALSE(decodeCTID(true)); } + +TEST_F(RPCHelpersTest, DeliverMaxAliasV1) +{ + std::array const inputArray = { + R"({ + "TransactionType": "Payment", + "Amount": { + "test": "test" + } + })", + R"({ + "TransactionType": "OfferCreate", + "Amount": { + "test": "test" + } + })", + R"({ + "TransactionType": "Payment", + "Amount1": { + "test": "test" + } + })"}; + + std::array outputArray = { + R"({ + "TransactionType": "Payment", + "Amount": { + "test": "test" + }, + "DeliverMax": { + "test": "test" + } + })", + R"({ + "TransactionType": "OfferCreate", + "Amount": { + "test": "test" + } + })", + R"({ + "TransactionType": "Payment", + "Amount1": { + "test": "test" + } + })"}; + + for (size_t i = 0; i < inputArray.size(); i++) { + auto req = boost::json::parse(inputArray[i]).as_object(); + insertDeliverMaxAlias(req, 1); + EXPECT_EQ(req, boost::json::parse(outputArray[i]).as_object()); + } +} + +TEST_F(RPCHelpersTest, DeliverMaxAliasV2) +{ + auto req = boost::json::parse( + R"({ + "TransactionType": "Payment", + "Amount": { + "test": "test" + } + })" + ) + .as_object(); + + insertDeliverMaxAlias(req, 2); + EXPECT_EQ( + req, + boost::json::parse( + R"({ + "TransactionType": "Payment", + "DeliverMax": { + "test": "test" + } + })" + ) + ); +} diff --git a/unittests/rpc/handlers/AccountTxTests.cpp b/unittests/rpc/handlers/AccountTxTests.cpp index fb833103..d695af35 100644 --- a/unittests/rpc/handlers/AccountTxTests.cpp +++ b/unittests/rpc/handlers/AccountTxTests.cpp @@ -1714,6 +1714,7 @@ generateTransactionTypeTestValues() "tx": { "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Amount": "1", + "DeliverMax": "1", "Destination": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "Fee": "1", "Sequence": 32, @@ -1763,7 +1764,7 @@ generateTransactionTypeTestValues() }, "tx": { "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", - "Amount": "1", + "DeliverMax": "1", "Destination": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "Fee": "1", "Sequence": 32, diff --git a/unittests/rpc/handlers/LedgerTests.cpp b/unittests/rpc/handlers/LedgerTests.cpp index b80a5e24..d54500f1 100644 --- a/unittests/rpc/handlers/LedgerTests.cpp +++ b/unittests/rpc/handlers/LedgerTests.cpp @@ -468,6 +468,7 @@ TEST_F(RPCLedgerHandlerTest, TransactionsExpandNotBinary) { "Account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Amount":"100", + "DeliverMax":"100", "Destination":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "Fee":"3", "Sequence":30, @@ -695,6 +696,7 @@ TEST_F(RPCLedgerHandlerTest, OwnerFundsEmtpy) { "Account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Amount":"100", + "DeliverMax":"100", "Destination":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "Fee":"3", "Sequence":30, diff --git a/unittests/rpc/handlers/TxTests.cpp b/unittests/rpc/handlers/TxTests.cpp index f22ed5ff..bb510956 100644 --- a/unittests/rpc/handlers/TxTests.cpp +++ b/unittests/rpc/handlers/TxTests.cpp @@ -288,6 +288,72 @@ TEST_F(RPCTxTest, DefaultParameter_API_v1) }); } +TEST_F(RPCTxTest, PaymentTx_API_v1) +{ + auto const rawBackendPtr = dynamic_cast(mockBackendPtr.get()); + ASSERT_NE(rawBackendPtr, nullptr); + + TransactionAndMetadata tx; + tx.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 2, 3, 300).getSerializer().peekData(); + tx.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30).getSerializer().peekData(); + tx.date = 123456; + tx.ledgerSequence = 100; + + EXPECT_CALL(*rawBackendPtr, fetchTransaction(ripple::uint256{TXNID}, _)).WillOnce(Return(tx)); + + auto const rawETLPtr = dynamic_cast(mockETLServicePtr.get()); + ASSERT_NE(rawETLPtr, nullptr); + EXPECT_CALL(*rawETLPtr, getETLState).WillOnce(Return(etl::ETLState{})); + + runSpawn([this](auto yield) { + auto const handler = AnyHandler{TestTxHandler{mockBackendPtr, mockETLServicePtr}}; + auto const req = json::parse(fmt::format( + R"({{ + "command": "tx", + "transaction": "{}" + }})", + TXNID + )); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1u}); + ASSERT_TRUE(output); + EXPECT_TRUE(output->as_object().contains("DeliverMax")); + EXPECT_EQ(output->at("Amount"), output->at("DeliverMax")); + }); +} + +TEST_F(RPCTxTest, PaymentTx_API_v2) +{ + auto const rawBackendPtr = dynamic_cast(mockBackendPtr.get()); + ASSERT_NE(rawBackendPtr, nullptr); + + TransactionAndMetadata tx; + tx.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 2, 3, 300).getSerializer().peekData(); + tx.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30).getSerializer().peekData(); + tx.date = 123456; + tx.ledgerSequence = 100; + + EXPECT_CALL(*rawBackendPtr, fetchTransaction(ripple::uint256{TXNID}, _)).WillOnce(Return(tx)); + + auto const rawETLPtr = dynamic_cast(mockETLServicePtr.get()); + ASSERT_NE(rawETLPtr, nullptr); + EXPECT_CALL(*rawETLPtr, getETLState).WillOnce(Return(etl::ETLState{})); + + runSpawn([this](auto yield) { + auto const handler = AnyHandler{TestTxHandler{mockBackendPtr, mockETLServicePtr}}; + auto const req = json::parse(fmt::format( + R"({{ + "command": "tx", + "transaction": "{}" + }})", + TXNID + )); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2u}); + ASSERT_TRUE(output); + EXPECT_TRUE(output->as_object().contains("DeliverMax")); + EXPECT_FALSE(output->as_object().contains("Amount")); + }); +} + TEST_F(RPCTxTest, DefaultParameter_API_v2) { auto const rawBackendPtr = dynamic_cast(mockBackendPtr.get());