From 7a58d95ce0de2c728643c380c378104f125508d9 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 1 Dec 2025 13:43:29 -0500 Subject: [PATCH 1/5] feat: add delegate filter param's to account_tx RPC --- src/data/BackendInterface.hpp | 2 + src/data/Types.hpp | 1 + src/data/cassandra/CassandraBackendFamily.hpp | 107 ++++++++++- src/rpc/RPCHelpers.cpp | 44 +++++ src/rpc/RPCHelpers.hpp | 19 ++ src/rpc/common/Types.hpp | 23 ++- src/rpc/common/Validators.cpp | 18 ++ src/rpc/common/Validators.hpp | 7 + src/rpc/handlers/AccountTx.cpp | 20 +- src/rpc/handlers/AccountTx.hpp | 6 +- tests/common/util/MockBackend.hpp | 2 + tests/common/util/MockBackendTestFixture.hpp | 1 - .../data/cassandra/BackendTests.cpp | 10 +- tests/unit/rpc/RPCHelpersTests.cpp | 87 +++++++++ tests/unit/rpc/handlers/AccountTxTests.cpp | 172 +++++++++++++++++- 15 files changed, 503 insertions(+), 16 deletions(-) diff --git a/src/data/BackendInterface.hpp b/src/data/BackendInterface.hpp index 7a823c76a3..0200fad0b1 100644 --- a/src/data/BackendInterface.hpp +++ b/src/data/BackendInterface.hpp @@ -23,6 +23,7 @@ #include "data/LedgerCacheInterface.hpp" #include "data/Types.hpp" #include "etl/CorruptionDetector.hpp" +#include "rpc/common/Types.hpp" #include "util/Spawn.hpp" #include "util/log/Logger.hpp" @@ -314,6 +315,7 @@ class BackendInterface { std::uint32_t limit, bool forward, std::optional const& txnCursor, + std::optional const& delegateFilter, boost::asio::yield_context yield ) const = 0; diff --git a/src/data/Types.hpp b/src/data/Types.hpp index 175583efa1..ca2bcf7418 100644 --- a/src/data/Types.hpp +++ b/src/data/Types.hpp @@ -70,6 +70,7 @@ struct TransactionAndMetadata { Blob metadata; std::uint32_t ledgerSequence = 0; std::uint32_t date = 0; + std::optional delegatedAccount; TransactionAndMetadata() = default; diff --git a/src/data/cassandra/CassandraBackendFamily.hpp b/src/data/cassandra/CassandraBackendFamily.hpp index ad917e1293..fc0c7978e5 100644 --- a/src/data/cassandra/CassandraBackendFamily.hpp +++ b/src/data/cassandra/CassandraBackendFamily.hpp @@ -28,6 +28,7 @@ #include "data/cassandra/Handle.hpp" #include "data/cassandra/Types.hpp" #include "data/cassandra/impl/ExecutionStrategy.hpp" +#include "rpc/common/Types.hpp" #include "util/Assert.hpp" #include "util/LedgerUtils.hpp" #include "util/Profiler.hpp" @@ -40,18 +41,25 @@ #include #include #include +#include #include #include #include #include #include +#include +#include +#include +#include #include +#include #include #include #include #include #include +#include #include #include #include @@ -152,6 +160,7 @@ class CassandraBackendFamily : public BackendInterface { std::uint32_t const limit, bool forward, std::optional const& txnCursor, + std::optional const& delegateFilter, boost::asio::yield_context yield ) const override { @@ -203,9 +212,66 @@ class CassandraBackendFamily : public BackendInterface { } } - auto const txns = fetchTransactions(hashes, yield); + auto txns = fetchTransactions(hashes, yield); LOG(log_.debug()) << "Txns = " << txns.size(); + std::optional filterCounterpartyID; + if (delegateFilter && delegateFilter->counterParty) { + filterCounterpartyID = ripple::parseBase58(*delegateFilter->counterParty); + + // If the counterparty string is invalid, we don't return anything. Error should have been caught by validators. + if (!filterCounterpartyID) { + LOG(log_.warn()) << "Invalid counterparty account in filter"; + return {.txns={}, .cursor={}}; + } + } + + std::vector resultTxns; + if (delegateFilter.has_value()) { + resultTxns.reserve(txns.size()); + + for (auto& txn : txns) { + try { + auto const delegationInfo = getDelegationInfo(txn.transaction, txn.metadata); + + if (delegationInfo) { + auto const& [delegatee, delegator] = *delegationInfo; + bool match = false; + + // Filter by "Delegator" ie. User wants to find the Owner (Delegator). + // This implies the User (account) must be the Signer (Delegatee) that acted on someone's behalf. + if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { + // The user (account) must be delegatee + if (account == delegatee) { + if (!filterCounterpartyID || *filterCounterpartyID == delegator) { + txn.delegatedAccount = delegator; + match = true; + } + } + } + // Filter by "Delegatee" ie. User wants to find the Signer (Delegatee). + // This implies the User (account) must be the Owner (Delegator). + else if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { + // The user (account) must be delegator + if (account == delegator) { + if (!filterCounterpartyID || *filterCounterpartyID == delegatee) { + txn.delegatedAccount = delegatee; + match = true; + } + } + } + + if (match) + resultTxns.push_back(txn); + + } + } catch (std::exception const& e) { + LOG(log_.warn()) << "Failed to parse tx for filter"; + } + } + return {.txns=resultTxns, .cursor=cursor}; + } + if (txns.size() == limit) { LOG(log_.debug()) << "Returning cursor"; return {txns, cursor}; @@ -970,6 +1036,45 @@ class CassandraBackendFamily : public BackendInterface { return true; } + +/** + * @brief Extracts delegation information from a transaction. + * + * Parses the transaction blob and checks whether the signer + * (derived from the SigningPubKey) differs from the delegator + * account. If so, returns {delegatee, delegator}. Otherwise returns null. + * + * @param txnBlob Serialized transaction blob. + * @param metaBlob Unused metadata blob fetched from rippled. + * @return pair of {delegatee, delegator} if delegated, otherwise std::nullopt + */ +static std::optional> +getDelegationInfo(ripple::Blob const& txnBlob, ripple::Blob const& /*metaBlob*/) +{ + + ripple::SerialIter it{txnBlob.data(), txnBlob.size()}; + ripple::STTx const txn{it}; + + auto const delegator = txn.getAccountID(ripple::sfAccount); + if (txn.isFieldPresent(ripple::sfSigningPubKey)) + { + auto const pubKeyBlob = txn.getFieldVL(ripple::sfSigningPubKey); + ripple::PublicKey const pubKey{ripple::Slice{pubKeyBlob.data(), pubKeyBlob.size()}}; + + auto const delegatee = ripple::calcAccountID(pubKey); + + // Delegation Check + // If the signer (delegatee) is NOT the account owner (delegator), it's delegated. + if (delegatee != delegator) + { + return std::make_pair(delegatee, delegator); + } + } + + return std::nullopt; + +} + }; } // namespace data::cassandra diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index 72a6555ff5..933b2190f3 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -1567,4 +1567,48 @@ toJsonWithBinaryTx(data::TransactionAndMetadata const& txnPlusMeta, std::uint32_ return obj; } +std::optional +parseDelegateType(boost::json::value const& delegateType) +{ + if (not delegateType.is_string()) + return {}; + + auto const& type = delegateType.as_string(); + + if (type == "delegator") + return DelegateFilter::Role::Delegator; + if (type == "delegatee") + return DelegateFilter::Role::Delegatee; + + return {}; +} + +std::optional +parseDelegateFilter(boost::json::object const& delegateObject) +{ + DelegateFilter delegate{}; + if (!delegateObject.contains("delegate_filter")) + return {}; + + auto const& filterVal = delegateObject.at("delegate_filter"); + if (!filterVal.is_string()) + return {}; + + auto const delegateTypeOpt = parseDelegateType(filterVal.as_string()); + if (!delegateTypeOpt.has_value()) + return {}; + + delegate.delegateType = *delegateTypeOpt; + if (delegateObject.contains("counterparty")) { + auto const& counterpartyVal = delegateObject.at("counterparty"); + + if (!counterpartyVal.is_string()) + return {}; + + delegate.counterParty = counterpartyVal.as_string(); + } + + return delegate; +} + } // namespace rpc diff --git a/src/rpc/RPCHelpers.hpp b/src/rpc/RPCHelpers.hpp index 5d3f9bb994..a339cd2f5f 100644 --- a/src/rpc/RPCHelpers.hpp +++ b/src/rpc/RPCHelpers.hpp @@ -861,4 +861,23 @@ getDeliveredAmount( uint32_t date ); +/** + * @brief Parse the delegate type from a JSON value + * + * @param delegateType The JSON value containing the delegate type string + * @return The parsed delegate type or std::nullopt if the input is invalid or not a string + */ +std::optional +parseDelegateType(boost::json::value const& delegateType); + +/** + * @brief Parse a delegate filter object from JSON + * + * @param delegateObject The JSON object containing the delegate filter input from user + * @return The constructed DelegateFilter or std::nullopt if parsing fails + */ +std::optional +parseDelegateFilter(boost::json::object const& delegateObject); + + } // namespace rpc diff --git a/src/rpc/common/Types.hpp b/src/rpc/common/Types.hpp index 85beda00b0..203c0730ad 100644 --- a/src/rpc/common/Types.hpp +++ b/src/rpc/common/Types.hpp @@ -33,9 +33,9 @@ #include #include +#include #include #include -#include namespace etl { class LoadBalancer; @@ -194,6 +194,27 @@ struct AccountCursor { } }; +/** + * @brief A delegate object used filter account_tx by specific delegate accounts + */ +struct DelegateFilter { + /** + * @brief A delegate type used in delegate filter + */ + enum class Role { + // This account is the *active* sender, acting on behalf of another party. + // e.g., Account A in "A sends payment to B on behalf of C." + Delegatee, + + // This account is the *passive* party whose funds are being moved from. + // e.g., Account C in "A sends payment to B on behalf of C." + Delegator + }; + + Role delegateType; + std::optional counterParty; +}; + /** * @brief Convert an empty output to a JSON object * diff --git a/src/rpc/common/Validators.cpp b/src/rpc/common/Validators.cpp index cb458252c8..4627938eae 100644 --- a/src/rpc/common/Validators.cpp +++ b/src/rpc/common/Validators.cpp @@ -357,4 +357,22 @@ CustomValidator CustomValidators::authorizeCredentialValidator = return MaybeError{}; }}; + CustomValidator CustomValidators::delegateValidator = + CustomValidator{[](boost::json::value const& value, std::string_view key) -> MaybeError { + if (!value.is_object()) + return Error{Status{RippledError::rpcINVALID_PARAMS, std::string(key) + " not object"}}; + + auto const& delegate = value.as_object(); + if (!delegate.contains("delegate_filter")) + return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' is required but missing."}}; + + if (!parseDelegateType(delegate.at("delegate_filter")).has_value()) + return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' value must be 'delegator' or 'delegatee'."}}; + + if (delegate.contains("counterparty") && !accountValidator.verify(delegate, "counterparty")) + return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'counterparty' value must be a valid account."}}; + + return MaybeError{}; + }}; + } // namespace rpc::validation diff --git a/src/rpc/common/Validators.hpp b/src/rpc/common/Validators.hpp index 633a6f6c93..e6d4b427db 100644 --- a/src/rpc/common/Validators.hpp +++ b/src/rpc/common/Validators.hpp @@ -592,6 +592,13 @@ struct CustomValidators final { * Used by AuthorizeCredentialValidator in deposit_preauth. */ static CustomValidator credentialTypeValidator; + + /** + * @brief Provides a validator for validating filtering by delegation. + * + * Used by account_tx if user wants to filter by delegation. + */ + static CustomValidator delegateValidator; }; /** diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index ad82919e6c..16edbd5ed5 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -122,7 +122,7 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c auto const limit = input.limit.value_or(kLIMIT_DEFAULT); auto const accountID = accountFromStringStrict(input.account); auto const [txnsAndCursor, timeDiff] = util::timed([&]() { - return sharedPtrBackend_->fetchAccountTransactions(*accountID, limit, input.forward, cursor, ctx.yield); + return sharedPtrBackend_->fetchAccountTransactions(*accountID, limit, input.forward, cursor, input.delegateFilter,ctx.yield); }); LOG(log_.info()) << "db fetch took " << timeDiff << " milliseconds - num blobs = " << txnsAndCursor.txns.size(); @@ -191,6 +191,20 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c obj[JS(close_time_iso)] = ripple::to_string_iso(ledgerHeader->closeTime); } } + + if (txnPlusMeta.delegatedAccount.has_value()) { + if (input.delegateFilter) { + if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { + // filtering by the txns where other accounts sent txns for this delegatedAccount + obj["delegator"] = to_string(*txnPlusMeta.delegatedAccount); + } + else if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { + // filtering by the txns where this delegatedAccount sent txn on behalf of other users + obj["delegatee"] = to_string(*txnPlusMeta.delegatedAccount); + } + } + } + obj[JS(validated)] = true; response.transactions.push_back(std::move(obj)); continue; @@ -286,6 +300,10 @@ tag_invoke(boost::json::value_to_tag, boost::json::valu if (jsonObject.contains("tx_type")) input.transactionTypeInLowercase = boost::json::value_to(jsonObject.at("tx_type")); + if (jsonObject.contains("delegate")){ + input.delegateFilter = parseDelegateFilter(jsonObject.at("delegate").as_object()); + } + return input; } diff --git a/src/rpc/handlers/AccountTx.hpp b/src/rpc/handlers/AccountTx.hpp index e29bb4355b..9bea51a85d 100644 --- a/src/rpc/handlers/AccountTx.hpp +++ b/src/rpc/handlers/AccountTx.hpp @@ -103,6 +103,7 @@ class AccountTxHandler { std::optional limit; std::optional marker; std::optional transactionTypeInLowercase; + std::optional delegateFilter; }; using Result = HandlerReturnType; @@ -156,7 +157,10 @@ class AccountTxHandler { validation::Type{}, modifiers::ToLower{}, validation::OneOf(typesKeysInLowercase.cbegin(), typesKeysInLowercase.cend()), - }, + }, { + "delegate", + validation::CustomValidators::delegateValidator + } }; static auto const kRPC_SPEC = RpcSpec{ diff --git a/tests/common/util/MockBackend.hpp b/tests/common/util/MockBackend.hpp index a6447d5da5..100442add1 100644 --- a/tests/common/util/MockBackend.hpp +++ b/tests/common/util/MockBackend.hpp @@ -23,6 +23,7 @@ #include "data/DBHelpers.hpp" #include "data/LedgerCache.hpp" #include "data/Types.hpp" +#include "rpc/common/Types.hpp" #include "util/config/ConfigDefinition.hpp" #include @@ -86,6 +87,7 @@ struct MockBackend : public BackendInterface { std::uint32_t const, bool, std::optional const&, + std::optional const&, boost::asio::yield_context), (const, override) ); diff --git a/tests/common/util/MockBackendTestFixture.hpp b/tests/common/util/MockBackendTestFixture.hpp index 06c84e3722..8489d0225a 100644 --- a/tests/common/util/MockBackendTestFixture.hpp +++ b/tests/common/util/MockBackendTestFixture.hpp @@ -20,7 +20,6 @@ #pragma once #include "data/BackendInterface.hpp" -#include "util/LoggerFixtures.hpp" #include "util/MockBackend.hpp" #include "util/config/ConfigDefinition.hpp" diff --git a/tests/integration/data/cassandra/BackendTests.cpp b/tests/integration/data/cassandra/BackendTests.cpp index 04909575fd..6ca004a305 100644 --- a/tests/integration/data/cassandra/BackendTests.cpp +++ b/tests/integration/data/cassandra/BackendTests.cpp @@ -481,7 +481,7 @@ TEST_F(BackendCassandraTest, Basic) EXPECT_EQ(hashes.size(), 1); EXPECT_EQ(ripple::strHex(hashes[0]), hashHex); for (auto& a : affectedAccounts) { - auto [accountTransactions, cursor] = backend_->fetchAccountTransactions(a, 100, true, {}, yield); + auto [accountTransactions, cursor] = backend_->fetchAccountTransactions(a, 100, true, {}, {}, yield); EXPECT_EQ(accountTransactions.size(), 1); EXPECT_EQ(accountTransactions[0], accountTransactions[0]); EXPECT_FALSE(cursor); @@ -709,7 +709,7 @@ TEST_F(BackendCassandraTest, Basic) auto retTxns = backend_->fetchAllTransactionsInLedger(seq, yield); for (auto [hash, txn, meta] : txns) { bool found = false; - for (auto [retTxn, retMeta, retSeq, retDate] : retTxns) { + for (auto [retTxn, retMeta, retSeq, retDate, retFilter] : retTxns) { if (std::strncmp( reinterpret_cast(retTxn.data()), static_cast(txn.data()), @@ -730,7 +730,7 @@ TEST_F(BackendCassandraTest, Basic) do { uint32_t const limit = 10; auto [accountTransactions, retCursor] = - backend_->fetchAccountTransactions(account, limit, false, cursor, yield); + backend_->fetchAccountTransactions(account, limit, false, cursor, {}, yield); if (retCursor) EXPECT_EQ(accountTransactions.size(), limit); retData.insert(retData.end(), accountTransactions.begin(), accountTransactions.end()); @@ -738,8 +738,8 @@ TEST_F(BackendCassandraTest, Basic) } while (cursor); EXPECT_EQ(retData.size(), data.size()); for (size_t i = 0; i < retData.size(); ++i) { - auto [txn, meta, _, _2] = retData[i]; - auto [_3, expTxn, expMeta] = data[i]; + auto [txn, meta, _, _2, _3] = retData[i]; + auto [_4, expTxn, expMeta] = data[i]; EXPECT_STREQ(reinterpret_cast(txn.data()), static_cast(expTxn.data())); EXPECT_STREQ(reinterpret_cast(meta.data()), static_cast(expMeta.data())); } diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp index 40189b9b66..02fc40fa5d 100644 --- a/tests/unit/rpc/RPCHelpersTests.cpp +++ b/tests/unit/rpc/RPCHelpersTests.cpp @@ -603,6 +603,93 @@ TEST_F(RPCHelpersTest, FetchAndCheckAnyFlagExists_TrustLineIsFrozenAndCheckFreez }); } +TEST_F(RPCHelpersTest, ParseDelegateType) +{ + auto result = parseDelegateType(boost::json::value("delegator")); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(*result, DelegateFilter::Role::Delegator); + + result = parseDelegateType(boost::json::value("delegatee")); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(*result, DelegateFilter::Role::Delegatee); + + // invalid types + result = parseDelegateType(boost::json::value("invalid_type")); + EXPECT_FALSE(result.has_value()); + + result = parseDelegateType(boost::json::value(123)); + EXPECT_FALSE(result.has_value()); + + result = parseDelegateType(boost::json::value(true)); + EXPECT_FALSE(result.has_value()); +} + +TEST_F(RPCHelpersTest, ParseDelegateFilter_Success) +{ + // only delegate agent is valid + { + auto const json = boost::json::parse(R"JSON({ + "delegate_filter": "delegator" + })JSON").as_object(); + + auto const result = parseDelegateFilter(json); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->delegateType, DelegateFilter::Role::Delegator); + EXPECT_FALSE(result->counterParty.has_value()); + } + + // delegate agent + counterparty is valid + { + auto const json = boost::json::parse(R"JSON({ + "delegate_filter": "delegatee", + "counterparty": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun" + })JSON").as_object(); + + auto const result = parseDelegateFilter(json); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->delegateType, DelegateFilter::Role::Delegatee); + ASSERT_TRUE(result->counterParty.has_value()); + EXPECT_EQ(*result->counterParty, "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun"); + } +} + +TEST_F(RPCHelpersTest, ParseDelegateFilter_Failures) +{ + // Missing required "delegate_filter" key + { + auto const json = boost::json::parse(R"JSON({ + "counterparty": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun" + })JSON").as_object(); + EXPECT_FALSE(parseDelegateFilter(json).has_value()); + } + + // "delegate_filter" is not a string (it's an integer) + { + auto const json = boost::json::parse(R"JSON({ + "delegate_filter": 123 + })JSON").as_object(); + EXPECT_FALSE(parseDelegateFilter(json).has_value()); + } + + // "delegate_filter" is a string but invalid value + { + auto const json = boost::json::parse(R"JSON({ + "delegate_filter": "random_string" + })JSON").as_object(); + EXPECT_FALSE(parseDelegateFilter(json).has_value()); + } + + // "counterparty" exists but is not a string (it's a number) + { + auto const json = boost::json::parse(R"JSON({ + "delegate_filter": "delegator", + "counterparty": 9999 + })JSON").as_object(); + EXPECT_FALSE(parseDelegateFilter(json).has_value()); + } +} + + TEST_F(RPCHelpersTest, isGlobalFrozen_AccountIsGlobalFrozen) { auto const account = getAccountIdWithString(kACCOUNT); diff --git a/tests/unit/rpc/handlers/AccountTxTests.cpp b/tests/unit/rpc/handlers/AccountTxTests.cpp index 20c6d9838b..1fe6f19e5a 100644 --- a/tests/unit/rpc/handlers/AccountTxTests.cpp +++ b/tests/unit/rpc/handlers/AccountTxTests.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -398,6 +399,56 @@ struct AccountTxParameterTest : public RPCAccountTxHandlerTest, })JSON", .expectedError = "invalidParams", .expectedErrorMessage = "Invalid field 'tx_type'." + }, + AccountTxParamTestCaseBundle{ + .testName = "DelegateNotObject", + .testJson = R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": 123 + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "delegate not object" + }, + AccountTxParamTestCaseBundle{ + .testName = "DelegateFilterMissing", + .testJson = R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": { "other_field": "value" } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Field 'delegate_filter' is required but missing." + }, + AccountTxParamTestCaseBundle{ + .testName = "DelegateFilterInvalidValue", + .testJson = R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": { "delegate_filter": "invalid_mode" } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Field 'delegate_filter' value must be 'delegator' or 'delegatee'." + }, + AccountTxParamTestCaseBundle{ + .testName = "DelegateCounterpartyInvalid", + .testJson = R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": { + "delegate_filter": "delegatee", + "counterparty": "not_an_account" + } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Field 'counterparty' value must be a valid account." + }, + AccountTxParamTestCaseBundle{ + .testName = "DelegateOnlyCounterparty", + .testJson = R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": { + "counterparty": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn" + } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Field 'delegate_filter' is required but missing." } }; }; @@ -503,6 +554,7 @@ TEST_F(RPCAccountTxHandlerTest, IndexSpecificForwardTrue) testing::_, true, testing::Optional(testing::Eq(TransactionsCursor{kMIN_SEQ, INT32_MAX})), + testing::_, testing::_ ) ); @@ -547,6 +599,7 @@ TEST_F(RPCAccountTxHandlerTest, IndexSpecificForwardFalse) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), + testing::_, testing::_ ) ); @@ -591,6 +644,7 @@ TEST_F(RPCAccountTxHandlerTest, IndexNotSpecificForwardTrue) testing::_, true, testing::Optional(testing::Eq(TransactionsCursor{kMIN_SEQ - 1, INT32_MAX})), + testing::_, testing::_ ) ); @@ -635,6 +689,7 @@ TEST_F(RPCAccountTxHandlerTest, IndexNotSpecificForwardFalse) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), + testing::_, testing::_ ) ); @@ -679,6 +734,7 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrue) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), + testing::_, testing::_ ) ); @@ -734,6 +790,7 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrueV2) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), + testing::_, testing::_ ) ) @@ -786,7 +843,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitAndMarker) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ ) ) .WillOnce(Return(transCursor)); @@ -825,7 +882,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitIsCapped) { auto const transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -859,7 +916,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitAllowedUpToCap) { auto const transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -903,6 +960,7 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerIndex) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), + testing::_, testing::_ ) ); @@ -995,6 +1053,7 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerHash) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), + testing::_, testing::_ ) ); @@ -1041,6 +1100,7 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerIndexValidated) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), + testing::_, testing::_ ) ); @@ -1084,6 +1144,7 @@ TEST_F(RPCAccountTxHandlerTest, TxLessThanMinSeq) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), + testing::_, testing::_ ) ); @@ -1128,6 +1189,7 @@ TEST_F(RPCAccountTxHandlerTest, TxLargerThanMaxSeq) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 2, INT32_MAX})), + testing::_, testing::_ ) ); @@ -1160,6 +1222,104 @@ TEST_F(RPCAccountTxHandlerTest, TxLargerThanMaxSeq) }); } +TEST_F(RPCAccountTxHandlerTest, WithDelegateAgent) +{ + auto transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); + auto const accountId = *ripple::parseBase58(kACCOUNT); + for (auto& txn : transactions) { + txn.delegatedAccount = accountId; + } + + auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; + EXPECT_CALL( + *backend_, + fetchAccountTransactions( + testing::_, + testing::_, + false, + testing::_, + testing::Optional(testing::AllOf( + testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegator), + testing::Field(&DelegateFilter::counterParty, testing::Eq(std::nullopt)) + )), + testing::_ + ) + ).WillOnce(Return(transCursor)); + + ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); + + runSpawn([&, this](auto yield) { + auto const handler = AnyHandler{AccountTxHandler{backend_, mockETLServicePtr_}}; + static auto const kINPUT = json::parse(R"JSON({ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": { + "delegate_filter": "delegator" + } + })JSON"); + + auto const output = handler.process(kINPUT, Context{yield}); + ASSERT_TRUE(output); + + EXPECT_EQ(output.result->at("account").as_string(), kACCOUNT); + auto const& txs = output.result->at("transactions").as_array(); + ASSERT_EQ(txs.size(), 2); + + // Check the transactions contains delegator + EXPECT_TRUE(txs[0].as_object().contains("delegator")); + EXPECT_TRUE(txs[1].as_object().contains("delegator")); + + }); +} + +TEST_F(RPCAccountTxHandlerTest, WithDelegateFromAndCounterparty) +{ + auto transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); + auto const kCOUNTERPARTY = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun"; + auto const counterpartyID = *ripple::parseBase58(kCOUNTERPARTY); + for (auto& txn : transactions) { + txn.delegatedAccount = counterpartyID; + } + + auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; + + EXPECT_CALL( + *backend_, + fetchAccountTransactions( + testing::_, + testing::_, + false, + testing::_, + testing::Optional(testing::AllOf( + testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegatee), + testing::Field(&DelegateFilter::counterParty, testing::Optional(std::string(kCOUNTERPARTY))) + )), + testing::_ + ) + ).WillOnce(Return(transCursor)); + + ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); + + runSpawn([&, this](auto yield) { + auto const handler = AnyHandler{AccountTxHandler{backend_, mockETLServicePtr_}}; + static auto const kINPUT = json::parse(fmt::format(R"JSON({{ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": {{ + "delegate_filter": "delegatee", + "counterparty": "{}" + }} + }})JSON", kCOUNTERPARTY)); + + auto const output = handler.process(kINPUT, Context{yield}); + ASSERT_TRUE(output); + + auto const& txs = output.result->at("transactions").as_array(); + ASSERT_EQ(txs.size(), 2); + + EXPECT_TRUE(txs[0].as_object().contains("delegatee")); + EXPECT_EQ(txs[0].at("delegatee").as_string(), kCOUNTERPARTY); + }); +} + TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v1) { auto const out = R"JSON({ @@ -1344,7 +1504,7 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v1) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ ) ); @@ -1563,7 +1723,7 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v2) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ ) ); @@ -2181,7 +2341,7 @@ TEST_P(AccountTxTransactionTypeTest, SpecificTransactionType) auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; ON_CALL(*backend_, fetchAccountTransactions).WillByDefault(Return(transCursor)); EXPECT_CALL( - *backend_, fetchAccountTransactions(_, _, false, Optional(Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), _) + *backend_, fetchAccountTransactions(_, _, false, Optional(Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), _, _) ); auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, kMAX_SEQ); From 141aa1477f8f03eada85a8623e64d79b39ee5be3 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 1 Dec 2025 16:57:24 -0500 Subject: [PATCH 2/5] fix doxygen --- src/data/BackendInterface.hpp | 1 + src/data/cassandra/CassandraBackendFamily.hpp | 71 +++++++------- src/rpc/RPCHelpers.cpp | 4 +- src/rpc/RPCHelpers.hpp | 1 - src/rpc/common/Types.hpp | 22 ++--- src/rpc/common/Validators.cpp | 38 ++++---- src/rpc/handlers/AccountTx.cpp | 11 ++- src/rpc/handlers/AccountTx.hpp | 6 +- tests/unit/rpc/RPCHelpersTests.cpp | 19 ++-- tests/unit/rpc/handlers/AccountTxTests.cpp | 97 ++++++++++++------- 10 files changed, 149 insertions(+), 121 deletions(-) diff --git a/src/data/BackendInterface.hpp b/src/data/BackendInterface.hpp index 0200fad0b1..f987d66900 100644 --- a/src/data/BackendInterface.hpp +++ b/src/data/BackendInterface.hpp @@ -306,6 +306,7 @@ class BackendInterface { * @param limit The maximum number of transactions per result page * @param forward Whether to fetch the page forwards or backwards from the given cursor * @param txnCursor The cursor to resume fetching from + * @param delegateFilter Delegate filter to restrict results to transactions involving permission delegation * @param yield The coroutine context * @return Results and a cursor to resume from */ diff --git a/src/data/cassandra/CassandraBackendFamily.hpp b/src/data/cassandra/CassandraBackendFamily.hpp index fc0c7978e5..1ee569ccb9 100644 --- a/src/data/cassandra/CassandraBackendFamily.hpp +++ b/src/data/cassandra/CassandraBackendFamily.hpp @@ -218,37 +218,39 @@ class CassandraBackendFamily : public BackendInterface { std::optional filterCounterpartyID; if (delegateFilter && delegateFilter->counterParty) { filterCounterpartyID = ripple::parseBase58(*delegateFilter->counterParty); - - // If the counterparty string is invalid, we don't return anything. Error should have been caught by validators. + + // If the counterparty string is invalid, we don't return anything. Error should have been caught by + // validators. if (!filterCounterpartyID) { - LOG(log_.warn()) << "Invalid counterparty account in filter"; - return {.txns={}, .cursor={}}; + LOG(log_.warn()) << "Invalid counterparty account in filter"; + return {.txns = {}, .cursor = {}}; } } std::vector resultTxns; if (delegateFilter.has_value()) { resultTxns.reserve(txns.size()); - - for (auto& txn : txns) { + + for (auto& txn : txns) { try { - auto const delegationInfo = getDelegationInfo(txn.transaction, txn.metadata); + auto const delegationInfo = getDelegationInfo(txn.transaction); if (delegationInfo) { auto const& [delegatee, delegator] = *delegationInfo; bool match = false; - + // Filter by "Delegator" ie. User wants to find the Owner (Delegator). - // This implies the User (account) must be the Signer (Delegatee) that acted on someone's behalf. + // This implies the User (account) must be the Signer (Delegatee) that acted on someone's + // behalf. if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { // The user (account) must be delegatee if (account == delegatee) { if (!filterCounterpartyID || *filterCounterpartyID == delegator) { - txn.delegatedAccount = delegator; + txn.delegatedAccount = delegator; match = true; } } - } + } // Filter by "Delegatee" ie. User wants to find the Signer (Delegatee). // This implies the User (account) must be the Owner (Delegator). else if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { @@ -260,16 +262,15 @@ class CassandraBackendFamily : public BackendInterface { } } } - - if (match) + + if (match) resultTxns.push_back(txn); - } } catch (std::exception const& e) { LOG(log_.warn()) << "Failed to parse tx for filter"; } } - return {.txns=resultTxns, .cursor=cursor}; + return {.txns = resultTxns, .cursor = cursor}; } if (txns.size() == limit) { @@ -1037,44 +1038,38 @@ class CassandraBackendFamily : public BackendInterface { return true; } -/** - * @brief Extracts delegation information from a transaction. - * - * Parses the transaction blob and checks whether the signer - * (derived from the SigningPubKey) differs from the delegator - * account. If so, returns {delegatee, delegator}. Otherwise returns null. - * - * @param txnBlob Serialized transaction blob. - * @param metaBlob Unused metadata blob fetched from rippled. - * @return pair of {delegatee, delegator} if delegated, otherwise std::nullopt - */ -static std::optional> -getDelegationInfo(ripple::Blob const& txnBlob, ripple::Blob const& /*metaBlob*/) -{ - + /** + * @brief Extracts delegation information from a transaction. + * + * Parses the transaction blob and checks whether the signer + * (derived from the SigningPubKey) differs from the delegator + * account. If so, returns {delegatee, delegator}. Otherwise returns null. + * + * @param txnBlob Serialized transaction blob. + * @return pair of {delegatee, delegator} if delegated, otherwise std::nullopt + */ + static std::optional> + getDelegationInfo(ripple::Blob const& txnBlob) + { ripple::SerialIter it{txnBlob.data(), txnBlob.size()}; ripple::STTx const txn{it}; auto const delegator = txn.getAccountID(ripple::sfAccount); - if (txn.isFieldPresent(ripple::sfSigningPubKey)) - { + if (txn.isFieldPresent(ripple::sfSigningPubKey)) { auto const pubKeyBlob = txn.getFieldVL(ripple::sfSigningPubKey); ripple::PublicKey const pubKey{ripple::Slice{pubKeyBlob.data(), pubKeyBlob.size()}}; - + auto const delegatee = ripple::calcAccountID(pubKey); // Delegation Check // If the signer (delegatee) is NOT the account owner (delegator), it's delegated. - if (delegatee != delegator) - { + if (delegatee != delegator) { return std::make_pair(delegatee, delegator); } } return std::nullopt; - -} - + } }; } // namespace data::cassandra diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index 933b2190f3..ed3e85ea81 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -1601,10 +1601,10 @@ parseDelegateFilter(boost::json::object const& delegateObject) delegate.delegateType = *delegateTypeOpt; if (delegateObject.contains("counterparty")) { auto const& counterpartyVal = delegateObject.at("counterparty"); - + if (!counterpartyVal.is_string()) return {}; - + delegate.counterParty = counterpartyVal.as_string(); } diff --git a/src/rpc/RPCHelpers.hpp b/src/rpc/RPCHelpers.hpp index a339cd2f5f..9f8524c85c 100644 --- a/src/rpc/RPCHelpers.hpp +++ b/src/rpc/RPCHelpers.hpp @@ -879,5 +879,4 @@ parseDelegateType(boost::json::value const& delegateType); std::optional parseDelegateFilter(boost::json::object const& delegateObject); - } // namespace rpc diff --git a/src/rpc/common/Types.hpp b/src/rpc/common/Types.hpp index 203c0730ad..7cb15c17c6 100644 --- a/src/rpc/common/Types.hpp +++ b/src/rpc/common/Types.hpp @@ -198,18 +198,18 @@ struct AccountCursor { * @brief A delegate object used filter account_tx by specific delegate accounts */ struct DelegateFilter { - /** - * @brief A delegate type used in delegate filter - */ + /** + * @brief A delegate type used in delegate filter + */ enum class Role { - // This account is the *active* sender, acting on behalf of another party. - // e.g., Account A in "A sends payment to B on behalf of C." - Delegatee, - - // This account is the *passive* party whose funds are being moved from. - // e.g., Account C in "A sends payment to B on behalf of C." - Delegator - }; + // This account is the *active* sender, acting on behalf of another party. + // e.g., Account A in "A sends payment to B on behalf of C." + Delegatee, + + // This account is the *passive* party whose funds are being moved from. + // e.g., Account C in "A sends payment to B on behalf of C." + Delegator + }; Role delegateType; std::optional counterParty; diff --git a/src/rpc/common/Validators.cpp b/src/rpc/common/Validators.cpp index 4627938eae..f67973b355 100644 --- a/src/rpc/common/Validators.cpp +++ b/src/rpc/common/Validators.cpp @@ -357,22 +357,26 @@ CustomValidator CustomValidators::authorizeCredentialValidator = return MaybeError{}; }}; - CustomValidator CustomValidators::delegateValidator = - CustomValidator{[](boost::json::value const& value, std::string_view key) -> MaybeError { - if (!value.is_object()) - return Error{Status{RippledError::rpcINVALID_PARAMS, std::string(key) + " not object"}}; - - auto const& delegate = value.as_object(); - if (!delegate.contains("delegate_filter")) - return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' is required but missing."}}; - - if (!parseDelegateType(delegate.at("delegate_filter")).has_value()) - return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' value must be 'delegator' or 'delegatee'."}}; - - if (delegate.contains("counterparty") && !accountValidator.verify(delegate, "counterparty")) - return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'counterparty' value must be a valid account."}}; - - return MaybeError{}; - }}; +CustomValidator CustomValidators::delegateValidator = + CustomValidator{[](boost::json::value const& value, std::string_view key) -> MaybeError { + if (!value.is_object()) + return Error{Status{RippledError::rpcINVALID_PARAMS, std::string(key) + " not object"}}; + + auto const& delegate = value.as_object(); + if (!delegate.contains("delegate_filter")) + return Error{Status{RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' is required but missing."}}; + + if (!parseDelegateType(delegate.at("delegate_filter")).has_value()) + return Error{Status{ + RippledError::rpcINVALID_PARAMS, "Field 'delegate_filter' value must be 'delegator' or 'delegatee'." + }}; + + if (delegate.contains("counterparty") && !accountValidator.verify(delegate, "counterparty")) + return Error{ + Status{RippledError::rpcINVALID_PARAMS, "Field 'counterparty' value must be a valid account."} + }; + + return MaybeError{}; + }}; } // namespace rpc::validation diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index 16edbd5ed5..a481785149 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -122,7 +122,9 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c auto const limit = input.limit.value_or(kLIMIT_DEFAULT); auto const accountID = accountFromStringStrict(input.account); auto const [txnsAndCursor, timeDiff] = util::timed([&]() { - return sharedPtrBackend_->fetchAccountTransactions(*accountID, limit, input.forward, cursor, input.delegateFilter,ctx.yield); + return sharedPtrBackend_->fetchAccountTransactions( + *accountID, limit, input.forward, cursor, input.delegateFilter, ctx.yield + ); }); LOG(log_.info()) << "db fetch took " << timeDiff << " milliseconds - num blobs = " << txnsAndCursor.txns.size(); @@ -197,14 +199,13 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { // filtering by the txns where other accounts sent txns for this delegatedAccount obj["delegator"] = to_string(*txnPlusMeta.delegatedAccount); - } - else if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { + } else if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { // filtering by the txns where this delegatedAccount sent txn on behalf of other users obj["delegatee"] = to_string(*txnPlusMeta.delegatedAccount); } } } - + obj[JS(validated)] = true; response.transactions.push_back(std::move(obj)); continue; @@ -300,7 +301,7 @@ tag_invoke(boost::json::value_to_tag, boost::json::valu if (jsonObject.contains("tx_type")) input.transactionTypeInLowercase = boost::json::value_to(jsonObject.at("tx_type")); - if (jsonObject.contains("delegate")){ + if (jsonObject.contains("delegate")) { input.delegateFilter = parseDelegateFilter(jsonObject.at("delegate").as_object()); } diff --git a/src/rpc/handlers/AccountTx.hpp b/src/rpc/handlers/AccountTx.hpp index 9bea51a85d..1815c35256 100644 --- a/src/rpc/handlers/AccountTx.hpp +++ b/src/rpc/handlers/AccountTx.hpp @@ -157,10 +157,8 @@ class AccountTxHandler { validation::Type{}, modifiers::ToLower{}, validation::OneOf(typesKeysInLowercase.cbegin(), typesKeysInLowercase.cend()), - }, { - "delegate", - validation::CustomValidators::delegateValidator - } + }, + {"delegate", validation::CustomValidators::delegateValidator} }; static auto const kRPC_SPEC = RpcSpec{ diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp index 02fc40fa5d..c3c5b76677 100644 --- a/tests/unit/rpc/RPCHelpersTests.cpp +++ b/tests/unit/rpc/RPCHelpersTests.cpp @@ -630,7 +630,8 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Success) { auto const json = boost::json::parse(R"JSON({ "delegate_filter": "delegator" - })JSON").as_object(); + })JSON") + .as_object(); auto const result = parseDelegateFilter(json); ASSERT_TRUE(result.has_value()); @@ -643,7 +644,8 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Success) auto const json = boost::json::parse(R"JSON({ "delegate_filter": "delegatee", "counterparty": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun" - })JSON").as_object(); + })JSON") + .as_object(); auto const result = parseDelegateFilter(json); ASSERT_TRUE(result.has_value()); @@ -659,7 +661,8 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Failures) { auto const json = boost::json::parse(R"JSON({ "counterparty": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun" - })JSON").as_object(); + })JSON") + .as_object(); EXPECT_FALSE(parseDelegateFilter(json).has_value()); } @@ -667,7 +670,8 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Failures) { auto const json = boost::json::parse(R"JSON({ "delegate_filter": 123 - })JSON").as_object(); + })JSON") + .as_object(); EXPECT_FALSE(parseDelegateFilter(json).has_value()); } @@ -675,7 +679,8 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Failures) { auto const json = boost::json::parse(R"JSON({ "delegate_filter": "random_string" - })JSON").as_object(); + })JSON") + .as_object(); EXPECT_FALSE(parseDelegateFilter(json).has_value()); } @@ -684,12 +689,12 @@ TEST_F(RPCHelpersTest, ParseDelegateFilter_Failures) auto const json = boost::json::parse(R"JSON({ "delegate_filter": "delegator", "counterparty": 9999 - })JSON").as_object(); + })JSON") + .as_object(); EXPECT_FALSE(parseDelegateFilter(json).has_value()); } } - TEST_F(RPCHelpersTest, isGlobalFrozen_AccountIsGlobalFrozen) { auto const account = getAccountIdWithString(kACCOUNT); diff --git a/tests/unit/rpc/handlers/AccountTxTests.cpp b/tests/unit/rpc/handlers/AccountTxTests.cpp index 1fe6f19e5a..0dd5983e8f 100644 --- a/tests/unit/rpc/handlers/AccountTxTests.cpp +++ b/tests/unit/rpc/handlers/AccountTxTests.cpp @@ -431,7 +431,7 @@ struct AccountTxParameterTest : public RPCAccountTxHandlerTest, .testName = "DelegateCounterpartyInvalid", .testJson = R"JSON({ "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", - "delegate": { + "delegate": { "delegate_filter": "delegatee", "counterparty": "not_an_account" } @@ -443,7 +443,7 @@ struct AccountTxParameterTest : public RPCAccountTxHandlerTest, .testName = "DelegateOnlyCounterparty", .testJson = R"JSON({ "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", - "delegate": { + "delegate": { "counterparty": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn" } })JSON", @@ -843,7 +843,12 @@ TEST_F(RPCAccountTxHandlerTest, LimitAndMarker) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ + testing::_, + testing::_, + false, + testing::Optional(testing::Eq(TransactionsCursor{10, 11})), + testing::_, + testing::_ ) ) .WillOnce(Return(transCursor)); @@ -1234,17 +1239,20 @@ TEST_F(RPCAccountTxHandlerTest, WithDelegateAgent) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, - testing::_, - false, testing::_, - testing::Optional(testing::AllOf( - testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegator), - testing::Field(&DelegateFilter::counterParty, testing::Eq(std::nullopt)) - )), - testing::_ + testing::_, + false, + testing::_, + testing::Optional( + testing::AllOf( + testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegator), + testing::Field(&DelegateFilter::counterParty, testing::Eq(std::nullopt)) + ) + ), + testing::_ ) - ).WillOnce(Return(transCursor)); + ) + .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -1256,18 +1264,17 @@ TEST_F(RPCAccountTxHandlerTest, WithDelegateAgent) "delegate_filter": "delegator" } })JSON"); - + auto const output = handler.process(kINPUT, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(output.result->at("account").as_string(), kACCOUNT); auto const& txs = output.result->at("transactions").as_array(); ASSERT_EQ(txs.size(), 2); - + // Check the transactions contains delegator EXPECT_TRUE(txs[0].as_object().contains("delegator")); EXPECT_TRUE(txs[1].as_object().contains("delegator")); - }); } @@ -1285,31 +1292,39 @@ TEST_F(RPCAccountTxHandlerTest, WithDelegateFromAndCounterparty) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::_, - testing::Optional(testing::AllOf( - testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegatee), - testing::Field(&DelegateFilter::counterParty, testing::Optional(std::string(kCOUNTERPARTY))) - )), - testing::_ + testing::_, + testing::_, + false, + testing::_, + testing::Optional( + testing::AllOf( + testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegatee), + testing::Field(&DelegateFilter::counterParty, testing::Optional(std::string(kCOUNTERPARTY))) + ) + ), + testing::_ ) - ).WillOnce(Return(transCursor)); + ) + .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); runSpawn([&, this](auto yield) { auto const handler = AnyHandler{AccountTxHandler{backend_, mockETLServicePtr_}}; - static auto const kINPUT = json::parse(fmt::format(R"JSON({{ - "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", - "delegate": {{ - "delegate_filter": "delegatee", - "counterparty": "{}" - }} - }})JSON", kCOUNTERPARTY)); - - auto const output = handler.process(kINPUT, Context{yield}); + static auto const kINPUT = json::parse( + fmt::format( + R"JSON({{ + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "delegate": {{ + "delegate_filter": "delegatee", + "counterparty": "{}" + }} + }})JSON", + kCOUNTERPARTY + ) + ); + + auto const output = handler.process(kINPUT, Context{yield}); ASSERT_TRUE(output); auto const& txs = output.result->at("transactions").as_array(); @@ -1504,7 +1519,12 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v1) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ + testing::_, + testing::_, + false, + testing::Optional(testing::Eq(TransactionsCursor{10, 11})), + testing::_, + testing::_ ) ); @@ -1723,7 +1743,12 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v2) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_, testing::_ + testing::_, + testing::_, + false, + testing::Optional(testing::Eq(TransactionsCursor{10, 11})), + testing::_, + testing::_ ) ); From 18bcdf3d674fdfd9dd7802c9b9f6d91d0b448f50 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 2 Dec 2025 11:11:09 -0500 Subject: [PATCH 3/5] remove try catch --- src/data/cassandra/CassandraBackendFamily.hpp | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/data/cassandra/CassandraBackendFamily.hpp b/src/data/cassandra/CassandraBackendFamily.hpp index 1ee569ccb9..9cbbec1a80 100644 --- a/src/data/cassandra/CassandraBackendFamily.hpp +++ b/src/data/cassandra/CassandraBackendFamily.hpp @@ -232,42 +232,38 @@ class CassandraBackendFamily : public BackendInterface { resultTxns.reserve(txns.size()); for (auto& txn : txns) { - try { - auto const delegationInfo = getDelegationInfo(txn.transaction); - - if (delegationInfo) { - auto const& [delegatee, delegator] = *delegationInfo; - bool match = false; - - // Filter by "Delegator" ie. User wants to find the Owner (Delegator). - // This implies the User (account) must be the Signer (Delegatee) that acted on someone's - // behalf. - if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { - // The user (account) must be delegatee - if (account == delegatee) { - if (!filterCounterpartyID || *filterCounterpartyID == delegator) { - txn.delegatedAccount = delegator; - match = true; - } + auto const delegationInfo = getDelegationInfo(txn.transaction); + + if (delegationInfo) { + auto const& [delegatee, delegator] = *delegationInfo; + bool match = false; + + // Filter by "Delegator" ie. User wants to find the Owner (Delegator). + // This implies the User (account) must be the Signer (Delegatee) that acted on someone's + // behalf. + if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { + // The user (account) must be delegatee + if (account == delegatee) { + if (!filterCounterpartyID || *filterCounterpartyID == delegator) { + txn.delegatedAccount = delegator; + match = true; } } - // Filter by "Delegatee" ie. User wants to find the Signer (Delegatee). - // This implies the User (account) must be the Owner (Delegator). - else if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { - // The user (account) must be delegator - if (account == delegator) { - if (!filterCounterpartyID || *filterCounterpartyID == delegatee) { - txn.delegatedAccount = delegatee; - match = true; - } + } + // Filter by "Delegatee" ie. User wants to find the Signer (Delegatee). + // This implies the User (account) must be the Owner (Delegator). + else if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { + // The user (account) must be delegator + if (account == delegator) { + if (!filterCounterpartyID || *filterCounterpartyID == delegatee) { + txn.delegatedAccount = delegatee; + match = true; } } - - if (match) - resultTxns.push_back(txn); } - } catch (std::exception const& e) { - LOG(log_.warn()) << "Failed to parse tx for filter"; + + if (match) + resultTxns.push_back(txn); } } return {.txns = resultTxns, .cursor = cursor}; From 55f596195bd25762a2bd3788dbaa43c5ab2bf3ce Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Fri, 5 Dec 2025 13:27:14 -0500 Subject: [PATCH 4/5] decouple filtering --- src/data/BackendInterface.hpp | 3 - src/data/cassandra/CassandraBackendFamily.hpp | 92 +------- src/rpc/CMakeLists.txt | 1 + src/rpc/common/Types.hpp | 10 +- src/rpc/filters/TransactionFilter.hpp | 54 +++++ .../impl/DelegateTransactionsFilter.cpp | 84 ++++++++ .../impl/DelegateTransactionsFilter.hpp | 53 +++++ src/rpc/handlers/AccountTx.cpp | 34 +-- tests/common/util/MockBackend.hpp | 2 - tests/common/util/TestObject.cpp | 26 +++ tests/common/util/TestObject.hpp | 3 + .../data/cassandra/BackendTests.cpp | 4 +- tests/unit/CMakeLists.txt | 1 + .../impl/DelegateTransactionsFilterTests.cpp | 196 ++++++++++++++++++ tests/unit/rpc/handlers/AccountTxTests.cpp | 82 ++------ 15 files changed, 460 insertions(+), 185 deletions(-) create mode 100644 src/rpc/filters/TransactionFilter.hpp create mode 100644 src/rpc/filters/impl/DelegateTransactionsFilter.cpp create mode 100644 src/rpc/filters/impl/DelegateTransactionsFilter.hpp create mode 100644 tests/unit/rpc/filters/impl/DelegateTransactionsFilterTests.cpp diff --git a/src/data/BackendInterface.hpp b/src/data/BackendInterface.hpp index f987d66900..7a823c76a3 100644 --- a/src/data/BackendInterface.hpp +++ b/src/data/BackendInterface.hpp @@ -23,7 +23,6 @@ #include "data/LedgerCacheInterface.hpp" #include "data/Types.hpp" #include "etl/CorruptionDetector.hpp" -#include "rpc/common/Types.hpp" #include "util/Spawn.hpp" #include "util/log/Logger.hpp" @@ -306,7 +305,6 @@ class BackendInterface { * @param limit The maximum number of transactions per result page * @param forward Whether to fetch the page forwards or backwards from the given cursor * @param txnCursor The cursor to resume fetching from - * @param delegateFilter Delegate filter to restrict results to transactions involving permission delegation * @param yield The coroutine context * @return Results and a cursor to resume from */ @@ -316,7 +314,6 @@ class BackendInterface { std::uint32_t limit, bool forward, std::optional const& txnCursor, - std::optional const& delegateFilter, boost::asio::yield_context yield ) const = 0; diff --git a/src/data/cassandra/CassandraBackendFamily.hpp b/src/data/cassandra/CassandraBackendFamily.hpp index 9cbbec1a80..12cf9845f8 100644 --- a/src/data/cassandra/CassandraBackendFamily.hpp +++ b/src/data/cassandra/CassandraBackendFamily.hpp @@ -28,7 +28,6 @@ #include "data/cassandra/Handle.hpp" #include "data/cassandra/Types.hpp" #include "data/cassandra/impl/ExecutionStrategy.hpp" -#include "rpc/common/Types.hpp" #include "util/Assert.hpp" #include "util/LedgerUtils.hpp" #include "util/Profiler.hpp" @@ -59,7 +58,6 @@ #include #include #include -#include #include #include #include @@ -160,7 +158,6 @@ class CassandraBackendFamily : public BackendInterface { std::uint32_t const limit, bool forward, std::optional const& txnCursor, - std::optional const& delegateFilter, boost::asio::yield_context yield ) const override { @@ -212,63 +209,9 @@ class CassandraBackendFamily : public BackendInterface { } } - auto txns = fetchTransactions(hashes, yield); + auto const txns = fetchTransactions(hashes, yield); LOG(log_.debug()) << "Txns = " << txns.size(); - std::optional filterCounterpartyID; - if (delegateFilter && delegateFilter->counterParty) { - filterCounterpartyID = ripple::parseBase58(*delegateFilter->counterParty); - - // If the counterparty string is invalid, we don't return anything. Error should have been caught by - // validators. - if (!filterCounterpartyID) { - LOG(log_.warn()) << "Invalid counterparty account in filter"; - return {.txns = {}, .cursor = {}}; - } - } - - std::vector resultTxns; - if (delegateFilter.has_value()) { - resultTxns.reserve(txns.size()); - - for (auto& txn : txns) { - auto const delegationInfo = getDelegationInfo(txn.transaction); - - if (delegationInfo) { - auto const& [delegatee, delegator] = *delegationInfo; - bool match = false; - - // Filter by "Delegator" ie. User wants to find the Owner (Delegator). - // This implies the User (account) must be the Signer (Delegatee) that acted on someone's - // behalf. - if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { - // The user (account) must be delegatee - if (account == delegatee) { - if (!filterCounterpartyID || *filterCounterpartyID == delegator) { - txn.delegatedAccount = delegator; - match = true; - } - } - } - // Filter by "Delegatee" ie. User wants to find the Signer (Delegatee). - // This implies the User (account) must be the Owner (Delegator). - else if (delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { - // The user (account) must be delegator - if (account == delegator) { - if (!filterCounterpartyID || *filterCounterpartyID == delegatee) { - txn.delegatedAccount = delegatee; - match = true; - } - } - } - - if (match) - resultTxns.push_back(txn); - } - } - return {.txns = resultTxns, .cursor = cursor}; - } - if (txns.size() == limit) { LOG(log_.debug()) << "Returning cursor"; return {txns, cursor}; @@ -1033,39 +976,6 @@ class CassandraBackendFamily : public BackendInterface { return true; } - - /** - * @brief Extracts delegation information from a transaction. - * - * Parses the transaction blob and checks whether the signer - * (derived from the SigningPubKey) differs from the delegator - * account. If so, returns {delegatee, delegator}. Otherwise returns null. - * - * @param txnBlob Serialized transaction blob. - * @return pair of {delegatee, delegator} if delegated, otherwise std::nullopt - */ - static std::optional> - getDelegationInfo(ripple::Blob const& txnBlob) - { - ripple::SerialIter it{txnBlob.data(), txnBlob.size()}; - ripple::STTx const txn{it}; - - auto const delegator = txn.getAccountID(ripple::sfAccount); - if (txn.isFieldPresent(ripple::sfSigningPubKey)) { - auto const pubKeyBlob = txn.getFieldVL(ripple::sfSigningPubKey); - ripple::PublicKey const pubKey{ripple::Slice{pubKeyBlob.data(), pubKeyBlob.size()}}; - - auto const delegatee = ripple::calcAccountID(pubKey); - - // Delegation Check - // If the signer (delegatee) is NOT the account owner (delegator), it's delegated. - if (delegatee != delegator) { - return std::make_pair(delegatee, delegator); - } - } - - return std::nullopt; - } }; } // namespace data::cassandra diff --git a/src/rpc/CMakeLists.txt b/src/rpc/CMakeLists.txt index f06752fc01..d800ea76c7 100644 --- a/src/rpc/CMakeLists.txt +++ b/src/rpc/CMakeLists.txt @@ -20,6 +20,7 @@ target_sources( common/MetaProcessors.cpp common/impl/APIVersionParser.cpp common/impl/HandlerProvider.cpp + filters/impl/DelegateTransactionsFilter.cpp handlers/AccountChannels.cpp handlers/AccountCurrencies.cpp handlers/AccountInfo.cpp diff --git a/src/rpc/common/Types.hpp b/src/rpc/common/Types.hpp index 7cb15c17c6..bbfa19dc8b 100644 --- a/src/rpc/common/Types.hpp +++ b/src/rpc/common/Types.hpp @@ -202,13 +202,11 @@ struct DelegateFilter { * @brief A delegate type used in delegate filter */ enum class Role { - // This account is the *active* sender, acting on behalf of another party. - // e.g., Account A in "A sends payment to B on behalf of C." - Delegatee, + Delegatee, /**< This account is the *active* sender, acting on behalf of another party. + * e.g., Account A in "A sends payment to B on behalf of C." */ - // This account is the *passive* party whose funds are being moved from. - // e.g., Account C in "A sends payment to B on behalf of C." - Delegator + Delegator /**< This account is the *passive* party whose funds are being moved from. + * e.g., Account C in "A sends payment to B on behalf of C." */ }; Role delegateType; diff --git a/src/rpc/filters/TransactionFilter.hpp b/src/rpc/filters/TransactionFilter.hpp new file mode 100644 index 0000000000..1872ec1431 --- /dev/null +++ b/src/rpc/filters/TransactionFilter.hpp @@ -0,0 +1,54 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include "data/Types.hpp" + +#include + +#include + +namespace rpc { + +/** + * @brief Result of a filter check. + */ +struct FilterResult { + bool shouldInclude; + std::optional relevantAccount; +}; + +/** + * @brief Interface for filtering transactions. + */ +class TransactionFilter { +public: + virtual ~TransactionFilter() = default; + + /** + * @brief Check if a transaction blob matches the filter criteria. + * @param txnPlusMeta The transaction and metadata blob from the backend. + * @return FilterResult indicating if the txn should be included in the output Json or not + */ + [[nodiscard]] virtual FilterResult + check(data::TransactionAndMetadata const& txnPlusMeta) const = 0; +}; + +} // namespace rpc diff --git a/src/rpc/filters/impl/DelegateTransactionsFilter.cpp b/src/rpc/filters/impl/DelegateTransactionsFilter.cpp new file mode 100644 index 0000000000..7160c0e275 --- /dev/null +++ b/src/rpc/filters/impl/DelegateTransactionsFilter.cpp @@ -0,0 +1,84 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "rpc/filters/impl/DelegateTransactionsFilter.hpp" + +#include "data/Types.hpp" +#include "rpc/common/Types.hpp" +#include "rpc/filters/TransactionFilter.hpp" + +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace rpc { + +DelegateTransactionFilter::DelegateTransactionFilter(rpc::DelegateFilter filter, ripple::AccountID queriedAccount) + : delegateFilter_(std::move(filter)), queriedAccount_(queriedAccount) +{ + if (delegateFilter_.counterParty) + counterparty_ = ripple::parseBase58(*delegateFilter_.counterParty); +} + +FilterResult +DelegateTransactionFilter::check(data::TransactionAndMetadata const& txnPlusMeta) const +{ + ripple::SerialIter sit{txnPlusMeta.transaction.data(), txnPlusMeta.transaction.size()}; + ripple::STTx const sttx{sit}; + + // The account where the funds are withdrawn is always delegator + auto const txAccount = sttx.getAccountID(ripple::sfAccount); + + std::optional txDelegate; + if (sttx.isFieldPresent(ripple::sfDelegate)) + txDelegate = sttx.getAccountID(ripple::sfDelegate); + + // txn with no delegate filter should return immediately + // Note: should already have been checked in handler code before calling this function though + if (not txDelegate.has_value()) + return {.shouldInclude = false, .relevantAccount = std::nullopt}; + + // Filter by "Delegator" ie. User wants to find the Owner. + // This implies the user must be the Delegatee that acted on someone's behalf. + if (delegateFilter_.delegateType == rpc::DelegateFilter::Role::Delegator) { + if (*txDelegate == queriedAccount_) { + if (!counterparty_ || *counterparty_ == txAccount) + return {.shouldInclude = true, .relevantAccount = txAccount}; + } + } + + // Filter by "Delegatee" ie. User wants to find the Signer who acted on behalf of the user. + // This implies the user must be the delegator. + else if (delegateFilter_.delegateType == rpc::DelegateFilter::Role::Delegatee) { + if (txAccount == queriedAccount_) { + if (!counterparty_ || *counterparty_ == *txDelegate) + return {.shouldInclude = true, .relevantAccount = txDelegate}; + } + } + + return {.shouldInclude = false, .relevantAccount = std::nullopt}; +} + +} // namespace rpc diff --git a/src/rpc/filters/impl/DelegateTransactionsFilter.hpp b/src/rpc/filters/impl/DelegateTransactionsFilter.hpp new file mode 100644 index 0000000000..9224544529 --- /dev/null +++ b/src/rpc/filters/impl/DelegateTransactionsFilter.hpp @@ -0,0 +1,53 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include "data/Types.hpp" +#include "rpc/common/Types.hpp" +#include "rpc/filters/TransactionFilter.hpp" + +#include + +#include + +namespace rpc { + +/** + * @brief Delegate transaction filter to filter txn based on permission delegate + */ +class DelegateTransactionFilter : public TransactionFilter { +public: + /** + * @brief Construct a new delegate transaction filter + * @param params The filter parameters from the JSON request (role, counterparty string) + * @param queriedAccount The account currently being queried in account_tx (input from account_tx handler) + */ + DelegateTransactionFilter(rpc::DelegateFilter filter, ripple::AccountID queriedAccount); + + FilterResult + check(data::TransactionAndMetadata const& txnPlusMeta) const override; + +private: + rpc::DelegateFilter delegateFilter_; + ripple::AccountID queriedAccount_; + std::optional counterparty_; +}; + +} // namespace rpc diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index a481785149..34a0d7c98d 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -25,6 +25,7 @@ #include "rpc/RPCHelpers.hpp" #include "rpc/common/JsonBool.hpp" #include "rpc/common/Types.hpp" +#include "rpc/filters/impl/DelegateTransactionsFilter.hpp" #include "util/Assert.hpp" #include "util/JsonUtils.hpp" #include "util/Profiler.hpp" @@ -119,12 +120,16 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c } } + std::optional txFilter; + if (input.delegateFilter) { + auto const accountID = accountFromStringStrict(input.account); + txFilter.emplace(*input.delegateFilter, *accountID); + } + auto const limit = input.limit.value_or(kLIMIT_DEFAULT); auto const accountID = accountFromStringStrict(input.account); auto const [txnsAndCursor, timeDiff] = util::timed([&]() { - return sharedPtrBackend_->fetchAccountTransactions( - *accountID, limit, input.forward, cursor, input.delegateFilter, ctx.yield - ); + return sharedPtrBackend_->fetchAccountTransactions(*accountID, limit, input.forward, cursor, ctx.yield); }); LOG(log_.info()) << "db fetch took " << timeDiff << " milliseconds - num blobs = " << txnsAndCursor.txns.size(); @@ -147,6 +152,15 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c continue; } + std::optional relevantAccount; + if (txFilter) { + auto const result = txFilter->check(txnPlusMeta); + if (not result.shouldInclude) + continue; + + relevantAccount = result.relevantAccount; + } + boost::json::object obj; // if binary is false or transactionType is specified, we need to expand the transaction @@ -194,15 +208,11 @@ AccountTxHandler::process(AccountTxHandler::Input const& input, Context const& c } } - if (txnPlusMeta.delegatedAccount.has_value()) { - if (input.delegateFilter) { - if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { - // filtering by the txns where other accounts sent txns for this delegatedAccount - obj["delegator"] = to_string(*txnPlusMeta.delegatedAccount); - } else if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegatee) { - // filtering by the txns where this delegatedAccount sent txn on behalf of other users - obj["delegatee"] = to_string(*txnPlusMeta.delegatedAccount); - } + if (relevantAccount) { + if (input.delegateFilter->delegateType == rpc::DelegateFilter::Role::Delegator) { + obj["delegator"] = ripple::to_string(*relevantAccount); + } else { + obj["delegatee"] = ripple::to_string(*relevantAccount); } } diff --git a/tests/common/util/MockBackend.hpp b/tests/common/util/MockBackend.hpp index 100442add1..a6447d5da5 100644 --- a/tests/common/util/MockBackend.hpp +++ b/tests/common/util/MockBackend.hpp @@ -23,7 +23,6 @@ #include "data/DBHelpers.hpp" #include "data/LedgerCache.hpp" #include "data/Types.hpp" -#include "rpc/common/Types.hpp" #include "util/config/ConfigDefinition.hpp" #include @@ -87,7 +86,6 @@ struct MockBackend : public BackendInterface { std::uint32_t const, bool, std::optional const&, - std::optional const&, boost::asio::yield_context), (const, override) ); diff --git a/tests/common/util/TestObject.cpp b/tests/common/util/TestObject.cpp index 7f28a97569..d2d6033f08 100644 --- a/tests/common/util/TestObject.cpp +++ b/tests/common/util/TestObject.cpp @@ -43,7 +43,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -1797,3 +1799,27 @@ createVault( return vault; } + +ripple::Blob +createDelegateBlob(std::string_view owner, std::string_view delegate) +{ + ripple::STObject obj(ripple::sfTransaction); + obj.setFieldU16(ripple::sfTransactionType, ripple::ttPAYMENT); + + if (auto const acc = ripple::parseBase58(std::string(owner))) { + obj.setAccountID(ripple::sfAccount, *acc); + obj.setAccountID(ripple::sfDestination, *acc); + } + if (auto const acc = ripple::parseBase58(std::string(delegate))) + obj.setAccountID(ripple::sfDelegate, *acc); + + obj.setFieldAmount(ripple::sfAmount, ripple::STAmount(100)); + obj.setFieldAmount(ripple::sfFee, ripple::STAmount(10)); + obj.setFieldU32(ripple::sfSequence, 1); + obj.setFieldVL(ripple::sfSigningPubKey, ripple::Slice(nullptr, 0)); + + ripple::STTx tx(std::move(obj)); + ripple::Serializer s; + tx.add(s); + return s.getData(); +} diff --git a/tests/common/util/TestObject.hpp b/tests/common/util/TestObject.hpp index fd811c59ab..3145253acd 100644 --- a/tests/common/util/TestObject.hpp +++ b/tests/common/util/TestObject.hpp @@ -580,3 +580,6 @@ createVault( ripple::uint256 previousTxId, uint32_t previousTxSeq ); + +[[nodiscard]] ripple::Blob +createDelegateBlob(std::string_view owner, std::string_view delegate); diff --git a/tests/integration/data/cassandra/BackendTests.cpp b/tests/integration/data/cassandra/BackendTests.cpp index 6ca004a305..ff889f043c 100644 --- a/tests/integration/data/cassandra/BackendTests.cpp +++ b/tests/integration/data/cassandra/BackendTests.cpp @@ -481,7 +481,7 @@ TEST_F(BackendCassandraTest, Basic) EXPECT_EQ(hashes.size(), 1); EXPECT_EQ(ripple::strHex(hashes[0]), hashHex); for (auto& a : affectedAccounts) { - auto [accountTransactions, cursor] = backend_->fetchAccountTransactions(a, 100, true, {}, {}, yield); + auto [accountTransactions, cursor] = backend_->fetchAccountTransactions(a, 100, true, {}, yield); EXPECT_EQ(accountTransactions.size(), 1); EXPECT_EQ(accountTransactions[0], accountTransactions[0]); EXPECT_FALSE(cursor); @@ -730,7 +730,7 @@ TEST_F(BackendCassandraTest, Basic) do { uint32_t const limit = 10; auto [accountTransactions, retCursor] = - backend_->fetchAccountTransactions(account, limit, false, cursor, {}, yield); + backend_->fetchAccountTransactions(account, limit, false, cursor, yield); if (retCursor) EXPECT_EQ(accountTransactions.size(), limit); retData.insert(retData.end(), accountTransactions.begin(), accountTransactions.end()); diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index cff464b7ac..01e287ab99 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -90,6 +90,7 @@ target_sources( rpc/common/SpecsTests.cpp rpc/common/TypesTests.cpp rpc/common/impl/HandlerProviderTests.cpp + rpc/filters/impl/DelegateTransactionsFilterTests.cpp rpc/handlers/AccountChannelsTests.cpp rpc/handlers/AccountCurrenciesTests.cpp rpc/handlers/AccountInfoTests.cpp diff --git a/tests/unit/rpc/filters/impl/DelegateTransactionsFilterTests.cpp b/tests/unit/rpc/filters/impl/DelegateTransactionsFilterTests.cpp new file mode 100644 index 0000000000..5a69a3b957 --- /dev/null +++ b/tests/unit/rpc/filters/impl/DelegateTransactionsFilterTests.cpp @@ -0,0 +1,196 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "data/Types.hpp" +#include "rpc/common/Types.hpp" +#include "rpc/filters/impl/DelegateTransactionsFilter.hpp" +#include "util/TestObject.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +using namespace rpc; +using namespace ripple; + +namespace { +auto const kACCOUNT_OWNER = *parseBase58("rnrx6w8Z2VJERMMpk9jv9Y2YZKTekFAZaK"); +auto const kACCOUNT_DELEGATOR = *parseBase58("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"); +auto const kACCOUNT_DESTINATION = *parseBase58("rMAXACCrp3Y8PpswXcg3bKggHX76V3F8M4"); +auto constexpr kMAX_SEQ = 30u; +} // namespace + +class DelegateTransactionFilterTest : public ::testing::Test { +protected: + static data::TransactionAndMetadata + createBlob(std::string_view owner, std::string_view delegate) + { + data::TransactionAndMetadata ret; + ret.transaction = createDelegateBlob(owner, delegate); + ret.ledgerSequence = kMAX_SEQ; + return ret; + } +}; + +TEST_F(DelegateTransactionFilterTest, ReturnsFalseIfNoDelegateField) +{ + DelegateFilter filterParams{.delegateType = DelegateFilter::Role::Delegator, .counterParty = std::nullopt}; + DelegateTransactionFilter filter(filterParams, kACCOUNT_OWNER); + + // Create standard tx (no delegate field) using standard TestObject helper + auto obj = createPaymentTransactionObject(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DESTINATION), 100, 10, 1); + + STTx tx(std::move(obj)); + Serializer s; + tx.add(s); + + data::TransactionAndMetadata blob; + blob.transaction = s.getData(); + + auto const& result = filter.check(blob); + EXPECT_FALSE(result.shouldInclude); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegator_MatchesWhenUserIsSigner) +{ + // I am Account B (Signer). I want to see transactions where I acted as delegator (signed for someone). + DelegateFilter filterParams{.delegateType = DelegateFilter::Role::Delegator, .counterParty = std::nullopt}; + DelegateTransactionFilter filter(filterParams, kACCOUNT_DELEGATOR); + + // Tx: Owner/delegator=A, Signer/delegatee=B + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_TRUE(result.shouldInclude); + ASSERT_TRUE(result.relevantAccount.has_value()); + EXPECT_EQ(*result.relevantAccount, kACCOUNT_OWNER); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegator_FailsWhenUserIsNotSigner) +{ + // I am Account C. I query for Delegator work. + DelegateFilter filterParams{.delegateType = DelegateFilter::Role::Delegator, .counterParty = std::nullopt}; + DelegateTransactionFilter filter(filterParams, kACCOUNT_DESTINATION); + + // Tx: Owner/delegator=A, Signer/delegatee=B (C is not involved) + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_FALSE(result.shouldInclude); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegator_WithCounterparty_Match) +{ + // I am Account B (Signer). I want to see work I did specifically for Account A. + DelegateFilter filterParams{ + .delegateType = DelegateFilter::Role::Delegator, .counterParty = to_string(kACCOUNT_OWNER) + }; + DelegateTransactionFilter filter(filterParams, kACCOUNT_DELEGATOR); + + // Tx: Owner/delegator=A, Signer/delegatee=B + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_TRUE(result.shouldInclude); + EXPECT_EQ(*result.relevantAccount, kACCOUNT_OWNER); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegator_WithCounterparty_Mismatch) +{ + // I am Account B (Signer). I want to see work I did for Account C. + DelegateFilter filterParams{ + .delegateType = DelegateFilter::Role::Delegator, .counterParty = to_string(kACCOUNT_DESTINATION) + }; + DelegateTransactionFilter filter(filterParams, kACCOUNT_DELEGATOR); + + // Tx: Owner/delegator=A, Signer/delegatee=B (Owner A != Counterparty C) + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_FALSE(result.shouldInclude); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegatee_MatchesWhenUserIsOwner) +{ + // I am Account A (Owner). I want to see who signed for me. + DelegateFilter filterParams{.delegateType = DelegateFilter::Role::Delegatee, .counterParty = std::nullopt}; + DelegateTransactionFilter filter(filterParams, kACCOUNT_OWNER); + + // Tx: Owner/delegator=A, Signer/delegatee=B + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_TRUE(result.shouldInclude); + ASSERT_TRUE(result.relevantAccount.has_value()); + EXPECT_EQ(*result.relevantAccount, kACCOUNT_DELEGATOR); // Should return Signer +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegatee_FailsWhenUserIsNotOwner) +{ + // I am Account C. I query for Delegatee work. + DelegateFilter filterParams{.delegateType = DelegateFilter::Role::Delegatee, .counterParty = std::nullopt}; + DelegateTransactionFilter filter(filterParams, kACCOUNT_DESTINATION); + + // Tx: Owner/delegator=A, Signer/delegatee=B (C is not involved) + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_FALSE(result.shouldInclude); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegatee_WithCounterparty_Match) +{ + // I am Account A (Owner). I want to see work signed specifically by B. + DelegateFilter filterParams{ + .delegateType = DelegateFilter::Role::Delegatee, .counterParty = to_string(kACCOUNT_DELEGATOR) + }; + DelegateTransactionFilter filter(filterParams, kACCOUNT_OWNER); + + // Tx: Owner/delegator=A, Signer/delegatee=B + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_TRUE(result.shouldInclude); + EXPECT_EQ(*result.relevantAccount, kACCOUNT_DELEGATOR); +} + +TEST_F(DelegateTransactionFilterTest, RoleDelegatee_WithCounterparty_Mismatch) +{ + // I am Account A (Owner). I want to see work signed by C. + DelegateFilter filterParams{ + .delegateType = DelegateFilter::Role::Delegatee, .counterParty = to_string(kACCOUNT_DESTINATION) + }; + DelegateTransactionFilter filter(filterParams, kACCOUNT_OWNER); + + // Tx: Owner/delegator=A, Signer/delegatee=B (Signer B != Counterparty C) + auto blob = createBlob(to_string(kACCOUNT_OWNER), to_string(kACCOUNT_DELEGATOR)); + + auto const& result = filter.check(blob); + EXPECT_FALSE(result.shouldInclude); +} diff --git a/tests/unit/rpc/handlers/AccountTxTests.cpp b/tests/unit/rpc/handlers/AccountTxTests.cpp index 0dd5983e8f..cfb2459673 100644 --- a/tests/unit/rpc/handlers/AccountTxTests.cpp +++ b/tests/unit/rpc/handlers/AccountTxTests.cpp @@ -554,7 +554,6 @@ TEST_F(RPCAccountTxHandlerTest, IndexSpecificForwardTrue) testing::_, true, testing::Optional(testing::Eq(TransactionsCursor{kMIN_SEQ, INT32_MAX})), - testing::_, testing::_ ) ); @@ -599,7 +598,6 @@ TEST_F(RPCAccountTxHandlerTest, IndexSpecificForwardFalse) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), - testing::_, testing::_ ) ); @@ -644,7 +642,6 @@ TEST_F(RPCAccountTxHandlerTest, IndexNotSpecificForwardTrue) testing::_, true, testing::Optional(testing::Eq(TransactionsCursor{kMIN_SEQ - 1, INT32_MAX})), - testing::_, testing::_ ) ); @@ -689,7 +686,6 @@ TEST_F(RPCAccountTxHandlerTest, IndexNotSpecificForwardFalse) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), - testing::_, testing::_ ) ); @@ -734,7 +730,6 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrue) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), - testing::_, testing::_ ) ); @@ -790,7 +785,6 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrueV2) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), - testing::_, testing::_ ) ) @@ -843,12 +837,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitAndMarker) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::Optional(testing::Eq(TransactionsCursor{10, 11})), - testing::_, - testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ ) ) .WillOnce(Return(transCursor)); @@ -887,7 +876,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitIsCapped) { auto const transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_, testing::_)) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -921,7 +910,7 @@ TEST_F(RPCAccountTxHandlerTest, LimitAllowedUpToCap) { auto const transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_, testing::_)) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -965,7 +954,6 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerIndex) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), - testing::_, testing::_ ) ); @@ -1058,7 +1046,6 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerHash) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), - testing::_, testing::_ ) ); @@ -1105,7 +1092,6 @@ TEST_F(RPCAccountTxHandlerTest, SpecificLedgerIndexValidated) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), - testing::_, testing::_ ) ); @@ -1149,7 +1135,6 @@ TEST_F(RPCAccountTxHandlerTest, TxLessThanMinSeq) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 1, INT32_MAX})), - testing::_, testing::_ ) ); @@ -1194,7 +1179,6 @@ TEST_F(RPCAccountTxHandlerTest, TxLargerThanMaxSeq) testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{kMAX_SEQ - 2, INT32_MAX})), - testing::_, testing::_ ) ); @@ -1230,28 +1214,13 @@ TEST_F(RPCAccountTxHandlerTest, TxLargerThanMaxSeq) TEST_F(RPCAccountTxHandlerTest, WithDelegateAgent) { auto transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); - auto const accountId = *ripple::parseBase58(kACCOUNT); + for (auto& txn : transactions) { - txn.delegatedAccount = accountId; + txn.transaction = createDelegateBlob(kACCOUNT2, kACCOUNT); } auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL( - *backend_, - fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::_, - testing::Optional( - testing::AllOf( - testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegator), - testing::Field(&DelegateFilter::counterParty, testing::Eq(std::nullopt)) - ) - ), - testing::_ - ) - ) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -1281,30 +1250,15 @@ TEST_F(RPCAccountTxHandlerTest, WithDelegateAgent) TEST_F(RPCAccountTxHandlerTest, WithDelegateFromAndCounterparty) { auto transactions = genTransactions(kMIN_SEQ + 1, kMAX_SEQ - 1); - auto const kCOUNTERPARTY = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun"; - auto const counterpartyID = *ripple::parseBase58(kCOUNTERPARTY); + auto constexpr kCOUNTERPARTY = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun"; + for (auto& txn : transactions) { - txn.delegatedAccount = counterpartyID; + txn.transaction = createDelegateBlob(kACCOUNT, kCOUNTERPARTY); } auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; - EXPECT_CALL( - *backend_, - fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::_, - testing::Optional( - testing::AllOf( - testing::Field(&DelegateFilter::delegateType, DelegateFilter::Role::Delegatee), - testing::Field(&DelegateFilter::counterParty, testing::Optional(std::string(kCOUNTERPARTY))) - ) - ), - testing::_ - ) - ) + EXPECT_CALL(*backend_, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_)) .WillOnce(Return(transCursor)); ON_CALL(*mockETLServicePtr_, getETLState).WillByDefault(Return(etl::ETLState{})); @@ -1519,12 +1473,7 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v1) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::Optional(testing::Eq(TransactionsCursor{10, 11})), - testing::_, - testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ ) ); @@ -1743,12 +1692,7 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v2) EXPECT_CALL( *backend_, fetchAccountTransactions( - testing::_, - testing::_, - false, - testing::Optional(testing::Eq(TransactionsCursor{10, 11})), - testing::_, - testing::_ + testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_ ) ); @@ -2366,7 +2310,7 @@ TEST_P(AccountTxTransactionTypeTest, SpecificTransactionType) auto const transCursor = TransactionsAndCursor{.txns = transactions, .cursor = TransactionsCursor{12, 34}}; ON_CALL(*backend_, fetchAccountTransactions).WillByDefault(Return(transCursor)); EXPECT_CALL( - *backend_, fetchAccountTransactions(_, _, false, Optional(Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), _, _) + *backend_, fetchAccountTransactions(_, _, false, Optional(Eq(TransactionsCursor{kMAX_SEQ, INT32_MAX})), _) ); auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, kMAX_SEQ); From 300219a4b1a260a3851fba17c1079778d86eb041 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Fri, 5 Dec 2025 13:39:44 -0500 Subject: [PATCH 5/5] fix doxygen --- src/rpc/filters/impl/DelegateTransactionsFilter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/filters/impl/DelegateTransactionsFilter.hpp b/src/rpc/filters/impl/DelegateTransactionsFilter.hpp index 9224544529..bfd91b8950 100644 --- a/src/rpc/filters/impl/DelegateTransactionsFilter.hpp +++ b/src/rpc/filters/impl/DelegateTransactionsFilter.hpp @@ -36,7 +36,7 @@ class DelegateTransactionFilter : public TransactionFilter { public: /** * @brief Construct a new delegate transaction filter - * @param params The filter parameters from the JSON request (role, counterparty string) + * @param filter The filter parameters from the JSON request (role, counterparty string) * @param queriedAccount The account currently being queried in account_tx (input from account_tx handler) */ DelegateTransactionFilter(rpc::DelegateFilter filter, ripple::AccountID queriedAccount);