Introduce interfaces signing providers and BaseSignatureCreator

Author: Liquid369

src/keystore.h

@@ -12,21 +12,26 @@
 #include "sapling/address.h"
 #include "sapling/zip32.h"
 #include "sync.h"
+#include <key.h>
+#include <pubkey.h>
+#include <script/script.h>
+#include <script/sign.h>
+#include <script/standard.h>
+#include <sync.h>
 
 #include <boost/signals2/signal.hpp>
 
 class CScript;
 class CScriptID;
 
 /** A virtual base class for key stores */
-class CKeyStore
+class CKeyStore : public SigningProvider
 {
 public:
     // todo: Make it protected again once we are more advanced in the wallet/spkm decoupling.
     mutable RecursiveMutex cs_KeyStore;
 
-    virtual ~CKeyStore() {}
-
+public:
     //! Add a key to the store.
     virtual bool AddKeyPubKey(const CKey& key, const CPubKey& pubkey) = 0;
     virtual bool AddKey(const CKey& key);

src/pivx-tx.cpp

@@ -544,7 +544,8 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mergedTx.vout.size()))
             ProduceSignature(
-                    MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType),
+                    keystore,
+                    MutableTransactionSignatureCreator(&mergedTx, i, amount, nHashType),
                     prevPubKey,
                     sigdata,
                     sigversion,

src/rpc/rawtransaction.cpp

@@ -766,7 +766,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
         SigVersion sigversion = mergedTx.GetRequiredSigVersion();
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mergedTx.vout.size()))
-            ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType),
+            ProduceSignature(keystore, MutableTransactionSignatureCreator(&mergedTx, i, amount, nHashType),
                     prevPubKey, sigdata, sigversion, fColdStake);
 
         // ... and merge in other signatures:

src/rpc/rpcevo.cpp

@@ -355,7 +355,9 @@ static OperationResult SignTransaction(CWallet* const pwallet, CMutableTransacti
         SigVersion sv = tx.GetRequiredSigVersion();
         txin.scriptSig.clear();
         SignatureData sigdata;
-        if (!ProduceSignature(MutableTransactionSignatureCreator(pwallet, &tx, i, coin.out.nValue, SIGHASH_ALL),
+        const CKeyStore& keys = *pwallet;
+        if (!ProduceSignature(keys,
+                              MutableTransactionSignatureCreator(&tx, i, coin.out.nValue, SIGHASH_ALL),
                               coin.out.scriptPubKey, sigdata, sv, false)) {
             return errorOut(TxInErrorToString(i, txin, "signature failed"));
         }

src/sapling/transaction_builder.cpp

@@ -332,9 +332,9 @@ TransactionBuilderResult TransactionBuilder::ProveAndSign()
         auto tIn = tIns[nIn];
         SignatureData sigdata;
         bool signSuccess = ProduceSignature(
-            TransactionSignatureCreator(
-                keystore, &txNewConst, nIn, tIn.value, SIGHASH_ALL),
-            tIn.scriptPubKey, sigdata, SIGVERSION_SAPLING, false);
+                        *keystore,
+                        TransactionSignatureCreator(&txNewConst, nIn, tIn.value, SIGHASH_ALL),
+                        tIn.scriptPubKey, sigdata, SIGVERSION_SAPLING, false);
 
         if (!signSuccess) {
             return TransactionBuilderResult("Failed to sign transaction");
@@ -366,7 +366,12 @@ TransactionBuilderResult TransactionBuilder::AddDummySignatures()
     for (int nIn = 0; nIn < (int) mtx.vin.size(); nIn++) {
         auto tIn = tIns[nIn];
         SignatureData sigdata;
-        if (!ProduceSignature(DummySignatureCreator(keystore), tIn.scriptPubKey, sigdata, SIGVERSION_SAPLING, false)) {
+        if (!ProduceSignature(*keystore,
+                              DUMMY_SIGNATURE_CREATOR,
+                              tIn.scriptPubKey,
+                              sigdata,
+                              SIGVERSION_SAPLING,
+                              false)) {
             return TransactionBuilderResult("Failed to sign transaction");
         } else {
             UpdateTransaction(mtx, nIn, sigdata);

src/script/sign.cpp

@@ -16,12 +16,12 @@
 
 typedef std::vector<unsigned char> valtype;
 
-TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
+TransactionSignatureCreator::TransactionSignatureCreator(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
 
-bool TransactionSignatureCreator::CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
+bool TransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
 {
     CKey key;
-    if (!keystore->GetKey(address, key))
+    if (!provider.GetKey(address, key))
         return false;
 
     uint256 hash;
@@ -37,24 +37,24 @@ bool TransactionSignatureCreator::CreateSig(std::vector<unsigned char>& vchSig,
     return true;
 }
 
-static bool Sign1(const CKeyID& address, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
+static bool Sign1(const SigningProvider& provider, const CKeyID& address, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
 {
     std::vector<unsigned char> vchSig;
-    if (!creator.CreateSig(vchSig, address, scriptCode, sigversion))
+    if (!creator.CreateSig(provider, vchSig, address, scriptCode, sigversion))
         return false;
     ret.emplace_back(vchSig);
     return true;
 }
 
-static bool SignN(const std::vector<valtype>& multisigdata, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
+static bool SignN(const SigningProvider& provider, const std::vector<valtype>& multisigdata, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
 {
     int nSigned = 0;
     int nRequired = multisigdata.front()[0];
     for (unsigned int i = 1; i < multisigdata.size()-1 && nSigned < nRequired; i++)
     {
         const valtype& pubkey = multisigdata[i];
         CKeyID keyID = CPubKey(pubkey).GetID();
-        if (Sign1(keyID, creator, scriptCode, ret, sigversion))
+        if (Sign1(provider, keyID, creator, scriptCode, ret, sigversion))
             ++nSigned;
     }
     return nSigned==nRequired;
@@ -66,7 +66,7 @@ static bool SignN(const std::vector<valtype>& multisigdata, const BaseSignatureC
  * unless whichTypeRet is TX_SCRIPTHASH, in which case scriptSigRet is the redemption script.
  * Returns false if scriptPubKey could not be completely satisfied.
  */
-static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptPubKey,
+static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& scriptPubKey,
                      std::vector<valtype>& ret, txnouttype& whichTypeRet, SigVersion sigversion, bool fColdStake)
 {
     CScript scriptRet;
@@ -85,28 +85,28 @@ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptP
         return false;
     case TX_PUBKEY:
         keyID = CPubKey(vSolutions[0]).GetID();
-        return Sign1(keyID, creator, scriptPubKey, ret, sigversion);
+        return Sign1(provider, keyID, creator, scriptPubKey, ret, sigversion);
     case TX_PUBKEYHASH:
         keyID = CKeyID(uint160(vSolutions[0]));
-        if (!Sign1(keyID, creator, scriptPubKey, ret, sigversion))
+        if (!Sign1(provider, keyID, creator, scriptPubKey, ret, sigversion))
             return false;
         else
         {
             CPubKey vch;
-            creator.KeyStore().GetPubKey(keyID, vch);
+            provider.GetPubKey(keyID, vch);
             ret.push_back(ToByteVector(vch));
         }
         return true;
     case TX_SCRIPTHASH:
-        if (creator.KeyStore().GetCScript(uint160(vSolutions[0]), scriptRet)) {
-            ret.emplace_back(scriptRet.begin(), scriptRet.end());
+        if (provider.GetCScript(uint160(vSolutions[0]), scriptRet)) {
+            ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end()));
             return true;
         }
         return false;
 
     case TX_MULTISIG:
         ret.push_back(valtype()); // workaround CHECKMULTISIG bug
-        return (SignN(vSolutions, creator, scriptPubKey, ret, sigversion));
+        return (SignN(provider, vSolutions, creator, scriptPubKey, ret, sigversion));
 
     case TX_COLDSTAKE:
         if (fColdStake) {
@@ -116,11 +116,11 @@ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptP
             // sign with the owner key
             keyID = CKeyID(uint160(vSolutions[1]));
         }
-        if (!Sign1(keyID, creator, scriptPubKey, ret, sigversion))
+        if (!Sign1(provider, keyID, creator, scriptPubKey, ret, sigversion))
             return error("*** %s: failed to sign with the %s key.",
                     __func__, fColdStake ? "cold staker" : "owner");
         CPubKey vch;
-        if (!creator.KeyStore().GetPubKey(keyID, vch))
+        if (!provider.GetPubKey(keyID, vch))
             return error("%s : Unable to get public key from keyID", __func__);
 
         valtype oper;
@@ -149,13 +149,13 @@ static CScript PushAll(const std::vector<valtype>& values)
     return result;
 }
 
-bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata, SigVersion sigversion, bool fColdStake, ScriptError* serror)
+bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata, SigVersion sigversion, bool fColdStake, ScriptError* serror)
 {
     CScript script = fromPubKey;
     bool solved = true;
     std::vector<valtype> result;
     txnouttype whichType;
-    solved = SignStep(creator, script, result, whichType, sigversion, fColdStake);
+    solved = SignStep(provider, creator, script, result, whichType, sigversion, fColdStake);
     CScript subscript;
 
     if (solved && whichType == TX_SCRIPTHASH)
@@ -164,7 +164,7 @@ bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPu
         // the final scriptSig is the signatures from that
         // and then the serialized subscript:
         script = subscript = CScript(result[0].begin(), result[0].end());
-        solved = solved && SignStep(creator, script, result, whichType, sigversion, fColdStake) && whichType != TX_SCRIPTHASH;
+        solved = solved && SignStep(provider, creator, script, result, whichType, sigversion, fColdStake) && whichType != TX_SCRIPTHASH;
         result.emplace_back(subscript.begin(), subscript.end());
     }
 
@@ -188,27 +188,27 @@ void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const Signatur
     tx.vin[nIn].scriptSig = data.scriptSig;
 }
 
-bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, bool fColdStake)
+bool SignSignature(const SigningProvider &provider,  const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, bool fColdStake)
 {
     assert(nIn < txTo.vin.size());
 
     CTransaction txToConst(txTo);
-    TransactionSignatureCreator creator(&keystore, &txToConst, nIn, amount, nHashType);
+    TransactionSignatureCreator creator(&txToConst, nIn, amount, nHashType);
 
     SignatureData sigdata;
-    bool ret = ProduceSignature(creator, fromPubKey, sigdata, txToConst.GetRequiredSigVersion(), fColdStake);
+    bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata, txToConst.GetRequiredSigVersion(), fColdStake);
     UpdateTransaction(txTo, nIn, sigdata);
     return ret;
 }
 
-bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, bool fColdStake)
+bool SignSignature(const SigningProvider &provider,  const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, bool fColdStake)
 {
     assert(nIn < txTo.vin.size());
     CTxIn& txin = txTo.vin[nIn];
     assert(txin.prevout.n < txFrom.vout.size());
     const CTxOut& txout = txFrom.vout[txin.prevout.n];
 
-    return SignSignature(keystore, txout.scriptPubKey, txTo, nIn, txout.nValue, nHashType, fColdStake);
+    return SignSignature(provider, txout.scriptPubKey, txTo, nIn, txout.nValue, nHashType, fColdStake);
 }
 
 static std::vector<valtype> CombineMultisig(const CScript& scriptPubKey, const BaseSignatureChecker& checker,
@@ -343,40 +343,39 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature
 }
 
 namespace {
-/** Dummy signature checker which accepts all signatures. */
-class DummySignatureChecker : public BaseSignatureChecker
-{
-public:
-    DummySignatureChecker() {}
+    /** Dummy signature checker which accepts all signatures. */
+    class DummySignatureChecker final : public BaseSignatureChecker {
+    public:
+        DummySignatureChecker() {}
+
+        bool CheckSig(const std::vector<unsigned char> &scriptSig, const std::vector<unsigned char> &vchPubKey,
+                      const CScript &scriptCode, SigVersion sigversion) const override { return true; }
+    };
 
-    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
+    const DummySignatureChecker DUMMY_CHECKER;
+}
+class DummySignatureCreator final : public BaseSignatureCreator {
+public:
+    DummySignatureCreator() {}
+    const BaseSignatureChecker& Checker() const override { return DUMMY_CHECKER; }
+    bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override
     {
+        // Create a dummy signature that is a valid DER-encoding
+        vchSig.assign(72, '\000');
+        vchSig[0] = 0x30;
+        vchSig[1] = 69;
+        vchSig[2] = 0x02;
+        vchSig[3] = 33;
+        vchSig[4] = 0x01;
+        vchSig[4 + 33] = 0x02;
+        vchSig[5 + 33] = 32;
+        vchSig[6 + 33] = 0x01;
+        vchSig[6 + 33 + 32] = SIGHASH_ALL;
         return true;
     }
 };
-const DummySignatureChecker dummyChecker;
-}
 
-const BaseSignatureChecker& DummySignatureCreator::Checker() const
-{
-    return dummyChecker;
-}
-
-bool DummySignatureCreator::CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const
-{
-    // Create a dummy signature that is a valid DER-encoding
-    vchSig.assign(72, '\000');
-    vchSig[0] = 0x30;
-    vchSig[1] = 69;
-    vchSig[2] = 0x02;
-    vchSig[3] = 33;
-    vchSig[4] = 0x01;
-    vchSig[4 + 33] = 0x02;
-    vchSig[5 + 33] = 32;
-    vchSig[6 + 33] = 0x01;
-    vchSig[6 + 33 + 32] = SIGHASH_ALL;
-    return true;
-}
+const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator();
 
 template<typename M, typename K, typename V>
 bool LookupHelper(const M& map, const K& key, V& value)
@@ -389,16 +388,15 @@ bool LookupHelper(const M& map, const K& key, V& value)
     return false;
 }
 
-bool IsSolvable(const CKeyStore& store, const CScript& script, bool fColdStaking)
+bool IsSolvable(const SigningProvider& provider, 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, SIGVERSION_BASE, fColdStaking)) {
+    if (ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, script, sigs, SIGVERSION_BASE, fColdStaking)) {
         // VerifyScript check is just defensive, and should never fail.
-        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker(), SIGVERSION_BASE));
+        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, DUMMY_CHECKER, SIGVERSION_BASE));
         return true;
     }
     return false;