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.

Sign up for GitHub

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

Precompute sighashes #8524

Merged
merged 3 commits into from
Sep 1, 2016
Merged

Precompute sighashes #8524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2c5d04
Precompute sighashes
sipa Aug 16, 2016
ab48c5e
Unit test for sighash caching
NicolasDorier Aug 16, 2016
35fe039
Rename to PrecomputedTransactionData
sipa Aug 26, 2016
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
Next Next commit
Precompute sighashes 
Original version by Nicolas Dorier. Precomputing version by Pieter Wuille.
  • Loading branch information
@sipa
sipa committed Aug 16, 2016
commit d2c5d044d00ec805957ab246a7863d83ca075805
22 changes: 13 additions & 9 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,12 +1492,13 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C

// Check against previous transactions
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true)) {
CachedHashes cachedHashes(tx);
if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, cachedHashes)) {
// SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
// need to turn both off, and compare against just turning off CLEANSTACK
// to see if the failure is specifically due to witness validation.
if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true) &&
!CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true)) {
if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, cachedHashes) &&
!CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, cachedHashes)) {
// Only the witness is wrong, so the transaction itself may be fine.
state.SetCorruptionPossible();
}
Expand All @@ -1513,7 +1514,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
// There is a similar check in CreateNewBlock() to prevent creating
// invalid blocks, however allowing such transactions into the mempool
// can be exploited as a DoS attack.
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true))
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes))
{
return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s",
__func__, hash.ToString(), FormatStateMessage(state));
Expand Down Expand Up @@ -1910,7 +1911,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight)
bool CScriptCheck::operator()() {
const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
const CScriptWitness *witness = (nIn < ptxTo->wit.vtxinwit.size()) ? &ptxTo->wit.vtxinwit[nIn].scriptWitness : NULL;
if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore), &error)) {
if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *cachedHashes), &error)) {
return false;
}
return true;
Expand Down Expand Up @@ -1969,7 +1970,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
}
}// namespace Consensus

bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks)
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector<CScriptCheck> *pvChecks)
{
if (!tx.IsCoinBase())
{
Expand All @@ -1996,7 +1997,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
assert(coins);

// Verify signature
CScriptCheck check(*coins, tx, i, flags, cacheStore);
CScriptCheck check(*coins, tx, i, flags, cacheStore, &cachedHashes);
if (pvChecks) {
pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back());
Expand All @@ -2009,7 +2010,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// avoid splitting the network between upgraded and
// non-upgraded nodes.
CScriptCheck check2(*coins, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore);
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes);
if (check2())
return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
}
Expand Down Expand Up @@ -2405,6 +2406,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
std::vector<std::pair<uint256, CDiskTxPos> > vPos;
vPos.reserve(block.vtx.size());
blockundo.vtxundo.reserve(block.vtx.size() - 1);
std::vector<CachedHashes> cachedHashes;
cachedHashes.reserve(block.vtx.size()); // Required so that pointers to individual CachedHashes don't get invalidated
for (unsigned int i = 0; i < block.vtx.size(); i++)
{
const CTransaction &tx = block.vtx[i];
Expand Down Expand Up @@ -2451,13 +2454,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
return state.DoS(100, error("ConnectBlock(): too many sigops"),
REJECT_INVALID, "bad-blk-sigops");

cachedHashes.emplace_back(tx);
if (!tx.IsCoinBase())
{
nFees += view.GetValueIn(tx)-tx.GetValueOut();

std::vector<CScriptCheck> vChecks;
bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL))
if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, cachedHashes[i], nScriptCheckThreads ? &vChecks : NULL))
return error("ConnectBlock(): CheckInputs on %s failed with %s",
tx.GetHash().ToString(), FormatStateMessage(state));
control.Add(vChecks);
Expand Down
9 changes: 6 additions & 3 deletions src/main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class CScriptCheck;
class CTxMemPool;
class CValidationInterface;
class CValidationState;
class CachedHashes;

struct CNodeStateStats;
struct LockPoints;
Expand Down Expand Up @@ -347,7 +348,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
* instead of being performed inline.
*/
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks,
unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks = NULL);
unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector<CScriptCheck> *pvChecks = NULL);

/** Apply the effects of this transaction on the UTXO set represented by view */
void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
Expand Down Expand Up @@ -421,12 +422,13 @@ class CScriptCheck
unsigned int nFlags;
bool cacheStore;
ScriptError error;
CachedHashes *cachedHashes;

public:
CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) :
CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, CachedHashes* cachedHashesIn) :
scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue),
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) { }
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), cachedHashes(cachedHashesIn) { }

bool operator()();

Expand All @@ -438,6 +440,7 @@ class CScriptCheck
std::swap(nFlags, check.nFlags);
std::swap(cacheStore, check.cacheStore);
std::swap(error, check.error);
std::swap(cachedHashes, check.cachedHashes);
}

ScriptError GetScriptError() const { return error; }
Expand Down
4 changes: 2 additions & 2 deletions src/script/bitcoinconsensus.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP

// Regardless of the verification result, the tx did not error.
set_error(err, bitcoinconsensus_ERR_OK);

return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount), NULL);
CachedHashes cachedHashes(tx);
return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount, cachedHashes), NULL);
} catch (const std::exception&) {
return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing
}
Expand Down
54 changes: 37 additions & 17 deletions src/script/interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1108,37 +1108,57 @@ class CTransactionSignatureSerializer {
}
};

uint256 GetPrevoutHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vin.size(); n++) {
ss << txTo.vin[n].prevout;
}
return ss.GetHash();
}

uint256 GetSequenceHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vin.size(); n++) {
ss << txTo.vin[n].nSequence;
}
return ss.GetHash();
}

uint256 GetOutputsHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vout.size(); n++) {
ss << txTo.vout[n];
}
return ss.GetHash();
}

} // anon namespace

uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion)
CachedHashes::CachedHashes(const CTransaction& txTo)
{
hashPrevouts = GetPrevoutHash(txTo);
hashSequence = GetSequenceHash(txTo);
hashOutputs = GetOutputsHash(txTo);
}

uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache)
{
if (sigversion == SIGVERSION_WITNESS_V0) {
uint256 hashPrevouts;
uint256 hashSequence;
uint256 hashOutputs;

if (!(nHashType & SIGHASH_ANYONECANPAY)) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vin.size(); n++) {
ss << txTo.vin[n].prevout;
}
hashPrevouts = ss.GetHash(); // TODO: cache this value for all signatures in a transaction
hashPrevouts = cache ? cache->hashPrevouts : GetPrevoutHash(txTo);
}

if (!(nHashType & SIGHASH_ANYONECANPAY) && (nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vin.size(); n++) {
ss << txTo.vin[n].nSequence;
}
hashSequence = ss.GetHash(); // TODO: cache this value for all signatures in a transaction
hashSequence = cache ? cache->hashSequence : GetSequenceHash(txTo);
}


if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) {
CHashWriter ss(SER_GETHASH, 0);
for (unsigned int n = 0; n < txTo.vout.size(); n++) {
ss << txTo.vout[n];
}
hashOutputs = ss.GetHash(); // TODO: cache this value for all signatures in a transaction
hashOutputs = cache ? cache->hashOutputs : GetOutputsHash(txTo);
} else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) {
CHashWriter ss(SER_GETHASH, 0);
ss << txTo.vout[nIn];
Expand Down Expand Up @@ -1209,7 +1229,7 @@ bool TransactionSignatureChecker::CheckSig(const vector<unsigned char>& vchSigIn
int nHashType = vchSig.back();
vchSig.pop_back();

uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion);
uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->cachedHashes);

if (!VerifySignature(vchSig, pubkey, sighash))
return false;
Expand Down
13 changes: 11 additions & 2 deletions src/script/interpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,20 @@ enum

bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned int flags, ScriptError* serror);

struct CachedHashes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this struct name is too general, maybe CachedTransactionHashes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually not a cache either anymore.

What about PrecomputedTransactionData ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commit.

{
uint256 hashPrevouts, hashSequence, hashOutputs;

CachedHashes(const CTransaction& tx);
};

enum SigVersion
{
SIGVERSION_BASE = 0,
SIGVERSION_WITNESS_V0 = 1,
};

uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion);
uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache = NULL);

class BaseSignatureChecker
{
Expand Down Expand Up @@ -133,12 +140,14 @@ class TransactionSignatureChecker : public BaseSignatureChecker
const CTransaction* txTo;
unsigned int nIn;
const CAmount amount;
const CachedHashes* cachedHashes;

protected:
virtual bool VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;

public:
TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn) {}
TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(NULL) {}
TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const CachedHashes& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(&cachedHashesIn) {}
bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const;
bool CheckLockTime(const CScriptNum& nLockTime) const;
bool CheckSequence(const CScriptNum& nSequence) const;
Expand Down
2 changes: 1 addition & 1 deletion src/script/sigcache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker
bool store;

public:
CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn) : TransactionSignatureChecker(txToIn, nInIn, amount), store(storeIn) {}
CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, CachedHashes& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {}

bool VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
};
Expand Down
6 changes: 4 additions & 2 deletions src/test/script_P2SH_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,20 @@ BOOST_AUTO_TEST_CASE(sign)
}
// All of the above should be OK, and the txTos have valid signatures
// Check to make sure signature verification fails if we use the wrong ScriptSig:
for (int i = 0; i < 8; i++)
for (int i = 0; i < 8; i++) {
CachedHashes cachedHashes(txTo[i]);
for (int j = 0; j < 8; j++)
{
CScript sigSave = txTo[i].vin[0].scriptSig;
txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig;
bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false)();
bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &cachedHashes)();
if (i == j)
BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j));
else
BOOST_CHECK_MESSAGE(!sigOK, strprintf("VerifySignature %d %d", i, j));
txTo[i].vin[0].scriptSig = sigSave;
}
}
}

BOOST_AUTO_TEST_CASE(norecurse)
Expand Down
6 changes: 4 additions & 2 deletions src/test/transaction_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest);
BOOST_CHECK(state.IsValid());

CachedHashes cachedHashes(tx);
for (unsigned int i = 0; i < tx.vin.size(); i++)
{
if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout))
Expand All @@ -168,7 +169,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
unsigned int verify_flags = ParseScriptFlags(test[2].get_str());
const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL;
BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout],
witness, verify_flags, TransactionSignatureChecker(&tx, i, amount), &err),
witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err),
strTest);
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err));
}
Expand Down Expand Up @@ -237,6 +238,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
CValidationState state;
fValid = CheckTransaction(tx, state) && state.IsValid();

CachedHashes cachedHashes(tx);
for (unsigned int i = 0; i < tx.vin.size() && fValid; i++)
{
if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout))
Expand All @@ -252,7 +254,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
}
const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL;
fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout],
witness, verify_flags, TransactionSignatureChecker(&tx, i, amount), &err);
witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err);
}
BOOST_CHECK_MESSAGE(!fValid, strTest);
BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err));
Expand Down