NEW: blacklisted tx inputs are rejected

Author: andvgal

qa/pull-tester/rpc-tests.py

@@ -179,6 +179,7 @@
     'pos-load.py',
     'pos-reindex.py',
     'spork-checkpoints.py',
+    'spork-blacklist.py',
 ]
 if ENABLE_ZMQ:
     testScripts.append('zmq_test.py')

qa/rpc-tests/spork-blacklist.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+# Copyright (c) 2019 The Energi Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import *
+import logging
+
+class SporkBlacklistTest(BitcoinTestFramework):
+    def __init__(self):
+        super().__init__()
+        self.setup_clean_chain = True
+
+    def advance_time(self):
+        # Spork override requires new time
+        set_mocktime(get_mocktime() + 30)
+        set_node_times(self.nodes, get_mocktime())
+
+    def run_test(self):
+        self.nodes[0].generate(101)
+        sync_chain(self.nodes, timeout=10)
+
+        addr0 = self.nodes[0].getaccountaddress("")
+        addr1 = self.nodes[1].getaccountaddress("")
+
+        logging.info("Sending OK")
+        self.nodes[0].sendtoaddress(addr1, 2)
+        self.nodes[0].generate(1)
+
+        logging.info("Issuing blacklist")
+        sync_chain(self.nodes, timeout=10)
+        self.nodes[1].sendtoaddress(addr0, Decimal("0.5"))
+        self.nodes[1].spork('blacklist', get_mocktime(), addr1)
+
+        logging.info("Failed send")
+        assert_raises_jsonrpc(None, "Error: The transaction was rejected! Reason given: blacklisted-input",
+                              self.nodes[1].sendtoaddress, addr0, Decimal("0.7"))
+        self.nodes[0].sendtoaddress(addr1, 2)
+        sync_mempools(self.nodes, timeout=10)
+        self.nodes[0].generate(1)
+
+        sync_chain(self.nodes, timeout=10)
+        assert_equal(self.nodes[1].getbalance(), 4)
+
+        logging.info("Disabling blacklist")
+        self.advance_time()
+        self.nodes[1].spork('blacklist', get_mocktime()+1, addr1)
+        self.nodes[1].sendtoaddress(addr0, 1)
+        sync_mempools(self.nodes, timeout=10)
+        self.nodes[0].generate(1)
+
+        sync_chain(self.nodes, timeout=10)
+        assert_greater_than(3, self.nodes[1].getbalance())
+
+
+if __name__ == '__main__':
+    SporkBlacklistTest().main()

src/protocol.cpp

@@ -71,7 +71,7 @@ const char *MNGOVERNANCEOBJECT="govobj";
 const char *MNGOVERNANCEOBJECTVOTE="govobjvote";
 const char *MNVERIFY="mnv";
 const char *CHECKPOINT="chkp";
-const char *BLACKLIST="black";
+const char *BLACKLIST="bll";
 };
 
 static const char* ppszTypeName[] =

src/rpc/misc.cpp

@@ -32,6 +32,8 @@
 
 #include <univalue.h>
 
+extern void ScriptPubKeyToJSON(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
+
 /**
  * @note Do not add or change anything in the information returned by this
  * method. `getinfo` exists for backwards-compatibility only. It combines
@@ -268,7 +270,7 @@ UniValue spork(const JSONRPCRequest& request)
                 auto& bl = item.second;
 
                 UniValue ret_item(UniValue::VOBJ);
-                ret_item.push_back(Pair("script", HexStr(bl.scriptPubKey)));
+                ScriptPubKeyToJSON(bl.scriptPubKey, ret_item, true);
                 ret_item.push_back(Pair("since", int64_t(bl.nTimeSince)));
                 ret_item.push_back(Pair("expires_in", int64_t(bl.nTimeSigned + CSporkBlacklist::MAX_AGE) - curr_time));
                 ret.push_back(ret_item);
@@ -355,7 +357,18 @@ UniValue spork(const JSONRPCRequest& request)
         }
 
         auto since = request.params[1].get_int64();
-        auto script = CScript(ParseHex(request.params[2].get_str()));
+        CScript script;
+
+        auto raw_dst = request.params[2].get_str();
+        auto addr = CBitcoinAddress(raw_dst);
+
+        if (addr.IsValid()) {
+            script = GetScriptForDestination(addr.Get());
+        } else {
+            auto raw_script = ParseHex(raw_dst);
+            // NOTE: another explicit c-tor will act as stream write!
+            script = CScript(raw_script.begin(), raw_script.end());
+        }
 
         if(sporkManager.UpdateBlacklist(script, since, *g_connman)){
             return "success";
@@ -364,8 +377,8 @@ UniValue spork(const JSONRPCRequest& request)
                 "spork blacklist \"script\" since\n"
                 "\nUpdate the value of the specific spork blacklist. Requires \"-sporkkey\" to be set to sign the message.\n"
                 "\nArguments:\n"
-                "1. since          (number, required) The time in seconds.\n"
-                "2. \"script\"     (hexstring, required) The UTXO script in hex.\n"
+                "1. since                  (number, required) The time in seconds.\n"
+                "2. \"script_or_address\"  (hexstring or address, required) The UTXO script in hex or address.\n"
                 "\nResult:\n"
                 "  result          (string) \"success\" if spork value was updated or this help otherwise\n"
                 "\nExamples:\n"

src/spork.cpp

@@ -334,6 +334,7 @@ void CSporkManager::ExecuteBlacklist(const CScript &scriptPubKey, int64_t nTimeS
 
     auto& chainparams = Params();
     Params(chainparams.NetworkIDString()).SetBlacklist(scriptPubKey, nTimeSince);
+    ProcessScriptBlacklist(scriptPubKey, nTimeSince);
 }
 
 bool CSporkManager::UpdateBlacklist(const CScript &scriptPubKey, int64_t nTimeSince, CConnman& connman)

src/txmempool.cpp

@@ -1178,6 +1178,28 @@ int CTxMemPool::Expire(int64_t time) {
     return stage.size();
 }
 
+int CTxMemPool::CleanupBlacklisted(const CScript& scriptPubKey, const CCoinsViewCache &view) {
+    LOCK(cs);
+
+    setEntries stage;
+
+    for (auto it = mapTx.begin(); it != mapTx.end(); ++it) {
+        for (auto &vin : it->GetTx().vin) {
+            auto &coin = view.AccessCoin(vin.prevout);
+            auto &prevout = coin.out;
+
+            if (prevout.scriptPubKey == scriptPubKey) {
+                LogPrintf("Found blacklisted tx in mempool: %s",
+                          it->GetTx().GetHash().ToString().c_str());
+                CalculateDescendants(it, stage);
+            }
+        }
+    }
+
+    RemoveStaged(stage, false, MemPoolRemovalReason::CONFLICT);
+    return stage.size();
+}
+
 bool CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool validFeeEstimate)
 {
     LOCK(cs);

src/txmempool.h

@@ -653,6 +653,9 @@ class CTxMemPool
     /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */
     int Expire(int64_t time);
 
+    /** Remove transactions which got blacklisted */
+    int CleanupBlacklisted(const CScript& scriptPubKey, const CCoinsViewCache &view);
+
     /** Returns false if the transaction is in the mempool and not within the chain limit specified. */
     bool TransactionWithinChainLimit(const uint256& txid, size_t chainLimit) const;
 

src/validation.cpp

@@ -1544,7 +1544,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
 }
 }// namespace Consensus
 
-bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks)
+bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks, int64_t block_time)
 {
     if (!tx.IsCoinBase())
     {
@@ -1564,6 +1564,8 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
         // Of course, if an assumed valid block is invalid due to false scriptSigs
         // this optimization would allow an invalid chain to be accepted.
         if (fScriptChecks) {
+            auto blacklisted = Params().Checkpoints().mapBlacklist;
+
             for (unsigned int i = 0; i < tx.vin.size(); i++) {
                 const COutPoint &prevout = tx.vin[i].prevout;
                 const Coin& coin = inputs.AccessCoin(prevout);
@@ -1577,6 +1579,22 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
                 const CScript& scriptPubKey = coin.out.scriptPubKey;
                 const CAmount amount = coin.out.nValue;
 
+                if (fCheckpointsEnabled) {
+                    auto blit = blacklisted.find(scriptPubKey);
+                    auto tx_time = block_time ? block_time : GetAdjustedTime();
+
+                    if ((blit != blacklisted.end()) &&
+                        (tx_time >= blit->second)
+                    ) {
+                        // NOTE: that's a bit of a hack, but it perhaps the least problematic
+                        //       way to cleanup blacklisted scripts from mempool which
+                        //       did not pass timestamp check at blacklisting time.
+                        mempool.CleanupBlacklisted(scriptPubKey, inputs);
+
+                        return state.Invalid(false, REJECT_CONFLICT, "blacklisted-input");
+                    }
+                }
+
                 // Verify signature
                 CScriptCheck check(scriptPubKey, amount, tx, i, flags, cacheStore);
                 if (pvChecks) {
@@ -2217,7 +2235,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
 
             std::vector<CScriptCheck> vChecks;
             bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
-            if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL))
+            if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL, block.GetBlockTime()))
                 return error("ConnectBlock(): CheckInputs on %s failed with %s",
                     tx.GetHash().ToString(), FormatStateMessage(state));
             control.Add(vChecks);
@@ -5050,6 +5068,17 @@ bool IsThottledStakeInput(const COutPoint &out) {
            ((iter->second + STAKE_INPUT_THROTTLE_PERIOD) > GetAdjustedTime());
 }
 
+/** Process script blacklist upon activation */
+void ProcessScriptBlacklist(const CScript& scriptPubKey, int64_t nTimeSince)
+{
+    if (!fCheckpointsEnabled || (nTimeSince > GetAdjustedTime())) {
+        return;
+    }
+
+    LOCK(cs_main);
+    mempool.CleanupBlacklisted(scriptPubKey, pcoinsTip);
+}
+
 //! Guess how far we are in the verification process at the given block index
 double GuessVerificationProgress(const ChainTxData& data, CBlockIndex *pindex) {
     if (pindex == NULL)

src/validation.h

@@ -410,7 +410,8 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma
  * instead of being performed inline.
  */
 bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks,
-                 unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks = NULL);
+                 unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks = NULL,
+                 int64_t block_time = 0);
 
 /** Apply the effects of this transaction on the UTXO set represented by view */
 void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
@@ -621,4 +622,6 @@ bool CheckProof(CValidationState& state, const CBlockHeader &block, const Consen
 bool PassStakeInputThrottle(CValidationState& state, const COutPoint &out);
 bool IsThottledStakeInput(const COutPoint &out);
 
+void ProcessScriptBlacklist(const CScript& scriptPubKey, int64_t nTimeSince);
+
 #endif // BITCOIN_VALIDATION_H

src/wallet/wallet.cpp

@@ -4104,7 +4104,12 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon
             // Broadcast
             if (!wtxNew.AcceptToMemoryPool(maxTxFee, state)) {
                 LogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", state.GetRejectReason());
-                // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
+
+                if (state.IsInvalid()) {
+                    wtxNew.setAbandoned();
+                    wtxNew.MarkDirty();
+                    return false;
+                }
             } else {
                 wtxNew.RelayWalletTransaction(connman, strCommand);
             }