Merge bitcoin#1890: [BUG][Script] Add fColdStaking boolean argument t…

…o IsSolvable

b425466 [Script] Add fColdStaking bool to IsSolvable (random-zebra)

Pull request description:

  Fixing a minor bug introduced in bitcoin#1757.
  `IsSolvable` always tries to produce a signature with the owner key, so it logs an error (and reports the coin as not solvable) for cold stakers.
  Fix it introducing a boolean argument in `IsSolvable`, to check the appropriate public key in the keystore.

ACKs for top commit:
  furszy:
    Great catch 👌 , ACK b425466
  Fuzzbawls:
    ACK b425466

Tree-SHA512: 9d4537218344c8176f1de500664905e78419fd852c9607b40cf83eedb55e5fb965977f31bba0a605fac35c9bee373613467d63c6d3f72386c598e7452b545101

src/script/sign.cpp

@@ -381,14 +381,14 @@ bool DummySignatureCreator::CreateSig(std::vector<unsigned char>& vchSig, const
     return true;
 }
 
-bool IsSolvable(const CKeyStore& store, const CScript& script)
+bool IsSolvable(const CKeyStore& store, const CScript& script, bool fColdStaking)
 {
     // This check is to make sure that the script we created can actually be solved for and signed by us
     // if we were to have the private keys. This is just to make sure that the script is valid and that,
     // if found in a transaction, we would still accept and relay that transaction. In particular,
     DummySignatureCreator creator(&store);
     SignatureData sigs;
-    if (ProduceSignature(creator, script, sigs, false)) {
+    if (ProduceSignature(creator, script, sigs, fColdStaking)) {
         // VerifyScript check is just defensive, and should never fail.
         assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()));
         return true;

src/script/sign.h

@@ -85,6 +85,6 @@ void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const Signatur
   * have all private keys. While this function does not need private keys, the passed
   * keystore is used to look up public keys and redeemscripts by hash.
   * Solvability is unrelated to whether we consider this output to be ours. */
-bool IsSolvable(const CKeyStore& store, const CScript& script);
+bool IsSolvable(const CKeyStore& store, const CScript& script, bool fColdStaking);
 
 #endif // BITCOIN_SCRIPT_SIGN_H

src/wallet/wallet.cpp

@@ -2065,7 +2065,7 @@ bool CWallet::AvailableCoins(std::vector<COutput>* pCoins,      // --> populates
                 // skip delegated coins
                 if (mine == ISMINE_SPENDABLE_DELEGATED && !fIncludeDelegated) continue;
 
-                bool solvable = IsSolvable(*this, pcoin->vout[i].scriptPubKey);
+                bool solvable = IsSolvable(*this, pcoin->vout[i].scriptPubKey, mine == ISMINE_COLD);
 
                 bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) ||
                         (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)) ||