Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce interfaces Signing Provider/BaseSignatureCreator #2837

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/keystore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/pivx-tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion src/rpc/rpcevo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand Down
13 changes: 9 additions & 4 deletions src/sapling/transaction_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
110 changes: 54 additions & 56 deletions src/script/sign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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());
}

Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down
Loading