Auto merge of #1895 - bitcartel:1857_tx_priority, r=str4d

Closes #1857. Fixes bug where a transaction sending from a zaddr would have a priority of zero.

Transactions sent from a zaddr should now be mined sooner as they no longer have a priority of zero

qa/rpc-tests/wallet_protectcoinbase.py

@@ -23,6 +23,7 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework):
         self.is_network_split=False
         self.sync_all()
 
+    # Returns txid if operation was a success or None
     def wait_and_assert_operationid_status(self, myopid, in_status='success', in_errormsg=None):
         print('waiting for async operation {}'.format(myopid))
         opids = []
@@ -30,6 +31,7 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework):
         timeout = 120
         status = None
         errormsg = None
+        txid = None
         for x in xrange(1, timeout):
             results = self.nodes[0].z_getoperationresult(opids)
             if len(results)==0:
@@ -38,6 +40,8 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework):
                 status = results[0]["status"]
                 if status == "failed":
                     errormsg = results[0]['error']['message']
+                elif status == "success":
+                    txid = results[0]['result']['txid']
                 break
         print('...returned status: {}'.format(status))
         assert_equal(in_status, status)
@@ -45,6 +49,7 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework):
             assert(in_errormsg is not None)
             assert_equal(in_errormsg in errormsg, True)
             print('...returned error: {}'.format(errormsg))
+        return txid
 
     def run_test (self):
         print "Mining blocks..."
@@ -116,8 +121,14 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework):
         recipients = []
         recipients.append({"address":mytaddr, "amount":Decimal('10.0')})
         myopid = self.nodes[0].z_sendmany(myzaddr, recipients)
-        self.wait_and_assert_operationid_status(myopid)
+        mytxid = self.wait_and_assert_operationid_status(myopid)
+        assert(mytxid is not None)
         self.sync_all()
+
+        # check that priority of the tx sending from a zaddr is not 0
+        mempool = self.nodes[0].getrawmempool(True)
+        assert(Decimal(mempool[mytxid]['startingpriority']) >= Decimal('1000000000000'))
+
         self.nodes[1].generate(1)
         self.sync_all()
 

src/coins.cpp

@@ -6,6 +6,7 @@
 
 #include "memusage.h"
 #include "random.h"
+#include "version.h"
 
 #include <assert.h>
 
@@ -442,6 +443,7 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const
 {
     if (tx.IsCoinBase())
         return 0.0;
+    CAmount nTotalIn = 0;
     double dResult = 0.0;
     BOOST_FOREACH(const CTxIn& txin, tx.vin)
     {
@@ -450,8 +452,34 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const
         if (!coins->IsAvailable(txin.prevout.n)) continue;
         if (coins->nHeight < nHeight) {
             dResult += coins->vout[txin.prevout.n].nValue * (nHeight-coins->nHeight);
+            nTotalIn += coins->vout[txin.prevout.n].nValue;
         }
     }
+
+    // If a transaction contains a joinsplit, we boost the priority of the transaction.
+    // Joinsplits do not reveal any information about the value or age of a note, so we
+    // cannot apply the priority algorithm used for transparent utxos.  Instead, we pick a
+    // very large number and multiply it by the transaction's fee per 1000 bytes of data.
+    // One trillion, 1000000000000, is equivalent to 1 ZEC utxo * 10000 blocks (~17 days).
+    if (tx.vjoinsplit.size() > 0) {
+        unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
+        nTotalIn += tx.GetJoinSplitValueIn();
+        CAmount fee = nTotalIn - tx.GetValueOut();
+        CFeeRate feeRate(fee, nTxSize);
+        CAmount feePerK = feeRate.GetFeePerK();
+
+        if (feePerK == 0) {
+            feePerK = 1;
+        }
+
+        dResult += 1000000000000 * double(feePerK);
+        // We cast feePerK from int64_t to double because if feePerK is a large number, say
+        // close to MAX_MONEY, the multiplication operation will result in an integer overflow.
+        // The variable dResult should never overflow since a 64-bit double in C++ is typically
+        // a double-precision floating-point number as specified by IEE 754, with a maximum
+        // value DBL_MAX of 1.79769e+308.
+    }
+
     return tx.ComputePriority(dResult);
 }