wallet: count the entries listtransactions skips instead of building them

listtransactions formatted every entry before "from" and then threw it
away. Paging through the history that way is quadratic: ObsidianDragon
pages up to 5000 rows at a time, so a 30k-row mining wallet built about
105k entries per full refresh (30k with this change). Skipped entries are
now counted after the same account and watch-only checks as before and
never formatted.

Output is unchanged for every valid input. When count + from overflows
(the build uses -fwrapv), the result is [] instead of an out-of-bounds read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 13:31:01 -05:00
co-authored by Claude Opus 5.5
parent 1541d045ee
commit c32c85b7f0
+37 -14
View File
@@ -1774,7 +1774,19 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
} }
} }
void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) // listtransactions skips its first `from` entries without building them: while *pnSkip > 0, an
// entry that would be emitted only decrements *pnSkip. The counter is shared across calls, so the
// skipped entries are exactly the first `from` ones in emission order. NULL: emit everything.
static bool ConsumeSkip(int* pnSkip)
{
if (pnSkip != NULL && *pnSkip > 0) {
--*pnSkip;
return true;
}
return false;
}
void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter, int* pnSkip = NULL)
{ {
CAmount nFee; CAmount nFee;
string strSentAccount; string strSentAccount;
@@ -1798,6 +1810,8 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe
{ {
BOOST_FOREACH(const COutputEntry& s, listSent) BOOST_FOREACH(const COutputEntry& s, listSent)
{ {
if (ConsumeSkip(pnSkip))
continue;
UniValue entry(UniValue::VOBJ); UniValue entry(UniValue::VOBJ);
if(involvesWatchonly || (::IsMine(*pwalletMain, s.destination) & ISMINE_WATCH_ONLY)) if(involvesWatchonly || (::IsMine(*pwalletMain, s.destination) & ISMINE_WATCH_ONLY))
entry.push_back(Pair("involvesWatchonly", true)); entry.push_back(Pair("involvesWatchonly", true));
@@ -1828,6 +1842,8 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe
account = pwalletMain->mapAddressBook[r.destination].name; account = pwalletMain->mapAddressBook[r.destination].name;
if (fAllAccounts || (account == strAccount)) if (fAllAccounts || (account == strAccount))
{ {
if (ConsumeSkip(pnSkip))
continue;
UniValue entry(UniValue::VOBJ); UniValue entry(UniValue::VOBJ);
if(involvesWatchonly || (::IsMine(*pwalletMain, r.destination) & ISMINE_WATCH_ONLY)) if(involvesWatchonly || (::IsMine(*pwalletMain, r.destination) & ISMINE_WATCH_ONLY))
entry.push_back(Pair("involvesWatchonly", true)); entry.push_back(Pair("involvesWatchonly", true));
@@ -1869,12 +1885,14 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe
} }
} }
void AcentryToJSON(const CAccountingEntry& acentry, const string& strAccount, UniValue& ret) void AcentryToJSON(const CAccountingEntry& acentry, const string& strAccount, UniValue& ret, int* pnSkip = NULL)
{ {
bool fAllAccounts = (strAccount == string("*")); bool fAllAccounts = (strAccount == string("*"));
if (fAllAccounts || acentry.strAccount == strAccount) if (fAllAccounts || acentry.strAccount == strAccount)
{ {
if (ConsumeSkip(pnSkip))
return;
UniValue entry(UniValue::VOBJ); UniValue entry(UniValue::VOBJ);
entry.push_back(Pair("account", acentry.strAccount)); entry.push_back(Pair("account", acentry.strAccount));
entry.push_back(Pair("category", "move")); entry.push_back(Pair("category", "move"));
@@ -1970,33 +1988,38 @@ UniValue listtransactions(const UniValue& params, bool fHelp, const CPubKey& myp
std::list<CAccountingEntry> acentries; std::list<CAccountingEntry> acentries;
CWallet::TxItems txOrdered = pwalletMain->OrderedTxItems(acentries, strAccount, true); CWallet::TxItems txOrdered = pwalletMain->OrderedTxItems(acentries, strAccount, true);
// iterate backwards until we have nCount items to return: // iterate backwards until we have nCount items to return. The first nFrom entries are only
// counted (nSkipLeft), never built, since they are not returned.
int nSkipLeft = nFrom;
for (CWallet::TxItems::reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) for (CWallet::TxItems::reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it)
{ {
CWalletTx *const pwtx = (*it).second.first; CWalletTx *const pwtx = (*it).second.first;
if (pwtx != 0) if (pwtx != 0)
{ {
ListTransactions(*pwtx, strAccount, 0, true, ret, filter); ListTransactions(*pwtx, strAccount, 0, true, ret, filter, &nSkipLeft);
} }
CAccountingEntry *const pacentry = (*it).second.second; CAccountingEntry *const pacentry = (*it).second.second;
if (pacentry != 0) if (pacentry != 0)
AcentryToJSON(*pacentry, strAccount, ret); AcentryToJSON(*pacentry, strAccount, ret, &nSkipLeft);
if ((int)ret.size() >= (nCount+nFrom)) break; // (skipped + built) is the entry count the loop used to test
if ((nFrom - nSkipLeft) + (int)ret.size() >= (nCount+nFrom)) break;
} }
// ret is newest to oldest // ret holds the entries after the first nSkipped ones, newest to oldest
const int nSkipped = nFrom - nSkipLeft;
const int nSeen = nSkipped + (int)ret.size();
if (nFrom > (int)ret.size()) if (nFrom > nSeen)
nFrom = ret.size(); nFrom = nSeen;
if ((nFrom + nCount) > (int)ret.size()) if ((nFrom + nCount) > nSeen)
nCount = ret.size() - nFrom; nCount = nSeen - nFrom;
// ret is newest to oldest; the reply is its entries [nFrom, nFrom+nCount), oldest to newest. // The reply is entries [nFrom, nFrom+nCount) of the newest-first sequence, oldest to newest.
// Copy them once, back to front, straight from ret's storage (no vector copy/erase/reverse). // Entry k of that sequence is ret[k - nSkipped] (nFrom >= nSkipped holds here).
UniValue result(UniValue::VARR); UniValue result(UniValue::VARR);
const std::vector<UniValue>& vNewestFirst = ret.getValues(); const std::vector<UniValue>& vNewestFirst = ret.getValues();
for (int i = nFrom + nCount; i > nFrom; --i) for (int i = nFrom + nCount; i > nFrom; --i)
result.push_back(vNewestFirst[i - 1]); result.push_back(vNewestFirst[i - 1 - nSkipped]);
return result; return result;
} }