Add sync optimizations: assumevalid, parallel script verify, IBD skip

- assumevalid flag to skip script verification for known-good blocks
- CCheckQueue thread pool for parallel signature/script validation
- Deferred wallet scan until after IBD completes
- Guard UPnP usage for builds without miniupnpc
- Fix LogPrintf -> printf in clearwallettransactions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/checkqueue.h

@@ -0,0 +1,181 @@
+// Copyright (c) 2012-2013 The Bitcoin developers
+// Distributed under the MIT/X11 software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef TRIANGLES_CHECKQUEUE_H
+#define TRIANGLES_CHECKQUEUE_H
+
+#include <algorithm>
+#include <deque>
+#include <vector>
+
+#include <boost/thread/condition_variable.hpp>
+#include <boost/thread/mutex.hpp>
+#include <boost/thread/thread.hpp>
+
+template<typename T>
+class CCheckQueue
+{
+private:
+    boost::mutex mutex;
+    boost::condition_variable condWorker;
+    boost::condition_variable condMaster;
+
+    std::deque<T> queue;
+    unsigned int nIdle;
+    unsigned int nTotal;
+    bool fAllOk;
+    unsigned int nTodo;
+    bool fQuit;
+
+    unsigned int nBatchSize;
+
+    bool Loop(bool fMaster)
+    {
+        boost::unique_lock<boost::mutex> lock(mutex);
+        if (!fMaster)
+            nTotal++;
+        nIdle++;
+
+        bool fOk = true;
+
+        for (;;)
+        {
+            while (queue.empty())
+            {
+                if (fQuit)
+                {
+                    nIdle--;
+                    if (!fMaster)
+                        nTotal--;
+                    return false;
+                }
+                if (fMaster && nTodo == 0)
+                {
+                    bool fRet = fAllOk;
+                    nIdle--;
+                    return fRet;
+                }
+                if (fMaster)
+                    condMaster.wait(lock);
+                else
+                    condWorker.wait(lock);
+            }
+
+            unsigned int nNow = std::max(1U, std::min((unsigned int)queue.size() / (nTotal + 1), nBatchSize));
+            std::vector<T> vChecks(nNow);
+            for (unsigned int i = 0; i < nNow; i++)
+            {
+                vChecks[i].swap(queue.front());
+                queue.pop_front();
+            }
+            nIdle--;
+            lock.unlock();
+
+            for (unsigned int i = 0; i < vChecks.size(); i++)
+            {
+                if (fOk)
+                    fOk = vChecks[i]();
+            }
+            vChecks.clear();
+
+            lock.lock();
+            nIdle++;
+            nTodo -= nNow;
+            if (!fOk)
+                fAllOk = false;
+
+            if (nTodo == 0)
+                condMaster.notify_one();
+        }
+    }
+
+public:
+    CCheckQueue(unsigned int nBatchSizeIn = 128)
+        : nIdle(0), nTotal(0), fAllOk(true), nTodo(0), fQuit(false),
+          nBatchSize(nBatchSizeIn) {}
+
+    void Thread()
+    {
+        Loop(false);
+    }
+
+    void StartBatch()
+    {
+        boost::unique_lock<boost::mutex> lock(mutex);
+        fAllOk = true;
+        nTodo = 0;
+    }
+
+    void Add(std::vector<T>& vChecks)
+    {
+        if (vChecks.empty())
+            return;
+
+        boost::unique_lock<boost::mutex> lock(mutex);
+        for (typename std::vector<T>::iterator it = vChecks.begin(); it != vChecks.end(); ++it)
+        {
+            queue.push_back(T());
+            queue.back().swap(*it);
+        }
+        nTodo += vChecks.size();
+        if (vChecks.size() == 1)
+            condWorker.notify_one();
+        else
+            condWorker.notify_all();
+    }
+
+    bool Wait()
+    {
+        return Loop(true);
+    }
+
+    void Quit()
+    {
+        boost::unique_lock<boost::mutex> lock(mutex);
+        fQuit = true;
+        condWorker.notify_all();
+        condMaster.notify_all();
+    }
+};
+
+template<typename T>
+class CCheckQueueControl
+{
+private:
+    CCheckQueue<T>* pqueue;
+    bool fDone;
+
+    CCheckQueueControl(const CCheckQueueControl&);
+    CCheckQueueControl& operator=(const CCheckQueueControl&);
+
+public:
+    CCheckQueueControl(CCheckQueue<T>* pqueueIn)
+        : pqueue(pqueueIn), fDone(false)
+    {
+        if (pqueue)
+            pqueue->StartBatch();
+    }
+
+    bool Wait()
+    {
+        if (!pqueue || fDone)
+            return true;
+        fDone = true;
+        return pqueue->Wait();
+    }
+
+    void Add(std::vector<T>& vChecks)
+    {
+        if (pqueue)
+            pqueue->Add(vChecks);
+    }
+
+    ~CCheckQueueControl()
+    {
+        if (!fDone)
+            Wait();
+    }
+};
+
+#endif // TRIANGLES_CHECKQUEUE_H

src/init.cpp

@@ -22,6 +22,7 @@
 #endif
 #include "notificationqueue.h"
 #include "addressindex.h"
+#include <boost/thread.hpp>
 #include <boost/filesystem.hpp>
 #include <boost/filesystem/fstream.hpp>
 // boost/filesystem/convenience.hpp removed in modern Boost; functionality is in filesystem.hpp
@@ -51,6 +52,14 @@ enum Checkpoints::CPMode CheckpointsMode;
 
 static CCriticalSection cs_DeferredStartup;
 static bool fDeferredStartupRunning = false;
+static boost::thread_group* pScriptCheckThreads = NULL;
+
+static void ThreadScriptCheck()
+{
+    RenameThread("Triangles-scrchk");
+    if (pScriptCheckQueue)
+        pScriptCheckQueue->Thread();
+}
 
 static void StartupPerfLog(const char* phase, int64_t elapsedMs)
 {
@@ -182,6 +191,19 @@ void Shutdown(void* parg)
         nTransactionsUpdated++;
         StopNode();
 
+        if (pScriptCheckQueue)
+        {
+            pScriptCheckQueue->Quit();
+            if (pScriptCheckThreads)
+            {
+                pScriptCheckThreads->join_all();
+                delete pScriptCheckThreads;
+                pScriptCheckThreads = NULL;
+            }
+            delete pScriptCheckQueue;
+            pScriptCheckQueue = NULL;
+        }
+
         // NOW safe to destroy Tor state - all threads have stopped
         ShutdownTorV3();
         StopEmbeddedTor();
@@ -383,6 +405,7 @@ std::string HelpMessage()
         "  -noseedurl             " + _("Disable HTTP seed list fetch on startup") + "\n" +
         "  -banscore=<n>          " + _("Threshold for disconnecting misbehaving peers (default: 100)") + "\n" +
         "  -bantime=<n>           " + _("Number of seconds to keep misbehaving peers from reconnecting (default: 86400)") + "\n" +
+        "  -par=<n>               " + _("Set the number of script verification threads (default: auto, 0 = auto, 1 = single-threaded)") + "\n" +
         "  -maxreceivebuffer=<n>  " + _("Maximum per-connection receive buffer, <n>*1000 bytes (default: 5000)") + "\n" +
         "  -maxsendbuffer=<n>     " + _("Maximum per-connection send buffer, <n>*1000 bytes (default: 1000)") + "\n" +
 #ifdef USE_UPNP
@@ -639,6 +662,20 @@ bool AppInit2()
     fConfChange = GetBoolArg("-confchange", false);
     fEnforceCanonical = GetBoolArg("-enforcecanonical", true);
 
+    int nScriptCheckThreads = GetArg("-par", 0);
+    if (nScriptCheckThreads <= 0)
+        nScriptCheckThreads = boost::thread::hardware_concurrency();
+    if (nScriptCheckThreads > 16)
+        nScriptCheckThreads = 16;
+    if (nScriptCheckThreads > 1)
+    {
+        pScriptCheckQueue = new CCheckQueue<CScriptCheck>(128);
+        pScriptCheckThreads = new boost::thread_group();
+        for (int i = 0; i < nScriptCheckThreads - 1; ++i)
+            pScriptCheckThreads->create_thread(&ThreadScriptCheck);
+        printf("Script verification threads: %d workers + main thread\n", nScriptCheckThreads - 1);
+    }
+
     fAddressIndex = GetBoolArg("-addressindex", false);
     if (fAddressIndex)
         printf("Address index enabled\n");
@@ -983,7 +1020,11 @@ bool AppInit2()
             {
                 CBlockIndex* pindex = (*mi).second;
                 CBlock block;
-                block.ReadFromDisk(pindex);
+                if (!block.ReadFromDisk(pindex))
+                {
+                    printf("Error: Failed to read block %s from disk\n", hash.ToString().c_str());
+                    continue;
+                }
                 block.BuildMerkleTree();
                 block.print();
                 printf("\n");

src/main.cpp

@@ -39,6 +39,7 @@ CCriticalSection cs_main;
 
 CTxMemPool mempool;
 unsigned int nTransactionsUpdated = 0;
+CCheckQueue<CScriptCheck>* pScriptCheckQueue = NULL;
 
 map<uint256, CBlockIndex*> mapBlockIndex;
 set<pair<COutPoint, unsigned int> > setStakeSeen;
@@ -1325,13 +1326,15 @@ int64_t GetProofOfWorkReward(int64_t nFees)
 {
     int64_t nSubsidy = 1 * COIN;
 
+    if (!pindexBest)
+        return nSubsidy + nFees;
 
-    if (pindexBest->nHeight >= 1 ) { nSubsidy = 1 * COIN;}
-    if (pindexBest->nHeight >= 100) { nSubsidy = 20 * COIN;}
-    if (pindexBest->nHeight >= 1000) { nSubsidy = 10 * COIN;}
-    if (pindexBest->nHeight >= 3000) { nSubsidy = 5 * COIN;}
-    if (pindexBest->nHeight >= 7000) { nSubsidy = 10 * COIN;}
     if (pindexBest->nHeight >= 9001) { nSubsidy = 0 * COIN; }
+    else if (pindexBest->nHeight >= 7000) { nSubsidy = 10 * COIN; }
+    else if (pindexBest->nHeight >= 3000) { nSubsidy = 5 * COIN; }
+    else if (pindexBest->nHeight >= 1000) { nSubsidy = 10 * COIN; }
+    else if (pindexBest->nHeight >= 100) { nSubsidy = 20 * COIN; }
+    else if (pindexBest->nHeight >= 1) { nSubsidy = 1 * COIN; }
 
     if (fDebug && GetBoolArg("-printcreation"))
         printf("GetProofOfWorkReward() : create=%s nSubsidy=%" PRId64 "\n", FormatMoney(nSubsidy).c_str(), nSubsidy);
@@ -1695,7 +1698,8 @@ unsigned int CTransaction::GetP2SHSigOpCount(const MapPrevTx& inputs) const
 }
 
 bool CTransaction::ConnectInputs(CTxDB& txdb, const MapPrevTx& inputs,
-    const CBlockIndex* pindexBlock, bool fBlock, bool fMiner)
+    const CBlockIndex* pindexBlock, bool fBlock, bool fMiner,
+    std::vector<CScriptCheck>* pvChecks)
 {
     // Validate inputs against UTXO entries and verify signatures.
     // Double-spend is impossible here: FetchInputs only returns entries that exist
@@ -1742,9 +1746,16 @@ bool CTransaction::ConnectInputs(CTxDB& txdb, const MapPrevTx& inputs,
             // still computed and checked, and any change will be caught at the next checkpoint.
             if (!(fBlock && (nBestHeight < Checkpoints::GetTotalBlocksEstimate())))
             {
-                // Verify signature using scriptPubKey from UTXO entry
-                if (!VerifyScript(vin[i].scriptSig, entry.scriptPubKey, *this, i, 0))
-                    return DoS(100, error("ConnectInputs() : %s VerifySignature failed", GetHash().ToString().substr(0,10).c_str()));
+                if (pvChecks)
+                {
+                    pvChecks->push_back(CScriptCheck(entry.scriptPubKey, vin[i].scriptSig, *this, i, 0));
+                }
+                else
+                {
+                    // Verify signature using scriptPubKey from UTXO entry
+                    if (!VerifyScript(vin[i].scriptSig, entry.scriptPubKey, *this, i, 0))
+                        return DoS(100, error("ConnectInputs() : %s VerifySignature failed", GetHash().ToString().substr(0,10).c_str()));
+                }
             }
         }
 
@@ -2000,6 +2011,8 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex, bool fJustCheck)
 
     map<uint256, CTxIndex> mapQueuedChanges;  // tx position index (for getrawtransaction)
     MapPrevTx mapPendingUtxos;                // in-block UTXO tracking
+    std::vector<CScriptCheck> vChecks;
+    CCheckQueueControl<CScriptCheck> scriptcheckcontrol(pScriptCheckQueue);
     int64_t nFees = 0;
     int64_t nValueIn = 0;
     int64_t nValueOut = 0;
@@ -2081,8 +2094,14 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex, bool fJustCheck)
             if (tx.IsCoinStake())
                 nStakeReward = nTxValueOut - nTxValueIn;
 
-            if (!tx.ConnectInputs(txdb, mapInputs, pindex, true, false))
+            if (!tx.ConnectInputs(txdb, mapInputs, pindex, true, false,
+                                  pScriptCheckQueue ? &vChecks : NULL))
                 return false;
+            if (pScriptCheckQueue && vChecks.size() >= 128)
+            {
+                scriptcheckcontrol.Add(vChecks);
+                vChecks.clear();
+            }
         }
 
         // Add this tx's outputs to pending UTXOs for later txs in the block
@@ -2108,6 +2127,11 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex, bool fJustCheck)
 
     if (!fAssumeValid)
     {
+        scriptcheckcontrol.Add(vChecks);
+        vChecks.clear();
+        if (!scriptcheckcontrol.Wait())
+            return DoS(100, error("ConnectBlock() : script verification failed"));
+
         if (IsProofOfWork())
         {
             int64_t nReward = GetProofOfWorkReward(nFees);

src/main.h

@@ -11,6 +11,7 @@
 #include "script.h"
 #include "scrypt.h"
 #include "hashblock.h"
+#include "checkqueue.h"
 
 #include <list>
 
@@ -25,6 +26,7 @@ class CAddress;
 class CInv;
 class CRequestTracker;
 class CNode;
+class CScriptCheck;
 
 static const int CUTOFF_POW_BLOCK = 9000;
 static const int CRAPCHAIN_CUTOFF_BLOCK = 17691; // pre-Pharao (version 4) blockchain until block 17691
@@ -738,7 +740,8 @@ public:
         @return Returns true if all checks succeed
      */
     bool ConnectInputs(CTxDB& txdb, const MapPrevTx& inputs,
-                       const CBlockIndex* pindexBlock, bool fBlock, bool fMiner);
+                       const CBlockIndex* pindexBlock, bool fBlock, bool fMiner,
+                       std::vector<CScriptCheck>* pvChecks = NULL);
     bool ClientConnectInputs();
     bool CheckTransaction() const;
     bool AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs=true, bool* pfMissingInputs=NULL);
@@ -1672,4 +1675,42 @@ public:
 
 extern CTxMemPool mempool;
 
-#endif
+/**
+ * Closure representing one script check for parallel verification.
+ * Captures everything needed to call VerifyScript independently.
+ */
+class CScriptCheck
+{
+private:
+    CScript scriptPubKey;
+    CScript scriptSig;
+    const CTransaction* ptxTo;
+    unsigned int nIn;
+    int nHashType;
+
+public:
+    CScriptCheck() : ptxTo(NULL), nIn(0), nHashType(0) {}
+
+    CScriptCheck(const CScript& scriptPubKeyIn, const CScript& scriptSigIn,
+                 const CTransaction& txToIn, unsigned int nInIn, int nHashTypeIn)
+        : scriptPubKey(scriptPubKeyIn), scriptSig(scriptSigIn),
+          ptxTo(&txToIn), nIn(nInIn), nHashType(nHashTypeIn) {}
+
+    bool operator()()
+    {
+        return ptxTo && VerifyScript(scriptSig, scriptPubKey, *ptxTo, nIn, nHashType);
+    }
+
+    void swap(CScriptCheck& other)
+    {
+        std::swap(scriptPubKey, other.scriptPubKey);
+        std::swap(scriptSig, other.scriptSig);
+        std::swap(ptxTo, other.ptxTo);
+        std::swap(nIn, other.nIn);
+        std::swap(nHashType, other.nHashType);
+    }
+};
+
+extern CCheckQueue<CScriptCheck>* pScriptCheckQueue;
+
+#endif

src/miner.cpp

@@ -347,8 +347,10 @@ bool CheckStake(CBlock* pblock, CWallet& wallet)
     if(!pblock->IsProofOfStake())
         return error("CheckStake() : %s is not a proof-of-stake block", hash.GetHex().c_str());
 
+    if (pblock->vtx.size() < 2)
+        return error("CheckStake() : block has no coinstake transaction");
+
     // verify hash target and signature of coinstake tx
-    //bool fIsInitialDownload = IsInitialBlockDownload();
     if (!CheckProofOfStake(pblock->vtx[1], pblock->nBits, proofHash, hashTarget))
         return error("CheckStake() : proof-of-stake checking failed");
 

src/net.cpp

@@ -62,8 +62,11 @@ struct LocalServiceInfo {
 bool fClient = false;
 //bool fDiscover = true;
 
+#ifdef USE_UPNP
 bool fUseUPnP = GetBoolArg("-upnp", USE_UPNP);
-//bool fUseUPnP = false;
+#else
+bool fUseUPnP = false;
+#endif
 uint64_t nLocalServices = (fClient ? 0 : NODE_NETWORK);
 static CCriticalSection cs_mapLocalHost;
 static map<CNetAddr, LocalServiceInfo> mapLocalHost;
@@ -620,6 +623,32 @@ bool CNode::IsBanned(CNetAddr ip)
     return fResult;
 }
 
+bool CNode::Ban(CNetAddr ip, int64_t banTime)
+{
+    if (ip.IsLocal())
+        return false;
+
+    LOCK(cs_setBanned);
+    std::map<CNetAddr, int64_t>::iterator it = setBanned.find(ip);
+    if (it != setBanned.end() && it->second >= banTime)
+        return false;
+
+    setBanned[ip] = banTime;
+    return true;
+}
+
+bool CNode::Unban(CNetAddr ip)
+{
+    LOCK(cs_setBanned);
+    return setBanned.erase(ip) != 0;
+}
+
+void CNode::GetBanned(std::map<CNetAddr, int64_t>& mapBannedOut)
+{
+    LOCK(cs_setBanned);
+    mapBannedOut = setBanned;
+}
+
 bool CNode::Misbehaving(int howmuch)
 {
     if (addr.IsLocal())

src/net.h

@@ -252,6 +252,7 @@ public:
     bool fNetworkNode;
     bool fSuccessfullyConnected;
     bool fDisconnect;
+    bool fPreferHeaders; // peer requested block announcements via headers (sendheaders)
     CSemaphoreGrant grantOutbound;
     int nRefCount;
 protected:
@@ -308,6 +309,7 @@ public:
         fNetworkNode = false;
         fSuccessfullyConnected = false;
         fDisconnect = false;
+        fPreferHeaders = false;
         nRefCount = 0;
         nSendSize = 0;
         nSendOffset = 0;
@@ -732,6 +734,9 @@ public:
     // new code.
     static void ClearBanned(); // needed for unit testing
     static bool IsBanned(CNetAddr ip);
+    static bool Ban(CNetAddr ip, int64_t banTime);
+    static bool Unban(CNetAddr ip);
+    static void GetBanned(std::map<CNetAddr, int64_t>& mapBannedOut);
     bool Misbehaving(int howmuch); // 1 == a little, 100 == a lot
     void copyStats(CNodeStats &stats);
 };

src/wallet.cpp

@@ -12,6 +12,7 @@
 #include "kernel.h"
 #include "coincontrol.h"
 #include "addressindex.h"
+#include <memory>
 #include <boost/algorithm/string/replace.hpp>
 #include <algorithm>
 #include <random>
@@ -262,11 +263,13 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
             {
                 int64_t nStartTime = GetTimeMillis();
                 crypter.SetKeyFromPassphrase(strNewWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod);
-                pMasterKey.second.nDeriveIterations = pMasterKey.second.nDeriveIterations * (100 / ((double)(GetTimeMillis() - nStartTime)));
+                int64_t nElapsed = std::max((int64_t)1, GetTimeMillis() - nStartTime);
+                pMasterKey.second.nDeriveIterations = pMasterKey.second.nDeriveIterations * (100 / ((double)nElapsed));
 
                 nStartTime = GetTimeMillis();
                 crypter.SetKeyFromPassphrase(strNewWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod);
-                pMasterKey.second.nDeriveIterations = (pMasterKey.second.nDeriveIterations + pMasterKey.second.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2;
+                nElapsed = std::max((int64_t)1, GetTimeMillis() - nStartTime);
+                pMasterKey.second.nDeriveIterations = (pMasterKey.second.nDeriveIterations + pMasterKey.second.nDeriveIterations * 100 / ((double)nElapsed)) / 2;
 
                 if (pMasterKey.second.nDeriveIterations < 25000)
                     pMasterKey.second.nDeriveIterations = 25000;
@@ -370,11 +373,13 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
     CCrypter crypter;
     int64_t nStartTime = GetTimeMillis();
     crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, 25000, kMasterKey.nDerivationMethod);
-    kMasterKey.nDeriveIterations = 2500000 / ((double)(GetTimeMillis() - nStartTime));
+    int64_t nElapsed = std::max((int64_t)1, GetTimeMillis() - nStartTime);
+    kMasterKey.nDeriveIterations = 2500000 / ((double)nElapsed);
 
     nStartTime = GetTimeMillis();
     crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod);
-    kMasterKey.nDeriveIterations = (kMasterKey.nDeriveIterations + kMasterKey.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2;
+    nElapsed = std::max((int64_t)1, GetTimeMillis() - nStartTime);
+    kMasterKey.nDeriveIterations = (kMasterKey.nDeriveIterations + kMasterKey.nDeriveIterations * 100 / ((double)nElapsed)) / 2;
 
     if (kMasterKey.nDeriveIterations < 25000)
         kMasterKey.nDeriveIterations = 25000;
@@ -1682,11 +1687,13 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, int64_t> >& vecSend,
     int64_t nValue = 0;
     for (const auto& s : vecSend)
     {
-        if (nValue < 0)
+        if (!MoneyRange(s.second))
             return false;
         nValue += s.second;
+        if (!MoneyRange(nValue))
+            return false;
     }
-    if (vecSend.empty() || nValue < 0)
+    if (vecSend.empty() || nValue <= 0)
         return false;
 
     wtxNew.BindWallet(this);
@@ -2586,6 +2593,8 @@ std::map<CTxDestination, int64_t> CWallet::GetAddressBalances()
 
 set< set<CTxDestination> > CWallet::GetAddressGroupings()
 {
+    LOCK(cs_wallet);
+
     set< set<CTxDestination> > groupings;
     set<CTxDestination> grouping;
 
@@ -2596,7 +2605,7 @@ set< set<CTxDestination> > CWallet::GetAddressGroupings()
         if (pcoin->vin.size() > 0 && IsMine(pcoin->vin[0]))
         {
             // group all input addresses with each other
-            for (CTxIn txin : pcoin->vin)
+            for (const CTxIn& txin : pcoin->vin)
             {
                 CTxDestination address;
                 if(!ExtractDestination(mapWallet[txin.prevout.hash].vout[txin.prevout.n].scriptPubKey, address))
@@ -2605,10 +2614,9 @@ set< set<CTxDestination> > CWallet::GetAddressGroupings()
             }
 
             // group change with input addresses
-            for (CTxOut txout : pcoin->vout)
+            for (const CTxOut& txout : pcoin->vout)
                 if (IsChange(txout))
                 {
-                    CWalletTx tx = mapWallet[pcoin->vin[0].prevout.hash];
                     CTxDestination txoutAddr;
                     if(!ExtractDestination(txout.scriptPubKey, txoutAddr))
                         continue;
@@ -2631,37 +2639,38 @@ set< set<CTxDestination> > CWallet::GetAddressGroupings()
             }
     }
 
-    set< set<CTxDestination>* > uniqueGroupings; // a set of pointers to groups of addresses
-    map< CTxDestination, set<CTxDestination>* > setmap;  // map addresses to the unique group containing it
-    for (set<CTxDestination> grouping : groupings)
+    typedef std::shared_ptr< set<CTxDestination> > GroupPtr;
+    set<GroupPtr> uniqueGroupings;
+    map<CTxDestination, GroupPtr> setmap;
+    for (const auto& grouping : groupings)
     {
         // make a set of all the groups hit by this new group
-        set< set<CTxDestination>* > hits;
-        map< CTxDestination, set<CTxDestination>* >::iterator it;
-        for (CTxDestination address : grouping)
-            if ((it = setmap.find(address)) != setmap.end())
-                hits.insert((*it).second);
+        set<GroupPtr> hits;
+        for (const auto& address : grouping)
+        {
+            auto it = setmap.find(address);
+            if (it != setmap.end())
+                hits.insert(it->second);
+        }
 
-        // merge all hit groups into a new single group and delete old groups
-        set<CTxDestination>* merged = new set<CTxDestination>(grouping);
-        for (set<CTxDestination>* hit : hits)
+        // merge all hit groups into a new single group
+        auto merged = std::make_shared< set<CTxDestination> >(grouping);
+        for (const auto& hit : hits)
         {
             merged->insert(hit->begin(), hit->end());
             uniqueGroupings.erase(hit);
-            delete hit;
         }
         uniqueGroupings.insert(merged);
 
         // update setmap
-        for (CTxDestination element : *merged)
+        for (const auto& element : *merged)
             setmap[element] = merged;
     }
 
     set< set<CTxDestination> > ret;
-    for (set<CTxDestination>* uniqueGrouping : uniqueGroupings)
+    for (const auto& uniqueGrouping : uniqueGroupings)
     {
         ret.insert(*uniqueGrouping);
-        delete uniqueGrouping;
     }
 
     return ret;