Improve Staking Icon

Improvements have been made to the miner code to use scoped enums
for the miner status rather than a concatenated string. (The string
is also retained for legacy purposes for now.)

The old staking icon/tooltip conflated two different states together:
"unable to stake" because the wallet is locked or the reserve is set
so that no coins can be used for staking, etc. vs. "currently not staking,
but able to stake", which is all of the coins currently on cooldown
(such as for a new wallet that just received it's first "deposit", or a
one UTXO wallet where the UTXO just staked and is on cooldown.

I have introduced a new staking icon, a red down arrow, which indicates
"unable to stake", with an appropriate tooltip description. The gray
down arrow is now used for "currently not staking" status, where staking
will occur without any further operator action (i.e. coins maturing).
The green up arrow is used for currently staking status as before.

Some minor improvements were also made, such as minor style cleanups
of the code where I was working, along with some lock optimizations.

Author: jamescowens

src/Makefile.qt.include

@@ -270,6 +270,7 @@ RES_ICONS = \
   qt/res/icons/rsz_chat.png \
   qt/res/icons/staking_off.svg \
   qt/res/icons/staking_on.svg \
+  qt/res/icons/staking_unable.svg \
   qt/res/icons/statistics.png \
   qt/res/icons/toolbar.png \
   qt/res/icons/transaction_conflicted.png \

src/main.cpp

@@ -377,9 +377,19 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
         return result;
     }
 
-    bool staking = MinerStatus.nLastCoinStakeSearchInterval && MinerStatus.WeightSum;
-    // Get out early if not staking and ignore_staking_status is false and set return value of 0.
-    if (!staking && !ignore_staking_status)
+    bool staking;
+    bool able_to_stake;
+
+    {
+        LOCK(MinerStatus.lock);
+
+        staking = MinerStatus.nLastCoinStakeSearchInterval && MinerStatus.WeightSum;
+
+        able_to_stake = MinerStatus.able_to_stake;
+    }
+
+    // Get out early if not staking, ignore_staking_status is false, and not able_to_stake and set return value of 0.
+    if (!ignore_staking_status && !staking && !able_to_stake)
     {
         if (fDebug10) LogPrintf("GetEstimatedTimetoStake debug: Not staking: ETTS = %f", result);
         return result;
@@ -447,11 +457,9 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
     {
         CTxIndex txindex;
         CBlock CoinBlock; //Block which contains CoinTx
-        if (!txdb.ReadTxIndex(out.tx->GetHash(), txindex))
-            continue; //error?
+        if (!txdb.ReadTxIndex(out.tx->GetHash(), txindex)) continue; //Ignore transactions that can't be read.
 
-        if (!CoinBlock.ReadFromDisk(txindex.pos.nFile, txindex.pos.nBlockPos, false))
-            continue;
+        if (!CoinBlock.ReadFromDisk(txindex.pos.nFile, txindex.pos.nBlockPos, false)) continue;
 
         // We are going to store as an event the time that the UTXO matures (is available for staking again.)
         nTime = (CoinBlock.GetBlockTime() & ~ETTS_TIMESTAMP_MASK) + nStakeMinAge;
@@ -462,14 +470,14 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
         // subtracting the reserve. Each UTXO also has to be greater than 1/80 GRC to result in a weight greater than zero in the CreateCoinStake loop,
         // so eliminate UTXO's with less than 0.0125 GRC balances right here. The test with Satoshi units for that is
         // nValue >= 1250000.
-        if(BalanceAvailForStaking >= nValue && nValue >= 1250000)
+        if (BalanceAvailForStaking >= nValue && nValue >= 1250000)
         {
         vUTXO.push_back(std::pair<int64_t, int64_t>( nTime, nValue));
         if (fDebug10) LogPrintf("GetEstimatedTimetoStake debug: pair (relative to current time: <%i, %i>", nTime - nCurrentTime, nValue);
 
         // Only record a time below if it is after nCurrentTime, because UTXO's that have matured already are already stakeable and can be grouped (will be found)
         // by the nCurrentTime record that was already injected above.
-        if(nTime > nCurrentTime) UniqueUTXOTimes.insert(nTime);
+        if (nTime > nCurrentTime) UniqueUTXOTimes.insert(nTime);
         }
     }
 
@@ -485,13 +493,13 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
     // become significant. The CDF of a compound geometric distribution as you do tosses with different probabilities follows the
     // recursion relation... CDF.i = 1 - (1 - CDF.i-1)(1 - p.i). If all probabilities are the same, this reduces to the familiar
     // CDF.k = 1 - (1 - p)^k where ^ is exponentiation.
-    for(const auto& itertime : UniqueUTXOTimes)
+    for (const auto& itertime : UniqueUTXOTimes)
     {
 
         nTime = itertime;
         dProbAccumulator = 0;
 
-        for( auto& iterUTXO : vUTXO)
+        for (auto& iterUTXO : vUTXO)
         {
 
             if (fDebug10) LogPrintf("GetEstimatedTimetoStake debug: Unique UTXO Time: %u, vector pair <%u, %u>", nTime, iterUTXO.first, iterUTXO.second);
@@ -515,8 +523,7 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
         dCumulativeProbability = 1 - ((1 - dCumulativeProbability) * pow((1 - dProbAccumulator), nThrows));
         if (fDebug10) LogPrintf("GetEstimatedTimetoStake debug: dCumulativeProbability = %e", dCumulativeProbability);
 
-        if(dCumulativeProbability >= dConfidence)
-            break;
+        if (dCumulativeProbability >= dConfidence) break;
 
         nTimePrev = nTime;
     }
@@ -549,16 +556,6 @@ double GetEstimatedTimetoStake(bool ignore_staking_status, double dDiff, double
     result = nDeltaTime + nTime - nCurrentTime;
     if (fDebug10) LogPrintf("GetEstimatedTimetoStake debug: ETTS at %d confidence = %i", dConfidence, result);
 
-    // The old calculation for comparative purposes, only done if fDebug10 set. Note that this is the "fixed" old
-    // calculation, because the old network weight calculation was wrong too...
-    if (fDebug10)
-    {
-        double oldETTS = 0;
-
-        oldETTS = GetTargetSpacing(nBestHeight) * GetEstimatedNetworkWeight(40) / MinerStatus.WeightSum;
-        LogPrintf("GetEstimatedTimetoStake debug: oldETTS = %f", oldETTS);
-    }
-
     return result;
 }
 
@@ -598,48 +595,57 @@ void GetGlobalStatus()
 
         // It is necessary to assign a local variable for ETTS to avoid an occasional deadlock between the lock below,
         // the lock on cs_main in GetEstimateTimetoStake(), and the corresponding lock in the stakeminer.
-        double dETTS = GetEstimatedTimetoStake()/86400.0;
+        double dETTS = GetEstimatedTimetoStake() / 86400.0;
+
         LOCK(GlobalStatusStruct.lock);
-        { LOCK(MinerStatus.lock);
-        GlobalStatusStruct.blocks = ToString(nBestHeight);
-        GlobalStatusStruct.difficulty = RoundToString(PORDiff,3);
-        GlobalStatusStruct.netWeight = RoundToString(GetEstimatedNetworkWeight() / 80.0,2);
-        //todo: use the real weight from miner status (requires scaling)
-        GlobalStatusStruct.coinWeight = sWeight;
-        GlobalStatusStruct.magnitude = RoundToString(boincmagnitude,2);
-        GlobalStatusStruct.ETTS = RoundToString(dETTS,3);
-        GlobalStatusStruct.ERRperday = RoundToString(boincmagnitude * NN::Tally::GetMagnitudeUnit(GetAdjustedTime()),2);
-        GlobalStatusStruct.cpid = NN::GetPrimaryCpid();
-        GlobalStatusStruct.poll = std::move(current_poll);
-
-        GlobalStatusStruct.status = msMiningErrors;
-
-        if(MinerStatus.WeightSum)
-            GlobalStatusStruct.coinWeight = RoundToString(MinerStatus.WeightSum / 80.0,2);
-
-        GlobalStatusStruct.errors.clear();
-        std::string Alerts = GetWarnings("statusbar");
-        if(!Alerts.empty())
-            GlobalStatusStruct.errors += _("Alert: ") + Alerts + "; ";
-
-        if (PORDiff < 0.1)
-            GlobalStatusStruct.errors +=  _("Low difficulty!; ");
-
-        if(!MinerStatus.ReasonNotStaking.empty())
-            GlobalStatusStruct.errors +=  _("Miner: ") + MinerStatus.ReasonNotStaking;
-
-        unsigned long stk_dropped = MinerStatus.KernelsFound - MinerStatus.AcceptedCnt;
-        if(stk_dropped)
-            GlobalStatusStruct.errors += "Rejected " + ToString(stk_dropped) + " stakes;";
-
-        if(!msMiningErrors6.empty())
-            GlobalStatusStruct.errors +=msMiningErrors6 + "; ";
-        if(!msMiningErrors7.empty())
-            GlobalStatusStruct.errors += msMiningErrors7 + "; ";
-        if(!msMiningErrors8.empty())
-            GlobalStatusStruct.errors += msMiningErrors8 + "; ";
 
+        {
+            GlobalStatusStruct.blocks = ToString(nBestHeight);
+            GlobalStatusStruct.difficulty = RoundToString(PORDiff,3);
+            GlobalStatusStruct.netWeight = RoundToString(GetEstimatedNetworkWeight() / 80.0,2);
+            //todo: use the real weight from miner status (requires scaling)
+            GlobalStatusStruct.coinWeight = sWeight;
+            GlobalStatusStruct.magnitude = RoundToString(boincmagnitude,2);
+            GlobalStatusStruct.ETTS = RoundToString(dETTS,3);
+            GlobalStatusStruct.ERRperday = RoundToString(boincmagnitude * NN::Tally::GetMagnitudeUnit(GetAdjustedTime()),2);
+            GlobalStatusStruct.cpid = NN::GetPrimaryCpid();
+            GlobalStatusStruct.poll = std::move(current_poll);
+
+            GlobalStatusStruct.status = msMiningErrors;
+
+            unsigned long stk_dropped;
+
+            {
+                LOCK(MinerStatus.lock);
+
+                if(MinerStatus.WeightSum)
+                    GlobalStatusStruct.coinWeight = RoundToString(MinerStatus.WeightSum / 80.0,2);
+
+                GlobalStatusStruct.errors.clear();
+                std::string Alerts = GetWarnings("statusbar");
+                if(!Alerts.empty())
+                    GlobalStatusStruct.errors += _("Alert: ") + Alerts + "; ";
+
+                if (PORDiff < 0.1)
+                    GlobalStatusStruct.errors +=  _("Low difficulty!; ");
+
+                if(!MinerStatus.ReasonNotStaking.empty())
+                    GlobalStatusStruct.errors +=  _("Miner: ") + MinerStatus.ReasonNotStaking;
+
+                stk_dropped = MinerStatus.KernelsFound - MinerStatus.AcceptedCnt;
+            }
+
+            if (stk_dropped)
+                GlobalStatusStruct.errors += "Rejected " + ToString(stk_dropped) + " stakes;";
+
+            if (!msMiningErrors6.empty())
+                GlobalStatusStruct.errors +=msMiningErrors6 + "; ";
+            if (!msMiningErrors7.empty())
+                GlobalStatusStruct.errors += msMiningErrors7 + "; ";
+            if (!msMiningErrors8.empty())
+                GlobalStatusStruct.errors += msMiningErrors8 + "; ";
         }
+
         return;
     }
     catch (std::exception& e)

src/miner.cpp

@@ -66,13 +66,13 @@ class COrphan
 //!
 //! \return Always \false - suitable for returning from the call directly.
 //!
-bool ReturnMinerError(CMinerStatus& status, const std::string& message)
+bool ReturnMinerError(CMinerStatus& status, CMinerStatus::ReasonNotStakingCategory& not_staking_error)
 {
     LOCK(status.lock);
 
     status.Clear();
-    status.ReasonNotStaking += message;
-    status.ReasonNotStaking += "; ";
+
+    status.SetReasonNotStaking(not_staking_error);
 
     if (fDebug) {
         LogPrintf("CreateCoinStake: %s", MinerStatus.ReasonNotStaking);
@@ -131,7 +131,7 @@ bool SignClaim(NN::Claim& claim, const uint256& last_block_hash)
 CMinerStatus::CMinerStatus(void)
 {
     Clear();
-    ReasonNotStaking= "";
+    ClearReasonsNotStaking();
     CreatedCnt= AcceptedCnt= KernelsFound= 0;
     KernelDiffMax= 0;
 }
@@ -144,6 +144,35 @@ void CMinerStatus::Clear()
     nLastCoinStakeSearchInterval = 0;
 }
 
+bool CMinerStatus::SetReasonNotStaking(ReasonNotStakingCategory not_staking_error)
+{
+    bool inserted = false;
+
+    if (std::find(vReasonNotStaking.begin(), vReasonNotStaking.end(), not_staking_error) == vReasonNotStaking.end())
+    {
+       vReasonNotStaking.insert(vReasonNotStaking.end(), not_staking_error);
+
+       if (not_staking_error != NONE)
+       {
+           if (!ReasonNotStaking.empty()) ReasonNotStaking += "; ";
+           ReasonNotStaking += vReasonNotStakingStrings[static_cast<int>(not_staking_error)];
+       }
+
+       if (not_staking_error > NO_MATURE_COINS) able_to_stake = false;
+
+       inserted = true;
+    }
+
+    return inserted;
+}
+
+void CMinerStatus::ClearReasonsNotStaking()
+{
+    vReasonNotStaking.clear();
+    ReasonNotStaking.clear();
+    able_to_stake = true;
+}
+
 CMinerStatus MinerStatus;
 
 // We want to sort transactions by priority and fee, so:
@@ -477,11 +506,11 @@ bool CreateCoinStake( CBlock &blocknew, CKey &key,
 
     // Choose coins to use
     vector<pair<const CWalletTx*,unsigned int>> CoinsToStake;
-    string sError = "";
+    CMinerStatus::ReasonNotStakingCategory not_staking_error;
 
-    if (!wallet.SelectCoinsForStaking(txnew.nTime, CoinsToStake, sError, true))
+    if (!wallet.SelectCoinsForStaking(txnew.nTime, CoinsToStake, not_staking_error, true))
     {
-        ReturnMinerError(MinerStatus, sError);
+        ReturnMinerError(MinerStatus, not_staking_error);
 
         return false;
     }
@@ -1053,14 +1082,14 @@ bool IsMiningAllowed(CWallet *pwallet)
     if(pwallet->IsLocked())
     {
         LOCK(MinerStatus.lock);
-        MinerStatus.ReasonNotStaking+=_("Wallet locked; ");
+        MinerStatus.SetReasonNotStaking(CMinerStatus::WALLET_LOCKED);
         status=false;
     }
 
     if(fDevbuildCripple)
     {
         LOCK(MinerStatus.lock);
-        MinerStatus.ReasonNotStaking+="Testnet-only version; ";
+        MinerStatus.SetReasonNotStaking(CMinerStatus::TESTNET_ONLY);
         status=false;
     }
 
@@ -1069,7 +1098,7 @@ bool IsMiningAllowed(CWallet *pwallet)
         )
     {
         LOCK(MinerStatus.lock);
-        MinerStatus.ReasonNotStaking+=_("Offline; ");
+        MinerStatus.SetReasonNotStaking(CMinerStatus::OFFLINE);
         status=false;
     }
 
@@ -1227,9 +1256,11 @@ void StakeMiner(CWallet *pwallet)
         CBlockIndex* pindexPrev = pindexBest;
         CBlock StakeBlock;
 
-        { LOCK(MinerStatus.lock);
+        {
+            LOCK(MinerStatus.lock);
+
             //clear miner messages
-            MinerStatus.ReasonNotStaking="";
+            MinerStatus.ClearReasonsNotStaking();
 
             //New versions
             if (IsV11Enabled(pindexPrev->nHeight + 1)) {
@@ -1240,11 +1271,15 @@ void StakeMiner(CWallet *pwallet)
             }
 
             MinerStatus.Version= StakeBlock.nVersion;
+
+            // This is needed due to early initialization of bitcoingui
+            miner_first_pass_complete = true;
         }
 
         if(!IsMiningAllowed(pwallet))
         {
             LOCK(MinerStatus.lock);
+
             MinerStatus.Clear();
             continue;
         }
@@ -1294,7 +1329,9 @@ void StakeMiner(CWallet *pwallet)
 
         LogPrintf("StakeMiner: signed boinchash, coinstake, wholeblock");
 
-        { LOCK(MinerStatus.lock);
+        {
+            LOCK(MinerStatus.lock);
+
             MinerStatus.CreatedCnt++;
         }
 
@@ -1306,7 +1343,10 @@ void StakeMiner(CWallet *pwallet)
         }
 
         LogPrintf("StakeMiner: block processed");
-        { LOCK(MinerStatus.lock);
+
+        {
+            LOCK(MinerStatus.lock);
+
             MinerStatus.AcceptedCnt++;
         }
 

src/miner.h

@@ -9,23 +9,7 @@
 #include "main.h"
 #include "wallet.h"
 
-struct CMinerStatus
-{
-    CCriticalSection lock;
-    std::string ReasonNotStaking;
-    uint64_t WeightSum,WeightMin,WeightMax;
-    double ValueSum;
-    int Version;
-    uint64_t CreatedCnt;
-    uint64_t AcceptedCnt;
-    uint64_t KernelsFound;
-    int64_t nLastCoinStakeSearchInterval;
-    double KernelDiffMax;
-    double KernelDiffSum;
-
-    void Clear();
-    CMinerStatus();
-};
+// struct CMinerStatus is in wallet.h to prevent a circular header reference issue
 
 typedef std::vector< std::pair<std::string, double> > SideStakeAlloc;
 

src/qt/bitcoin.qrc

@@ -71,6 +71,7 @@
         <file alias="notsynced">res/icons/notsynced.svg</file>
         <file alias="synced">res/icons/green_check.svg</file>
         <file alias="white_and_red_x">res/icons/white_and_red_x.svg</file>
+        <file alias="staking_unable">res/icons/staking_unable.svg</file>
     </qresource>
     <qresource prefix="/images">
         <file alias="splash">res/images/splash3.png</file>