Added mapSerials consensus rules to prohibit double-spending.

qa/pull-tester/rpc-tests.sh

@@ -28,6 +28,7 @@ testScripts=(
     'signrawtransactions.py'
     'walletbackup.py'
     'zcpour.py'
+    'zcpourdoublespend.py'
 );
 testScriptsExt=(
     'bipdersig-p2p.py'

qa/rpc-tests/zcpourdoublespend.py

@@ -0,0 +1,135 @@
+#!/usr/bin/env python2
+
+#
+# Tests a Pour double-spend and a subsequent reorg.
+#
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import *
+from decimal import Decimal
+import os
+import shutil
+import sys
+
+class PourTxTest(BitcoinTestFramework):
+    def setup_network(self):
+        # Start with split network:
+        return super(PourTxTest, self).setup_network(True)
+
+    def expect_cannot_pour(self, node, txn):
+        exception_triggered = False
+
+        try:
+            node.sendrawtransaction(txn)
+        except JSONRPCException:
+            exception_triggered = True
+
+        assert_equal(exception_triggered, True)
+
+    def run_test(self):
+        # All nodes should start with 1,250 BTC:
+        starting_balance = 1250
+        for i in range(4):
+            assert_equal(self.nodes[i].getbalance(), starting_balance)
+            self.nodes[i].getnewaddress("")  # bug workaround, coins generated assigned to first getnewaddress!
+        
+        # Generate zcaddress keypairs
+        zckeypair = self.nodes[0].zcrawkeygen()
+        zcsecretkey = zckeypair["zcsecretkey"]
+        zcaddress = zckeypair["zcaddress"]
+
+        pool = [0, 1, 2, 3]
+        for i in range(4):
+            (total_in, inputs) = gather_inputs(self.nodes[i], 50)
+            pool[i] = self.nodes[i].createrawtransaction(inputs, {})
+            pool[i] = self.nodes[i].zcrawpour(pool[i], {}, {zcaddress:49.9}, 50, 0.1)
+            signed = self.nodes[i].signrawtransaction(pool[i]["rawtxn"])
+            self.nodes[0].sendrawtransaction(signed["hex"])
+            self.nodes[0].generate(1)
+            self.nodes[1].sendrawtransaction(signed["hex"])
+            self.nodes[1].generate(1)
+            pool[i] = pool[i]["encryptedbucket1"]
+
+        # Confirm that the protects have taken place
+        for i in range(4):
+            enc_bucket = pool[i]
+            receive_result = self.nodes[0].zcrawreceive(zcsecretkey, enc_bucket)
+            assert_equal(receive_result["exists"], True)
+            pool[i] = receive_result["bucket"]
+
+            # Extra confirmation on Node 1
+            receive_result = self.nodes[1].zcrawreceive(zcsecretkey, enc_bucket)
+            assert_equal(receive_result["exists"], True)
+
+        blank_tx = self.nodes[0].createrawtransaction([], {})
+        # Create pour {A, B}->{*}
+        pour_AB = self.nodes[0].zcrawpour(blank_tx,
+                                          {pool[0] : zcsecretkey, pool[1] : zcsecretkey},
+                                          {zcaddress:(49.9*2)-0.1},
+                                          0, 0.1)
+
+        # Create pour {B, C}->{*}
+        pour_BC = self.nodes[0].zcrawpour(blank_tx,
+                                          {pool[1] : zcsecretkey, pool[2] : zcsecretkey},
+                                          {zcaddress:(49.9*2)-0.1},
+                                          0, 0.1)
+
+        # Create pour {C, D}->{*}
+        pour_CD = self.nodes[0].zcrawpour(blank_tx,
+                                          {pool[2] : zcsecretkey, pool[3] : zcsecretkey},
+                                          {zcaddress:(49.9*2)-0.1},
+                                          0, 0.1)
+
+        # Create pour {A, D}->{*}
+        pour_AD = self.nodes[0].zcrawpour(blank_tx,
+                                          {pool[0] : zcsecretkey, pool[3] : zcsecretkey},
+                                          {zcaddress:(49.9*2)-0.1},
+                                          0, 0.1)
+
+        # (a)    Node 1 will spend pour AB, then attempt to
+        # double-spend it with BC. It should fail before and
+        # after Node 1 mines blocks.
+        #
+        # (b)    Then, Node 2 will spend BC, and mine 5 blocks.
+        # Node 1 connects, and AB will be reorg'd from the chain.
+        # Any attempts to spend AB or CD should fail for
+        # both nodes.
+        #
+        # (c)    Then, Node 1 will spend AD, which should work
+        # because the previous spend for A (AB) is considered
+        # invalid.
+
+        # (a)
+
+        self.nodes[0].sendrawtransaction(pour_AB["rawtxn"])
+
+        self.expect_cannot_pour(self.nodes[0], pour_BC["rawtxn"])
+
+        # Generate a block
+        self.nodes[0].generate(1)
+
+        self.expect_cannot_pour(self.nodes[0], pour_BC["rawtxn"])
+
+        # (b)
+        self.nodes[1].sendrawtransaction(pour_BC["rawtxn"])
+        self.nodes[1].generate(5)
+
+        # Connect the two nodes
+
+        connect_nodes(self.nodes[1], 0)
+        sync_blocks(self.nodes[0:2])
+
+        # AB, BC, CD should all be impossible to spend for each node.
+        self.expect_cannot_pour(self.nodes[0], pour_AB["rawtxn"])
+        self.expect_cannot_pour(self.nodes[0], pour_CD["rawtxn"])
+
+        self.expect_cannot_pour(self.nodes[1], pour_AB["rawtxn"])
+        self.expect_cannot_pour(self.nodes[1], pour_CD["rawtxn"])
+
+        # (c)
+
+        self.nodes[0].sendrawtransaction(pour_AD["rawtxn"])
+        self.nodes[0].generate(1)
+
+if __name__ == '__main__':
+    PourTxTest().main()

src/coins.cpp

@@ -391,6 +391,15 @@ bool CCoinsViewCache::HavePourRequirements(const CTransaction& tx) const
 {
     BOOST_FOREACH(const CPourTx &pour, tx.vpour)
     {
+        BOOST_FOREACH(const uint256& serial, pour.serials)
+        {
+            if (GetSerial(serial)) {
+                // If the serial is set, this transaction
+                // double-spends!
+                return false;
+            }
+        }
+
         libzerocash::IncrementalMerkleTree tree(INCREMENTAL_MERKLE_TREE_DEPTH);
         if (!GetAnchorAt(pour.anchor, tree)) {
             // If we do not have the anchor for the pour,

src/init.cpp

@@ -47,6 +47,8 @@
 #include <boost/thread.hpp>
 #include <openssl/crypto.h>
 
+#include "libsnark/common/profiling.hpp"
+
 using namespace std;
 
 libzerocash::ZerocashParams *pzerocashParams = NULL;
@@ -1266,6 +1268,11 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     // ********************************************************* Step 7i: Load zcash params
     ZC_LoadParams();
 
+    // These must be disabled for now, they are buggy and we probably don't
+    // want any of libsnark's profiling in production anyway.
+    libsnark::inhibit_profiling_info = true;
+    libsnark::inhibit_profiling_counters = true;
+
     // ********************************************************* Step 8: load wallet
 #ifdef ENABLE_WALLET
     if (fDisableWallet) {

src/main.cpp

@@ -1036,6 +1036,14 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
             return false;
         }
     }
+    BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+        BOOST_FOREACH(const uint256 &serial, pour.serials) {
+            if (pool.mapSerials.count(serial))
+            {
+                return false;
+            }
+        }
+    }
     }
 
     {
@@ -1491,6 +1499,13 @@ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCach
         }
     }
 
+    // spend serials
+    BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+        BOOST_FOREACH(const uint256 &serial, pour.serials) {
+            inputs.SetSerial(serial, true);
+        }
+    }
+
     // add outputs
     inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight);
 }
@@ -1772,6 +1787,13 @@ bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex
         outs->Clear();
         }
 
+        // unspend serials
+        BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+            BOOST_FOREACH(const uint256 &serial, pour.serials) {
+                view.SetSerial(serial, false);
+            }
+        }
+
         // restore inputs
         if (i > 0) { // not coinbases
             const CTxUndo &txundo = blockUndo.vtxundo[i-1];

src/txdb.cpp

@@ -93,10 +93,7 @@ bool CCoinsViewDB::GetSerial(const uint256 &serial) const {
     bool spent = false;
     bool read = db.Read(make_pair(DB_SERIAL, serial), spent);
 
-    // We should never read false from the database.
+    return read;
-    assert(spent != read);
-
-    return spent;
 }
 
 bool CCoinsViewDB::GetCoins(const uint256 &txid, CCoins &coins) const {

src/txmempool.cpp

@@ -99,6 +99,11 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
     const CTransaction& tx = mapTx[hash].GetTx();
     for (unsigned int i = 0; i < tx.vin.size(); i++)
         mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i);
+    BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+        BOOST_FOREACH(const uint256 &serial, pour.serials) {
+            mapSerials[serial] = &tx;
+        }
+    }
     nTransactionsUpdated++;
     totalTxSize += entry.GetTxSize();
     minerPolicyEstimator->processTransaction(entry, fCurrentEstimate);
@@ -143,6 +148,11 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list<CTransaction>& rem
             }
             BOOST_FOREACH(const CTxIn& txin, tx.vin)
                 mapNextTx.erase(txin.prevout);
+            BOOST_FOREACH(const CPourTx& pour, tx.vpour) {
+                BOOST_FOREACH(const uint256& serial, pour.serials) {
+                    mapSerials.erase(serial);
+                }
+            }
 
             removed.push_back(tx);
             totalTxSize -= mapTx[hash].GetTxSize();
@@ -219,6 +229,19 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction>
             }
         }
     }
+
+    BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+        BOOST_FOREACH(const uint256 &serial, pour.serials) {
+            std::map<uint256, const CTransaction*>::iterator it = mapSerials.find(serial);
+            if (it != mapSerials.end()) {
+                const CTransaction &txConflict = *it->second;
+                if (txConflict != tx)
+                {
+                    remove(txConflict, removed, true);
+                }
+            }
+        }
+    }
 }
 
 /**
@@ -291,6 +314,15 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
             assert(it3->second.n == i);
             i++;
         }
+        BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+            BOOST_FOREACH(const uint256 &serial, pour.serials) {
+                assert(!pcoins->GetSerial(serial));
+            }
+
+            // TODO: chained pours
+            libzerocash::IncrementalMerkleTree tree(INCREMENTAL_MERKLE_TREE_DEPTH);
+            assert(pcoins->GetAnchorAt(pour.anchor, tree));
+        }
         if (fDependsWait)
             waitingOnDependants.push_back(&it->second);
         else {
@@ -324,6 +356,14 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
         assert(it->first == it->second.ptx->vin[it->second.n].prevout);
     }
 
+    for (std::map<uint256, const CTransaction*>::const_iterator it = mapSerials.begin(); it != mapSerials.end(); it++) {
+        uint256 hash = it->second->GetHash();
+        map<uint256, CTxMemPoolEntry>::const_iterator it2 = mapTx.find(hash);
+        const CTransaction& tx = it2->second.GetTx();
+        assert(it2 != mapTx.end());
+        assert(&tx == it->second);
+    }
+
     assert(totalTxSize == checkTotal);
 }
 
@@ -430,6 +470,13 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const
 
 CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { }
 
+bool CCoinsViewMemPool::GetSerial(const uint256 &serial) const {
+    if (mempool.mapSerials.count(serial))
+        return true;
+
+    return base->GetSerial(serial);
+}
+
 bool CCoinsViewMemPool::GetCoins(const uint256 &txid, CCoins &coins) const {
     // If an entry in the mempool exists, always return that one, as it's guaranteed to never
     // conflict with the underlying cache, and it cannot have pruned entries (as it contains full)

src/txmempool.h

@@ -98,6 +98,7 @@ public:
     mutable CCriticalSection cs;
     std::map<uint256, CTxMemPoolEntry> mapTx;
     std::map<COutPoint, CInPoint> mapNextTx;
+    std::map<uint256, const CTransaction*> mapSerials;
     std::map<uint256, std::pair<double, CAmount> > mapDeltas;
 
     CTxMemPool(const CFeeRate& _minRelayFee);
@@ -176,6 +177,7 @@ protected:
 
 public:
     CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn);
+    bool GetSerial(const uint256 &txid) const;
     bool GetCoins(const uint256 &txid, CCoins &coins) const;
     bool HaveCoins(const uint256 &txid) const;
 };