diff --git a/src/Makefile.test.include b/src/Makefile.test.include --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -119,6 +119,8 @@ test/serialize_tests.cpp \ test/sighash_tests.cpp \ test/sigopcount_tests.cpp \ + test/sigutil.cpp \ + test/sigutil.h \ test/skiplist_tests.cpp \ test/streams_tests.cpp \ test/test_bitcoin.cpp \ diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -12,6 +12,7 @@ #include "script/script_error.h" #include "script/sign.h" #include "test/scriptflags.h" +#include "test/sigutil.h" #include "test/test_bitcoin.h" #include "util.h" #include "utilstrencodings.h" @@ -200,49 +201,6 @@ #endif } -static void NegateSignatureS(std::vector &vchSig) { - // Parse the signature. - std::vector r, s; - r = std::vector(vchSig.begin() + 4, - vchSig.begin() + 4 + vchSig[3]); - s = std::vector(vchSig.begin() + 6 + vchSig[3], - vchSig.begin() + 6 + vchSig[3] + - vchSig[5 + vchSig[3]]); - - // Really ugly to implement mod-n negation here, but it would be feature - // creep to expose such functionality from libsecp256k1. - static const uint8_t order[33] = { - 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE, 0xBA, 0xAE, 0xDC, 0xE6, 0xAF, - 0x48, 0xA0, 0x3B, 0xBF, 0xD2, 0x5E, 0x8C, 0xD0, 0x36, 0x41, 0x41}; - while (s.size() < 33) { - s.insert(s.begin(), 0x00); - } - - int carry = 0; - for (int p = 32; p >= 1; p--) { - int n = (int)order[p] - s[p] - carry; - s[p] = (n + 256) & 0xFF; - carry = (n < 0); - } - - assert(carry == 0); - if (s.size() > 1 && s[0] == 0 && s[1] < 0x80) { - s.erase(s.begin()); - } - - // Reconstruct the signature. - vchSig.clear(); - vchSig.push_back(0x30); - vchSig.push_back(4 + r.size() + s.size()); - vchSig.push_back(0x02); - vchSig.push_back(r.size()); - vchSig.insert(vchSig.end(), r.begin(), r.end()); - vchSig.push_back(0x02); - vchSig.push_back(s.size()); - vchSig.insert(vchSig.end(), s.begin(), s.end()); -} - namespace { const uint8_t vchKey0[32] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; diff --git a/src/test/sigutil.h b/src/test/sigutil.h new file mode 100644 --- /dev/null +++ b/src/test/sigutil.h @@ -0,0 +1,13 @@ +// Copyright (c) 2017 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_SIGUTIL_H +#define BITCOIN_TEST_SIGUTIL_H + +#include +#include + +void NegateSignatureS(std::vector &vchSig); + +#endif // BITCOIN_TEST_SIGUTIL_H diff --git a/src/test/sigutil.cpp b/src/test/sigutil.cpp new file mode 100644 --- /dev/null +++ b/src/test/sigutil.cpp @@ -0,0 +1,50 @@ +// Copyright (c) 2017 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "test/sigutil.h" + +#include + +void NegateSignatureS(std::vector &vchSig) { + // Parse the signature. + std::vector r, s; + r = std::vector(vchSig.begin() + 4, + vchSig.begin() + 4 + vchSig[3]); + s = std::vector(vchSig.begin() + 6 + vchSig[3], + vchSig.begin() + 6 + vchSig[3] + + vchSig[5 + vchSig[3]]); + + // Really ugly to implement mod-n negation here, but it would be feature + // creep to expose such functionality from libsecp256k1. + static const uint8_t order[33] = { + 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE, 0xBA, 0xAE, 0xDC, 0xE6, 0xAF, + 0x48, 0xA0, 0x3B, 0xBF, 0xD2, 0x5E, 0x8C, 0xD0, 0x36, 0x41, 0x41}; + while (s.size() < 33) { + s.insert(s.begin(), 0x00); + } + + int carry = 0; + for (int p = 32; p >= 1; p--) { + int n = (int)order[p] - s[p] - carry; + s[p] = (n + 256) & 0xFF; + carry = (n < 0); + } + + assert(carry == 0); + if (s.size() > 1 && s[0] == 0 && s[1] < 0x80) { + s.erase(s.begin()); + } + + // Reconstruct the signature. + vchSig.clear(); + vchSig.push_back(0x30); + vchSig.push_back(4 + r.size() + s.size()); + vchSig.push_back(0x02); + vchSig.push_back(r.size()); + vchSig.insert(vchSig.end(), r.begin(), r.end()); + vchSig.push_back(0x02); + vchSig.push_back(s.size()); + vchSig.insert(vchSig.end(), s.begin(), s.end()); +} diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -5,10 +5,14 @@ #include "config.h" #include "consensus/validation.h" #include "key.h" +#include "keystore.h" #include "miner.h" #include "pubkey.h" #include "random.h" +#include "script/scriptcache.h" +#include "script/sign.h" #include "script/standard.h" +#include "test/sigutil.h" #include "test/test_bitcoin.h" #include "txmempool.h" #include "utiltime.h" @@ -31,7 +35,6 @@ // Make sure skipping validation of transctions that were validated going // into the memory pool does not allow double-spends in blocks to pass // validation when they should not. - CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; @@ -86,4 +89,302 @@ BOOST_CHECK_EQUAL(mempool.size(), 0); } +// Run CheckInputs (using pcoinsTip) on the given transaction, for all script +// flags. Test that CheckInputs passes for all flags that don't overlap with the +// failing_flags argument, but otherwise fails. +// CHECKLOCKTIMEVERIFY and CHECKSEQUENCEVERIFY (and future NOP codes that may +// get reassigned) have an interaction with DISCOURAGE_UPGRADABLE_NOPS: if the +// script flags used contain DISCOURAGE_UPGRADABLE_NOPS but don't contain +// CHECKLOCKTIMEVERIFY (or CHECKSEQUENCEVERIFY), but the script does contain +// OP_CHECKLOCKTIMEVERIFY (or OP_CHECKSEQUENCEVERIFY), then script execution +// should fail. +// Capture this interaction with the upgraded_nop argument: set it when +// evaluating any script flag that is implemented as an upgraded NOP code. +void ValidateCheckInputsForAllFlags(CMutableTransaction &tx, + uint32_t failing_flags, bool add_to_cache, + bool upgraded_nop) { + PrecomputedTransactionData txdata(tx); + // If we add many more flags, this loop can get too expensive, but we can + // rewrite in the future to randomly pick a set of flags to evaluate. + for (size_t test_flags = 0; test_flags < (1U << 17); test_flags += 1) { + CValidationState state; + // Make sure the mandatory flags are enabled. + test_flags |= MANDATORY_SCRIPT_VERIFY_FLAGS; + + bool ret = CheckInputs(tx, state, pcoinsTip, true, test_flags, true, + add_to_cache, txdata, nullptr); + + // CheckInputs should succeed iff test_flags doesn't intersect with + // failing_flags + bool expected_return_value = !(test_flags & failing_flags); + if (expected_return_value && upgraded_nop) { + // If the script flag being tested corresponds to an upgraded NOP, + // then script execution should fail if DISCOURAGE_UPGRADABLE_NOPS + // is set. + expected_return_value = + !(test_flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS); + } + + BOOST_CHECK_EQUAL(ret, expected_return_value); + + // Test the caching + if (ret && add_to_cache) { + // Check that we get a cache hit if the tx was valid + std::vector scriptchecks; + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip, true, test_flags, + true, add_to_cache, txdata, &scriptchecks)); + BOOST_CHECK(scriptchecks.empty()); + } else { + // Check that we get script executions to check, if the transaction + // was invalid, or we didn't add to cache. + std::vector scriptchecks; + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip, true, test_flags, + true, add_to_cache, txdata, &scriptchecks)); + BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); + } + } +} + +BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) { + // Test that passing CheckInputs with one set of script flags doesn't imply + // that we would pass again with a different set of flags. + InitScriptExecutionCache(); + + CScript p2pk_scriptPubKey = + CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; + CScript p2sh_scriptPubKey = + GetScriptForDestination(CScriptID(p2pk_scriptPubKey)); + CScript p2pkh_scriptPubKey = + GetScriptForDestination(coinbaseKey.GetPubKey().GetID()); + + CBasicKeyStore keystore; + keystore.AddKey(coinbaseKey); + keystore.AddCScript(p2pk_scriptPubKey); + + // flags to test: SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, + // SCRIPT_VERIFY_CHECKSEQUENCE_VERIFY, SCRIPT_VERIFY_NULLDUMMY, uncompressed + // pubkey thing + + // Create 2 outputs that match the three scripts above, spending the first + // coinbase tx. + CMutableTransaction spend_tx; + + spend_tx.nVersion = 1; + spend_tx.vin.resize(1); + spend_tx.vin[0].prevout.hash = coinbaseTxns[0].GetId(); + spend_tx.vin[0].prevout.n = 0; + spend_tx.vout.resize(4); + spend_tx.vout[0].nValue = 11 * CENT; + spend_tx.vout[0].scriptPubKey = p2sh_scriptPubKey; + spend_tx.vout[1].nValue = 11 * CENT; + spend_tx.vout[1].scriptPubKey = + CScript() << OP_CHECKLOCKTIMEVERIFY << OP_DROP + << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; + spend_tx.vout[2].nValue = 11 * CENT; + spend_tx.vout[2].scriptPubKey = + CScript() << OP_CHECKSEQUENCEVERIFY << OP_DROP + << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; + spend_tx.vout[3].nValue = 11 * CENT; + spend_tx.vout[3].scriptPubKey = p2sh_scriptPubKey; + + // Sign, with a non-DER signature + { + std::vector vchSig; + uint256 hash = SignatureHash(p2pk_scriptPubKey, spend_tx, 0, + SIGHASH_ALL | SIGHASH_FORKID, + coinbaseTxns[0].vout[0].nValue); + BOOST_CHECK(coinbaseKey.Sign(hash, vchSig)); + // Negate S to ensure the signature is valid but non standard. + NegateSignatureS(vchSig); + vchSig.push_back(uint8_t(SIGHASH_ALL | SIGHASH_FORKID)); + spend_tx.vin[0].scriptSig << vchSig; + } + + LOCK(cs_main); + + // Test that invalidity under a set of flags doesn't preclude validity under + // other (eg consensus) flags. + // spend_tx is invalid according to DERSIG + CValidationState state; + { + PrecomputedTransactionData ptd_spend_tx(spend_tx); + + BOOST_CHECK( + !CheckInputs(spend_tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_LOW_S, + true, true, ptd_spend_tx, nullptr)); + + // If we call again asking for scriptchecks (as happens in + // ConnectBlock), we should add a script check object for this -- we're + // not caching invalidity (if that changes, delete this test case). + std::vector scriptchecks; + BOOST_CHECK( + CheckInputs(spend_tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_LOW_S, + true, true, ptd_spend_tx, &scriptchecks)); + BOOST_CHECK_EQUAL(scriptchecks.size(), 1); + + // Test that CheckInputs returns true iff LOW_S-enforcing flags are not + // present. Don't add these checks to the cache, so that we can test + // later that block validation works fine in the absence of cached + // successes. + ValidateCheckInputsForAllFlags(spend_tx, SCRIPT_VERIFY_LOW_S, false, + false); + + // And if we produce a block with this tx, it should be valid (LOW_S not + // enabled yet), even though there's no cache entry. + CBlock block; + + block = CreateAndProcessBlock({spend_tx}, p2pk_scriptPubKey); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() == block.GetHash()); + BOOST_CHECK(pcoinsTip->GetBestBlock() == block.GetHash()); + } + + // Test P2SH: construct a transaction that is valid without P2SH, and then + // test validity with P2SH. + { + CMutableTransaction invalid_under_p2sh_tx; + invalid_under_p2sh_tx.nVersion = 1; + invalid_under_p2sh_tx.vin.resize(1); + invalid_under_p2sh_tx.vin[0].prevout.hash = spend_tx.GetId(); + invalid_under_p2sh_tx.vin[0].prevout.n = 0; + invalid_under_p2sh_tx.vout.resize(1); + invalid_under_p2sh_tx.vout[0].nValue = 11 * CENT; + invalid_under_p2sh_tx.vout[0].scriptPubKey = p2pk_scriptPubKey; + std::vector vchSig2(p2pk_scriptPubKey.begin(), + p2pk_scriptPubKey.end()); + invalid_under_p2sh_tx.vin[0].scriptSig << vchSig2; + + ValidateCheckInputsForAllFlags(invalid_under_p2sh_tx, + SCRIPT_VERIFY_P2SH, true, false); + } + + // Test CHECKLOCKTIMEVERIFY + { + CMutableTransaction invalid_with_cltv_tx; + invalid_with_cltv_tx.nVersion = 1; + invalid_with_cltv_tx.nLockTime = 100; + invalid_with_cltv_tx.vin.resize(1); + invalid_with_cltv_tx.vin[0].prevout.hash = spend_tx.GetId(); + invalid_with_cltv_tx.vin[0].prevout.n = 1; + invalid_with_cltv_tx.vin[0].nSequence = 0; + invalid_with_cltv_tx.vout.resize(1); + invalid_with_cltv_tx.vout[0].nValue = 11 * CENT; + invalid_with_cltv_tx.vout[0].scriptPubKey = p2pk_scriptPubKey; + + // Sign + std::vector vchSig; + uint256 hash = SignatureHash( + spend_tx.vout[1].scriptPubKey, invalid_with_cltv_tx, 0, + SIGHASH_ALL | SIGHASH_FORKID, spend_tx.vout[1].nValue); + BOOST_CHECK(coinbaseKey.Sign(hash, vchSig)); + vchSig.push_back(uint8_t(SIGHASH_ALL | SIGHASH_FORKID)); + invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 101; + + ValidateCheckInputsForAllFlags(invalid_with_cltv_tx, + SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, + true); + + // Make it valid, and check again + invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100; + CValidationState state; + PrecomputedTransactionData txdata(invalid_with_cltv_tx); + BOOST_CHECK(CheckInputs(invalid_with_cltv_tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS | + SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, + true, true, txdata, nullptr)); + } + + // TEST CHECKSEQUENCEVERIFY + { + CMutableTransaction invalid_with_csv_tx; + invalid_with_csv_tx.nVersion = 2; + invalid_with_csv_tx.vin.resize(1); + invalid_with_csv_tx.vin[0].prevout.hash = spend_tx.GetId(); + invalid_with_csv_tx.vin[0].prevout.n = 2; + invalid_with_csv_tx.vin[0].nSequence = 100; + invalid_with_csv_tx.vout.resize(1); + invalid_with_csv_tx.vout[0].nValue = 11 * CENT; + invalid_with_csv_tx.vout[0].scriptPubKey = p2pk_scriptPubKey; + + // Sign + std::vector vchSig; + uint256 hash = SignatureHash( + spend_tx.vout[2].scriptPubKey, invalid_with_csv_tx, 0, + SIGHASH_ALL | SIGHASH_FORKID, spend_tx.vout[2].nValue); + BOOST_CHECK(coinbaseKey.Sign(hash, vchSig)); + vchSig.push_back(uint8_t(SIGHASH_ALL | SIGHASH_FORKID)); + invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 101; + + ValidateCheckInputsForAllFlags( + invalid_with_csv_tx, SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true); + + // Make it valid, and check again + invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; + CValidationState state; + PrecomputedTransactionData txdata(invalid_with_csv_tx); + BOOST_CHECK(CheckInputs(invalid_with_csv_tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS | + SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, + true, true, txdata, nullptr)); + } + + // TODO: add tests for remaining script flags + + { + // Test a transaction with multiple inputs. + CMutableTransaction tx; + + tx.nVersion = 1; + tx.vin.resize(2); + tx.vin[0].prevout.hash = spend_tx.GetId(); + tx.vin[0].prevout.n = 0; + tx.vin[1].prevout.hash = spend_tx.GetId(); + tx.vin[1].prevout.n = 3; + tx.vout.resize(1); + tx.vout[0].nValue = 22 * CENT; + tx.vout[0].scriptPubKey = p2pk_scriptPubKey; + + // Sign + SignatureData sigdata; + ProduceSignature( + MutableTransactionSignatureCreator(&keystore, &tx, 0, 11 * CENT, + SIGHASH_ALL | SIGHASH_FORKID), + spend_tx.vout[0].scriptPubKey, sigdata); + UpdateTransaction(tx, 0, sigdata); + ProduceSignature( + MutableTransactionSignatureCreator(&keystore, &tx, 1, 11 * CENT, + SIGHASH_ALL | SIGHASH_FORKID), + spend_tx.vout[3].scriptPubKey, sigdata); + UpdateTransaction(tx, 1, sigdata); + + // This should be valid under all script flags + ValidateCheckInputsForAllFlags(tx, 0, true, false); + + // Check that if the second input is invalid, but the first input is + // valid, the transaction is not cached. + // Invalidate vin[1] + tx.vin[1].scriptSig = CScript(); + + CValidationState state; + PrecomputedTransactionData txdata(tx); + + // This transaction is now invalid because the second signature is + // missing. + BOOST_CHECK(!CheckInputs(tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS, true, true, + txdata, nullptr)); + + // Make sure this transaction was not cached (ie becausethe first input + // was valid) + std::vector scriptchecks; + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip, true, + MANDATORY_SCRIPT_VERIFY_FLAGS, true, true, + txdata, &scriptchecks)); + // Should get 2 script checks back -- caching is on a whole-transaction + // basis. + BOOST_CHECK_EQUAL(scriptchecks.size(), 2); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -427,6 +427,24 @@ uint64_t GetTransactionSigOpCount(const CTransaction &tx, const CCoinsViewCache &inputs, int flags); +/** + * Check whether all inputs of this transaction are valid (no double spends, + * scripts & sigs, amounts). This does not modify the UTXO set. + * + * If pvChecks is not nullptr, script checks are pushed onto it instead of being + * performed inline. Any script checks which are not necessary (eg due to script + * execution cache hits) are, obviously, not pushed onto pvChecks/run. + * + * Setting sigCacheStore/scriptCacheStore to false will remove elements from the + * corresponding cache which are matched. This is useful for checking blocks + * where we will likely never need the cache entry again. + */ +bool CheckInputs(const CTransaction &tx, CValidationState &state, + const CCoinsViewCache &view, bool fScriptChecks, + uint32_t flags, bool sigCacheStore, bool scriptCacheStore, + const PrecomputedTransactionData &txdata, + std::vector *pvChecks = nullptr); + /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction &tx, CCoinsViewCache &inputs, int nHeight); void UpdateCoins(const CTransaction &tx, CCoinsViewCache &inputs, diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -226,12 +226,6 @@ int nManualPruneHeight = 0); static void FindFilesToPruneManual(std::set &setFilesToPrune, int nManualPruneHeight); -static bool CheckInputs(const CTransaction &tx, CValidationState &state, - const CCoinsViewCache &view, bool fScriptChecks, - uint32_t flags, bool sigCacheStore, - bool scriptCacheStore, - const PrecomputedTransactionData &txdata, - std::vector *pvChecks = nullptr); static uint32_t GetBlockScriptFlags(const CBlockIndex *pindex, const Config &config); @@ -1414,24 +1408,11 @@ } } // namespace Consensus -/** - * Check whether all inputs of this transaction are valid (no double spends, - * scripts & sigs, amounts). This does not modify the UTXO set. - * - * If pvChecks is not nullptr, script checks are pushed onto it instead of being - * performed inline. Any script checks which are not necessary (eg due to script - * execution cache hits) are, obviously, not pushed onto pvChecks/run. - * - * Setting sigCacheStore/scriptCacheStore to false will remove elements from the - * corresponding cache which are matched. This is useful for checking blocks - * where we will likely never need the cache entry again. - */ -static bool CheckInputs(const CTransaction &tx, CValidationState &state, - const CCoinsViewCache &inputs, bool fScriptChecks, - uint32_t flags, bool sigCacheStore, - bool scriptCacheStore, - const PrecomputedTransactionData &txdata, - std::vector *pvChecks) { +bool CheckInputs(const CTransaction &tx, CValidationState &state, + const CCoinsViewCache &inputs, bool fScriptChecks, + uint32_t flags, bool sigCacheStore, bool scriptCacheStore, + const PrecomputedTransactionData &txdata, + std::vector *pvChecks) { assert(!tx.IsCoinBase()); if (!Consensus::CheckTxInputs(tx, state, inputs, GetSpendHeight(inputs))) {