diff options
-rw-r--r-- | .travis.yml | 2 | ||||
-rw-r--r-- | ci/test/00_setup_env_mac_host.sh (renamed from ci/test/00_setup_env_mac_functional.sh) | 2 | ||||
-rw-r--r-- | depends/packages/boost.mk | 5 | ||||
-rw-r--r-- | src/consensus/tx_check.cpp | 19 | ||||
-rw-r--r-- | src/consensus/tx_check.h | 2 | ||||
-rw-r--r-- | src/key.h | 19 | ||||
-rw-r--r-- | src/net_processing.cpp | 18 | ||||
-rw-r--r-- | src/pubkey.h | 25 | ||||
-rw-r--r-- | src/test/bip32_tests.cpp | 16 | ||||
-rw-r--r-- | src/test/compress_tests.cpp | 73 | ||||
-rw-r--r-- | src/test/fuzz/transaction.cpp | 7 | ||||
-rw-r--r-- | src/validation.cpp | 3 |
12 files changed, 104 insertions, 87 deletions
diff --git a/.travis.yml b/.travis.yml index 0a14ddf34c..60d0481d62 100644 --- a/.travis.yml +++ b/.travis.yml @@ -152,4 +152,4 @@ jobs: # https://docs.travis-ci.com/user/reference/osx/#macos-version osx_image: xcode11 env: >- - FILE_ENV="./ci/test/00_setup_env_mac_functional.sh" + FILE_ENV="./ci/test/00_setup_env_mac_host.sh" diff --git a/ci/test/00_setup_env_mac_functional.sh b/ci/test/00_setup_env_mac_host.sh index e9e68c47a8..3033f4cdd9 100644 --- a/ci/test/00_setup_env_mac_functional.sh +++ b/ci/test/00_setup_env_mac_host.sh @@ -11,7 +11,7 @@ export BREW_PACKAGES="automake berkeley-db4 libtool boost miniupnpc pkg-config p export PIP_PACKAGES="zmq" export RUN_CI_ON_HOST=true export RUN_UNIT_TESTS=true -export RUN_FUNCTIONAL_TESTS=true +export RUN_FUNCTIONAL_TESTS=false export GOAL="install" export BITCOIN_CONFIG="--enable-gui --enable-bip70 --enable-reduce-exports --enable-werror" # Run without depends diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index 5df49b2af8..766b8d224e 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -10,15 +10,14 @@ $(package)_config_opts_debug=variant=debug $(package)_config_opts=--layout=tagged --build-type=complete --user-config=user-config.jam $(package)_config_opts+=threading=multi link=static -sNO_BZIP2=1 -sNO_ZLIB=1 $(package)_config_opts_linux=threadapi=pthread runtime-link=shared -$(package)_config_opts_darwin=--toolset=darwin-4.2.1 runtime-link=shared +$(package)_config_opts_darwin=--toolset=clang-darwin runtime-link=shared $(package)_config_opts_mingw32=binary-format=pe target-os=windows threadapi=win32 runtime-link=static $(package)_config_opts_x86_64_mingw32=address-model=64 $(package)_config_opts_i686_mingw32=address-model=32 $(package)_config_opts_i686_linux=address-model=32 architecture=x86 $(package)_toolset_$(host_os)=gcc $(package)_archiver_$(host_os)=$($(package)_ar) -$(package)_toolset_darwin=darwin -$(package)_archiver_darwin=$($(package)_libtool) +$(package)_toolset_darwin=clang-darwin $(package)_config_libraries=chrono,filesystem,system,thread,test $(package)_cxxflags=-std=c++11 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 1206035839..6793f871cf 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -7,7 +7,7 @@ #include <primitives/transaction.h> #include <consensus/validation.h> -bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs) +bool CheckTransaction(const CTransaction& tx, CValidationState& state) { // Basic checks that don't depend on any context if (tx.vin.empty()) @@ -31,14 +31,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge"); } - // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock - if (fCheckDuplicateInputs) { - std::set<COutPoint> vInOutPoints; - for (const auto& txin : tx.vin) - { - if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate"); - } + // Check for duplicate inputs (see CVE-2018-17144) + // While Consensus::CheckTxInputs does check if all inputs of a tx are available, and UpdateCoins marks all inputs + // of a tx as spent, it does not check if the tx has duplicate inputs. + // Failure to run this check will result in either a crash or an inflation bug, depending on the implementation of + // the underlying coins database. + std::set<COutPoint> vInOutPoints; + for (const auto& txin : tx.vin) { + if (!vInOutPoints.insert(txin.prevout).second) + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) diff --git a/src/consensus/tx_check.h b/src/consensus/tx_check.h index bcfdf36bf9..6f3f8fe969 100644 --- a/src/consensus/tx_check.h +++ b/src/consensus/tx_check.h @@ -15,6 +15,6 @@ class CTransaction; class CValidationState; -bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true); +bool CheckTransaction(const CTransaction& tx, CValidationState& state); #endif // BITCOIN_CONSENSUS_TX_CHECK_H @@ -162,25 +162,6 @@ struct CExtKey { bool Derive(CExtKey& out, unsigned int nChild) const; CExtPubKey Neuter() const; void SetSeed(const unsigned char* seed, unsigned int nSeedLen); - template <typename Stream> - void Serialize(Stream& s) const - { - unsigned int len = BIP32_EXTKEY_SIZE; - ::WriteCompactSize(s, len); - unsigned char code[BIP32_EXTKEY_SIZE]; - Encode(code); - s.write((const char *)&code[0], len); - } - template <typename Stream> - void Unserialize(Stream& s) - { - unsigned int len = ::ReadCompactSize(s); - unsigned char code[BIP32_EXTKEY_SIZE]; - if (len != BIP32_EXTKEY_SIZE) - throw std::runtime_error("Invalid extended key size\n"); - s.read((char *)&code[0], len); - Decode(code); - } }; /** Initialize the elliptic curve support. May not be called twice without calling ECC_Stop first. */ diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f173af0fa1..4a3076e3c7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -38,6 +38,8 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; +/** How long to cache transactions in mapRelay for normal relay */ +static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME{15 * 60}; /** Headers download timeout expressed in microseconds * Timeout = base + per_header * (expected number of headers) */ static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes @@ -1513,6 +1515,10 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm // messages from this peer (likely resulting in our peer eventually // disconnecting us). if (pfrom->m_tx_relay != nullptr) { + // mempool entries added before this time have likely expired from mapRelay + const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME; + const std::chrono::seconds mempool_req = pfrom->m_tx_relay->m_last_mempool_req.load(); + LOCK(cs_main); while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { @@ -1532,11 +1538,15 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm if (mi != mapRelay.end()) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); push = true; - } else if (pfrom->m_tx_relay->m_last_mempool_req.load().count()) { + } else { auto txinfo = mempool.info(inv.hash); // To protect privacy, do not answer getdata using the mempool when - // that TX couldn't have been INVed in reply to a MEMPOOL request. - if (txinfo.tx && txinfo.m_time <= pfrom->m_tx_relay->m_last_mempool_req.load()) { + // that TX couldn't have been INVed in reply to a MEMPOOL request, + // or when it's too recent to have expired from mapRelay. + if (txinfo.tx && ( + (mempool_req.count() && txinfo.m_time <= mempool_req) + || (txinfo.m_time <= longlived_mempool_time))) + { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx)); push = true; } @@ -3876,7 +3886,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx))); if (ret.second) { - vRelayExpiration.push_back(std::make_pair(nNow + 15 * 60 * 1000000, ret.first)); + vRelayExpiration.push_back(std::make_pair(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first)); } } if (vInv.size() == MAX_INV_SZ) { diff --git a/src/pubkey.h b/src/pubkey.h index 089324ffda..fd815a871b 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -222,31 +222,6 @@ struct CExtPubKey { void Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const; void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]); bool Derive(CExtPubKey& out, unsigned int nChild) const; - - void Serialize(CSizeComputer& s) const - { - // Optimized implementation for ::GetSerializeSize that avoids copying. - s.seek(BIP32_EXTKEY_SIZE + 1); // add one byte for the size (compact int) - } - template <typename Stream> - void Serialize(Stream& s) const - { - unsigned int len = BIP32_EXTKEY_SIZE; - ::WriteCompactSize(s, len); - unsigned char code[BIP32_EXTKEY_SIZE]; - Encode(code); - s.write((const char *)&code[0], len); - } - template <typename Stream> - void Unserialize(Stream& s) - { - unsigned int len = ::ReadCompactSize(s); - unsigned char code[BIP32_EXTKEY_SIZE]; - if (len != BIP32_EXTKEY_SIZE) - throw std::runtime_error("Invalid extended key size\n"); - s.read((char *)&code[0], len); - Decode(code); - } }; /** Users of this module must hold an ECCVerifyHandle. The constructor and diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 660df00964..e46cf624cf 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -118,22 +118,6 @@ static void RunTest(const TestVector &test) { } key = keyNew; pubkey = pubkeyNew; - - CDataStream ssPub(SER_DISK, CLIENT_VERSION); - ssPub << pubkeyNew; - BOOST_CHECK(ssPub.size() == 75); - - CDataStream ssPriv(SER_DISK, CLIENT_VERSION); - ssPriv << keyNew; - BOOST_CHECK(ssPriv.size() == 75); - - CExtPubKey pubCheck; - CExtKey privCheck; - ssPub >> pubCheck; - ssPriv >> privCheck; - - BOOST_CHECK(pubCheck == pubkeyNew); - BOOST_CHECK(privCheck == keyNew); } } diff --git a/src/test/compress_tests.cpp b/src/test/compress_tests.cpp index e8f149470e..c6a08b293f 100644 --- a/src/test/compress_tests.cpp +++ b/src/test/compress_tests.cpp @@ -4,6 +4,7 @@ #include <compressor.h> #include <test/setup_common.h> +#include <script/standard.h> #include <stdint.h> @@ -61,4 +62,76 @@ BOOST_AUTO_TEST_CASE(compress_amounts) BOOST_CHECK(TestDecode(i)); } +BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id) +{ + // case CKeyID + CKey key; + key.MakeNewKey(true); + CPubKey pubkey = key.GetPubKey(); + + CScript script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; + BOOST_CHECK_EQUAL(script.size(), 25); + + std::vector<unsigned char> out; + bool done = CompressScript(script, out); + BOOST_CHECK_EQUAL(done, true); + + // Check compressed script + BOOST_CHECK_EQUAL(out.size(), 21); + BOOST_CHECK_EQUAL(out[0], 0x00); + BOOST_CHECK_EQUAL(memcmp(&out[1], &script[3], 20), 0); // compare the 20 relevant chars of the CKeyId in the script +} + +BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id) +{ + // case CScriptID + CScript script, redeemScript; + script << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL; + BOOST_CHECK_EQUAL(script.size(), 23); + + std::vector<unsigned char> out; + bool done = CompressScript(script, out); + BOOST_CHECK_EQUAL(done, true); + + // Check compressed script + BOOST_CHECK_EQUAL(out.size(), 21); + BOOST_CHECK_EQUAL(out[0], 0x01); + BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 20), 0); // compare the 20 relevant chars of the CScriptId in the script +} + +BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) +{ + CKey key; + key.MakeNewKey(true); // case compressed PubKeyID + + CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // COMPRESSED_PUBLIC_KEY_SIZE (33) + BOOST_CHECK_EQUAL(script.size(), 35); + + std::vector<unsigned char> out; + bool done = CompressScript(script, out); + BOOST_CHECK_EQUAL(done, true); + + // Check compressed script + BOOST_CHECK_EQUAL(out.size(), 33); + BOOST_CHECK_EQUAL(memcmp(&out[0], &script[1], 1), 0); + BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // compare the 32 chars of the compressed CPubKey +} + +BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id) +{ + CKey key; + key.MakeNewKey(false); // case uncompressed PubKeyID + CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // PUBLIC_KEY_SIZE (65) + BOOST_CHECK_EQUAL(script.size(), 67); // 1 char code + 65 char pubkey + OP_CHECKSIG + + std::vector<unsigned char> out; + bool done = CompressScript(script, out); + BOOST_CHECK_EQUAL(done, true); + + // Check compressed script + BOOST_CHECK_EQUAL(out.size(), 33); + BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // first 32 chars of CPubKey are copied into out[1:] + BOOST_CHECK_EQUAL(out[0], 0x04 | (script[65] & 0x01)); // least significant bit (lsb) of last char of pubkey is mapped into out[0] +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 96d7947b07..383d879040 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -43,12 +43,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) } CValidationState state_with_dupe_check; - const bool valid_with_dupe_check = CheckTransaction(tx, state_with_dupe_check, /* fCheckDuplicateInputs= */ true); - CValidationState state_without_dupe_check; - const bool valid_without_dupe_check = CheckTransaction(tx, state_without_dupe_check, /* fCheckDuplicateInputs= */ false); - if (valid_with_dupe_check) { - assert(valid_without_dupe_check); - } + (void)CheckTransaction(tx, state_with_dupe_check); const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE}; std::string reason; diff --git a/src/validation.cpp b/src/validation.cpp index 29747befe6..9301066c6a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3302,9 +3302,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); // Check transactions - // Must check for duplicate inputs (see CVE-2018-17144) for (const auto& tx : block.vtx) - if (!CheckTransaction(*tx, state, true)) + if (!CheckTransaction(*tx, state)) return state.Invalid(state.GetReason(), false, state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); |