diff options
40 files changed, 476 insertions, 178 deletions
diff --git a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj index 6ded6895cd..dbc977f643 100644 --- a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj +++ b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj @@ -20,6 +20,7 @@ <ClCompile Include="..\..\src\bench\checkqueue.cpp" /> <ClCompile Include="..\..\src\bench\coin_selection.cpp" /> <ClCompile Include="..\..\src\bench\crypto_hash.cpp" /> + <ClCompile Include="..\..\src\bench\data.cpp" /> <ClCompile Include="..\..\src\bench\examples.cpp" /> <ClCompile Include="..\..\src\bench\lockedpool.cpp" /> <ClCompile Include="..\..\src\bench\mempool_eviction.cpp" /> @@ -68,7 +69,7 @@ <ItemGroup> <RawBenchFile Include="..\..\src\bench\data\*.raw" /> </ItemGroup> - <HeaderFromHexdump RawFilePath="%(RawBenchFile.FullPath)" HeaderFilePath="%(RawBenchFile.FullPath).h" SourceHeader="static unsigned const char %(RawBenchFile.Filename)[] = {" SourceFooter="};" /> + <HeaderFromHexdump RawFilePath="%(RawBenchFile.FullPath)" HeaderFilePath="%(RawBenchFile.FullPath).h" SourceHeader="static unsigned const char %(RawBenchFile.Filename)_raw[] = {" SourceFooter="};" /> </Target> <Import Label="hexdumpTarget" Project="..\msbuild\tasks\hexdump.targets" /> <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> diff --git a/contrib/devtools/copyright_header.py b/contrib/devtools/copyright_header.py index f2987f2260..fc01e570aa 100755 --- a/contrib/devtools/copyright_header.py +++ b/contrib/devtools/copyright_header.py @@ -34,7 +34,7 @@ EXCLUDE_DIRS = [ "src/univalue/", ] -INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py'] +INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.mm', '*.py'] INCLUDE_COMPILED = re.compile('|'.join([fnmatch.translate(m) for m in INCLUDE])) def applies_to_file(filename): @@ -90,14 +90,12 @@ def compile_copyright_regex(copyright_style, year_style, name): EXPECTED_HOLDER_NAMES = [ "Satoshi Nakamoto\n", "The Bitcoin Core developers\n", - "Bitcoin Core Developers\n", "BitPay Inc\.\n", "University of Illinois at Urbana-Champaign\.\n", "Pieter Wuille\n", "Wladimir J. van der Laan\n", "Jeff Garzik\n", "Jan-Klaas Kollhof\n", - "Sam Rushing\n", "ArtForz -- public domain half-a-node\n", "Intel Corporation", "The Zcash developers", diff --git a/contrib/devtools/github-merge.py b/contrib/devtools/github-merge.py index cd7a271e83..78ac671bfe 100755 --- a/contrib/devtools/github-merge.py +++ b/contrib/devtools/github-merge.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2016-2017 Bitcoin Core Developers +# Copyright (c) 2016-2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -411,4 +411,3 @@ def main(): if __name__ == '__main__': main() - diff --git a/contrib/verifybinaries/README.md b/contrib/verifybinaries/README.md index 3ffe0a2f28..4209fdb364 100644 --- a/contrib/verifybinaries/README.md +++ b/contrib/verifybinaries/README.md @@ -6,7 +6,7 @@ Make sure you obtain the proper release signing key and verify the fingerprint w ```sh $ gpg --fingerprint "Bitcoin Core binary release signing key" -pub 4096R/36C2E964 2015-06-24 [expires: 2017-02-13] +pub 4096R/36C2E964 2015-06-24 [expires: YYYY-MM-DD] Key fingerprint = 01EA 5486 DE18 A882 D4C2 6845 90C8 019E 36C2 E964 uid Wladimir J. van der Laan (Bitcoin Core binary release signing key) <laanwj@gmail.com> ``` diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index c6162b5caa..1b01b50b07 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -18,6 +18,8 @@ bench_bench_bitcoin_SOURCES = \ bench/block_assemble.cpp \ bench/checkblock.cpp \ bench/checkqueue.cpp \ + bench/data.h \ + bench/data.cpp \ bench/duplicate_inputs.cpp \ bench/examples.cpp \ bench/rollingbloom.cpp \ @@ -76,7 +78,7 @@ CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES) CLEANFILES += $(CLEAN_BITCOIN_BENCH) -bench/checkblock.cpp: bench/data/block413567.raw.h +bench/data.cpp: bench/data/block413567.raw.h bitcoin_bench: $(BENCH_BINARY) @@ -89,7 +91,7 @@ bitcoin_bench_clean : FORCE %.raw.h: %.raw @$(MKDIR_P) $(@D) @{ \ - echo "static unsigned const char $(*F)[] = {" && \ + echo "static unsigned const char $(*F)_raw[] = {" && \ $(HEXDUMP) -v -e '8/1 "0x%02x, "' -e '"\n"' $< | $(SED) -e 's/0x ,//g' && \ echo "};"; \ } > "$@.new" && mv -f "$@.new" "$@" diff --git a/src/bench/checkblock.cpp b/src/bench/checkblock.cpp index e325333c01..4b13381e16 100644 --- a/src/bench/checkblock.cpp +++ b/src/bench/checkblock.cpp @@ -3,41 +3,34 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <bench/bench.h> +#include <bench/data.h> #include <chainparams.h> #include <validation.h> #include <streams.h> #include <consensus/validation.h> -namespace block_bench { -#include <bench/data/block413567.raw.h> -} // namespace block_bench - // These are the two major time-sinks which happen after we have fully received // a block off the wire, but before we can relay the block on to peers using // compact block relay. static void DeserializeBlockTest(benchmark::State& state) { - CDataStream stream((const char*)block_bench::block413567, - (const char*)block_bench::block413567 + sizeof(block_bench::block413567), - SER_NETWORK, PROTOCOL_VERSION); + CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION); char a = '\0'; stream.write(&a, 1); // Prevent compaction while (state.KeepRunning()) { CBlock block; stream >> block; - bool rewound = stream.Rewind(sizeof(block_bench::block413567)); + bool rewound = stream.Rewind(benchmark::data::block413567.size()); assert(rewound); } } static void DeserializeAndCheckBlockTest(benchmark::State& state) { - CDataStream stream((const char*)block_bench::block413567, - (const char*)block_bench::block413567 + sizeof(block_bench::block413567), - SER_NETWORK, PROTOCOL_VERSION); + CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION); char a = '\0'; stream.write(&a, 1); // Prevent compaction @@ -46,7 +39,7 @@ static void DeserializeAndCheckBlockTest(benchmark::State& state) while (state.KeepRunning()) { CBlock block; // Note that CBlock caches its checked state, so we need to recreate it here stream >> block; - bool rewound = stream.Rewind(sizeof(block_bench::block413567)); + bool rewound = stream.Rewind(benchmark::data::block413567.size()); assert(rewound); CValidationState validationState; diff --git a/src/bench/data.cpp b/src/bench/data.cpp new file mode 100644 index 0000000000..0ae4c7cad4 --- /dev/null +++ b/src/bench/data.cpp @@ -0,0 +1,14 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <bench/data.h> + +namespace benchmark { +namespace data { + +#include <bench/data/block413567.raw.h> +const std::vector<uint8_t> block413567{block413567_raw, block413567_raw + sizeof(block413567_raw) / sizeof(block413567_raw[0])}; + +} // namespace data +} // namespace benchmark diff --git a/src/bench/data.h b/src/bench/data.h new file mode 100644 index 0000000000..5f13d766ea --- /dev/null +++ b/src/bench/data.h @@ -0,0 +1,19 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_BENCH_DATA_H +#define BITCOIN_BENCH_DATA_H + +#include <cstdint> +#include <vector> + +namespace benchmark { +namespace data { + +extern const std::vector<uint8_t> block413567; + +} // namespace data +} // namespace benchmark + +#endif // BITCOIN_BENCH_DATA_H diff --git a/src/psbt.cpp b/src/psbt.cpp index d765133190..fe74002e82 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -212,6 +212,25 @@ bool PSBTInputSigned(const PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index) +{ + const CTxOut& out = psbt.tx->vout.at(index); + PSBTOutput& psbt_out = psbt.outputs.at(index); + + // Fill a SignatureData with output info + SignatureData sigdata; + psbt_out.FillSignatureData(sigdata); + + // Construct a would-be spend of this output, to update sigdata with. + // Note that ProduceSignature is used to fill in metadata (not actual signatures), + // so provider does not need to provide any private keys (it can be a HidingSigningProvider). + MutableTransactionSignatureCreator creator(psbt.tx.get_ptr(), /* index */ 0, out.nValue, SIGHASH_ALL); + ProduceSignature(provider, creator, out.scriptPubKey, sigdata); + + // Put redeem_script, witness_script, key paths, into PSBTOutput. + psbt_out.FromSignatureData(sigdata); +} + bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash, SignatureData* out_sigdata, bool use_dummy) { PSBTInput& input = psbt.inputs.at(index); diff --git a/src/psbt.h b/src/psbt.h index 1bc1e91a84..f3840b9ed3 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -565,6 +565,12 @@ bool PSBTInputSigned(const PSBTInput& input); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false); +/** Updates a PSBTOutput with information from provider. + * + * This fills in the redeem_script, witness_script, and hd_keypaths where possible. + */ +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index); + /** * Finalizes a PSBT if possible, combining partial signatures. * diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index a900ec0198..49e9e072a8 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -11,6 +11,8 @@ #include <qt/networkstyle.h> #include <qt/rpcconsole.h> #include <shutdown.h> +#include <test/setup_common.h> +#include <univalue.h> #include <validation.h> #if defined(HAVE_CONFIG_H) @@ -26,8 +28,6 @@ #include <QtGlobal> #include <QtTest/QtTestWidgets> #include <QtTest/QtTestGui> -#include <string> -#include <univalue.h> namespace { //! Call getblockchaininfo RPC and check first field of JSON output. @@ -62,6 +62,7 @@ void AppTests::appTests() } #endif + BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference LogInstance().DisconnectTestLogger(); diff --git a/src/qt/test/paymentservertests.cpp b/src/qt/test/paymentservertests.cpp index f0eca899fc..6cafe05461 100644 --- a/src/qt/test/paymentservertests.cpp +++ b/src/qt/test/paymentservertests.cpp @@ -13,7 +13,7 @@ #include <random.h> #include <script/script.h> #include <script/standard.h> -#include <util/system.h> +#include <test/setup_common.h> #include <util/strencodings.h> #include <openssl/x509.h> @@ -66,7 +66,7 @@ static SendCoinsRecipient handleRequest(PaymentServer* server, std::vector<unsig void PaymentServerTests::paymentServerTests() { - SelectParams(CBaseChainParams::MAIN); + BasicTestingSetup testing_setup(CBaseChainParams::MAIN); auto node = interfaces::MakeNode(); OptionsModel optionsModel(*node); PaymentServer* server = new PaymentServer(nullptr, false); diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 3735f41f9d..3c2ffa6c00 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -34,9 +34,6 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); - ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference - LogInstance().DisconnectTestLogger(); - TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 79d88ab742..6bda8dc6eb 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -42,12 +42,18 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); #endif #endif -extern void noui_connect(); - // This is all you need to run all the tests int main(int argc, char *argv[]) { - BasicTestingSetup test{CBaseChainParams::REGTEST}; + // Initialize persistent globals with the testing setup state for sanity. + // E.g. -datadir in gArgs is set to a temp directory dummy value (instead + // of defaulting to the default datadir), or globalChainParams is set to + // regtest params. + // + // All tests must use their own testing setup (if needed). + { + BasicTestingSetup dummy{CBaseChainParams::REGTEST}; + } auto node = interfaces::MakeNode(); diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index 1588be8da3..006007be63 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -104,6 +104,7 @@ void TrafficGraphWidget::paintEvent(QPaintEvent *) } } + painter.setRenderHint(QPainter::Antialiasing); if(!vSamplesIn.empty()) { QPainterPath p; paintPath(p, vSamplesIn); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 50c4589d9f..743efc1e65 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -14,7 +14,6 @@ #include <core_io.h> #include <hash.h> #include <index/blockfilterindex.h> -#include <key_io.h> #include <policy/feerate.h> #include <policy/policy.h> #include <policy/rbf.h> @@ -42,9 +41,9 @@ #include <boost/thread/thread.hpp> // boost::thread::interrupt +#include <condition_variable> #include <memory> #include <mutex> -#include <condition_variable> struct CUpdatedBlock { @@ -169,7 +168,8 @@ static UniValue getblockcount(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 0) throw std::runtime_error( RPCHelpMan{"getblockcount", - "\nReturns the number of blocks in the longest blockchain.\n", + "\nReturns the height of the most-work fully-validated chain.\n" + "The genesis block has height 0.\n", {}, RPCResult{ "n (numeric) The current block count\n" @@ -189,7 +189,7 @@ static UniValue getbestblockhash(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 0) throw std::runtime_error( RPCHelpMan{"getbestblockhash", - "\nReturns the hash of the best (tip) block in the longest blockchain.\n", + "\nReturns the hash of the best (tip) block in the most-work fully-validated chain.\n", {}, RPCResult{ "\"hex\" (string) the block hash, hex-encoded\n" @@ -1305,7 +1305,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) RPCResult{ "{\n" " \"chain\": \"xxxx\", (string) current network name as defined in BIP70 (main, test, regtest)\n" - " \"blocks\": xxxxxx, (numeric) the current number of blocks processed in the server\n" + " \"blocks\": xxxxxx, (numeric) the height of the most-work fully-validated chain. The genesis block has height 0\n" " \"headers\": xxxxxx, (numeric) the current number of headers we have validated\n" " \"bestblockhash\": \"...\", (string) the hash of the currently best block\n" " \"difficulty\": xxxxxx, (numeric) the current difficulty\n" @@ -2247,41 +2247,12 @@ UniValue scantxoutset(const JSONRPCRequest& request) // loop through the scan objects for (const UniValue& scanobject : request.params[1].get_array().getValues()) { - std::string desc_str; - std::pair<int64_t, int64_t> range = {0, 1000}; - if (scanobject.isStr()) { - desc_str = scanobject.get_str(); - } else if (scanobject.isObject()) { - UniValue desc_uni = find_value(scanobject, "desc"); - if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); - desc_str = desc_uni.get_str(); - UniValue range_uni = find_value(scanobject, "range"); - if (!range_uni.isNull()) { - range = ParseDescriptorRange(range_uni); - } - } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); - } - FlatSigningProvider provider; - auto desc = Parse(desc_str, provider); - if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); - } - if (!desc->IsRange()) { - range.first = 0; - range.second = 0; - } - for (int i = range.first; i <= range.second; ++i) { - std::vector<CScript> scripts; - if (!desc->Expand(i, provider, scripts, provider)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); - } - for (const auto& script : scripts) { - std::string inferred = InferDescriptor(script, provider)->ToString(); - needles.emplace(script); - descriptors.emplace(std::move(script), std::move(inferred)); - } + auto scripts = EvalDescriptorStringOrObject(scanobject, provider); + for (const auto& script : scripts) { + std::string inferred = InferDescriptor(script, provider)->ToString(); + needles.emplace(script); + descriptors.emplace(std::move(script), std::move(inferred)); } } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 477f05f46c..6636a8a29a 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -103,7 +103,6 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request) static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) { - static const int nInnerLoopCount = 0x10000; int nHeightEnd = 0; int nHeight = 0; @@ -124,14 +123,14 @@ static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, ui LOCK(cs_main); IncrementExtraNonce(pblock, ::ChainActive().Tip(), nExtraNonce); } - while (nMaxTries > 0 && pblock->nNonce < nInnerLoopCount && !CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus())) { + while (nMaxTries > 0 && pblock->nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus()) && !ShutdownRequested()) { ++pblock->nNonce; --nMaxTries; } - if (nMaxTries == 0) { + if (nMaxTries == 0 || ShutdownRequested()) { break; } - if (pblock->nNonce == nInnerLoopCount) { + if (pblock->nNonce == std::numeric_limits<uint32_t>::max()) { continue; } std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock); @@ -469,7 +468,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } - // Release the wallet and main lock while waiting + // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); @@ -480,6 +479,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) { // Timeout: Check transactions for update + // without holding ::mempool.cs to avoid deadlocks if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime += std::chrono::seconds(10); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 9da24afe79..966ff3fedc 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1495,12 +1495,19 @@ UniValue converttopsbt(const JSONRPCRequest& request) UniValue utxoupdatepsbt(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) { + if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { throw std::runtime_error( RPCHelpMan{"utxoupdatepsbt", - "\nUpdates a PSBT with witness UTXOs retrieved from the UTXO set or the mempool.\n", + "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set or the mempool.\n", { - {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"} + {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"}, + {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of either strings or objects", { + {"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with an output descriptor and extra information", { + {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, + {"range", RPCArg::Type::RANGE, "1000", "Up to what index HD chains should be explored (either end or [begin,end])"}, + }}, + }}, }, RPCResult { " \"psbt\" (string) The base64-encoded partially signed transaction with inputs updated\n" @@ -1510,7 +1517,7 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) }}.ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR}, true); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR}, true); // Unserialize the transactions PartiallySignedTransaction psbtx; @@ -1519,6 +1526,17 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + // Parse descriptors, if any. + FlatSigningProvider provider; + if (!request.params[1].isNull()) { + auto descs = request.params[1].get_array(); + for (size_t i = 0; i < descs.size(); ++i) { + EvalDescriptorStringOrObject(descs[i], provider); + } + } + // We don't actually need private keys further on; hide them as a precaution. + HidingSigningProvider public_provider(&provider, /* nosign */ true, /* nobip32derivs */ false); + // Fetch previous transactions (inputs): CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); @@ -1545,11 +1563,19 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) const Coin& coin = view.AccessCoin(psbtx.tx->vin[i].prevout); - std::vector<std::vector<unsigned char>> solutions_data; - txnouttype which_type = Solver(coin.out.scriptPubKey, solutions_data); - if (which_type == TX_WITNESS_V0_SCRIPTHASH || which_type == TX_WITNESS_V0_KEYHASH || which_type == TX_WITNESS_UNKNOWN) { + if (IsSegWitOutput(provider, coin.out.scriptPubKey)) { input.witness_utxo = coin.out; } + + // Update script/keypath information using descriptor data. + // Note that SignPSBTInput does a lot more than just constructing ECDSA signatures + // we don't actually care about those here, in fact. + SignPSBTInput(public_provider, psbtx, i, /* sighash_type */ 1); + } + + // Update script/keypath information using descriptor data. + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + UpdatePSBTOutput(public_provider, psbtx, i); } CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 9c4cdc3a90..14fcb628eb 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -221,6 +221,9 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). keystore->AddCScript(GetScriptForWitness(witnessScript)); } + if (rs.isNull() && ws.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript"); + } } } } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index ca17d379bc..6fdab2c4cd 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -232,6 +232,10 @@ static UniValue getrpcinfo(const JSONRPCRequest& request) UniValue result(UniValue::VOBJ); result.pushKV("active_commands", active_commands); + const std::string path = LogInstance().m_file_path.string(); + UniValue log_path(UniValue::VSTR, path); + result.pushKV("logpath", log_path); + return result; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4642cf16b1..67ccb225b5 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -6,6 +6,7 @@ #include <keystore.h> #include <outputtype.h> #include <rpc/util.h> +#include <script/descriptor.h> #include <tinyformat.h> #include <util/strencodings.h> @@ -697,3 +698,40 @@ std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value) } return {low, high}; } + +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider) +{ + std::string desc_str; + std::pair<int64_t, int64_t> range = {0, 1000}; + if (scanobject.isStr()) { + desc_str = scanobject.get_str(); + } else if (scanobject.isObject()) { + UniValue desc_uni = find_value(scanobject, "desc"); + if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); + desc_str = desc_uni.get_str(); + UniValue range_uni = find_value(scanobject, "range"); + if (!range_uni.isNull()) { + range = ParseDescriptorRange(range_uni); + } + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); + } + + auto desc = Parse(desc_str, provider); + if (!desc) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); + } + if (!desc->IsRange()) { + range.first = 0; + range.second = 0; + } + std::vector<CScript> ret; + for (int i = range.first; i <= range.second; ++i) { + std::vector<CScript> scripts; + if (!desc->Expand(i, provider, scripts, provider)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); + } + std::move(scripts.begin(), scripts.end(), std::back_inserter(ret)); + } + return ret; +} diff --git a/src/rpc/util.h b/src/rpc/util.h index 0eb2fef5c3..8762281d73 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -9,6 +9,8 @@ #include <outputtype.h> #include <pubkey.h> #include <rpc/protocol.h> +#include <script/script.h> +#include <script/sign.h> #include <script/standard.h> #include <univalue.h> @@ -84,6 +86,9 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s //! Parse a JSON range specified as int64, or [int64, int64] std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value); +/** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */ +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider); + struct RPCArg { enum class Type { OBJ, diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 36dd68a3d8..5320dc0876 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -505,3 +505,19 @@ FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvide ret.origins.insert(b.origins.begin(), b.origins.end()); return ret; } + +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) +{ + std::vector<valtype> solutions; + auto whichtype = Solver(script, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + if (whichtype == TX_SCRIPTHASH) { + auto h160 = uint160(solutions[0]); + CScript subscript; + if (provider.GetCScript(h160, subscript)) { + whichtype = Solver(subscript, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + } + } + return false; +} diff --git a/src/script/sign.h b/src/script/sign.h index f746325b90..e5c0329a61 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -232,4 +232,7 @@ void UpdateInput(CTxIn& input, const SignatureData& data); * Solvability is unrelated to whether we consider this output to be ours. */ bool IsSolvable(const SigningProvider& provider, const CScript& script); +/** Check whether a scriptPubKey is known to be segwit. */ +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/support/cleanse.cpp b/src/support/cleanse.cpp index 17a4a4c2b2..ecb00510f7 100644 --- a/src/support/cleanse.cpp +++ b/src/support/cleanse.cpp @@ -11,33 +11,25 @@ #include <Windows.h> // For SecureZeroMemory. #endif -/* Compilers have a bad habit of removing "superfluous" memset calls that - * are trying to zero memory. For example, when memset()ing a buffer and - * then free()ing it, the compiler might decide that the memset is - * unobservable and thus can be removed. - * - * Previously we used OpenSSL which tried to stop this by a) implementing - * memset in assembly on x86 and b) putting the function in its own file - * for other platforms. - * - * This change removes those tricks in favour of using asm directives to - * scare the compiler away. As best as our compiler folks can tell, this is - * sufficient and will continue to be so. - * - * Adam Langley <agl@google.com> - * Commit: ad1907fe73334d6c696c8539646c21b11178f20f - * BoringSSL (LICENSE: ISC) - */ void memory_cleanse(void *ptr, size_t len) { - std::memset(ptr, 0, len); - - /* As best as we can tell, this is sufficient to break any optimisations that - might try to eliminate "superfluous" memsets. If there's an easy way to - detect memset_s, it would be better to use that. */ #if defined(_MSC_VER) + /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ SecureZeroMemory(ptr, len); #else + std::memset(ptr, 0, len); + + /* Memory barrier that scares the compiler away from optimizing out the memset. + * + * Quoting Adam Langley <agl@google.com> in commit ad1907fe73334d6c696c8539646c21b11178f20f + * in BoringSSL (ISC License): + * As best as we can tell, this is sufficient to break any optimisations that + * might try to eliminate "superfluous" memsets. + * This method is used in memzero_explicit() the Linux kernel, too. Its advantage is that it + * is pretty efficient because the compiler can still implement the memset() efficiently, + * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by + * Yang et al. (USENIX Security 2017) for more background. + */ __asm__ __volatile__("" : : "r"(ptr) : "memory"); #endif } diff --git a/src/support/cleanse.h b/src/support/cleanse.h index 5298214e44..b03520315d 100644 --- a/src/support/cleanse.h +++ b/src/support/cleanse.h @@ -8,7 +8,8 @@ #include <stdlib.h> -// Attempt to overwrite data in the specified memory span. +/** Secure overwrite a buffer (possibly containing secret data) with zero-bytes. The write + * operation will not be optimized out by the compiler. */ void memory_cleanse(void *ptr, size_t len); #endif // BITCOIN_SUPPORT_CLEANSE_H diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index e3ba9cddb0..24c7d51898 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -73,7 +73,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. - RegisterAllCoreRPCCommands(tableRPC); // We have to run a scheduler thread to prevent ActivateBestChain diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 6a9813442b..b3368d44b6 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -10,6 +10,7 @@ #include <miner.h> #include <pow.h> #include <random.h> +#include <script/standard.h> #include <test/setup_common.h> #include <util/time.h> #include <validation.h> @@ -21,6 +22,8 @@ struct RegtestingSetup : public TestingSetup { RegtestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {} }; +static const std::vector<unsigned char> V_OP_TRUE{OP_TRUE}; + BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegtestingSetup) struct TestSubscriber : public CValidationInterface { @@ -62,8 +65,21 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; + pubKey.clear(); + { + WitnessV0ScriptHash witness_program; + CSHA256().Write(&V_OP_TRUE[0], V_OP_TRUE.size()).Finalize(witness_program.begin()); + pubKey << OP_0 << ToByteVector(witness_program); + } + + // Make the coinbase transaction with two outputs: + // One zero-value one that has a unique pubkey to make sure that blocks at the same height can have a different hash + // Another one that has the coinbase reward in a P2WSH with OP_TRUE as witness program to make it easy to spend CMutableTransaction txCoinbase(*pblock->vtx[0]); - txCoinbase.vout.resize(1); + txCoinbase.vout.resize(2); + txCoinbase.vout[1].scriptPubKey = pubKey; + txCoinbase.vout[1].nValue = txCoinbase.vout[0].nValue; + txCoinbase.vout[0].nValue = 0; txCoinbase.vin[0].scriptWitness.SetNull(); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); @@ -72,6 +88,9 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) { + LOCK(cs_main); // For LookupBlockIndex + GenerateCoinbaseCommitment(*pblock, LookupBlockIndex(pblock->hashPrevBlock), Params().GetConsensus()); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus())) { @@ -82,13 +101,13 @@ std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) } // construct a valid block -const std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) { return FinalizeBlock(Block(prev_hash)); } // construct an invalid block (but with a valid header) -const std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) { auto pblock = Block(prev_hash); @@ -188,4 +207,131 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash()); } +/** + * Test that mempool updates happen atomically with reorgs. + * + * This prevents RPC clients, among others, from retrieving immediately-out-of-date mempool data + * during large reorgs. + * + * The test verifies this by creating a chain of `num_txs` blocks, matures their coinbases, and then + * submits txns spending from their coinbase to the mempool. A fork chain is then processed, + * invalidating the txns and evicting them from the mempool. + * + * We verify that the mempool updates atomically by polling it continuously + * from another thread during the reorg and checking that its size only changes + * once. The size changing exactly once indicates that the polling thread's + * view of the mempool is either consistent with the chain state before reorg, + * or consistent with the chain state after the reorg, and not just consistent + * with some intermediate state during the reorg. + */ +BOOST_AUTO_TEST_CASE(mempool_locks_reorg) +{ + bool ignored; + auto ProcessBlock = [&ignored](std::shared_ptr<const CBlock> block) -> bool { + return ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored); + }; + + // Process all mined blocks + BOOST_REQUIRE(ProcessBlock(std::make_shared<CBlock>(Params().GenesisBlock()))); + auto last_mined = GoodBlock(Params().GenesisBlock().GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + + // Run the test multiple times + for (int test_runs = 3; test_runs > 0; --test_runs) { + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // Later on split from here + const uint256 split_hash{last_mined->hashPrevBlock}; + + // Create a bunch of transactions to spend the miner rewards of the + // most recent blocks + std::vector<CTransactionRef> txs; + for (int num_txs = 22; num_txs > 0; --num_txs) { + CMutableTransaction mtx; + mtx.vin.push_back(CTxIn{COutPoint{last_mined->vtx[0]->GetHash(), 1}, CScript{}}); + mtx.vin[0].scriptWitness.stack.push_back(V_OP_TRUE); + mtx.vout.push_back(last_mined->vtx[0]->vout[1]); + mtx.vout[0].nValue -= 1000; + txs.push_back(MakeTransactionRef(mtx)); + + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mature the inputs of the txs + for (int j = COINBASE_MATURITY; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mine a reorg (and hold it back) before adding the txs to the mempool + const uint256 tip_init{last_mined->GetHash()}; + + std::vector<std::shared_ptr<const CBlock>> reorg; + last_mined = GoodBlock(split_hash); + reorg.push_back(last_mined); + for (size_t j = COINBASE_MATURITY + txs.size() + 1; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + reorg.push_back(last_mined); + } + + // Add the txs to the tx pool + { + LOCK(cs_main); + CValidationState state; + std::list<CTransactionRef> plTxnReplaced; + for (const auto& tx : txs) { + BOOST_REQUIRE(AcceptToMemoryPool( + ::mempool, + state, + tx, + /* pfMissingInputs */ &ignored, + &plTxnReplaced, + /* bypass_limits */ false, + /* nAbsurdFee */ 0)); + } + } + + // Check that all txs are in the pool + { + LOCK(::mempool.cs); + BOOST_CHECK_EQUAL(::mempool.mapTx.size(), txs.size()); + } + + // Run a thread that simulates an RPC caller that is polling while + // validation is doing a reorg + std::thread rpc_thread{[&]() { + // This thread is checking that the mempool either contains all of + // the transactions invalidated by the reorg, or none of them, and + // not some intermediate amount. + while (true) { + LOCK(::mempool.cs); + if (::mempool.mapTx.size() == 0) { + // We are done with the reorg + break; + } + // Internally, we might be in the middle of the reorg, but + // externally the reorg to the most-proof-of-work chain should + // be atomic. So the caller assumes that the returned mempool + // is consistent. That is, it has all txs that were there + // before the reorg. + assert(::mempool.mapTx.size() == txs.size()); + continue; + } + LOCK(cs_main); + // We are done with the reorg, so the tip must have changed + assert(tip_init != ::ChainActive().Tip()->GetBlockHash()); + }}; + + // Submit the reorg in this thread to invalidate and remove the txs from the tx pool + for (const auto& b : reorg) { + ProcessBlock(b); + } + // Check that the reorg was eventually successful + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // We can join the other thread, which returns when the reorg was successful + rpc_thread.join(); + } +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cac7beb6a1..9257cff718 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -104,7 +104,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan // for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) { - LOCK(cs); + AssertLockHeld(cs); // For each entry in vHashesToUpdate, store the set of in-mempool, but not // in-vHashesToUpdate transactions, so that we don't have to recalculate // descendants when we come across a previously seen entry. @@ -322,8 +322,8 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, assert(int(nSigOpCostWithAncestors) >= 0); } -CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) : - nTransactionsUpdated(0), minerPolicyEstimator(estimator) +CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) + : nTransactionsUpdated(0), minerPolicyEstimator(estimator) { _clear(); //lock free clear @@ -341,13 +341,11 @@ bool CTxMemPool::isSpent(const COutPoint& outpoint) const unsigned int CTxMemPool::GetTransactionsUpdated() const { - LOCK(cs); return nTransactionsUpdated; } void CTxMemPool::AddTransactionsUpdated(unsigned int n) { - LOCK(cs); nTransactionsUpdated += n; } @@ -459,8 +457,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool - { - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { @@ -485,13 +482,12 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso } RemoveStaged(setAllRemoves, false, reason); - } } void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) { // Remove transactions spending a coinbase which are now immature and no-longer-final transactions - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); @@ -547,7 +543,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) */ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); std::vector<const CTxMemPoolEntry*> entries; for (const auto& tx : vtx) { @@ -922,7 +918,7 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool } int CTxMemPool::Expire(int64_t time) { - LOCK(cs); + AssertLockHeld(cs); indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin(); setEntries toremove; while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) { @@ -1015,7 +1011,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining) { - LOCK(cs); + AssertLockHeld(cs); unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(0); diff --git a/src/txmempool.h b/src/txmempool.h index ce0b762336..565dd61f0f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -6,12 +6,13 @@ #ifndef BITCOIN_TXMEMPOOL_H #define BITCOIN_TXMEMPOOL_H +#include <atomic> +#include <map> #include <memory> #include <set> -#include <map> -#include <vector> -#include <utility> #include <string> +#include <utility> +#include <vector> #include <amount.h> #include <coins.h> @@ -443,7 +444,7 @@ class CTxMemPool { private: uint32_t nCheckFrequency GUARDED_BY(cs); //!< Value n means that n times in 2^32 we check. - unsigned int nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation + std::atomic<unsigned int> nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation CBlockPolicyEstimator* minerPolicyEstimator; uint64_t totalTxSize; //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141. @@ -513,21 +514,12 @@ public: * `mempool.cs` whenever adding transactions to the mempool and whenever * changing the chain tip. It's necessary to keep both mutexes locked until * the mempool is consistent with the new chain tip and fully populated. - * - * @par Consistency bug - * - * The second guarantee above is not currently enforced, but - * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code - * in bitcoin currently depends on second guarantee, but it is important to - * fix for third party code that needs be able to frequently poll the - * mempool without locking `cs_main` and without encountering missing - * transactions during reorgs. */ mutable RecursiveMutex cs; indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; - std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order + std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order struct CompareIteratorByHash { bool operator()(const txiter &a, const txiter &b) const { @@ -582,10 +574,10 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); - void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight); + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free @@ -598,7 +590,7 @@ public: * Check that none of this transactions inputs are in the mempool, and thus * the tx is not dependent on other mempool transactions to be included in a block. */ - bool HasNoInputsOf(const CTransaction& tx) const; + bool HasNoInputsOf(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); @@ -632,7 +624,7 @@ public: * for). Note: vHashesToUpdate should be the set of transactions from the * disconnected block that have been accepted back into the mempool. */ - void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); /** Try to calculate all in-mempool ancestors of entry. * (these are all calculated including the tx itself) @@ -663,10 +655,10 @@ public: * pvNoSpendsRemaining, if set, will be populated with the list of outpoints * which are not in mempool which no longer have any spends in this mempool. */ - void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining=nullptr); + void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ - int Expire(int64_t time); + int Expire(int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Calculate the ancestor and descendant count for the given transaction. diff --git a/src/uint256.cpp b/src/uint256.cpp index e3bc9712e8..ea7164c1f0 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -37,16 +37,15 @@ void base_blob<BITS>::SetHex(const char* psz) psz += 2; // hex string to uint - const char* pbegin = psz; - while (::HexDigit(*psz) != -1) - psz++; - psz--; + size_t digits = 0; + while (::HexDigit(psz[digits]) != -1) + digits++; unsigned char* p1 = (unsigned char*)data; unsigned char* pend = p1 + WIDTH; - while (psz >= pbegin && p1 < pend) { - *p1 = ::HexDigit(*psz--); - if (psz >= pbegin) { - *p1 |= ((unsigned char)::HexDigit(*psz--) << 4); + while (digits > 0 && p1 < pend) { + *p1 = ::HexDigit(psz[--digits]); + if (digits > 0) { + *p1 |= ((unsigned char)::HexDigit(psz[--digits]) << 4); p1++; } } diff --git a/src/validation.cpp b/src/validation.cpp index d39b78614c..c6995ed24b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -304,7 +304,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +{ int expired = pool.Expire(GetTime() - age); if (expired != 0) { LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); @@ -341,7 +342,7 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) { AssertLockHeld(cs_main); std::vector<uint256> vHashUpdate; @@ -2547,7 +2548,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& LimitValidationInterfaceQueue(); { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -2667,6 +2668,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c LimitValidationInterfaceQueue(); LOCK(cs_main); + LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -4100,7 +4102,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; diff --git a/src/validation.h b/src/validation.h index 638229952d..9573d62048 100644 --- a/src/validation.h +++ b/src/validation.h @@ -18,6 +18,7 @@ #include <protocol.h> // For CMessageHeader::MessageStartChars #include <script/script_error.h> #include <sync.h> +#include <txmempool.h> // For CTxMemPool::cs #include <versionbits.h> #include <algorithm> @@ -553,7 +554,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block disconnection on our pcoinsTip: - bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -572,8 +573,8 @@ public: bool IsInitialBlockDownload() const; private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index ce4788dee1..721a244afb 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -44,16 +44,7 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { - const CTxOut& out = psbtx.tx->vout.at(i); - PSBTOutput& psbt_out = psbtx.outputs.at(i); - - // Fill a SignatureData with output info - SignatureData sigdata; - psbt_out.FillSignatureData(sigdata); - - MutableTransactionSignatureCreator creator(psbtx.tx.get_ptr(), 0, out.nValue, 1); - ProduceSignature(HidingSigningProvider(pwallet, true, !bip32derivs), creator, out.scriptPubKey, sigdata); - psbt_out.FromSignatureData(sigdata); + UpdatePSBTOutput(HidingSigningProvider(pwallet, true, !bip32derivs), psbtx, i); } return TransactionError::OK; diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index b16eafccca..7712e8bdf6 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -64,9 +64,23 @@ class BIP65Test(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_cltv_info(self, *, is_active): + assert_equal( + next(s for s in self.nodes[0].getblockchaininfo()['softforks'] if s['id'] == 'bip65'), + { + "id": "bip65", + "version": 4, + "reject": { + "status": is_active + } + }, + ) + def run_test(self): self.nodes[0].add_p2p_connection(P2PInterface()) + self.test_cltv_info(is_active=False) + self.log.info("Mining %d blocks", CLTV_HEIGHT - 2) self.coinbase_txids = [self.nodes[0].getblock(b)['tx'][0] for b in self.nodes[0].generate(CLTV_HEIGHT - 2)] self.nodeaddress = self.nodes[0].getnewaddress() @@ -86,7 +100,9 @@ class BIP65Test(BitcoinTestFramework): block.hashMerkleRoot = block.calc_merkle_root() block.solve() + self.test_cltv_info(is_active=False) self.nodes[0].p2p.send_and_ping(msg_block(block)) + self.test_cltv_info(is_active=False) # Not active as of current tip, but next block must obey rules assert_equal(self.nodes[0].getbestblockhash(), block.hash) self.log.info("Test that blocks must now be at least version 4") @@ -135,7 +151,9 @@ class BIP65Test(BitcoinTestFramework): block.hashMerkleRoot = block.calc_merkle_root() block.solve() + self.test_cltv_info(is_active=False) # Not active as of current tip, but next block must obey rules self.nodes[0].p2p.send_and_ping(msg_block(block)) + self.test_cltv_info(is_active=True) # Active as of current tip assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.sha256) diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 7480e5c5ba..067e3be1f4 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -51,9 +51,23 @@ class BIP66Test(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_dersig_info(self, *, is_active): + assert_equal( + next(s for s in self.nodes[0].getblockchaininfo()['softforks'] if s['id'] == 'bip66'), + { + "id": "bip66", + "version": 3, + "reject": { + "status": is_active + } + }, + ) + def run_test(self): self.nodes[0].add_p2p_connection(P2PInterface()) + self.test_dersig_info(is_active=False) + self.log.info("Mining %d blocks", DERSIG_HEIGHT - 2) self.coinbase_txids = [self.nodes[0].getblock(b)['tx'][0] for b in self.nodes[0].generate(DERSIG_HEIGHT - 2)] self.nodeaddress = self.nodes[0].getnewaddress() @@ -74,7 +88,9 @@ class BIP66Test(BitcoinTestFramework): block.rehash() block.solve() + self.test_dersig_info(is_active=False) self.nodes[0].p2p.send_and_ping(msg_block(block)) + self.test_dersig_info(is_active=False) # Not active as of current tip, but next block must obey rules assert_equal(self.nodes[0].getbestblockhash(), block.hash) self.log.info("Test that blocks must now be at least version 3") @@ -128,8 +144,11 @@ class BIP66Test(BitcoinTestFramework): block.rehash() block.solve() + self.test_dersig_info(is_active=False) # Not active as of current tip, but next block must obey rules self.nodes[0].p2p.send_and_ping(msg_block(block)) + self.test_dersig_info(is_active=True) # Active as of current tip assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.sha256) + if __name__ == '__main__': BIP66Test().main() diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 49ae0fb1a9..e99fa22646 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests some generic aspects of the RPC interface.""" +import os from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than_or_equal @@ -31,6 +32,7 @@ class RPCInterfaceTest(BitcoinTestFramework): command = info['active_commands'][0] assert_equal(command['method'], 'getrpcinfo') assert_greater_than_or_equal(command['duration'], 0) + assert_equal(info['logpath'], os.path.join(self.nodes[0].datadir, 'regtest', 'debug.log')) def test_batch_request(self): self.log.info("Testing basic JSON-RPC batch request...") diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 58010f7c2e..62f3843756 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -129,6 +129,11 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): outval = value - decimal.Decimal("0.00001000") rawtx = node2.createrawtransaction([{"txid": txid, "vout": vout}], [{self.final: outval}]) + prevtx_err = dict(prevtxs[0]) + del prevtx_err["redeemScript"] + + assert_raises_rpc_error(-8, "Missing redeemScript/witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err]) + rawtx2 = node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs - 1], prevtxs) rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [self.priv[-1]], prevtxs) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 8bfa7a0238..2fffe96ebe 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -325,18 +325,32 @@ class PSBTTest(BitcoinTestFramework): vout3 = find_output(self.nodes[0], txid3, 11) self.sync_all() - # Update a PSBT with UTXOs from the node - # Bech32 inputs should be filled with witness UTXO. Other inputs should not be filled because they are non-witness + def test_psbt_input_keys(psbt_input, keys): + """Check that the psbt input has only the expected keys.""" + assert_equal(set(keys), set(psbt_input.keys())) + + # Create a PSBT. None of the inputs are filled initially psbt = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1},{"txid":txid2, "vout":vout2},{"txid":txid3, "vout":vout3}], {self.nodes[0].getnewaddress():32.999}) decoded = self.nodes[1].decodepsbt(psbt) - assert "witness_utxo" not in decoded['inputs'][0] and "non_witness_utxo" not in decoded['inputs'][0] - assert "witness_utxo" not in decoded['inputs'][1] and "non_witness_utxo" not in decoded['inputs'][1] - assert "witness_utxo" not in decoded['inputs'][2] and "non_witness_utxo" not in decoded['inputs'][2] + test_psbt_input_keys(decoded['inputs'][0], []) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], []) + + # Update a PSBT with UTXOs from the node + # Bech32 inputs should be filled with witness UTXO. Other inputs should not be filled because they are non-witness updated = self.nodes[1].utxoupdatepsbt(psbt) decoded = self.nodes[1].decodepsbt(updated) - assert "witness_utxo" in decoded['inputs'][0] and "non_witness_utxo" not in decoded['inputs'][0] - assert "witness_utxo" not in decoded['inputs'][1] and "non_witness_utxo" not in decoded['inputs'][1] - assert "witness_utxo" not in decoded['inputs'][2] and "non_witness_utxo" not in decoded['inputs'][2] + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo']) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], []) + + # Try again, now while providing descriptors, making P2SH-segwit work, and causing bip32_derivs and redeem_script to be filled in + descs = [self.nodes[1].getaddressinfo(addr)['desc'] for addr in [addr1,addr2,addr3]] + updated = self.nodes[1].utxoupdatepsbt(psbt, descs) + decoded = self.nodes[1].decodepsbt(updated) + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo', 'bip32_derivs']) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], ['witness_utxo', 'bip32_derivs', 'redeem_script']) # Two PSBTs with a common input should not be joinable psbt1 = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1}], {self.nodes[0].getnewaddress():Decimal('10.999')}) diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index 5810e94938..91d26e9cb3 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -57,8 +57,7 @@ class ResendWalletTransactionsTest(BitcoinTestFramework): # after the last time we tried to broadcast. Use mocktime and give an extra minute to be sure. block_time = int(time.time()) + 6 * 60 node.setmocktime(block_time) - block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockchaininfo()['blocks']), block_time) - block.nVersion = 3 + block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time) block.rehash() block.solve() node.submitblock(ToHex(block)) |