From c32c85b7f054a23bb53235f2bde5990045f667fc Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 9 Oct 2026 13:31:01 -0500 Subject: [PATCH] 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 --- src/wallet/rpcwallet.cpp | 51 +++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 81de3d1e9..d673ac974 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -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; string strSentAccount; @@ -1798,6 +1810,8 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe { BOOST_FOREACH(const COutputEntry& s, listSent) { + if (ConsumeSkip(pnSkip)) + continue; UniValue entry(UniValue::VOBJ); if(involvesWatchonly || (::IsMine(*pwalletMain, s.destination) & ISMINE_WATCH_ONLY)) 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; if (fAllAccounts || (account == strAccount)) { + if (ConsumeSkip(pnSkip)) + continue; UniValue entry(UniValue::VOBJ); if(involvesWatchonly || (::IsMine(*pwalletMain, r.destination) & ISMINE_WATCH_ONLY)) 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("*")); if (fAllAccounts || acentry.strAccount == strAccount) { + if (ConsumeSkip(pnSkip)) + return; UniValue entry(UniValue::VOBJ); entry.push_back(Pair("account", acentry.strAccount)); entry.push_back(Pair("category", "move")); @@ -1970,33 +1988,38 @@ UniValue listtransactions(const UniValue& params, bool fHelp, const CPubKey& myp std::list acentries; 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) { CWalletTx *const pwtx = (*it).second.first; if (pwtx != 0) { - ListTransactions(*pwtx, strAccount, 0, true, ret, filter); + ListTransactions(*pwtx, strAccount, 0, true, ret, filter, &nSkipLeft); } CAccountingEntry *const pacentry = (*it).second.second; 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()) - nFrom = ret.size(); - if ((nFrom + nCount) > (int)ret.size()) - nCount = ret.size() - nFrom; + if (nFrom > nSeen) + nFrom = nSeen; + if ((nFrom + nCount) > nSeen) + nCount = nSeen - nFrom; - // ret is newest to oldest; the reply is its entries [nFrom, nFrom+nCount), oldest to newest. - // Copy them once, back to front, straight from ret's storage (no vector copy/erase/reverse). + // The reply is entries [nFrom, nFrom+nCount) of the newest-first sequence, oldest to newest. + // Entry k of that sequence is ret[k - nSkipped] (nFrom >= nSkipped holds here). UniValue result(UniValue::VARR); const std::vector& vNewestFirst = ret.getValues(); for (int i = nFrom + nCount; i > nFrom; --i) - result.push_back(vNewestFirst[i - 1]); + result.push_back(vNewestFirst[i - 1 - nSkipped]); return result; }