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

View File

@@ -43,6 +43,62 @@ private:
std::map<uint256, AccountTx> transactionMap_;
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:
RWDBDatabase(Application& app, Config const& config, JobQueue& jobQueue)
: app_(app), useTxTables_(config.useTxTables())
@@ -193,7 +249,17 @@ public:
std::size_t count = 0;
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;
}
@@ -607,12 +673,17 @@ public:
{
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)
{
++skipped;
continue;
}
AccountTx const accountTx = accountData.transactions[txIndex];
std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>(
accountTx.second->getLgrSeq());
accountTx.first->setStatus(COMMITTED);
@@ -652,13 +723,18 @@ public:
innerRIt != rIt->second.rend();
++innerRIt)
{
AccountTx const accountTx =
accountData.transactions[innerRIt->second];
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
if (skipped < options.offset)
{
++skipped;
continue;
}
AccountTx const accountTx =
accountData.transactions[innerRIt->second];
std::uint32_t const inLedger = rangeCheckedCast<std::uint32_t>(
accountTx.second->getLgrSeq());
accountTx.first->setLedger(inLedger);
@@ -694,12 +770,19 @@ public:
{
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)
{
++skipped;
continue;
}
const auto& [txn, txMeta] = accountData.transactions[txIndex];
result.emplace_back(
txn->getSTransaction()->getSerializer().peekData(),
txMeta->getAsObject().getSerializer().peekData(),
@@ -738,13 +821,20 @@ public:
innerRIt != rIt->second.rend();
++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)
{
++skipped;
continue;
}
const auto& [txn, txMeta] =
accountData.transactions[innerRIt->second];
result.emplace_back(
txn->getSTransaction()->getSerializer().peekData(),
txMeta->getAsObject().getSerializer().peekData(),
@@ -838,18 +928,23 @@ public:
return {newmarker, total};
}
Blob rawTxn = accountData.transactions[index]
.first->getSTransaction()
AccountTx const& accountTx =
accountData.transactions[index];
Blob rawTxn = accountTx.first->getSTransaction()
->getSerializer()
.peekData();
Blob rawMeta = accountData.transactions[index]
.second->getAsObject()
Blob rawMeta = accountTx.second->getAsObject()
.getSerializer()
.peekData();
if (rawMeta.size() == 0)
onUnsavedLedger(ledgerSeq);
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
onTransaction(
rangeCheckedCast<std::uint32_t>(ledgerSeq),
"COMMITTED",
@@ -893,18 +988,23 @@ public:
return {newmarker, total};
}
Blob rawTxn = accountData.transactions[index]
.first->getSTransaction()
AccountTx const& accountTx =
accountData.transactions[index];
Blob rawTxn = accountTx.first->getSTransaction()
->getSerializer()
.peekData();
Blob rawMeta = accountData.transactions[index]
.second->getAsObject()
Blob rawMeta = accountTx.second->getAsObject()
.getSerializer()
.peekData();
if (rawMeta.size() == 0)
onUnsavedLedger(ledgerSeq);
if (options.strict &&
!isAccountInvolvedInTx(options.account, accountTx))
continue;
onTransaction(
rangeCheckedCast<std::uint32_t>(ledgerSeq),
"COMMITTED",

View File

@@ -27,6 +27,7 @@
#include <ripple/app/rdb/backend/detail/Node.h>
#include <ripple/basics/BasicConfig.h>
#include <ripple/basics/StringUtilities.h>
#include <ripple/basics/strHex.h>
#include <ripple/core/DatabaseCon.h>
#include <ripple/core/SociDB.h>
#include <ripple/json/to_string.h>
@@ -758,14 +759,34 @@ transactionsSQL(
options.minLedger);
}
// Convert account ID to hex string for binary search
std::string accountHex =
strHex(options.account.data(), options.account.size());
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)
sql = boost::str(
boost::format("SELECT %s FROM AccountTransactions "
"WHERE Account = '%s' %s %s LIMIT %u, %u;") %
selection % toBase58(options.account) % maxClause % minClause %
beast::lexicalCastThrow<std::string>(options.offset) %
"INNER JOIN Transactions ON Transactions.TransID = "
"AccountTransactions.TransID "
"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));
else
sql = boost::str(
@@ -773,15 +794,16 @@ transactionsSQL(
"SELECT %s FROM "
"AccountTransactions INNER JOIN Transactions "
"ON Transactions.TransID = AccountTransactions.TransID "
"WHERE Account = '%s' %s %s "
"WHERE Account = '%s' %s %s %s "
"ORDER BY AccountTransactions.LedgerSeq %s, "
"AccountTransactions.TxnSeq %s, AccountTransactions.TransID %s "
"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") %
beast::lexicalCastThrow<std::string>(options.offset) %
beast::lexicalCastThrow<std::string>(numberOfResults));
JLOG(j.trace()) << "txSQL query: " << sql;
return sql;
}
@@ -1114,6 +1136,21 @@ accountTxPage(
if (limit_used > 0)
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(
R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,
Status,RawTxn,TxnMeta
@@ -1132,12 +1169,12 @@ accountTxPage(
{
sql = boost::str(
boost::format(
prefix + (R"(AccountTransactions.LedgerSeq BETWEEN %u AND %u
prefix + (R"(AccountTransactions.LedgerSeq BETWEEN %u AND %u %s
ORDER BY AccountTransactions.LedgerSeq %s,
AccountTransactions.TxnSeq %s
LIMIT %u;)")) %
toBase58(options.account) % options.minLedger % options.maxLedger %
order % order % queryLimit);
filterClause % order % order % queryLimit);
}
else
{
@@ -1150,25 +1187,25 @@ accountTxPage(
auto b58acct = toBase58(options.account);
sql = boost::str(
boost::format((
R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,
Status,RawTxn,TxnMeta
R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
FROM AccountTransactions, Transactions WHERE
(AccountTransactions.TransID = Transactions.TransID AND
AccountTransactions.Account = '%s' AND
AccountTransactions.LedgerSeq BETWEEN %u AND %u)
AccountTransactions.LedgerSeq BETWEEN %u AND %u) %s
UNION
SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
FROM AccountTransactions, Transactions WHERE
(AccountTransactions.TransID = Transactions.TransID AND
AccountTransactions.Account = '%s' AND
AccountTransactions.LedgerSeq = %u AND
AccountTransactions.TxnSeq %s %u)
AccountTransactions.TxnSeq %s %u) %s
ORDER BY AccountTransactions.LedgerSeq %s,
AccountTransactions.TxnSeq %s
LIMIT %u;
)")) %
b58acct % minLedger % maxLedger % b58acct % findLedger % compare %
findSeq % order % order % queryLimit);
b58acct % minLedger % maxLedger % filterClause % b58acct %
findLedger % compare % findSeq % filterClause % order % order %
queryLimit);
}
{

View File

@@ -40,6 +40,17 @@ strHex(FwdIt begin, FwdIt end)
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())>
std::string
strHex(T const& from)

View File

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