Merge #13309: Directly operate with CMutableTransaction in SignSignature

6b8b63af1461dc11ffd813401e2c36fa44656715 Generic TransactionSignatureCreator works with both CTransaction and CMutableTransaction (Martin Ankerl)

Pull request description:

  Refactored `TransactionSignatureCreator` into a templated `GenericTransactionSignatureCreator` that works with both `CMutableTransaction` and `CTransaction`.

  The advantage is that now in `SignSignature`, the `MutableTransactionSignatureCreator` can now operate directly with the `CMutableTransaction` without the need to copy the data into a `CTransaction`.

  Running all unit tests brings a very noticable speedup on my machine:

      48.4 sec before this change
      36.4 sec with this change
      --------
      12.0 seconds saved

  running only `--run_test=transaction_tests/test_big_witness_transaction`:

      16.7 sec before this change
       5.9 sec with this change
      --------
      10.8 seconds saved

  This relates to my first attempt with the const_cast hack #13202, and to the slow unit test issue #10026.

  Also see #13050 which modifies the tests but not the production code (like this PR) to get a speedup.

Tree-SHA512: 2cff0e9699f484f26120a40e431a24c8bc8f9e780fd89cb0ecf20c5be3eab6c43f9c359cde244abd9f3620d06c7c354e3b9dd3da41fa2ca1ac1e09386fea25fb
This commit is contained in:
Wladimir J. van der Laan 2018-05-31 10:39:09 +02:00 committed by UdjinM6
parent ff9623ec1c
commit 3d4017d30d
No known key found for this signature in database
GPG Key ID: 83592BD1400D58D9
5 changed files with 61 additions and 52 deletions

View File

@ -1032,7 +1032,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
CSHA256() CSHA256()
.Write(vchMessage.data(), vchMessage.size()) .Write(vchMessage.data(), vchMessage.size())
.Finalize(vchHash.data()); .Finalize(vchHash.data());
fSuccess = checker.VerifySignature(vchSig, CPubKey(vchPubKey), uint256(vchHash)); fSuccess = CPubKey(vchPubKey).Verify(uint256(vchHash), vchSig);
} }
if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size()) { if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size()) {
@ -1308,9 +1308,11 @@ namespace {
* Wrapper that serializes like CTransaction, but with the modifications * Wrapper that serializes like CTransaction, but with the modifications
* required for the signature hash done in-place * required for the signature hash done in-place
*/ */
class CTransactionSignatureSerializer { template <class T>
class CTransactionSignatureSerializer
{
private: private:
const CTransaction& txTo; //!< reference to the spending transaction (the one being serialized) const T& txTo; //!< reference to the spending transaction (the one being serialized)
const CScript& scriptCode; //!< output script being consumed const CScript& scriptCode; //!< output script being consumed
const unsigned int nIn; //!< input index of txTo being signed const unsigned int nIn; //!< input index of txTo being signed
const bool fAnyoneCanPay; //!< whether the hashtype has the SIGHASH_ANYONECANPAY flag set const bool fAnyoneCanPay; //!< whether the hashtype has the SIGHASH_ANYONECANPAY flag set
@ -1318,7 +1320,7 @@ private:
const bool fHashNone; //!< whether the hashtype is SIGHASH_NONE const bool fHashNone; //!< whether the hashtype is SIGHASH_NONE
public: public:
CTransactionSignatureSerializer(const CTransaction &txToIn, const CScript &scriptCodeIn, unsigned int nInIn, int nHashTypeIn) : CTransactionSignatureSerializer(const T& txToIn, const CScript& scriptCodeIn, unsigned int nInIn, int nHashTypeIn) :
txTo(txToIn), scriptCode(scriptCodeIn), nIn(nInIn), txTo(txToIn), scriptCode(scriptCodeIn), nIn(nInIn),
fAnyoneCanPay(!!(nHashTypeIn & SIGHASH_ANYONECANPAY)), fAnyoneCanPay(!!(nHashTypeIn & SIGHASH_ANYONECANPAY)),
fHashSingle((nHashTypeIn & 0x1f) == SIGHASH_SINGLE), fHashSingle((nHashTypeIn & 0x1f) == SIGHASH_SINGLE),
@ -1402,7 +1404,9 @@ public:
} }
}; };
uint256 GetPrevoutHash(const CTransaction& txTo) { template <class T>
uint256 GetPrevoutHash(const T& txTo)
{
CHashWriter ss(SER_GETHASH, 0); CHashWriter ss(SER_GETHASH, 0);
for (const auto& txin : txTo.vin) { for (const auto& txin : txTo.vin) {
ss << txin.prevout; ss << txin.prevout;
@ -1410,7 +1414,9 @@ uint256 GetPrevoutHash(const CTransaction& txTo) {
return ss.GetHash(); return ss.GetHash();
} }
uint256 GetSequenceHash(const CTransaction& txTo) { template <class T>
uint256 GetSequenceHash(const T& txTo)
{
CHashWriter ss(SER_GETHASH, 0); CHashWriter ss(SER_GETHASH, 0);
for (const auto& txin : txTo.vin) { for (const auto& txin : txTo.vin) {
ss << txin.nSequence; ss << txin.nSequence;
@ -1418,7 +1424,9 @@ uint256 GetSequenceHash(const CTransaction& txTo) {
return ss.GetHash(); return ss.GetHash();
} }
uint256 GetOutputsHash(const CTransaction& txTo) { template <class T>
uint256 GetOutputsHash(const T& txTo)
{
CHashWriter ss(SER_GETHASH, 0); CHashWriter ss(SER_GETHASH, 0);
for (const auto& txout : txTo.vout) { for (const auto& txout : txTo.vout) {
ss << txout; ss << txout;
@ -1428,14 +1436,20 @@ uint256 GetOutputsHash(const CTransaction& txTo) {
} // namespace } // namespace
PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo) template <class T>
PrecomputedTransactionData::PrecomputedTransactionData(const T& txTo)
{ {
hashPrevouts = GetPrevoutHash(txTo); hashPrevouts = GetPrevoutHash(txTo);
hashSequence = GetSequenceHash(txTo); hashSequence = GetSequenceHash(txTo);
hashOutputs = GetOutputsHash(txTo); hashOutputs = GetOutputsHash(txTo);
} }
uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache) // explicit instantiation
template PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo);
template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo);
template <class T>
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache)
{ {
assert(nIn < txTo.vin.size()); assert(nIn < txTo.vin.size());
@ -1450,7 +1464,7 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig
} }
// Wrapper to serialize only the necessary parts of the transaction being signed // Wrapper to serialize only the necessary parts of the transaction being signed
CTransactionSignatureSerializer txTmp(txTo, scriptCode, nIn, nHashType); CTransactionSignatureSerializer<T> txTmp(txTo, scriptCode, nIn, nHashType);
// Serialize and hash // Serialize and hash
CHashWriter ss(SER_GETHASH, 0); CHashWriter ss(SER_GETHASH, 0);
@ -1458,12 +1472,14 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig
return ss.GetHash(); return ss.GetHash();
} }
bool BaseSignatureChecker::VerifySignature(const std::vector<uint8_t>& vchSig, const CPubKey& pubkey, const uint256& sighash) const template <class T>
bool GenericTransactionSignatureChecker<T>::VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
{ {
return pubkey.Verify(sighash, vchSig); return pubkey.Verify(sighash, vchSig);
} }
bool TransactionSignatureChecker::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const template <class T>
bool GenericTransactionSignatureChecker<T>::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
{ {
CPubKey pubkey(vchPubKey); CPubKey pubkey(vchPubKey);
if (!pubkey.IsValid()) if (!pubkey.IsValid())
@ -1484,7 +1500,8 @@ bool TransactionSignatureChecker::CheckSig(const std::vector<unsigned char>& vch
return true; return true;
} }
bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) const template <class T>
bool GenericTransactionSignatureChecker<T>::CheckLockTime(const CScriptNum& nLockTime) const
{ {
// There are two kinds of nLockTime: lock-by-blockheight // There are two kinds of nLockTime: lock-by-blockheight
// and lock-by-blocktime, distinguished by whether // and lock-by-blocktime, distinguished by whether
@ -1520,7 +1537,8 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con
return true; return true;
} }
bool TransactionSignatureChecker::CheckSequence(const CScriptNum& nSequence) const template <class T>
bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSequence) const
{ {
// Relative lock times are supported by comparing the passed // Relative lock times are supported by comparing the passed
// in operand to the sequence number of the input. // in operand to the sequence number of the input.
@ -1566,6 +1584,10 @@ bool TransactionSignatureChecker::CheckSequence(const CScriptNum& nSequence) con
return true; return true;
} }
// explicit instantiation
template class GenericTransactionSignatureChecker<CTransaction>;
template class GenericTransactionSignatureChecker<CMutableTransaction>;
bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror) bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror)
{ {
set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR); set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR);

View File

@ -110,7 +110,8 @@ struct PrecomputedTransactionData
{ {
uint256 hashPrevouts, hashSequence, hashOutputs; uint256 hashPrevouts, hashSequence, hashOutputs;
explicit PrecomputedTransactionData(const CTransaction& tx); template <class T>
explicit PrecomputedTransactionData(const T& tx);
}; };
enum class SigVersion enum class SigVersion
@ -118,13 +119,12 @@ enum class SigVersion
BASE = 0, BASE = 0,
}; };
uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr); template <class T>
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr);
class BaseSignatureChecker class BaseSignatureChecker
{ {
public: public:
virtual bool VerifySignature(const std::vector<uint8_t>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
{ {
return false; return false;
@ -143,33 +143,29 @@ public:
virtual ~BaseSignatureChecker() {} virtual ~BaseSignatureChecker() {}
}; };
class TransactionSignatureChecker : public BaseSignatureChecker template <class T>
class GenericTransactionSignatureChecker : public BaseSignatureChecker
{ {
private: private:
const CTransaction* txTo; const T* txTo;
unsigned int nIn; unsigned int nIn;
const CAmount amount; const CAmount amount;
const PrecomputedTransactionData* txdata; const PrecomputedTransactionData* txdata;
public: protected:
TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {} virtual bool VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
// The overriden functions are now final.
bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const final;
bool CheckLockTime(const CScriptNum& nLockTime) const final;
bool CheckSequence(const CScriptNum& nSequence) const final;
};
class MutableTransactionSignatureChecker : public TransactionSignatureChecker
{
private:
const CTransaction txTo;
public: public:
MutableTransactionSignatureChecker(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : TransactionSignatureChecker(&txTo, nInIn, amount), txTo(*txToIn) {} GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
bool CheckLockTime(const CScriptNum& nLockTime) const override;
bool CheckSequence(const CScriptNum& nSequence) const override;
}; };
using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>;
using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CMutableTransaction>;
bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr); bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr);
bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr); bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr);

View File

@ -14,9 +14,9 @@
typedef std::vector<unsigned char> valtype; typedef std::vector<unsigned char> valtype;
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) {} MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
bool TransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
{ {
CKey key; CKey key;
if (!provider.GetKey(address, key)) if (!provider.GetKey(address, key))
@ -170,8 +170,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
{ {
assert(nIn < txTo.vin.size()); assert(nIn < txTo.vin.size());
CTransaction txToConst(txTo); MutableTransactionSignatureCreator creator(&txTo, nIn, amount, nHashType);
TransactionSignatureCreator creator(&txToConst, nIn, amount, nHashType);
SignatureData sigdata; SignatureData sigdata;
bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata); bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);

View File

@ -37,26 +37,19 @@ public:
}; };
/** A signature creator for transactions. */ /** A signature creator for transactions. */
class TransactionSignatureCreator : public BaseSignatureCreator { class MutableTransactionSignatureCreator : public BaseSignatureCreator {
const CTransaction* txTo; const CMutableTransaction* txTo;
unsigned int nIn; unsigned int nIn;
int nHashType; int nHashType;
CAmount amount; CAmount amount;
const TransactionSignatureChecker checker; const MutableTransactionSignatureChecker checker;
public: public:
TransactionSignatureCreator(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL); MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn = SIGHASH_ALL);
const BaseSignatureChecker& Checker() const override{ return checker; } const BaseSignatureChecker& Checker() const override{ return checker; }
bool CreateSig(const SigningProvider& provider, 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 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. */ /** A signature creator that just produces 72-byte empty signatures. */
extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR; extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR;

View File

@ -3891,14 +3891,13 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
if (sign) if (sign)
{ {
CTransaction txNewConst(txNew);
int nIn = 0; int nIn = 0;
for(const auto& coin : vecCoins) for(const auto& coin : vecCoins)
{ {
const CScript& scriptPubKey = coin.txout.scriptPubKey; const CScript& scriptPubKey = coin.txout.scriptPubKey;
SignatureData sigdata; SignatureData sigdata;
if (!ProduceSignature(*this, TransactionSignatureCreator(&txNewConst, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata)) if (!ProduceSignature(*this, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
{ {
strFailReason = _("Signing transaction failed"); strFailReason = _("Signing transaction failed");
return false; return false;