From e32e2ebee4e48b027e6d1c381fba8e31222b82d8 Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Mon, 9 Oct 2023 10:19:07 +0100 Subject: [PATCH] Fix account_tx response both both ledger range and ledger index/hash are specified (#904) Fix mismatch with rippled --- src/rpc/handlers/AccountTx.cpp | 21 ++++++++++++--------- unittests/rpc/handlers/AccountTxTests.cpp | 10 +--------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index e8da674bd..04b2785cc 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -105,15 +105,21 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con if (input.ledgerHash || input.ledgerIndex || input.usingValidatedLedger) { if (ctx.apiVersion > 1u && (input.ledgerIndexMax || input.ledgerIndexMin)) + { return Error{Status{RippledError::rpcINVALID_PARAMS, "containsLedgerSpecifierAndRange"}}; + } + else if (!input.ledgerIndexMax && !input.ledgerIndexMin) + { + // mimic rippled, when both range and index specified, respect the range. + // take ledger from ledgerHash or ledgerIndex only when range is not specified + auto const lgrInfoOrStatus = getLedgerInfoFromHashOrSeq( + *sharedPtrBackend_, ctx.yield, input.ledgerHash, input.ledgerIndex, range->maxSequence); - auto const lgrInfoOrStatus = getLedgerInfoFromHashOrSeq( - *sharedPtrBackend_, ctx.yield, input.ledgerHash, input.ledgerIndex, range->maxSequence); + if (auto status = std::get_if(&lgrInfoOrStatus)) + return Error{*status}; - if (auto status = std::get_if(&lgrInfoOrStatus)) - return Error{*status}; - - maxIndex = minIndex = std::get(lgrInfoOrStatus).seq; + maxIndex = minIndex = std::get(lgrInfoOrStatus).seq; + } } std::optional cursor; @@ -191,9 +197,6 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con obj[JS(meta)] = ripple::strHex(txnPlusMeta.metadata); obj[JS(tx_blob)] = ripple::strHex(txnPlusMeta.transaction); obj[JS(ledger_index)] = txnPlusMeta.ledgerSequence; - - if (ctx.apiVersion < 2u) - obj[JS(inLedger)] = txnPlusMeta.ledgerSequence; } obj[JS(validated)] = true; diff --git a/unittests/rpc/handlers/AccountTxTests.cpp b/unittests/rpc/handlers/AccountTxTests.cpp index 4612f4153..da61b0c37 100644 --- a/unittests/rpc/handlers/AccountTxTests.cpp +++ b/unittests/rpc/handlers/AccountTxTests.cpp @@ -371,14 +371,6 @@ TEST_P(AccountTxParameterTest, CheckParams) } else { - if (req.as_object().contains("ledger_hash")) - { - EXPECT_CALL(*rawBackendPtr, fetchLedgerByHash).WillOnce(testing::Return(ripple::LedgerHeader{})); - } - else if (req.as_object().contains("ledger_index")) - { - EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence).WillOnce(testing::Return(ripple::LedgerHeader{})); - } EXPECT_CALL(*rawBackendPtr, fetchAccountTransactions); runSpawn([&, this](auto yield) { @@ -655,7 +647,7 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrue) "144B4E9C06F24296074F7BC48F92A97916C6DC5EA98314D31252CF902EF8DD8451" "243869B38667CBD89DF3"); EXPECT_FALSE(output->at("transactions").as_array()[0].as_object().contains("date")); - + EXPECT_FALSE(output->at("transactions").as_array()[0].as_object().contains("inLedger")); EXPECT_FALSE(output->as_object().contains("limit")); }); }