From 1541d045ee4c608f296b7ceec70e9bc198835934 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 9 Oct 2026 13:31:01 -0500 Subject: [PATCH] wallet: cut per-entry copies in listtransactions and gettransaction - ListTransactions computes a tx's "size" once, lazily, through a const CTransaction reference instead of a slicing copy per entry. - listtransactions copies the returned window once from the built array. - OrderedTxItems can open the wallet DB read-only without a flush on close; listtransactions uses it, so a read-only RPC no longer forces a BDB checkpoint (ThreadFlushWalletDB still flushes about 2 s later). - gettransaction hex-encodes the tx without copying it. Output is unchanged. Co-Authored-By: Claude Opus 5.5 --- src/wallet/rpcwallet.cpp | 44 ++++++++++++++++++++++------------------ src/wallet/wallet.cpp | 6 ++++-- src/wallet/wallet.h | 4 +++- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d71f35d98..81de3d1e9 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1788,6 +1788,11 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe bool fAllAccounts = (strAccount == string("*")); bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY); + // "size" is a property of the transaction, not of an entry: compute it once, when the first + // entry is built, through a const reference instead of a by-value CTransaction copy. + bool fHaveTxSize = false; + uint64_t nTxSize = 0; + // Sent if ((!listSent.empty() || nFee != 0) && (fAllAccounts || strAccount == strSentAccount)) { @@ -1804,7 +1809,11 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe entry.push_back(Pair("fee", ValueFromAmount(-nFee))); if (fLong) WalletTxToJSON(wtx, entry); - entry.push_back(Pair("size", static_cast(GetSerializeSize(static_cast(wtx), SER_NETWORK, PROTOCOL_VERSION)))); + if (!fHaveTxSize) { + nTxSize = static_cast(GetSerializeSize(static_cast(wtx), SER_NETWORK, PROTOCOL_VERSION)); + fHaveTxSize = true; + } + entry.push_back(Pair("size", nTxSize)); ret.push_back(entry); } } @@ -1849,7 +1858,11 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe entry.push_back(Pair("vout", r.vout)); if (fLong) WalletTxToJSON(wtx, entry); - entry.push_back(Pair("size", static_cast(GetSerializeSize(static_cast(wtx), SER_NETWORK, PROTOCOL_VERSION)))); + if (!fHaveTxSize) { + nTxSize = static_cast(GetSerializeSize(static_cast(wtx), SER_NETWORK, PROTOCOL_VERSION)); + fHaveTxSize = true; + } + entry.push_back(Pair("size", nTxSize)); ret.push_back(entry); } } @@ -1955,7 +1968,7 @@ UniValue listtransactions(const UniValue& params, bool fHelp, const CPubKey& myp UniValue ret(UniValue::VARR); std::list acentries; - CWallet::TxItems txOrdered = pwalletMain->OrderedTxItems(acentries, strAccount); + CWallet::TxItems txOrdered = pwalletMain->OrderedTxItems(acentries, strAccount, true); // iterate backwards until we have nCount items to return: for (CWallet::TxItems::reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) @@ -1978,23 +1991,14 @@ UniValue listtransactions(const UniValue& params, bool fHelp, const CPubKey& myp if ((nFrom + nCount) > (int)ret.size()) nCount = ret.size() - nFrom; - vector arrTmp = ret.getValues(); + // 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). + UniValue result(UniValue::VARR); + const std::vector& vNewestFirst = ret.getValues(); + for (int i = nFrom + nCount; i > nFrom; --i) + result.push_back(vNewestFirst[i - 1]); - vector::iterator first = arrTmp.begin(); - std::advance(first, nFrom); - vector::iterator last = arrTmp.begin(); - std::advance(last, nFrom+nCount); - - if (last != arrTmp.end()) arrTmp.erase(last, arrTmp.end()); - if (first != arrTmp.begin()) arrTmp.erase(arrTmp.begin(), first); - - std::reverse(arrTmp.begin(), arrTmp.end()); // Return oldest to newest - - ret.clear(); - ret.setArray(); - ret.push_backV(arrTmp); - - return ret; + return result; } UniValue listaccounts(const UniValue& params, bool fHelp, const CPubKey& mypk) @@ -2239,7 +2243,7 @@ UniValue gettransaction(const UniValue& params, bool fHelp, const CPubKey& mypk) ListTransactions(wtx, "*", 0, false, details, filter); entry.push_back(Pair("details", details)); - string strHex = EncodeHexTx(static_cast(wtx)); + string strHex = EncodeHexTx(static_cast(wtx)); entry.push_back(Pair("hex", strHex)); return entry; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1e8a22cb8..e596767d7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1701,10 +1701,12 @@ int64_t CWallet::IncOrderPosNext(CWalletDB *pwalletdb) return nRet; } -CWallet::TxItems CWallet::OrderedTxItems(std::list& acentries, std::string strAccount) +CWallet::TxItems CWallet::OrderedTxItems(std::list& acentries, std::string strAccount, bool fReadOnlyDb) { AssertLockHeld(cs_wallet); // mapWallet - CWalletDB walletdb(strWalletFile); + // Only a read cursor over the accounting entries runs on this handle. Pure readers open it + // read-only and without flush-on-close, so the call does not end in a txn_checkpoint. + CWalletDB walletdb(strWalletFile, fReadOnlyDb ? "r" : "r+", !fReadOnlyDb); // First: get all CWalletTx and CAccountingEntry into a sorted-by-order multimap. TxItems txOrdered; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 107814971..3e11b9921 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1210,10 +1210,12 @@ public: /** * Get the wallet's activity log + * @param fReadOnlyDb open wallet.dat read-only and without flush-on-close (pure readers such as + * listtransactions: no txn_checkpoint per call); false = read-write, flush-on-close * @return multimap of ordered transactions and accounting entries * @warning Returned pointers are *only* valid within the scope of passed acentries */ - TxItems OrderedTxItems(std::list& acentries, std::string strAccount = ""); + TxItems OrderedTxItems(std::list& acentries, std::string strAccount = "", bool fReadOnlyDb = false); void MarkDirty(); bool UpdateNullifierNoteMap();