Compare commits

...

5 Commits

Author SHA1 Message Date
Richard Holland
000136d283 test case 2025-02-03 15:58:59 +11:00
Richard Holland
c4696715f1 add strict filtering to missing function 2025-02-03 13:59:01 +11:00
RichardAH
bd3216d9c5 Merge branch 'dev' into account-tx-fix 2025-02-03 10:36:59 +10:00
Richard Holland
2c4ef51106 better filter to remove false positives on regular key 2025-02-01 14:52:03 +11:00
Richard Holland
053ffd7b18 add strict filtering to account_tx api 2025-02-01 11:56:46 +11:00
6 changed files with 247 additions and 30 deletions

View File

@@ -69,6 +69,7 @@ public:
std::uint32_t offset; std::uint32_t offset;
std::uint32_t limit; std::uint32_t limit;
bool bUnlimited; bool bUnlimited;
bool strict;
}; };
struct AccountTxPageOptions struct AccountTxPageOptions
@@ -79,6 +80,7 @@ public:
std::optional<AccountTxMarker> marker; std::optional<AccountTxMarker> marker;
std::uint32_t limit; std::uint32_t limit;
bool bAdmin; bool bAdmin;
bool strict;
}; };
using AccountTx = using AccountTx =
@@ -101,6 +103,7 @@ public:
bool forward = false; bool forward = false;
uint32_t limit = 0; uint32_t limit = 0;
std::optional<AccountTxMarker> marker; std::optional<AccountTxMarker> marker;
bool strict;
}; };
struct AccountTxResult struct AccountTxResult

View File

@@ -43,6 +43,62 @@ private:
std::map<uint256, AccountTx> transactionMap_; std::map<uint256, AccountTx> transactionMap_;
std::map<AccountID, AccountTxData> accountTxMap_; std::map<AccountID, AccountTxData> accountTxMap_;
// helper function to scan for an account ID inside the tx and meta blobs
// used for strict filtering of account_tx
bool
isAccountInvolvedInTx(AccountID const& account, AccountTx const& accountTx)
{
auto const& txn = accountTx.first;
auto const& meta = accountTx.second;
// Search metadata, excluding RegularKey false positives
Blob const metaBlob = meta->getAsObject().getSerializer().peekData();
if (metaBlob.size() >= account.size())
{
auto it = metaBlob.begin();
while (true)
{
// Find next occurrence of account
it = std::search(
it,
metaBlob.end(),
account.data(),
account.data() + account.size());
if (it == metaBlob.end())
break;
// Check if this is a RegularKey field (0x8814 prefix)
if (it >= metaBlob.begin() + 2)
{
auto prefix = *(it - 2);
auto prefix2 = *(it - 1);
if (prefix != 0x88 || prefix2 != 0x14)
{
// Found account not preceded by RegularKey prefix
return true;
}
}
else
{
// Too close to start to be RegularKey
return true;
}
++it; // Move past this occurrence
}
}
// Search transaction blob
Blob const txnBlob = txn->getSTransaction()->getSerializer().peekData();
return txnBlob.size() >= account.size() &&
std::search(
txnBlob.begin(),
txnBlob.end(),
account.data(),
account.data() + account.size()) != txnBlob.end();
}
public: public:
RWDBDatabase(Application& app, Config const& config, JobQueue& jobQueue) RWDBDatabase(Application& app, Config const& config, JobQueue& jobQueue)
: app_(app), useTxTables_(config.useTxTables()) : app_(app), useTxTables_(config.useTxTables())
@@ -193,7 +249,17 @@ public:
std::size_t count = 0; std::size_t count = 0;
for (const auto& [_, accountData] : accountTxMap_) for (const auto& [_, accountData] : accountTxMap_)
{ {
count += accountData.transactions.size(); for (const auto& tx : accountData.transactions)
{
// RH NOTE: options isn't provided to this function
// but this function is probably only used internally
// so make it reflect the true number (unfiltered)
// if (options.strict &&
// !isAccountInvolvedInTx(options.account, tx))
// continue;
count++;
}
} }
return count; return count;
} }
@@ -607,12 +673,17 @@ public:
{ {
for (const auto& [txSeq, txIndex] : txIt->second) for (const auto& [txSeq, txIndex] : txIt->second)
{ {
AccountTx const accountTx = accountData.transactions[txIndex];
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
if (skipped < options.offset) if (skipped < options.offset)
{ {
++skipped; ++skipped;
continue; continue;
} }
AccountTx const accountTx = accountData.transactions[txIndex];
std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>( std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>(
accountTx.second->getLgrSeq()); accountTx.second->getLgrSeq());
accountTx.first->setStatus(COMMITTED); accountTx.first->setStatus(COMMITTED);
@@ -652,13 +723,18 @@ public:
innerRIt != rIt->second.rend(); innerRIt != rIt->second.rend();
++innerRIt) ++innerRIt)
{ {
AccountTx const accountTx =
accountData.transactions[innerRIt->second];
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
if (skipped < options.offset) if (skipped < options.offset)
{ {
++skipped; ++skipped;
continue; continue;
} }
AccountTx const accountTx =
accountData.transactions[innerRIt->second];
std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>( std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>(
accountTx.second->getLgrSeq()); accountTx.second->getLgrSeq());
accountTx.first->setLedger(inLedger); accountTx.first->setLedger(inLedger);
@@ -694,12 +770,19 @@ public:
{ {
for (const auto& [txSeq, txIndex] : txIt->second) for (const auto& [txSeq, txIndex] : txIt->second)
{ {
AccountTx const accountTx = accountData.transactions[txIndex];
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
const auto& [txn, txMeta] = accountTx;
if (skipped < options.offset) if (skipped < options.offset)
{ {
++skipped; ++skipped;
continue; continue;
} }
const auto& [txn, txMeta] = accountData.transactions[txIndex];
result.emplace_back( result.emplace_back(
txn->getSTransaction()->getSerializer().peekData(), txn->getSTransaction()->getSerializer().peekData(),
txMeta->getAsObject().getSerializer().peekData(), txMeta->getAsObject().getSerializer().peekData(),
@@ -738,13 +821,20 @@ public:
innerRIt != rIt->second.rend(); innerRIt != rIt->second.rend();
++innerRIt) ++innerRIt)
{ {
AccountTx const accountTx =
accountData.transactions[innerRIt->second];
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
const auto& [txn, txMeta] = accountTx;
if (skipped < options.offset) if (skipped < options.offset)
{ {
++skipped; ++skipped;
continue; continue;
} }
const auto& [txn, txMeta] =
accountData.transactions[innerRIt->second];
result.emplace_back( result.emplace_back(
txn->getSTransaction()->getSerializer().peekData(), txn->getSTransaction()->getSerializer().peekData(),
txMeta->getAsObject().getSerializer().peekData(), txMeta->getAsObject().getSerializer().peekData(),
@@ -838,18 +928,23 @@ public:
return {newmarker, total}; return {newmarker, total};
} }
Blob rawTxn = accountData.transactions[index] AccountTx const& accountTx =
.first->getSTransaction() accountData.transactions[index];
Blob rawTxn = accountTx.first->getSTransaction()
->getSerializer() ->getSerializer()
.peekData(); .peekData();
Blob rawMeta = accountData.transactions[index] Blob rawMeta = accountTx.second->getAsObject()
.second->getAsObject()
.getSerializer() .getSerializer()
.peekData(); .peekData();
if (rawMeta.size() == 0) if (rawMeta.size() == 0)
onUnsavedLedger(ledgerSeq); onUnsavedLedger(ledgerSeq);
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
onTransaction( onTransaction(
rangeCheckedCast<std::uint32_t>(ledgerSeq), rangeCheckedCast<std::uint32_t>(ledgerSeq),
"COMMITTED", "COMMITTED",
@@ -893,18 +988,23 @@ public:
return {newmarker, total}; return {newmarker, total};
} }
Blob rawTxn = accountData.transactions[index] AccountTx const& accountTx =
.first->getSTransaction() accountData.transactions[index];
Blob rawTxn = accountTx.first->getSTransaction()
->getSerializer() ->getSerializer()
.peekData(); .peekData();
Blob rawMeta = accountData.transactions[index] Blob rawMeta = accountTx.second->getAsObject()
.second->getAsObject()
.getSerializer() .getSerializer()
.peekData(); .peekData();
if (rawMeta.size() == 0) if (rawMeta.size() == 0)
onUnsavedLedger(ledgerSeq); onUnsavedLedger(ledgerSeq);
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
onTransaction( onTransaction(
rangeCheckedCast<std::uint32_t>(ledgerSeq), rangeCheckedCast<std::uint32_t>(ledgerSeq),
"COMMITTED", "COMMITTED",

View File

@@ -27,6 +27,7 @@
#include <ripple/app/rdb/backend/detail/Node.h> #include <ripple/app/rdb/backend/detail/Node.h>
#include <ripple/basics/BasicConfig.h> #include <ripple/basics/BasicConfig.h>
#include <ripple/basics/StringUtilities.h> #include <ripple/basics/StringUtilities.h>
#include <ripple/basics/strHex.h>
#include <ripple/core/DatabaseCon.h> #include <ripple/core/DatabaseCon.h>
#include <ripple/core/SociDB.h> #include <ripple/core/SociDB.h>
#include <ripple/json/to_string.h> #include <ripple/json/to_string.h>
@@ -758,14 +759,34 @@ transactionsSQL(
options.minLedger); options.minLedger);
} }
// Convert account ID to hex string for binary search
std::string accountHex =
strHex(options.account.data(), options.account.size());
std::string sql; std::string sql;
// For metadata search:
// 1. Look for account ID not preceded by 8814 (RegularKey field)
// 2. OR look for account in raw transaction
std::string filterClause = options.strict ? "AND (("
"hex(TxnMeta) LIKE '%" +
accountHex +
"%' AND "
"hex(TxnMeta) NOT LIKE '%8814" +
accountHex +
"%'"
") OR hex(RawTxn) LIKE '%" +
accountHex + "%')"
: "";
if (count) if (count)
sql = boost::str( sql = boost::str(
boost::format("SELECT %s FROM AccountTransactions " boost::format("SELECT %s FROM AccountTransactions "
"WHERE Account = '%s' %s %s LIMIT %u, %u;") % "INNER JOIN Transactions ON Transactions.TransID = "
selection % toBase58(options.account) % maxClause % minClause % "AccountTransactions.TransID "
beast::lexicalCastThrow<std::string>(options.offset) % "WHERE Account = '%s' %s %s %s LIMIT %u, %u;") %
selection % toBase58(options.account) % filterClause % maxClause %
minClause % beast::lexicalCastThrow<std::string>(options.offset) %
beast::lexicalCastThrow<std::string>(numberOfResults)); beast::lexicalCastThrow<std::string>(numberOfResults));
else else
sql = boost::str( sql = boost::str(
@@ -773,15 +794,16 @@ transactionsSQL(
"SELECT %s FROM " "SELECT %s FROM "
"AccountTransactions INNER JOIN Transactions " "AccountTransactions INNER JOIN Transactions "
"ON Transactions.TransID = AccountTransactions.TransID " "ON Transactions.TransID = AccountTransactions.TransID "
"WHERE Account = '%s' %s %s " "WHERE Account = '%s' %s %s %s "
"ORDER BY AccountTransactions.LedgerSeq %s, " "ORDER BY AccountTransactions.LedgerSeq %s, "
"AccountTransactions.TxnSeq %s, AccountTransactions.TransID %s " "AccountTransactions.TxnSeq %s, AccountTransactions.TransID %s "
"LIMIT %u, %u;") % "LIMIT %u, %u;") %
selection % toBase58(options.account) % maxClause % minClause % selection % toBase58(options.account) % filterClause % maxClause %
minClause % (descending ? "DESC" : "ASC") %
(descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") %
(descending ? "DESC" : "ASC") %
beast::lexicalCastThrow<std::string>(options.offset) % beast::lexicalCastThrow<std::string>(options.offset) %
beast::lexicalCastThrow<std::string>(numberOfResults)); beast::lexicalCastThrow<std::string>(numberOfResults));
JLOG(j.trace()) << "txSQL query: " << sql; JLOG(j.trace()) << "txSQL query: " << sql;
return sql; return sql;
} }
@@ -1114,6 +1136,21 @@ accountTxPage(
if (limit_used > 0) if (limit_used > 0)
newmarker = options.marker; newmarker = options.marker;
// Convert account ID to hex string for binary search
std::string accountHex =
strHex(options.account.data(), options.account.size());
// Add metadata search filter similar to transactionsSQL
std::string filterClause = options.strict
? " AND ((hex(TxnMeta) LIKE '%" + accountHex +
"%' "
"AND hex(TxnMeta) NOT LIKE '%8814" +
accountHex +
"%') "
"OR hex(RawTxn) LIKE '%" +
accountHex + "%')"
: "";
static std::string const prefix( static std::string const prefix(
R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq, R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,
Status,RawTxn,TxnMeta Status,RawTxn,TxnMeta
@@ -1132,12 +1169,12 @@ accountTxPage(
{ {
sql = boost::str( sql = boost::str(
boost::format( boost::format(
prefix + (R"(AccountTransactions.LedgerSeq BETWEEN %u AND %u prefix + (R"(AccountTransactions.LedgerSeq BETWEEN %u AND %u %s
ORDER BY AccountTransactions.LedgerSeq %s, ORDER BY AccountTransactions.LedgerSeq %s,
AccountTransactions.TxnSeq %s AccountTransactions.TxnSeq %s
LIMIT %u;)")) % LIMIT %u;)")) %
toBase58(options.account) % options.minLedger % options.maxLedger % toBase58(options.account) % options.minLedger % options.maxLedger %
order % order % queryLimit); filterClause % order % order % queryLimit);
} }
else else
{ {
@@ -1150,25 +1187,25 @@ accountTxPage(
auto b58acct = toBase58(options.account); auto b58acct = toBase58(options.account);
sql = boost::str( sql = boost::str(
boost::format(( boost::format((
R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq, R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
Status,RawTxn,TxnMeta
FROM AccountTransactions, Transactions WHERE FROM AccountTransactions, Transactions WHERE
(AccountTransactions.TransID = Transactions.TransID AND (AccountTransactions.TransID = Transactions.TransID AND
AccountTransactions.Account = '%s' AND AccountTransactions.Account = '%s' AND
AccountTransactions.LedgerSeq BETWEEN %u AND %u) AccountTransactions.LedgerSeq BETWEEN %u AND %u) %s
UNION UNION
SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
FROM AccountTransactions, Transactions WHERE FROM AccountTransactions, Transactions WHERE
(AccountTransactions.TransID = Transactions.TransID AND (AccountTransactions.TransID = Transactions.TransID AND
AccountTransactions.Account = '%s' AND AccountTransactions.Account = '%s' AND
AccountTransactions.LedgerSeq = %u AND AccountTransactions.LedgerSeq = %u AND
AccountTransactions.TxnSeq %s %u) AccountTransactions.TxnSeq %s %u) %s
ORDER BY AccountTransactions.LedgerSeq %s, ORDER BY AccountTransactions.LedgerSeq %s,
AccountTransactions.TxnSeq %s AccountTransactions.TxnSeq %s
LIMIT %u; LIMIT %u;
)")) % )")) %
b58acct % minLedger % maxLedger % b58acct % findLedger % compare % b58acct % minLedger % maxLedger % filterClause % b58acct %
findSeq % order % order % queryLimit); findLedger % compare % findSeq % filterClause % order % order %
queryLimit);
} }
{ {

View File

@@ -40,6 +40,17 @@ strHex(FwdIt begin, FwdIt end)
return result; return result;
} }
template <class FwdIt>
std::string
strHex(FwdIt begin, std::size_t length)
{
std::string result;
result.reserve(2 * length);
boost::algorithm::hex(
begin, std::next(begin, length), std::back_inserter(result));
return result;
}
template <class T, class = decltype(std::declval<T>().begin())> template <class T, class = decltype(std::declval<T>().begin())>
std::string std::string
strHex(T const& from) strHex(T const& from)

View File

@@ -223,7 +223,8 @@ doAccountTxHelp(RPC::Context& context, AccountTxArgs const& args)
result.ledgerRange.max, result.ledgerRange.max,
result.marker, result.marker,
args.limit, args.limit,
isUnlimited(context.role)}; isUnlimited(context.role),
args.strict};
auto const db = auto const db =
dynamic_cast<SQLiteDatabase*>(&context.app.getRelationalDatabase()); dynamic_cast<SQLiteDatabase*>(&context.app.getRelationalDatabase());
@@ -369,6 +370,9 @@ doAccountTxJson(RPC::JsonContext& context)
args.forward = args.forward =
params.isMember(jss::forward) && params[jss::forward].asBool(); params.isMember(jss::forward) && params[jss::forward].asBool();
args.strict =
params.isMember(jss::strict) ? params[jss::strict].asBool() : true;
if (!params.isMember(jss::account)) if (!params.isMember(jss::account))
return rpcError(rpcINVALID_PARAMS); return rpcError(rpcINVALID_PARAMS);

View File

@@ -245,6 +245,68 @@ class AccountTx_test : public beast::unit_test::suite
p[jss::ledger_hash] = to_string(env.closed()->info().parentHash); p[jss::ledger_hash] = to_string(env.closed()->info().parentHash);
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p)))); BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
} }
// Strict
{
Account S1{"S1"};
Account S2{"S2"};
Account S3{"S3"};
env.fund(XRP(10000), S1);
env.fund(XRP(10000), S2);
env.fund(XRP(10000), S3);
env.close();
// Regular key set
env(regkey(S1, S2));
env.close();
// we'll make a payment between S1 and S3
env(pay(S1, S3, XRP(100)));
env.close();
auto hasTxs = [](Json::Value const& j, bool strict) {
if (!j.isMember(jss::result) ||
j[jss::result][jss::status] != "success")
return false;
if (strict)
{
return (j[jss::result][jss::transactions].size() == 3) &&
(j[jss::result][jss::transactions][0u][jss::tx]
[jss::TransactionType] == jss::SetRegularKey) &&
(j[jss::result][jss::transactions][1u][jss::tx]
[jss::TransactionType] == jss::AccountSet) &&
(j[jss::result][jss::transactions][2u][jss::tx]
[jss::TransactionType] == jss::Payment);
}
return (j[jss::result][jss::transactions].size() == 4) &&
(j[jss::result][jss::transactions][0u][jss::tx]
[jss::TransactionType] == jss::Payment) &&
(j[jss::result][jss::transactions][1u][jss::tx]
[jss::TransactionType] == jss::SetRegularKey) &&
(j[jss::result][jss::transactions][2u][jss::tx]
[jss::TransactionType] == jss::AccountSet) &&
(j[jss::result][jss::transactions][3u][jss::tx]
[jss::TransactionType] == jss::Payment);
};
Json::Value p{jParms};
p[jss::account] = S2.human();
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p)), true));
p[jss::strict] = true;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p)), true));
p[jss::strict] = false;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p)), false));
}
} }
void void