Skip to content

Commit

Permalink
Merge bitcoin#12803: Make BaseSignatureCreator a pure interface
Browse files Browse the repository at this point in the history 
be67831 Make DummySignatureCreator a singleton (Pieter Wuille)
190b8d2 Make BaseSignatureCreator a pure interface (Pieter Wuille)

Pull request description:

  * Removes the `m_provider` field from `BaseSignatureCreator`. Instead both a `SigningProvider` (which provides keys and scripts) and a `BaseSignatureCreator` (which implements the transaction-specific (or other) signing logic) are passed into and down in `ProduceSignature`, making the two concepts orthogonal.
  * Makes `BaseSignatureCreator` a pure interface without constructor, making it easier to implement new derivations of it (for example for message signing).
  * As `DummySignatureCreator` now becomes a stateless object, turn it into a singleton `DUMMY_SIGNATURE_CREATOR`.

Tree-SHA512: 5f1f4512e4ea7d02a31df7b9ede55008efa716c5b74a2630ca1c2fc6599584d8bf5f5641487266127f4b3788033803539fbd22b03ef1219c83c10da2d3da3dcd
  • Loading branch information
@laanwj @PastaPastaPasta
laanwj authored and PastaPastaPasta committed Apr 17, 2021
1 parent 79da39a commit 71944f5
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 65 deletions.
2 changes: 1 addition & 1 deletion src/coinjoin/coinjoin-util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, con
for (const auto& coin : tallyItem.vecInputCoins) {
const CScript& scriptPubKey = coin.txout.scriptPubKey;
SignatureData sigdata;
bool res = ProduceSignature(DummySignatureCreator(pwallet.get()), scriptPubKey, sigdata);
bool res = ProduceSignature(*pwallet, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata);
assert(res);
UpdateTransaction(dummyTx, nIn, sigdata);
nIn++;
Expand Down
2 changes: 1 addition & 1 deletion src/dash-tx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
SignatureData sigdata;
// Only sign SIGHASH_SINGLE if there's a corresponding output:
if (!fHashSingle || (i < mergedTx.vout.size()))
ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType), prevPubKey, sigdata);
ProduceSignature(keystore, MutableTransactionSignatureCreator(&mergedTx, i, amount, nHashType), prevPubKey, sigdata);

// ... and merge in other signatures:
sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rawtransaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
SignatureData sigdata;
// Only sign SIGHASH_SINGLE if there's a corresponding output:
if (!fHashSingle || (i < mtx.vout.size())) {
ProduceSignature(MutableTransactionSignatureCreator(keystore, &mtx, i, amount, nHashType), prevPubKey, sigdata);
ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata);
}
sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));

Expand Down
2 changes: 1 addition & 1 deletion src/script/ismine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey)
if (keystore.HaveWatchOnly(scriptPubKey)) {
// TODO: This could be optimized some by doing some work after the above solver
SignatureData sigs;
return ProduceSignature(DummySignatureCreator(&keystore), scriptPubKey, sigs) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE;
return ProduceSignature(keystore, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigs) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE;
}
return ISMINE_NO;
}
80 changes: 39 additions & 41 deletions src/script/sign.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@

typedef std::vector<unsigned char> valtype;

TransactionSignatureCreator::TransactionSignatureCreator(const SigningProvider* provider, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : BaseSignatureCreator(provider), 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 (!m_provider->GetKey(address, key))
if (!provider.GetKey(address, key))
return false;

uint256 hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion);
Expand All @@ -29,24 +29,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.push_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 @@ -58,7 +58,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)
{
CScript scriptRet;
Expand All @@ -77,28 +77,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.Provider().GetPubKey(keyID, vch);
provider.GetPubKey(keyID, vch);
ret.push_back(ToByteVector(vch));
}
return true;
case TX_SCRIPTHASH:
if (creator.Provider().GetCScript(uint160(vSolutions[0]), scriptRet)) {
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));

default:
return false;
Expand All @@ -120,11 +120,11 @@ static CScript PushAll(const std::vector<valtype>& values)
return result;
}

bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata)
bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata)
{
std::vector<valtype> result;
txnouttype whichType;
bool solved = SignStep(creator, fromPubKey, result, whichType, SigVersion::BASE);
bool solved = SignStep(provider, creator, fromPubKey, result, whichType, SigVersion::BASE);
bool P2SH = false;
CScript subscript;

Expand All @@ -134,7 +134,7 @@ bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPu
// the final scriptSig is the signatures from that
// and then the serialized subscript:
subscript = CScript(result[0].begin(), result[0].end());
solved = solved && SignStep(creator, subscript, result, whichType, SigVersion::BASE) && whichType != TX_SCRIPTHASH;
solved = solved && SignStep(provider, creator, subscript, result, whichType, SigVersion::BASE) && whichType != TX_SCRIPTHASH;
P2SH = true;
}

Expand Down Expand Up @@ -171,10 +171,10 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
assert(nIn < txTo.vin.size());

CTransaction txToConst(txTo);
TransactionSignatureCreator creator(&provider, &txToConst, nIn, amount, nHashType);
TransactionSignatureCreator creator(&txToConst, nIn, amount, nHashType);

SignatureData sigdata;
bool ret = ProduceSignature(creator, fromPubKey, sigdata);
bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
UpdateTransaction(txTo, nIn, sigdata);
return ret;
}
Expand Down Expand Up @@ -321,36 +321,34 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature

namespace {
/** Dummy signature checker which accepts all signatures. */
class DummySignatureChecker : public BaseSignatureChecker
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; }
};
const DummySignatureChecker DUMMY_CHECKER;

bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
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;
} // namespace

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();
24 changes: 7 additions & 17 deletions src/script/sign.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,14 @@ class SigningProvider
virtual bool GetKey(const CKeyID &address, CKey& key) const =0;
};

/** Virtual base class for signature creators. */
/** Interface for signature creators. */
class BaseSignatureCreator {
protected:
const SigningProvider* m_provider;

public:
explicit BaseSignatureCreator(const SigningProvider* provider) : m_provider(provider) {}
const SigningProvider& Provider() const { return *m_provider; }
virtual ~BaseSignatureCreator() {}
virtual const BaseSignatureChecker& Checker() const =0;

/** Create a singular (non-script) signature. */
virtual bool CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0;
virtual bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0;
};

/** A signature creator for transactions. */
Expand All @@ -50,25 +45,20 @@ class TransactionSignatureCreator : public BaseSignatureCreator {
const TransactionSignatureChecker checker;

public:
TransactionSignatureCreator(const SigningProvider* provider, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL);
TransactionSignatureCreator(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL);
const BaseSignatureChecker& Checker() const override{ return checker; }
bool CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
};

class MutableTransactionSignatureCreator : public TransactionSignatureCreator {
CTransaction tx;

public:
MutableTransactionSignatureCreator(const SigningProvider* provider, const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount, int nHashTypeIn) : TransactionSignatureCreator(provider, &tx, nInIn, amount, nHashTypeIn), tx(*txToIn) {}
MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount, int nHashTypeIn) : TransactionSignatureCreator(&tx, nInIn, amount, nHashTypeIn), tx(*txToIn) {}
};

/** A signature creator that just produces 72-byte empty signatures. */
class DummySignatureCreator : public BaseSignatureCreator {
public:
explicit DummySignatureCreator(const SigningProvider* provider) : BaseSignatureCreator(provider) {}
const BaseSignatureChecker& Checker() const override;
bool CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
};
extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR;

struct SignatureData {
CScript scriptSig;
Expand All @@ -78,7 +68,7 @@ struct SignatureData {
};

/** Produce a script signature using a generic signature creator. */
bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& scriptPubKey, SignatureData& sigdata);
bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& scriptPubKey, SignatureData& sigdata);

/** Produce a script signature for a transaction. */
bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType);
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,7 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const
const CScript& scriptPubKey = txout.scriptPubKey;
SignatureData sigdata;

if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata))
if (!ProduceSignature(*this, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata))
{
return false;
} else {
Expand Down Expand Up @@ -3738,7 +3738,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
for (const auto& coin : vecCoins) {
const CScript& scriptPubKey = coin.txout.scriptPubKey;
SignatureData sigdata;
if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) {
if (!ProduceSignature(*this, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) {
strFailReason = _("Signing transaction failed");
return false;
} else {
Expand Down Expand Up @@ -3927,7 +3927,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
const CScript& scriptPubKey = coin.txout.scriptPubKey;
SignatureData sigdata;

if (!ProduceSignature(TransactionSignatureCreator(this, &txNewConst, nIn, SIGHASH_ALL), scriptPubKey, sigdata))
if (!ProduceSignature(*this, TransactionSignatureCreator(&txNewConst, nIn, SIGHASH_ALL), scriptPubKey, sigdata))
{
strFailReason = _("Signing transaction failed");
return false;
Expand Down

0 comments on commit 71944f5

Please sign in to comment.