When validation is waiting for parent chain daemon, "stall".

doc/man/elements-qt.1

@@ -854,12 +854,6 @@ Entries in the PAK list. Order of entries matter.
 .IP
 Pegin claims must be this deep to be considered valid. (default: 8)
 .HP
-\fB\-recheckpeginblockinterval=\fR<n>
-.IP
-The interval in seconds at which a peg\-in witness failing block is
-re\-evaluated in case of intermittent peg\-in witness failure. 0
-means never. (default: 120)
-.HP
 \fB\-validatepegin\fR
 .IP
 Validate peg\-in claims. An RPC connection will be attempted to the

doc/man/elementsd.1

@@ -828,12 +828,6 @@ Entries in the PAK list. Order of entries matter.
 .IP
 Pegin claims must be this deep to be considered valid. (default: 8)
 .HP
-\fB\-recheckpeginblockinterval=\fR<n>
-.IP
-The interval in seconds at which a peg\-in witness failing block is
-re\-evaluated in case of intermittent peg\-in witness failure. 0
-means never. (default: 120)
-.HP
 \fB\-validatepegin\fR
 .IP
 Validate peg\-in claims. An RPC connection will be attempted to the

src/init.cpp

@@ -582,7 +582,6 @@ void SetupServerArgs()
     gArgs.AddArg("-mainchainrpccookiefile=<file>", "The bitcoind cookie auth path which the daemon will use to connect to the trusted mainchain daemon to validate peg-ins. (default: `<datadir>/regtest/.cookie`)", false, OptionsCategory::ELEMENTS);
     gArgs.AddArg("-mainchainrpctimeout=<n>", strprintf("Timeout in seconds during mainchain RPC requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), false, OptionsCategory::ELEMENTS);
     gArgs.AddArg("-peginconfirmationdepth=<n>", strprintf("Pegin claims must be this deep to be considered valid. (default: %d)", DEFAULT_PEGIN_CONFIRMATION_DEPTH), false, OptionsCategory::ELEMENTS);
-    gArgs.AddArg("-recheckpeginblockinterval=<n>", strprintf("The interval in seconds at which a peg-in witness failing block is re-evaluated in case of intermittent peg-in witness failure. 0 means never. (default: %u)", 120), false, OptionsCategory::ELEMENTS);
     gArgs.AddArg("-parentpubkeyprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 pubkey address. (default: %d)", 111), false, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-parentscriptprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 script address. (default: %d)", 196), false, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-parent_bech32_hrp", strprintf("The human-readable part of the parent chain's bech32 encoding. (default: %s)", "bc"), false, OptionsCategory::CHAINPARAMS);
@@ -1279,6 +1278,61 @@ bool AppInitLockDataDirectory()
     return true;
 }
 
+/* This function checks that the RPC connection to the parent chain node
+ * can be attained, and is returning back reasonable answers.
+ */
+bool MainchainRPCCheck()
+{
+    // Check for working and valid rpc
+    // Retry until a non-RPC_IN_WARMUP result
+    while (true) {
+        try {
+            // The first thing we have to check is the version of the node.
+            UniValue params(UniValue::VARR);
+            UniValue reply = CallMainChainRPC("getnetworkinfo", params);
+            UniValue error = reply["error"];
+            if (!error.isNull()) {
+                // On the first call, it's possible to node is still in
+                // warmup; in that case, just wait and retry.
+                if (error["code"].get_int() == RPC_IN_WARMUP) {
+                    MilliSleep(1000);
+                    continue;
+                }
+                else {
+                    LogPrintf("ERROR: Mainchain daemon RPC check returned 'error' response.\n");
+                    return false;
+                }
+            }
+            UniValue result = reply["result"];
+            if (!result.isObject() || !result.get_obj()["version"].isNum() ||
+                    result.get_obj()["version"].get_int() < MIN_MAINCHAIN_NODE_VERSION) {
+                LogPrintf("ERROR: Parent chain daemon too old; need Bitcoin Core version 0.16.3 or newer.\n");
+                return false;
+            }
+
+            // Then check the genesis block to correspond to parent chain.
+            params.push_back(UniValue(0));
+            reply = CallMainChainRPC("getblockhash", params);
+            error = reply["error"];
+            if (!error.isNull()) {
+                LogPrintf("ERROR: Mainchain daemon RPC check returned 'error' response.\n");
+                return false;
+            }
+            result = reply["result"];
+            if (!result.isStr() || result.get_str() != Params().ParentGenesisBlockHash().GetHex()) {
+                LogPrintf("ERROR: Invalid parent genesis block hash response via RPC. Contacting wrong parent daemon?\n");
+                return false;
+            }
+        } catch (const std::runtime_error& re) {
+            LogPrintf("ERROR: Failure connecting to mainchain daemon RPC: %s\n", std::string(re.what()));
+            return false;
+        }
+
+        // Success
+        return true;
+    }
+}
+
 bool AppInitMain(InitInterfaces& interfaces)
 {
     const CChainParams& chainparams = Params();
@@ -1879,29 +1933,34 @@ bool AppInitMain(InitInterfaces& interfaces)
     // ELEMENTS:
     if (gArgs.GetBoolArg("-validatepegin", Params().GetConsensus().has_parent_chain)) {
         uiInterface.InitMessage(_("Awaiting mainchain RPC warmup"));
-    }
-    if (!MainchainRPCCheck(true)) { //Initial check only
-        const std::string err_msg = "ERROR: elements is set to verify pegins but cannot get valid response from the mainchain daemon. Please check debug.log for more information.\n\nIf you haven't setup a bitcoind please get the latest stable version from https://bitcoincore.org/en/download/ or if you do not need to validate pegins set in your elements configuration validatepegin=0";
-        // We fail immediately if this node has RPC server enabled
-        if (gArgs.GetBoolArg("-server", false)) {
-            InitError(err_msg);
-            return false;
-        } else {
-            // Or gently warn the user, and continue
-            InitWarning(err_msg);
-            gArgs.SoftSetArg("-validatepegin", "0");
+        if (!MainchainRPCCheck()) {
+            const std::string err_msg = "ERROR: elements is set to verify pegins but cannot get valid response from the mainchain daemon. Please check debug.log for more information.\n\nIf you haven't setup a bitcoind please get the latest stable version from https://bitcoincore.org/en/download/ or if you do not need to validate pegins set in your elements configuration validatepegin=0";
+            // We fail immediately if this node has RPC server enabled
+            if (gArgs.GetBoolArg("-server", false)) {
+                InitError(err_msg);
+                return false;
+            } else {
+                // Or gently warn the user, and continue
+                InitWarning(err_msg);
+                gArgs.SoftSetArg("-validatepegin", "0");
+            }
         }
     }
 
-    // Start the lightweight block re-evaluation scheduler thread
-    CScheduler::Function reevaluationLoop = std::bind(&CScheduler::serviceQueue, &reverification_scheduler);
-    threadGroup.create_thread(std::bind(&TraceThread<CScheduler::Function>, "reevaluation_scheduler", reevaluationLoop));
-
-    CScheduler::Function f2 = boost::bind(&MainchainRPCCheck, false);
-    unsigned int check_rpc_every = gArgs.GetArg("-recheckpeginblockinterval", 120) * 1000;
-    if (check_rpc_every) {
-        reverification_scheduler.scheduleEvery(f2, check_rpc_every);
-    }
+    // Call ActivateBestChain every 30 seconds. This is almost always a
+    // harmless no-op. It is necessary in the unusual case where:
+    // (1) Our connection to bitcoind is lost, and
+    // (2) we build up a queue of blocks to validate in the meantime, and then
+    // (3) our connection to bitcoind is restored, but
+    // (4) nothing after that causes ActivateBestChain to be called, including
+    //     no further blocks arriving for us to validate.
+    // Unfortunately, this unusual case happens in the functional test suite.
+    reverification_scheduler.scheduleEvery([]{
+        CValidationState state;
+        if (!ActivateBestChain(state, Params())) {
+            LogPrintf("Failed to periodically activate best chain (%s)\n", FormatStateMessage(state));
+        }
+    }, 30 * 1000);
 
     uiInterface.InitMessage(_("Done loading"));
 

src/mainchainrpc.cpp

@@ -152,18 +152,24 @@ UniValue CallMainChainRPC(const std::string& strMethod, const UniValue& params)
 
 bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDepth, const int nbTxs)
 {
+    LogPrintf("Checking for confirmed bitcoin block with hash %s, mindepth %d, nbtxs %d\n", hash.ToString().c_str(), nMinConfirmationDepth, nbTxs);
     try {
         UniValue params(UniValue::VARR);
         params.push_back(hash.GetHex());
         UniValue reply = CallMainChainRPC("getblockheader", params);
-        if (!find_value(reply, "error").isNull())
+        if (!find_value(reply, "error").isNull()) {
+            LogPrintf("ERROR: Got error reply from bitcoind getblockheader.\n");
             return false;
+        }
         UniValue result = find_value(reply, "result");
-        if (!result.isObject())
+        if (!result.isObject()) {
+            LogPrintf("ERROR: bitcoind getblockheader result was malformed (not object).\n");
             return false;
+        }
 
         UniValue confirmations = find_value(result.get_obj(), "confirmations");
         if (!confirmations.isNum() || confirmations.get_int64() < nMinConfirmationDepth) {
+            LogPrintf("Insufficient confirmations (got %s).\n", confirmations.write());
             return false;
         }
 

src/pegins.cpp

@@ -240,7 +240,11 @@ bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::P
     return true;
 }
 
-bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector<std::pair<CScript, CScript>>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth) {
+bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector<std::pair<CScript, CScript>>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth, bool* depth_failed) {
+    if (depth_failed) {
+        *depth_failed = false;
+    }
+
     // 0) Return false if !consensus.has_parent_chain
     if (!Params().GetConsensus().has_parent_chain) {
         err_msg = "Parent chain is not enabled on this network.";
@@ -372,6 +376,9 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector<
         LogPrintf("Required depth: %d\n", required_depth);
         if (!IsConfirmedBitcoinBlock(block_hash, required_depth, num_txs)) {
             err_msg = "Needs more confirmations.";
+            if (depth_failed) {
+                *depth_failed = true;
+            }
             return false;
         }
     }

src/pegins.h

@@ -19,7 +19,7 @@ bool GetAmountFromParentChainPegin(CAmount& amount, const CTransaction& txBTC, u
 /** Check whether a parent chain block hash satisfies the proof-of-work requirement specified by nBits */
 bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params&);
 /** Checks pegin witness for validity */
-bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector<std::pair<CScript, CScript>>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth);
+bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector<std::pair<CScript, CScript>>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth, bool* depth_failed = nullptr);
 // Constructs unblinded output to be used in amount and scriptpubkey checks during pegin
 CTxOut GetPeginOutputFromWitness(const CScriptWitness& pegin_witness);
 

src/rpc/rawtransaction.cpp

@@ -1060,8 +1060,9 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con
             continue;
         }
         // Report warning about immature peg-in though
-        if(txin.m_is_pegin && !IsValidPeginWitness(txConst.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, txin.prevout, err, true)) {
-            assert(err == "Needs more confirmations.");
+        bool depth_failed = false;
+        if(txin.m_is_pegin && !IsValidPeginWitness(txConst.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, txin.prevout, err, true, &depth_failed)) {
+            assert(depth_failed);
             immature_pegin = true;
         }
 

src/txdb.cpp

@@ -34,7 +34,7 @@ static const char DB_LAST_BLOCK = 'l';
 
 // ELEMENTS:
 static const char DB_PEGIN_FLAG = 'w';
-static const char DB_INVALID_BLOCK_Q = 'q';
+// static const char DB_INVALID_BLOCK_Q = 'q';  // No longer used, but avoid reuse.
 static const char DB_PAK = 'p';
 
 namespace {
@@ -272,14 +272,6 @@ bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) {
     return true;
 }
 
-// ELEMENTS:
-bool CBlockTreeDB::ReadInvalidBlockQueue(std::vector<uint256> &vBlocks) {
-    return Read(std::make_pair(DB_INVALID_BLOCK_Q, uint256S("0")), vBlocks);//FIXME: why uint 56 and not ""
-}
-bool CBlockTreeDB::WriteInvalidBlockQueue(const std::vector<uint256> &vBlocks) {
-    return Write(std::make_pair(DB_INVALID_BLOCK_Q, uint256S("0")), vBlocks);
-}
-
 bool CBlockTreeDB::ReadPAKList(std::vector<std::vector<unsigned char> >& offline_list, std::vector<std::vector<unsigned char> >& online_list, bool& reject)
 {
         return Read(std::make_pair(DB_PAK, uint256S("1")), offline_list) && Read(std::make_pair(DB_PAK, uint256S("2")), online_list) && Read(std::make_pair(DB_PAK, uint256S("3")), reject);

src/txdb.h

@@ -99,8 +99,6 @@ class CBlockTreeDB : public CDBWrapper
     bool ReadFlag(const std::string &name, bool &fValue);
     bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex);
     // ELEMENTS:
-    bool ReadInvalidBlockQueue(std::vector<uint256> &vBlocks);
-    bool WriteInvalidBlockQueue(const std::vector<uint256> &vBlocks);
     bool ReadPAKList(std::vector<std::vector<unsigned char> >& offline_list, std::vector<std::vector<unsigned char> >& online_list, bool& reject);
     bool WritePAKList(const std::vector<std::vector<unsigned char> >& offline_list, const std::vector<std::vector<unsigned char> >& online_list, bool reject);
 };