Remove dummy confirmations in RPC API and GUI for InstantSend transactions (#2040)

* remove instandsenddepth option

* remove CInstantSend::GetConfirmations

* Explicit IXlocks processing in the code using GetDepthInMainChain

* remove dummy confirmations for IX locks

* remove unused constant

* Track IS status in TransactionStatus

* Highlight IS in tx list

* Customize confirmation icons for IS txes

* Fix issues:

- bring back removed comment;
- simplify 2 complex condition to ose `IsTrusted()` function only;
- rename `IsLockedIX` function to 'IsLockedByInstantSend`;

* Revert condition with IsTrusted()

* code style fix

* remove dummy confirmations from instantsend.md

* fix instantsend.md

Author: gladcow

doc/instantsend.md

@@ -25,56 +25,4 @@ When a wallet InstantSend transaction is successfully locked a shell command pro
 
 #### RPC
 
-Details pertaining to an observed "Transaction Lock" can also be retrieved through RPC, it’s important however to understand the underlying mechanism.
-
-By default, the Dash Core daemon will launch using the following constant:
-
-```
-static const int DEFAULT_INSTANTSEND_DEPTH = 5;
-```
-
-This value can be overridden by passing the following argument to the Dash Core daemon:
-
-```
--instantsenddepth=<n>
-```
-
-The key thing to understand is that this value indicates the number of "confirmations" a successful Transaction Lock represents. When Wallet RPC commands which support `minconf` and `addlockconf` parameters (such as `listreceivedbyaddress`) are performed and `addlockconf` is `true`, then `instantsenddepth` attribute is taken into account when returning information about the transaction. In this case the value in `confirmations` field you see through RPC is showing the number of `"Blockchain Confirmations" + "InstantSend Depth"` (assuming the funds were sent via InstantSend).
-
-There is also a field named `instantlock` (that is present in commands such as `listsinceblock`). The value in this field indicates whether a given transaction is locked via InstantSend.
-
-**Examples**
-
-1. `listreceivedbyaddress 0 true`
-   * InstantSend transaction just occurred:
-        * confirmations: 5
-   * InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 6
-   * non-InstantSend transaction just occurred:
-        * confirmations: 0
-   * non-InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 1
-
-2. `listreceivedbyaddress 0`
-   * InstantSend transaction just occurred:
-        * confirmations: 0
-   * InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 1
-   * non-InstantSend transaction just occurred:
-        * confirmations: 0
-   * non-InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 1
-
-3. `listsinceblock`
-    * InstantSend transaction just occurred:
-        * confirmations: 0
-        * instantlock: true
-    * InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 1
-        * instantlock: true
-    * non-InstantSend transaction just occurred:
-        * confirmations: 0
-        * instantlock: false
-    * non-InstantSend transaction received one confirmation from blockchain:
-        * confirmations: 1
-        * instantlock: false
+Details pertaining to an observed "Transaction Lock" can also be retrieved through RPC. There is a boolean field named `instantlock` which indicates whether a given transaction is locked via InstantSend. This field is present in the output of some wallet RPC commands e.g. `listsinceblock`, `gettransaction` etc. as well as in the output of some mempool RPC commands e.g. `getmempoolentry` and a couple of others like `getrawmempool` (for `verbose=true` only).

src/governance-object.cpp

@@ -605,7 +605,7 @@ bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingC
     // GET CONFIRMATIONS FOR TRANSACTION
 
     AssertLockHeld(cs_main);
-    int nConfirmationsIn = instantsend.GetConfirmations(nCollateralHash);
+    int nConfirmationsIn = 0;
     if (nBlockHash != uint256()) {
         BlockMap::iterator mi = mapBlockIndex.find(nBlockHash);
         if (mi != mapBlockIndex.end() && (*mi).second) {
@@ -616,7 +616,8 @@ bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingC
         }
     }
 
-    if(nConfirmationsIn < GOVERNANCE_FEE_CONFIRMATIONS) {
+    if((nConfirmationsIn < GOVERNANCE_FEE_CONFIRMATIONS)  &&
+       (!instantsend.IsLockedInstantSendTransaction(nCollateralHash))){
         strError = strprintf("Collateral requires at least %d confirmations to be relayed throughout the network (it has only %d)", GOVERNANCE_FEE_CONFIRMATIONS, nConfirmationsIn);
         if (nConfirmationsIn >= GOVERNANCE_MIN_RELAY_FEE_CONFIRMATIONS) {
             fMissingConfirmations = true;

src/init.cpp

@@ -576,7 +576,6 @@ std::string HelpMessage(HelpMessageMode mode)
 
     strUsage += HelpMessageGroup(_("InstantSend options:"));
     strUsage += HelpMessageOpt("-enableinstantsend=<n>", strprintf(_("Enable InstantSend, show confirmations for locked transactions (0-1, default: %u)"), 1));
-    strUsage += HelpMessageOpt("-instantsenddepth=<n>", strprintf(_("Show N confirmations for a successfully locked transaction (%u-%u, default: %u)"), MIN_INSTANTSEND_DEPTH, MAX_INSTANTSEND_DEPTH, DEFAULT_INSTANTSEND_DEPTH));
     strUsage += HelpMessageOpt("-instantsendnotify=<cmd>", _("Execute command when a wallet InstantSend transaction is successfully locked (%s in cmd is replaced by TxID)"));
 
 
@@ -1873,11 +1872,8 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
 #endif // ENABLE_WALLET
 
     fEnableInstantSend = GetBoolArg("-enableinstantsend", 1);
-    nInstantSendDepth = GetArg("-instantsenddepth", DEFAULT_INSTANTSEND_DEPTH);
-    nInstantSendDepth = std::min(std::max(nInstantSendDepth, MIN_INSTANTSEND_DEPTH), MAX_INSTANTSEND_DEPTH);
 
     LogPrintf("fLiteMode %d\n", fLiteMode);
-    LogPrintf("nInstantSendDepth %d\n", nInstantSendDepth);
 #ifdef ENABLE_WALLET
     LogPrintf("PrivateSend liquidityprovider: %d\n", privateSendClient.nLiquidityProvider);
     LogPrintf("PrivateSend rounds: %d\n", privateSendClient.nPrivateSendRounds);

src/instantx.cpp

@@ -33,7 +33,6 @@ extern CWallet* pwalletMain;
 extern CTxMemPool mempool;
 
 bool fEnableInstantSend = true;
-int nInstantSendDepth = DEFAULT_INSTANTSEND_DEPTH;
 int nCompleteTXLocks;
 
 CInstantSend instantsend;
@@ -838,11 +837,6 @@ int CInstantSend::GetTransactionLockSignatures(const uint256& txHash)
     return -1;
 }
 
-int CInstantSend::GetConfirmations(const uint256 &nTXHash)
-{
-    return IsLockedInstantSendTransaction(nTXHash) ? nInstantSendDepth : 0;
-}
-
 bool CInstantSend::IsTxLockCandidateTimedOut(const uint256& txHash)
 {
     if(!fEnableInstantSend) return false;

src/instantx.h

@@ -26,14 +26,6 @@ extern CInstantSend instantsend;
     (1000/2900.0)**5 = 0.004875397277841433
 */
 
-// The INSTANTSEND_DEPTH is the "pseudo block depth" level assigned to locked
-// txs to indicate the degree of confidence in their eventual confirmation and
-// inability to be double-spent (adjustable via command line argument)
-static const int MIN_INSTANTSEND_DEPTH              = 0;
-static const int MAX_INSTANTSEND_DEPTH              = 60;
-/// Default number of "pseudo-confirmations" for an InstantSend tx
-static const int DEFAULT_INSTANTSEND_DEPTH          = 5;
-
 static const int MIN_INSTANTSEND_PROTO_VERSION      = 70208;
 
 /// For how long we are going to accept votes/locks
@@ -145,8 +137,6 @@ class CInstantSend
     bool IsLockedInstantSendTransaction(const uint256& txHash);
     /// Get the actual number of accepted lock signatures
     int GetTransactionLockSignatures(const uint256& txHash);
-    /// Get instantsend confirmations (only)
-    int GetConfirmations(const uint256 &nTXHash);
 
     /// Remove expired entries from maps
     void CheckAndRemove();

src/qt/guiconstants.h

@@ -34,6 +34,8 @@ static const bool DEFAULT_SPLASHSCREEN = true;
 #define COLOR_TX_STATUS_DANGER QColor(200, 100, 100)
 /* Transaction list -- TX status decoration - default color */
 #define COLOR_BLACK QColor(0, 0, 0)
+/* Transaction list -- TX status decoration - LockedByInstantSend color */
+#define COLOR_TX_STATUS_LOCKED QColor(0, 128, 255)
 
 /* Tooltips longer than this (in characters) are converted into rich text,
    so that they can be word-wrapped.

src/qt/transactionrecord.cpp

@@ -291,6 +291,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx)
     }
     else
     {
+        status.lockedByInstantSend = wtx.IsLockedByInstantSend();
         if (status.depth < 0)
         {
             status.status = TransactionStatus::Conflicted;

src/qt/transactionrecord.h

@@ -42,6 +42,8 @@ class TransactionStatus
 
     /// Transaction counts towards available balance
     bool countsForBalance;
+    /// Transaction was locked via InstantSend
+    bool lockedByInstantSend;
     /// Sorting key based on status
     std::string sortKey;
 

src/qt/transactiontablemodel.cpp

@@ -484,6 +484,24 @@ QString TransactionTableModel::formatTxAmount(const TransactionRecord *wtx, bool
     return QString(str);
 }
 
+QIcon IconWithShiftedColor(const QString& filename, bool shift)
+{
+    if (!shift)
+        return QIcon(filename);
+
+    QImage img(filename);
+    img = img.convertToFormat(QImage::Format_ARGB32);
+    for (int x = img.width(); x--; )
+    {
+        for (int y = img.height(); y--; )
+        {
+            const QRgb rgb = img.pixel(x, y);
+            img.setPixel(x, y, qRgba(qRed(rgb), qGreen(rgb), qBlue(rgb)+128, qAlpha(rgb)));
+        }
+    }
+    return QIcon(QPixmap::fromImage(img));
+}
+
 QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx) const
 {
     QString theme = GUIUtil::getThemeName();
@@ -495,17 +513,19 @@ QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx)
     case TransactionStatus::Offline:
         return COLOR_TX_STATUS_OFFLINE;
     case TransactionStatus::Unconfirmed:
+        // TODO: use special icon for InstantSend 0-conf
         return QIcon(":/icons/" + theme + "/transaction_0");
     case TransactionStatus::Abandoned:
         return QIcon(":/icons/" + theme + "/transaction_abandoned");
     case TransactionStatus::Confirming:
         switch(wtx->status.depth)
         {
-        case 1: return QIcon(":/icons/" + theme + "/transaction_1");
-        case 2: return QIcon(":/icons/" + theme + "/transaction_2");
-        case 3: return QIcon(":/icons/" + theme + "/transaction_3");
-        case 4: return QIcon(":/icons/" + theme + "/transaction_4");
-        default: return QIcon(":/icons/" + theme + "/transaction_5");
+        // TODO: use special icons for InstantSend instead of color shifting
+        case 1: return IconWithShiftedColor(":/icons/" + theme + "/transaction_1", wtx->status.lockedByInstantSend);
+        case 2: return IconWithShiftedColor(":/icons/" + theme + "/transaction_2", wtx->status.lockedByInstantSend);
+        case 3: return IconWithShiftedColor(":/icons/" + theme + "/transaction_3", wtx->status.lockedByInstantSend);
+        case 4: return IconWithShiftedColor(":/icons/" + theme + "/transaction_4", wtx->status.lockedByInstantSend);
+        default: return IconWithShiftedColor(":/icons/" + theme + "/transaction_5", wtx->status.lockedByInstantSend);
         };
     case TransactionStatus::Confirmed:
         return QIcon(":/icons/" + theme + "/transaction_confirmed");
@@ -608,6 +628,10 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
         {
             return COLOR_TX_STATUS_DANGER;
         }
+        if(rec->status.lockedByInstantSend)
+        {
+            return COLOR_TX_STATUS_LOCKED;
+        }
         // Non-confirmed (but not immature) as transactions are grey
         if(!rec->status.countsForBalance && rec->status.status != TransactionStatus::Immature)
         {

src/qt/walletmodel.cpp

@@ -754,7 +754,7 @@ bool WalletModel::transactionCanBeAbandoned(uint256 hash) const
 {
     LOCK2(cs_main, wallet->cs_wallet);
     const CWalletTx *wtx = wallet->GetWalletTx(hash);
-    if (!wtx || wtx->isAbandoned() || wtx->GetDepthInMainChain() > 0 || wtx->InMempool())
+    if (!wtx || wtx->isAbandoned() || wtx->GetDepthInMainChain() > 0 || wtx->IsLockedByInstantSend() || wtx->InMempool())
         return false;
     return true;
 }