Merge bitcoin/bitcoin#25464: rpc: Reduce Univalue push_backV peak memory usage in listtransactions

fa8a1c06961f4b1826696e0db8dce81dce627721 rpc: Fix Univalue push_backV OOM in listtransactions (MacroFake)

Pull request description:

  Related to, but not intended as a fix for #25229.

  Currently the RPC will have the same data stored thrice:

  * `UniValue ret` (memory filled by `ListTransactions`)
  * `std::vector<UniValue> vec` (constructed by calling `push_backV`)
  * `UniValue result` (the actual result, memory filled by `push_backV`)

  Fix this by filling the memory only once:

  * `std::vector<UniValue> ret` (memory filled by `ListTransactions`)
  * Pass iterators to `push_backV` instead of creating a full copy
  * Move memory into `UniValue result` instead of copying it

ACKs for top commit:
  shaavan:
    Code Review ACK fa8a1c06961f4b1826696e0db8dce81dce627721

Tree-SHA512: 1c3ca40fc8497134a4141195160e4aa9fe72b3c00c5998c972b58ad0eb498ebea05013f9105bb80e7264c9db1d0e7a2032396a8a4af1f326d831fbee20f32ea3

2 files changed, 16 insertions, 6 deletions

diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h
index 22be0311e8..7f9a6aaffd 100644
--- a/src/univalue/include/univalue.h
+++ b/src/univalue/include/univalue.h
@@ -84,6 +84,8 @@ public:
 
     bool push_back(const UniValue& val);
     bool push_backV(const std::vector<UniValue>& vec);
+    template <class It>
+    bool push_backV(It first, It last);
 
     void __pushKV(const std::string& key, const UniValue& val);
     bool pushKV(const std::string& key, const UniValue& val);
@@ -137,6 +139,14 @@ public:
     friend const UniValue& find_value( const UniValue& obj, const std::string& name);
 };
 
+template <class It>
+bool UniValue::push_backV(It first, It last)
+{
+    if (typ != VARR) return false;
+    values.insert(values.end(), first, last);
+    return true;
+}
+
 enum jtokentype {
     JTOK_ERR        = -1,
     JTOK_NONE       = 0,                           // eof
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index d44b62c934..0b52dcc001 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -310,11 +310,12 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
  * @param  wtx            The wallet transaction.
  * @param  nMinDepth      The minimum confirmation depth.
  * @param  fLong          Whether to include the JSON version of the transaction.
- * @param  ret            The UniValue into which the result is stored.
+ * @param  ret            The vector into which the result is stored.
  * @param  filter_ismine  The "is mine" filter flags.
  * @param  filter_label   Optional label string to filter incoming transactions.
  */
-static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+template <class Vec>
+static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     CAmount nFee;
     std::list<COutputEntry> listReceived;
@@ -500,8 +501,7 @@ RPCHelpMan listtransactions()
     if (nFrom < 0)
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative from");
 
-    UniValue ret(UniValue::VARR);
-
+    std::vector<UniValue> ret;
     {
         LOCK(pwallet->cs_wallet);
 
@@ -523,9 +523,9 @@ RPCHelpMan listtransactions()
     if ((nFrom + nCount) > (int)ret.size())
         nCount = ret.size() - nFrom;
 
-    const std::vector<UniValue>& txs = ret.getValues();
+    auto txs_rev_it{std::make_move_iterator(ret.rend())};
     UniValue result{UniValue::VARR};
-    result.push_backV({ txs.rend() - nFrom - nCount, txs.rend() - nFrom }); // Return oldest to newest
+    result.push_backV(txs_rev_it - nFrom - nCount, txs_rev_it - nFrom); // Return oldest to newest
     return result;
 },
     };