Merge pull request #226 from instagibbs/mempoollock

Fix claimpegins from mempool locks

Author: instagibbs

qa/rpc-tests/pegging.py

@@ -100,7 +100,7 @@
     subprocess.Popen(sidechain2start.split(), stdout=subprocess.PIPE)
 
     print("Daemons started")
-    time.sleep(2)
+    time.sleep(3)
 
     bitcoin = AuthServiceProxy("http://bitcoinrpc:"+bitcoin_pass+"@127.0.0.1:"+str(bitcoin_port))
     sidechain = AuthServiceProxy("http://sidechainrpc:"+sidechain_pass+"@127.0.0.1:"+str(sidechain_port))
@@ -114,13 +114,17 @@
 
     # Lockup some funds to unlock later
     sidechain.sendtomainchain(addr, 50)
-    sidechain.generate(101)
+    # Tests withdrawlock tracking in database
+    sidechain.generate(1)
+    # Tests withdrawlock in mempool
+    sidechain.sendtomainchain(addr, 50)
 
     addrs = sidechain.getpeginaddress()
-    txid = bitcoin.sendtoaddress(addrs["mainchain_address"], 49)
+    txid1 = bitcoin.sendtoaddress(addrs["mainchain_address"], 24)
+    txid2 = bitcoin.sendtoaddress(addrs["mainchain_address"], 24)
     bitcoin.generate(10)
-    proof = bitcoin.gettxoutproof([txid])
-    raw = bitcoin.getrawtransaction(txid)
+    proof = bitcoin.gettxoutproof([txid1])
+    raw = bitcoin.getrawtransaction(txid1)
 
     print("Attempting peg-in")
 
@@ -132,8 +136,13 @@
         pass
 
     timeout = 20
-    # Should succeed via wallet lookup for address match
-    pegtxid = sidechain.claimpegin(raw, proof)
+    # Both should succeed via wallet lookup for address match, and when given
+    pegtxid1 = sidechain.claimpegin(raw, proof)
+
+    proof = bitcoin.gettxoutproof([txid2])
+    raw = bitcoin.getrawtransaction(txid2)
+    pegtxid2 = sidechain.claimpegin(raw, proof, addrs["sidechain_address"])
+
     while len(sidechain.getrawmempool()) != len(sidechain2.getrawmempool()):
         time.sleep(1)
         timeout -= 1
@@ -147,13 +156,42 @@
             raise Exception("Blocks are not propagating.")
 
 
-    tx = sidechain.gettransaction(pegtxid)
+    tx1 = sidechain.gettransaction(pegtxid1)
+    tx2 = sidechain.gettransaction(pegtxid2)
 
-    if "confirmations" in tx and tx["confirmations"] > 0:
+    if "confirmations" in tx1 and tx1["confirmations"] > 0 and "confirmations" in tx2 and tx2["confirmations"] > 0:
         print("Peg-in is confirmed: Success!")
     else:
         raise Exception("Peg-in confirmation has failed.")
 
+    # Make a few large locks, then do many claims in mempool
+    n_locks = 10
+    n_claims = 30
+
+    print("Flooding mempool with many small claims")
+    pegtxs = []
+    for i in range(n_locks):
+        # Lockup some funds to unlock later
+        sidechain.sendtomainchain(addr, 50)
+        sidechain.generate(1)
+    sidechain.generate(101)
+
+    for i in range(n_claims):
+        addrs = sidechain.getpeginaddress()
+        txid = bitcoin.sendtoaddress(addrs["mainchain_address"], 1)
+        bitcoin.generate(10)
+        proof = bitcoin.gettxoutproof([txid])
+        raw = bitcoin.getrawtransaction(txid)
+        pegtxs += [sidechain.claimpegin(raw, proof)]
+
+    sidechain.generate(1)
+    for pegtxid in pegtxs:
+        tx = sidechain.gettransaction(pegtxid)
+        if "confirmations" not in tx or tx["confirmations"] == 0:
+            raise Exception("Peg-in confirmation has failed.")
+
+    print("Success!")
+
 except JSONRPCException as e:
         print("Pegging testing failed, aborting:")
         print(e.error)

src/validation.cpp

@@ -1210,15 +1210,21 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
         CAmount inChainInputValue;
         double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue);
 
+        bool fIsWithdrawLockSpender = false;
+
         // Keep track of transactions that spend a coinbase, which we re-scan
         // during reorgs to ensure COINBASE_MATURITY is still met.
+        // Also track withdraw lock spends to allow them through free relay
         bool fSpendsCoinbase = false;
         BOOST_FOREACH(const CTxIn &txin, tx.vin) {
             const CCoins *coins = view.AccessCoins(txin.prevout.hash);
             if (coins->IsCoinBase()) {
                 fSpendsCoinbase = true;
                 break;
             }
+            if (coins->vout[txin.prevout.n].scriptPubKey.IsWithdrawLock()) {
+                fIsWithdrawLockSpender = true;
+            }
         }
 
         CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, dPriority, chainActive.Height(),
@@ -1237,32 +1243,12 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
         CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize);
         if (mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) {
             return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nFees, mempoolRejectFee));
-        } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nModifiedFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(entry.GetPriority(chainActive.Height() + 1))) {
-            // Require that free transactions have sufficient priority to be mined in the next block.
-            return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient priority");
         }
 
-        // Continuously rate-limit free (really, very-low-fee) transactions
-        // This mitigates 'penny-flooding' -- sending thousands of free transactions just to
-        // be annoying or make others' transactions take longer to confirm.
-        if (fLimitFree && nModifiedFees < ::minRelayTxFee.GetFee(nSize))
+        // No transactions are allowed below minRelayTxFee except from disconnected blocks and withdraw lock spends
+        if (fLimitFree && nModifiedFees < ::minRelayTxFee.GetFee(nSize) && !fIsWithdrawLockSpender)
         {
-            static CCriticalSection csFreeLimiter;
-            static double dFreeCount;
-            static int64_t nLastTime;
-            int64_t nNow = GetTime();
-
-            LOCK(csFreeLimiter);
-
-            // Use an exponentially decaying ~10-minute window:
-            dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime));
-            nLastTime = nNow;
-            // -limitfreerelay unit is thousand-bytes-per-minute
-            // At default rate it would take over a month to fill 1GB
-            if (dFreeCount + nSize >= GetArg("-limitfreerelay", DEFAULT_LIMITFREERELAY) * 10 * 1000)
-                return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "rate limited free transaction");
-            LogPrint("mempool", "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize);
-            dFreeCount += nSize;
+            return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "min relay fee not met");
         }
 
         if (nAbsurdFee && nFees > nAbsurdFee)
@@ -1605,8 +1591,10 @@ bool GetLockedOutputs(const uint256 &genesisHash, const CAmount &nAmount, std::v
 
     std::vector<std::pair<COutPoint, CAmount> > locksCreated;
     bool locksChanged = false;
-    if (!pblocktree->ReadLocksCreated(genesisHash, locksCreated))
-        return false;
+    // If this fails, we should still try to grab from mempool.
+    if (!pblocktree->ReadLocksCreated(genesisHash, locksCreated)) {
+        locksCreated.clear();
+    }
 
     //For faster random esasure
     std::list<std::pair<COutPoint, CAmount> > locksList;
@@ -1646,8 +1634,9 @@ bool GetLockedOutputs(const uint256 &genesisHash, const CAmount &nAmount, std::v
         pblocktree->ReWriteLocksCreated(genesisHash, vChangedLocks);
     }
     //Found single lock large enough
-    if (res.size())
+    if (res.size()) {
         return true;
+    }
 
     CAmount nTotal = 0;
     //Gather up smaller locked outputs for aggregation.
@@ -1660,8 +1649,9 @@ bool GetLockedOutputs(const uint256 &genesisHash, const CAmount &nAmount, std::v
             continue;
         }
 
-        if (mempool.mapNextTx.count(COutPoint(it->first.hash, it->first.n)))
+        if (mempool.mapNextTx.count(COutPoint(it->first.hash, it->first.n))) {
             continue;
+        }
 
         assert(coins.vout[it->first.n].nValue.IsExplicit() && coins.vout[it->first.n].nValue.GetAmount() == it->second);
         res.push_back(*it);
@@ -1689,20 +1679,28 @@ bool GetLockedOutputs(const uint256 &genesisHash, const CAmount &nAmount, std::v
             const CTransaction& tx = *ptx;
             for (unsigned int j = 0; j < tx.vout.size() && nTotal < nAmount; j++) {
                 CTxOut txout = tx.vout[j];
-                if (!txout.scriptPubKey.IsWithdrawLock())
+                if (!txout.scriptPubKey.IsWithdrawLock()) {
                     continue;
+                }
 
                 uint256 withdrawGenHash = txout.scriptPubKey.GetWithdrawLockGenesisHash();
-                if (genesisHash != withdrawGenHash)
+                if (genesisHash != withdrawGenHash) {
                     continue;
-                if (mempool.mapWithdrawsSpentToTxid.count(std::make_pair(withdrawGenHash, COutPoint(tx.GetHash(), j))))
+                }
+
+                // Only written locks are filtered previously for invalid type
+                if (txout.nValue.IsCommitment() || txout.nAsset.IsCommitment() || txout.nAsset.GetAsset() != BITCOINID) {
+                    continue;
+                }
+
+                if (mempool.mapNextTx.count(COutPoint(tx.GetHash(), j))) {
                     continue;
+                }
 
-                if (txout.scriptPubKey.IsWithdrawLock() && txout.nValue.IsExplicit()) {
-                    res.push_back(std::make_pair(COutPoint(tx.GetHash(), j), txout.nValue.GetAmount()));
-                    nTotal += txout.nValue.GetAmount();
-                    if (nTotal >= nAmount)
-                        return true;
+                res.push_back(std::make_pair(COutPoint(tx.GetHash(), j), txout.nValue.GetAmount()));
+                nTotal += txout.nValue.GetAmount();
+                if (nTotal >= nAmount) {
+                    return true;
                 }
             }
        }
@@ -1712,8 +1710,9 @@ bool GetLockedOutputs(const uint256 &genesisHash, const CAmount &nAmount, std::v
     //res list is already randomized
     nTotal = 0;
     size_t i = 0;
-    for (; i < res.size() && nTotal < nAmount; i++)
+    for (; i < res.size() && nTotal < nAmount; i++) {
         nTotal += res[i].second;
+    }
 
     res.resize(i);
     return true;