From f36d1d5b8934aac60d3097047ecedeb58bae2185 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 5 Jun 2019 15:54:11 -0700 Subject: [PATCH 001/791] Use void* throughout support/lockedpool.h Replace uses of char* with void* in Arena's member variables. Instead, cast to char* where needed in the implementation. Certain compiler environments disallow std::hash specializations to prevent hashing the pointer's value instead of the string contents. Thus, compilation fails when std::unordered_map is keyed by char*. Explicitly using void* is a workaround in such environments. For consistency, void* is used throughout all member variables similarly to the public interface. --- src/support/lockedpool.cpp | 18 ++++++++++-------- src/support/lockedpool.h | 10 +++++----- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 85e3351e72576..6d767ca210638 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -27,6 +27,7 @@ #include #include #endif +#include LockedPoolManager* LockedPoolManager::_instance = nullptr; std::once_flag LockedPoolManager::init_flag; @@ -44,12 +45,12 @@ static inline size_t align_up(size_t x, size_t align) // Implementation: Arena Arena::Arena(void *base_in, size_t size_in, size_t alignment_in): - base(static_cast(base_in)), end(static_cast(base_in) + size_in), alignment(alignment_in) + base(base_in), end(static_cast(base_in) + size_in), alignment(alignment_in) { // Start with one free chunk that covers the entire arena auto it = size_to_free_chunk.emplace(size_in, base); chunks_free.emplace(base, it); - chunks_free_end.emplace(base + size_in, it); + chunks_free_end.emplace(static_cast(base) + size_in, it); } Arena::~Arena() @@ -75,8 +76,9 @@ void* Arena::alloc(size_t size) // Create the used-chunk, taking its space from the end of the free-chunk const size_t size_remaining = size_ptr_it->first - size; - auto allocated = chunks_used.emplace(size_ptr_it->second + size_remaining, size).first; - chunks_free_end.erase(size_ptr_it->second + size_ptr_it->first); + char* const free_chunk = static_cast(size_ptr_it->second); + auto allocated = chunks_used.emplace(free_chunk + size_remaining, size).first; + chunks_free_end.erase(free_chunk + size_ptr_it->first); if (size_ptr_it->first == size) { // whole chunk is used up chunks_free.erase(size_ptr_it->second); @@ -84,11 +86,11 @@ void* Arena::alloc(size_t size) // still some memory left in the chunk auto it_remaining = size_to_free_chunk.emplace(size_remaining, size_ptr_it->second); chunks_free[size_ptr_it->second] = it_remaining; - chunks_free_end.emplace(size_ptr_it->second + size_remaining, it_remaining); + chunks_free_end.emplace(free_chunk + size_remaining, it_remaining); } size_to_free_chunk.erase(size_ptr_it); - return reinterpret_cast(allocated->first); + return allocated->first; } void Arena::free(void *ptr) @@ -99,11 +101,11 @@ void Arena::free(void *ptr) } // Remove chunk from used map - auto i = chunks_used.find(static_cast(ptr)); + auto i = chunks_used.find(ptr); if (i == chunks_used.end()) { throw std::runtime_error("Arena: invalid or double free"); } - std::pair freed = *i; + auto freed = std::make_pair(static_cast(i->first), i->second); chunks_used.erase(i); // coalesce freed with previous chunk diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index b420c909fc53a..ce6fedc8e8bf5 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -89,23 +89,23 @@ class Arena */ bool addressInArena(void *ptr) const { return ptr >= base && ptr < end; } private: - typedef std::multimap SizeToChunkSortedMap; + typedef std::multimap SizeToChunkSortedMap; /** Map to enable O(log(n)) best-fit allocation, as it's sorted by size */ SizeToChunkSortedMap size_to_free_chunk; - typedef std::unordered_map ChunkToSizeMap; + typedef std::unordered_map ChunkToSizeMap; /** Map from begin of free chunk to its node in size_to_free_chunk */ ChunkToSizeMap chunks_free; /** Map from end of free chunk to its node in size_to_free_chunk */ ChunkToSizeMap chunks_free_end; /** Map from begin of used chunk to its size */ - std::unordered_map chunks_used; + std::unordered_map chunks_used; /** Base address of arena */ - char* base; + void* base; /** End address of arena */ - char* end; + void* end; /** Minimum chunk alignment */ size_t alignment; }; From 75347236f212f327a5bba10d8a900cc58ebe5de0 Mon Sep 17 00:00:00 2001 From: Pasta Date: Fri, 17 Dec 2021 23:40:06 -0500 Subject: [PATCH 002/791] docs: document c-style cast prohibition --- doc/developer-notes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 188889785686f..76aebc55b7d56 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -104,6 +104,10 @@ code. - `++i` is preferred over `i++`. - `nullptr` is preferred over `NULL` or `(void*)0`. - `static_assert` is preferred over `assert` where possible. Generally; compile-time checking is preferred over run-time checking. + - Use a named cast or functional cast, not a C-Style cast. When casting + between integer types, use functional casts such as `int(x)` or `int{x}` + instead of `(int) x`. When casting between more complex types, use static_cast. + Use reinterpret_cast and const_cast as appropriate. Block style example: ```c++ From 071eef1e974f128131afe6c6b5c68a430c64687a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 28 Nov 2021 19:25:01 +0200 Subject: [PATCH 003/791] build: Propagate user-defined flags to host packages --- depends/hosts/default.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/depends/hosts/default.mk b/depends/hosts/default.mk index 258619a9d0590..57e71ad55f124 100644 --- a/depends/hosts/default.mk +++ b/depends/hosts/default.mk @@ -29,8 +29,13 @@ host_$1=$$($(host_arch)_$(host_os)_$1) endef define add_host_flags_func +ifeq ($(filter $(origin $1),undefined default),) +$(host_arch)_$(host_os)_$1 = +$(host_arch)_$(host_os)_$(release_type)_$1 = $($1) +else $(host_arch)_$(host_os)_$1 += $($(host_os)_$1) $(host_arch)_$(host_os)_$(release_type)_$1 += $($(host_os)_$(release_type)_$1) +endif host_$1 = $$($(host_arch)_$(host_os)_$1) host_$(release_type)_$1 = $$($(host_arch)_$(host_os)_$(release_type)_$1) endef From a3a2bd9e8ad360a63cc8bdfc365d8bfd25ecc720 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 28 Nov 2021 19:30:09 +0200 Subject: [PATCH 004/791] ci: Drop no longer needed package-specific flags --- ci/test/00_setup_env_native_fuzz_with_msan.sh | 2 +- ci/test/00_setup_env_native_msan.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 071bac8fb3343..90c4d29737569 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -15,7 +15,7 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}" export CONTAINER_NAME="ci_native_msan" export PACKAGES="clang-12 llvm-12 cmake" # BDB generates false-positives and will be removed in future -export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' libevent_cflags='${MSAN_FLAGS}' sqlite_cflags='${MSAN_FLAGS}' zeromq_cxxflags='-std=c++17 ${MSAN_AND_LIBCXX_FLAGS}'" +export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no --prefix=${DEPENDS_DIR}/x86_64-pc-linux-gnu/ CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export USE_MEMORY_SANITIZER="true" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 34a792ec8ff26..33e7a4e6645f2 100755 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -15,7 +15,7 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}" export CONTAINER_NAME="ci_native_msan" export PACKAGES="clang-12 llvm-12 cmake" # BDB generates false-positives and will be removed in future -export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' libevent_cflags='${MSAN_FLAGS}' sqlite_cflags='${MSAN_FLAGS}' zeromq_cxxflags='-std=c++17 ${MSAN_AND_LIBCXX_FLAGS}'" +export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no --prefix=${DEPENDS_DIR}/x86_64-pc-linux-gnu/ CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export USE_MEMORY_SANITIZER="true" From 304ece994504220c355577170409b9200941f2af Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 31 May 2021 16:55:56 +0200 Subject: [PATCH 005/791] rpc: document bools in FillPSBT() calls --- src/wallet/rpc/spend.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 07119133b7ad6..13df213065273 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -82,10 +82,10 @@ static UniValue FinishTransaction(const std::shared_ptr pwallet, const PartiallySignedTransaction psbtx(rawTx); // First fill transaction with our data without signing, - // so external signers are not asked sign more than once. + // so external signers are not asked to sign more than once. bool complete; - pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, false, true); - const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, true, false)}; + pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true); + const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -1100,7 +1100,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) } else { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, false /* sign */, true /* bip32derivs */); + const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true); CHECK_NONFATAL(err == TransactionError::OK); CHECK_NONFATAL(!complete); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); @@ -1675,7 +1675,7 @@ RPCHelpMan walletcreatefundedpsbt() // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool(); bool complete = true; - const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, false, bip32derivs)}; + const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } From 7e02a3329797211ed5d35e5f5e7b919c099b78ba Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 25 Apr 2022 18:13:23 +0200 Subject: [PATCH 006/791] rpc: bumpfee signer support --- src/wallet/feebumper.cpp | 17 ++++++++++++++++- src/wallet/rpc/spend.cpp | 5 ++++- test/functional/wallet_signer.py | 23 ++++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 73042424ad498..c66754bfcb43b 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -239,7 +239,22 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) { LOCK(wallet.cs_wallet); - return wallet.SignTransaction(mtx); + + if (wallet.IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { + // Make a blank psbt + PartiallySignedTransaction psbtx(mtx); + + // First fill transaction with our data without signing, + // so external signers are not asked to sign more than once. + bool complete; + wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); + const TransactionError err = wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */); + if (err != TransactionError::OK) return false; + complete = FinalizeAndExtractPSBT(psbtx, mtx); + return complete; + } else { + return wallet.SignTransaction(mtx); + } } Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector& errors, uint256& bumped_txid) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 13df213065273..602d7aec96e31 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1010,7 +1010,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) && !want_psbt) { throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead."); } @@ -1088,6 +1088,9 @@ static RPCHelpMan bumpfee_helper(std::string method_name) // For psbtbumpfee, return the base64-encoded unsigned PSBT of the new transaction. if (!want_psbt) { if (!feebumper::SignTransaction(*pwallet, mtx)) { + if (pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction incomplete. Try psbtbumpfee instead."); + } throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction."); } diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 8e4e1f5d36b67..0839dbf9f4925 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -13,6 +13,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) @@ -150,7 +151,7 @@ def test_valid_signer(self): assert_equal(result[1], {'success': True}) assert_equal(mock_wallet.getwalletinfo()["txcount"], 1) dest = self.nodes[0].getnewaddress(address_type='bech32') - mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {}, True)['psbt'] + mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {'replaceable': True}, True)['psbt'] mock_psbt_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt, sign=True, sighashtype="ALL", bip32derivs=True) mock_psbt_final = mock_wallet.finalizepsbt(mock_psbt_signed["psbt"]) mock_tx = mock_psbt_final["hex"] @@ -190,6 +191,7 @@ def test_valid_signer(self): self.log.info('Test send using hww1') + # Don't broadcast transaction yet so the RPC returns the raw hex res = hww.send(outputs={dest:0.5},options={"add_to_wallet": False}) assert(res["complete"]) assert_equal(res["hex"], mock_tx) @@ -199,6 +201,25 @@ def test_valid_signer(self): res = hww.sendall(recipients=[{dest:0.5}, hww.getrawchangeaddress()],options={"add_to_wallet": False}) assert(res["complete"]) assert_equal(res["hex"], mock_tx) + # Broadcast transaction so we can bump the fee + hww.sendrawtransaction(res["hex"]) + + self.log.info('Prepare fee bumped mock PSBT') + + # Now that the transaction is broadcast, bump fee in mock wallet: + orig_tx_id = res["txid"] + mock_psbt_bumped = mock_wallet.psbtbumpfee(orig_tx_id)["psbt"] + mock_psbt_bumped_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt_bumped, sign=True, sighashtype="ALL", bip32derivs=True) + + with open(os.path.join(self.nodes[1].cwd, "mock_psbt"), "w", encoding="utf8") as f: + f.write(mock_psbt_bumped_signed["psbt"]) + + self.log.info('Test bumpfee using hww1') + + # Bump fee + res = hww.bumpfee(orig_tx_id) + assert_greater_than(res["fee"], res["origfee"]) + assert_equal(res["errors"], []) # # Handle error thrown by script # self.set_mock_result(self.nodes[4], "2") From 2c07cfacd1745844a1d3c57f2e8617549b9815d7 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 25 Apr 2022 18:14:28 +0200 Subject: [PATCH 007/791] gui: bumpfee signer support Specifically this enables the Send button in the fee bump dialog for wallets with external signer support. Similar to 2efdfb88aab6496dcf2b98e0de30635bc6bade85. --- src/qt/walletmodel.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 5ee32e79d5d4e..8c2f8e568d7df 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -508,7 +508,9 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy.")); } - auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString, "", "", SEND_CONFIRM_DELAY, !m_wallet->privateKeysDisabled(), getOptionsModel()->getEnablePSBTControls(), nullptr); + const bool enable_send{!wallet().privateKeysDisabled() || wallet().hasExternalSigner()}; + const bool always_show_unsigned{getOptionsModel()->getEnablePSBTControls()}; + auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString, "", "", SEND_CONFIRM_DELAY, enable_send, always_show_unsigned, nullptr); confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); // TODO: Replace QDialog::exec() with safer QDialog::show(). const auto retval = static_cast(confirmationDialog->exec()); @@ -526,6 +528,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) // Short-circuit if we are returning a bumped transaction PSBT to clipboard if (retval == QMessageBox::Save) { + // "Create Unsigned" clicked PartiallySignedTransaction psbtx(mtx); bool complete = false; const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, nullptr, psbtx, complete); @@ -541,7 +544,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } - assert(!m_wallet->privateKeysDisabled()); + assert(!m_wallet->privateKeysDisabled() || wallet().hasExternalSigner()); // sign bumped transaction if (!m_wallet->signBumpTransaction(mtx)) { From 6d58117a31a88eec3f0a103f9d1fc26cf0b48348 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 2 Jan 2022 11:10:25 +0200 Subject: [PATCH 008/791] build: Build minisketch test in `make check`, not in `make` --- ci/test/06_script_b.sh | 2 ++ src/Makefile.am | 1 + src/Makefile.minisketch.include | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index b60c9b6d30a77..9b8f3a02445f8 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -9,12 +9,14 @@ export LC_ALL=C.UTF-8 if [[ $HOST = *-mingw32 ]]; then # Generate all binaries, so that they can be wrapped CI_EXEC make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1 + CI_EXEC make "$MAKEJOBS" -C src minisketch/test.exe VERBOSE=1 CI_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-wine.sh" fi if [ -n "$QEMU_USER_CMD" ]; then # Generate all binaries, so that they can be wrapped CI_EXEC make "$MAKEJOBS" -C src/secp256k1 VERBOSE=1 + CI_EXEC make "$MAKEJOBS" -C src minisketch/test VERBOSE=1 CI_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-qemu.sh" fi diff --git a/src/Makefile.am b/src/Makefile.am index 23bc1800953a2..3b49e0cb01be4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ noinst_LTLIBRARIES = bin_PROGRAMS = noinst_PROGRAMS = +check_PROGRAMS = TESTS = BENCHMARKS = diff --git a/src/Makefile.minisketch.include b/src/Makefile.minisketch.include index b337f483498ed..1363bec34eac2 100644 --- a/src/Makefile.minisketch.include +++ b/src/Makefile.minisketch.include @@ -31,7 +31,7 @@ if ENABLE_TESTS if !ENABLE_FUZZ MINISKETCH_TEST = minisketch/test TESTS += $(MINISKETCH_TEST) -noinst_PROGRAMS += $(MINISKETCH_TEST) +check_PROGRAMS += $(MINISKETCH_TEST) minisketch_test_SOURCES = $(MINISKETCH_TEST_SOURCES_INT) minisketch_test_CPPFLAGS = $(AM_CPPFLAGS) $(LIBMINISKETCH_CPPFLAGS) From fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Sun, 14 Aug 2022 15:58:47 -0400 Subject: [PATCH 009/791] gui: Show watchonly balance only for Legacy wallets Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets. --- src/qt/sendcoinsdialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index bd44d127818b9..b9623d2041019 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -703,7 +703,7 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances) CAmount balance = balances.balance; if (model->wallet().hasExternalSigner()) { ui->labelBalanceName->setText(tr("External balance:")); - } else if (model->wallet().privateKeysDisabled()) { + } else if (model->wallet().isLegacy() && model->wallet().privateKeysDisabled()) { balance = balances.watch_only_balance; ui->labelBalanceName->setText(tr("Watch-only balance:")); } From fb1c6c14c11ccd4833c1a24f77c507f098d369ad Mon Sep 17 00:00:00 2001 From: yancy Date: Wed, 31 Aug 2022 14:20:37 +0200 Subject: [PATCH 010/791] test: Remove redundant test --- src/wallet/test/coinselector_tests.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 30a7fb9eb6ec8..0b29ef4c79aad 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -231,17 +231,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 10 * CENT); expected_result.Clear(); - // Negative effective value - // Select 10 Cent but have 1 Cent not be possible because too small - add_coin(5 * CENT, 5, expected_result); - add_coin(3 * CENT, 3, expected_result); - add_coin(2 * CENT, 2, expected_result); - const auto result6 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000); - BOOST_CHECK(result6); - BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 10 * CENT); - // FIXME: this test is redundant with the above, because 1 Cent is selected, not "too small" - // BOOST_CHECK(EquivalentResult(expected_result, *result)); - // Select 0.25 Cent, not possible BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT)); expected_result.Clear(); From b942c94d153f83b77ef5d603211252d9abadde95 Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 6 Sep 2022 13:56:30 +0200 Subject: [PATCH 011/791] test: Change coinselection parameter location to make tests independent --- src/wallet/test/coinselector_tests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 30a7fb9eb6ec8..1fa54b0103874 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -301,6 +301,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_selection_params_bnb.m_change_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_output_size); coin_selection_params_bnb.m_cost_of_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size) + coin_selection_params_bnb.m_change_fee; coin_selection_params_bnb.min_viable_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size); + coin_selection_params_bnb.m_subtract_fee_outputs = true; + { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); wallet->LoadWallet(); @@ -318,7 +320,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) available_coins.Clear(); add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate); available_coins.All().at(0).input_bytes = 40; - coin_selection_params_bnb.m_subtract_fee_outputs = true; const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); BOOST_CHECK(result9); BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT); From 5669afb80e350027705dc378d2ab16232567511a Mon Sep 17 00:00:00 2001 From: sinetek Date: Fri, 26 Aug 2022 17:45:54 +0200 Subject: [PATCH 012/791] fs: drop old WSL1 hack. --- src/fs.cpp | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/fs.cpp b/src/fs.cpp index 74b167e313559..bfc95879f57a3 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -60,36 +60,20 @@ FileLock::~FileLock() } } -static bool IsWSL() -{ - struct utsname uname_data; - return uname(&uname_data) == 0 && std::string(uname_data.version).find("Microsoft") != std::string::npos; -} - bool FileLock::TryLock() { if (fd == -1) { return false; } - // Exclusive file locking is broken on WSL using fcntl (issue #18622) - // This workaround can be removed once the bug on WSL is fixed - static const bool is_wsl = IsWSL(); - if (is_wsl) { - if (flock(fd, LOCK_EX | LOCK_NB) == -1) { - reason = GetErrorReason(); - return false; - } - } else { - struct flock lock; - lock.l_type = F_WRLCK; - lock.l_whence = SEEK_SET; - lock.l_start = 0; - lock.l_len = 0; - if (fcntl(fd, F_SETLK, &lock) == -1) { - reason = GetErrorReason(); - return false; - } + struct flock lock; + lock.l_type = F_WRLCK; + lock.l_whence = SEEK_SET; + lock.l_start = 0; + lock.l_len = 0; + if (fcntl(fd, F_SETLK, &lock) == -1) { + reason = GetErrorReason(); + return false; } return true; From 365aca40453995163bbd17231251512f9f9a103b Mon Sep 17 00:00:00 2001 From: yancy Date: Fri, 16 Sep 2022 14:22:42 +0200 Subject: [PATCH 013/791] refactor: Simplify feerate comparison statement --- src/wallet/coinselection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index b568e90998490..718bced07d690 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -93,7 +93,7 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo bool backtrack = false; if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value. curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch - (curr_waste > best_waste && (utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0)) { // Don't select things which we know will be more wasteful if the waste is increasing + (curr_waste > best_waste && (utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee))) { // Don't select things which we know will be more wasteful if the waste is increasing backtrack = true; } else if (curr_value >= selection_target) { // Selected value is within range curr_waste += (curr_value - selection_target); // This is the excess value which is added to the waste for the below comparison From 81d4a2b14ff65fe07085ef2a967a466015370ce3 Mon Sep 17 00:00:00 2001 From: yancy Date: Fri, 16 Sep 2022 14:26:00 +0200 Subject: [PATCH 014/791] refactor: Move feerate comparison invariant outside of the loop --- src/wallet/coinselection.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 718bced07d690..c281c915855e1 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -87,13 +87,15 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo std::vector best_selection; CAmount best_waste = MAX_MONEY; + bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee; + // Depth First search loop for choosing the UTXOs for (size_t curr_try = 0, utxo_pool_index = 0; curr_try < TOTAL_TRIES; ++curr_try, ++utxo_pool_index) { // Conditions for starting a backtrack bool backtrack = false; if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value. curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch - (curr_waste > best_waste && (utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee))) { // Don't select things which we know will be more wasteful if the waste is increasing + (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing backtrack = true; } else if (curr_value >= selection_target) { // Selected value is within range curr_waste += (curr_value - selection_target); // This is the excess value which is added to the waste for the below comparison From 9d3127b11e34131409dab7c08bde5b444d90b2cb Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 22 Apr 2022 19:15:17 -0400 Subject: [PATCH 015/791] Add settings.json prune-prev, proxy-prev, onion-prev settings This provides a way for the GUI settings dialog box to retain previous pruning and proxy settings when they are disabled, as requested by vasild: https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-937685759 https://github.com/bitcoin/bitcoin/pull/15936#discussion_r850568749 https://github.com/bitcoin/bitcoin/pull/15936#discussion_r852998379 Importantly, while this PR changes the settings.json format, it changes it in a fully backwards compatible way, so previous versious of bitcoind and bitcoin-qt will correctly interpret prune, proxy, and onion settins written by new versions of bitcoin-qt. --- src/qt/optionsmodel.cpp | 130 +++++++++++++++++++++++----------------- src/qt/optionsmodel.h | 13 +--- 2 files changed, 77 insertions(+), 66 deletions(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 0b4359a9179fc..cf35dbfcb0c9f 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -59,7 +59,7 @@ static const char* SettingName(OptionsModel::OptionID option) } /** Call node.updateRwSetting() with Bitcoin 22.x workaround. */ -static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const util::SettingsValue& value) +static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const std::string& suffix, const util::SettingsValue& value) { if (value.isNum() && (option == OptionsModel::DatabaseCache || @@ -73,9 +73,9 @@ static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID optio // in later releases by https://github.com/bitcoin/bitcoin/pull/24498. // If new numeric settings are added, they can be written as numbers // instead of strings, because bitcoin 22.x will not try to read these. - node.updateRwSetting(SettingName(option), value.getValStr()); + node.updateRwSetting(SettingName(option) + suffix, value.getValStr()); } else { - node.updateRwSetting(SettingName(option), value); + node.updateRwSetting(SettingName(option) + suffix, value); } } @@ -131,13 +131,6 @@ void OptionsModel::addOverriddenOption(const std::string &option) bool OptionsModel::Init(bilingual_str& error) { // Initialize display settings from stored settings. - m_prune_size_gb = PruneSizeGB(node().getPersistentSetting("prune")); - ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString())); - m_proxy_ip = proxy.ip; - m_proxy_port = proxy.port; - ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString())); - m_onion_ip = onion.ip; - m_onion_port = onion.port; language = QString::fromStdString(SettingToString(node().getPersistentSetting("lang"), "")); checkAndMigrate(); @@ -318,8 +311,6 @@ void OptionsModel::SetPruneTargetGB(int prune_target_gb) const util::SettingsValue cur_value = node().getPersistentSetting("prune"); const util::SettingsValue new_value = PruneSetting(prune_target_gb > 0, prune_target_gb); - m_prune_size_gb = prune_target_gb; - // Force setting to take effect. It is still safe to change the value at // this point because this function is only called after the intro screen is // shown, before the node starts. @@ -332,7 +323,12 @@ void OptionsModel::SetPruneTargetGB(int prune_target_gb) PruneSizeGB(cur_value) != PruneSizeGB(new_value)) { // Call UpdateRwSetting() instead of setOption() to avoid setting // RestartRequired flag - UpdateRwSetting(node(), Prune, new_value); + UpdateRwSetting(node(), Prune, "", new_value); + } + + // Keep previous pruning size, if pruning was disabled. + if (PruneEnabled(cur_value)) { + UpdateRwSetting(node(), Prune, "-prev", PruneEnabled(new_value) ? util::SettingsValue{} : cur_value); } } @@ -360,9 +356,9 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in return successful; } -QVariant OptionsModel::getOption(OptionID option) const +QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) const { - auto setting = [&]{ return node().getPersistentSetting(SettingName(option)); }; + auto setting = [&]{ return node().getPersistentSetting(SettingName(option) + suffix); }; QSettings settings; switch (option) { @@ -389,19 +385,30 @@ QVariant OptionsModel::getOption(OptionID option) const // default proxy case ProxyUse: + case ProxyUseTor: return ParseProxyString(SettingToString(setting(), "")).is_set; case ProxyIP: - return m_proxy_ip; + case ProxyIPTor: { + ProxySetting proxy = ParseProxyString(SettingToString(setting(), "")); + if (proxy.is_set) { + return proxy.ip; + } else if (suffix.empty()) { + return getOption(option, "-prev"); + } else { + return ParseProxyString(GetDefaultProxyAddress().toStdString()).ip; + } + } case ProxyPort: - return m_proxy_port; - - // separate Tor proxy - case ProxyUseTor: - return ParseProxyString(SettingToString(setting(), "")).is_set; - case ProxyIPTor: - return m_onion_ip; - case ProxyPortTor: - return m_onion_port; + case ProxyPortTor: { + ProxySetting proxy = ParseProxyString(SettingToString(setting(), "")); + if (proxy.is_set) { + return proxy.port; + } else if (suffix.empty()) { + return getOption(option, "-prev"); + } else { + return ParseProxyString(GetDefaultProxyAddress().toStdString()).port; + } + } #ifdef ENABLE_WALLET case SpendZeroConfChange: @@ -426,7 +433,9 @@ QVariant OptionsModel::getOption(OptionID option) const case Prune: return PruneEnabled(setting()); case PruneSize: - return m_prune_size_gb; + return PruneEnabled(setting()) ? PruneSizeGB(setting()) : + suffix.empty() ? getOption(option, "-prev") : + DEFAULT_PRUNE_TARGET_GB; case DatabaseCache: return qlonglong(SettingToInt(setting(), nDefaultDbCache)); case ThreadsScriptVerif: @@ -440,10 +449,10 @@ QVariant OptionsModel::getOption(OptionID option) const } } -bool OptionsModel::setOption(OptionID option, const QVariant& value) +bool OptionsModel::setOption(OptionID option, const QVariant& value, const std::string& suffix) { - auto changed = [&] { return value.isValid() && value != getOption(option); }; - auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, value); }; + auto changed = [&] { return value.isValid() && value != getOption(option, suffix); }; + auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, suffix, value); }; bool successful = true; /* set to false on parse error */ QSettings settings; @@ -481,52 +490,60 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) // default proxy case ProxyUse: if (changed()) { - update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port)); - setRestartRequired(true); + if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev"); + update(ProxyString(value.toBool(), getOption(ProxyIP).toString(), getOption(ProxyPort).toString())); + if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {}); + if (suffix.empty()) setRestartRequired(true); } break; case ProxyIP: if (changed()) { - m_proxy_ip = value.toString(); - if (getOption(ProxyUse).toBool()) { - update(ProxyString(true, m_proxy_ip, m_proxy_port)); - setRestartRequired(true); + if (suffix.empty() && !getOption(ProxyUse).toBool()) { + setOption(option, value, "-prev"); + } else { + update(ProxyString(true, value.toString(), getOption(ProxyPort).toString())); } + if (suffix.empty() && getOption(ProxyUse).toBool()) setRestartRequired(true); } break; case ProxyPort: if (changed()) { - m_proxy_port = value.toString(); - if (getOption(ProxyUse).toBool()) { - update(ProxyString(true, m_proxy_ip, m_proxy_port)); - setRestartRequired(true); + if (suffix.empty() && !getOption(ProxyUse).toBool()) { + setOption(option, value, "-prev"); + } else { + update(ProxyString(true, getOption(ProxyIP).toString(), value.toString())); } + if (suffix.empty() && getOption(ProxyUse).toBool()) setRestartRequired(true); } break; // separate Tor proxy case ProxyUseTor: if (changed()) { - update(ProxyString(value.toBool(), m_onion_ip, m_onion_port)); - setRestartRequired(true); + if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev"); + update(ProxyString(value.toBool(), getOption(ProxyIPTor).toString(), getOption(ProxyPortTor).toString())); + if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {}); + if (suffix.empty()) setRestartRequired(true); } break; case ProxyIPTor: if (changed()) { - m_onion_ip = value.toString(); - if (getOption(ProxyUseTor).toBool()) { - update(ProxyString(true, m_onion_ip, m_onion_port)); - setRestartRequired(true); + if (suffix.empty() && !getOption(ProxyUseTor).toBool()) { + setOption(option, value, "-prev"); + } else { + update(ProxyString(true, value.toString(), getOption(ProxyPortTor).toString())); } + if (suffix.empty() && getOption(ProxyUseTor).toBool()) setRestartRequired(true); } break; case ProxyPortTor: if (changed()) { - m_onion_port = value.toString(); - if (getOption(ProxyUseTor).toBool()) { - update(ProxyString(true, m_onion_ip, m_onion_port)); - setRestartRequired(true); + if (suffix.empty() && !getOption(ProxyUseTor).toBool()) { + setOption(option, value, "-prev"); + } else { + update(ProxyString(true, getOption(ProxyIPTor).toString(), value.toString())); } + if (suffix.empty() && getOption(ProxyUseTor).toBool()) setRestartRequired(true); } break; @@ -580,17 +597,20 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) break; case Prune: if (changed()) { - update(PruneSetting(value.toBool(), m_prune_size_gb)); - setRestartRequired(true); + if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev"); + update(PruneSetting(value.toBool(), getOption(PruneSize).toInt())); + if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {}); + if (suffix.empty()) setRestartRequired(true); } break; case PruneSize: if (changed()) { - m_prune_size_gb = ParsePruneSizeGB(value); - if (getOption(Prune).toBool()) { - update(PruneSetting(true, m_prune_size_gb)); - setRestartRequired(true); + if (suffix.empty() && !getOption(Prune).toBool()) { + setOption(option, value, "-prev"); + } else { + update(PruneSetting(true, ParsePruneSizeGB(value))); } + if (suffix.empty() && getOption(Prune).toBool()) setRestartRequired(true); } break; case DatabaseCache: diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 42b89c50293f7..0020b89e21c5e 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -81,8 +81,8 @@ class OptionsModel : public QAbstractListModel int rowCount(const QModelIndex & parent = QModelIndex()) const override; QVariant data(const QModelIndex & index, int role = Qt::DisplayRole) const override; bool setData(const QModelIndex & index, const QVariant & value, int role = Qt::EditRole) override; - QVariant getOption(OptionID option) const; - bool setOption(OptionID option, const QVariant& value); + QVariant getOption(OptionID option, const std::string& suffix="") const; + bool setOption(OptionID option, const QVariant& value, const std::string& suffix=""); /** Updates current unit in memory, settings and emits displayUnitChanged(new_unit) signal */ void setDisplayUnit(const QVariant& new_unit); @@ -121,15 +121,6 @@ class OptionsModel : public QAbstractListModel bool m_sub_fee_from_amount; bool m_enable_psbt_controls; - //! In-memory settings for display. These are stored persistently by the - //! bitcoin node but it's also nice to store them in memory to prevent them - //! getting cleared when enable/disable toggles are used in the GUI. - int m_prune_size_gb; - QString m_proxy_ip; - QString m_proxy_port; - QString m_onion_ip; - QString m_onion_port; - /* settings that were overridden by command-line */ QString strOverriddenByCommandLine; From 3c43d9db1e0f84279b08a93afd730caeece061a9 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Wed, 9 Dec 2020 14:38:21 +0200 Subject: [PATCH 016/791] p2p: Don't self-advertise during VERSION processing Previously, we would prepare to self-announce to a new peer while parsing a VERSION message from that peer. This is redundant, because we do something very similar in MaybeSendAddr(), which is called from SendMessages() after the version handshake is finished. There are a couple of differences: 1) MaybeSendAddr() self-advertises to all peers we do address relay with, not just outbound ones. 2) GetLocalAddrForPeer() called from MaybeSendAddr() makes a probabilistic decision to either advertise what they think we are or what we think we are, while PushAddress(self) on VERSION deterministically only does the former if the address from the latter is unroutable. 3) During VERSION processing, we haven't received a potential sendaddrv2 message from our peer yet, so self-advertisements with addresses from addrV2-only networks would always be dropped in PushAddress(). Since it's confusing to have two slightly different mechanisms for self-advertising, and the one in MaybeSendAddr() is better, remove the one in VERSION. Co-authored-by: Martin Zumsande --- src/net_processing.cpp | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index eca6263392c2a..1adcb3695b299 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3274,39 +3274,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_num_preferred_download_peers += state->fPreferredDownload; } - // Self advertisement & GETADDR logic if (!pfrom.IsInboundConn() && SetupAddressRelay(pfrom, *peer)) { - // For outbound peers, we try to relay our address (so that other - // nodes can try to find us more quickly, as we have no guarantee - // that an outbound peer is even aware of how to reach us) and do a - // one-time address fetch (to help populate/update our addrman). If - // we're starting up for the first time, our addrman may be pretty - // empty and no one will know who we are, so these mechanisms are + // For outbound peers, we do a one-time address fetch + // (to help populate/update our addrman). + // If we're starting up for the first time, our addrman may be pretty + // empty and no one will know who we are, so this mechanism is // important to help us connect to the network. // // We skip this for block-relay-only peers. We want to avoid // potentially leaking addr information and we do not want to // indicate to the peer that we will participate in addr relay. - if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) - { - CAddress addr{GetLocalAddress(pfrom.addr), peer->m_our_services, Now()}; - FastRandomContext insecure_rand; - if (addr.IsRoutable()) - { - LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString()); - PushAddress(*peer, addr, insecure_rand); - } else if (IsPeerAddrLocalGood(&pfrom)) { - // Override just the address with whatever the peer sees us as. - // Leave the port in addr as it was returned by GetLocalAddress() - // above, as this is an outbound connection and the peer cannot - // observe our listening port. - addr.SetIP(addrMe); - LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString()); - PushAddress(*peer, addr, insecure_rand); - } - } - - // Get recent addresses m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::GETADDR)); peer->m_getaddr_sent = true; // When requesting a getaddr, accept an additional MAX_ADDR_TO_SEND addresses in response From a518fff0f2e479064dd4cff6c29fb54c72c1407b Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 3 Oct 2022 23:28:56 -0400 Subject: [PATCH 017/791] rest: add verbose and mempool_sequence query params for mempool/contents --- src/rest.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/rest.cpp b/src/rest.cpp index 7f00db2222c22..c3c79433690f4 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -608,7 +608,20 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s case RESTResponseFormat::JSON: { std::string str_json; if (param == "contents") { - str_json = MempoolToJSON(*mempool, true).write() + "\n"; + const std::string raw_verbose{req->GetQueryParameter("verbose").value_or("true")}; + if (raw_verbose != "true" && raw_verbose != "false") { + return RESTERR(req, HTTP_BAD_REQUEST, "The \"verbose\" query parameter must be either \"true\" or \"false\"."); + } + const std::string raw_mempool_sequence{req->GetQueryParameter("mempool_sequence").value_or("false")}; + if (raw_mempool_sequence != "true" && raw_mempool_sequence != "false") { + return RESTERR(req, HTTP_BAD_REQUEST, "The \"mempool_sequence\" query parameter must be either \"true\" or \"false\"."); + } + const bool verbose{raw_verbose == "true"}; + const bool mempool_sequence{raw_mempool_sequence == "true"}; + if (verbose && mempool_sequence) { + return RESTERR(req, HTTP_BAD_REQUEST, "Verbose results cannot contain mempool sequence values. (hint: set \"verbose=false\")"); + } + str_json = MempoolToJSON(*mempool, verbose, mempool_sequence).write() + "\n"; } else { str_json = MempoolInfoToJSON(*mempool).write() + "\n"; } From 52a31dccc92366efb36db3b94920bdf8b05b264c Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 3 Oct 2022 23:29:21 -0400 Subject: [PATCH 018/791] tests: mempool/contents verbose and mempool_sequence query params tests --- test/functional/interface_rest.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 610a28c56b9f8..ebefb8bd6a28e 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -348,6 +348,34 @@ def run_test(self): assert_equal(json_obj[tx]['spentby'], txs[i + 1:i + 2]) assert_equal(json_obj[tx]['depends'], txs[i - 1:i]) + # Check the mempool response for explicit parameters + json_obj = self.test_rest_request("/mempool/contents", query_params={"verbose": "true", "mempool_sequence": "false"}) + assert_equal(json_obj, raw_mempool_verbose) + + # Check the mempool response for not verbose + json_obj = self.test_rest_request("/mempool/contents", query_params={"verbose": "false"}) + raw_mempool = self.nodes[0].getrawmempool(verbose=False) + + assert_equal(json_obj, raw_mempool) + + # Check the mempool response for sequence + json_obj = self.test_rest_request("/mempool/contents", query_params={"verbose": "false", "mempool_sequence": "true"}) + raw_mempool = self.nodes[0].getrawmempool(verbose=False, mempool_sequence=True) + + assert_equal(json_obj, raw_mempool) + + # Check for error response if verbose=true and mempool_sequence=true + resp = self.test_rest_request("/mempool/contents", ret_type=RetType.OBJ, status=400, query_params={"verbose": "true", "mempool_sequence": "true"}) + assert_equal(resp.read().decode('utf-8').strip(), 'Verbose results cannot contain mempool sequence values. (hint: set "verbose=false")') + + # Check for error response if verbose is not "true" or "false" + resp = self.test_rest_request("/mempool/contents", ret_type=RetType.OBJ, status=400, query_params={"verbose": "TRUE"}) + assert_equal(resp.read().decode('utf-8').strip(), 'The "verbose" query parameter must be either "true" or "false".') + + # Check for error response if mempool_sequence is not "true" or "false" + resp = self.test_rest_request("/mempool/contents", ret_type=RetType.OBJ, status=400, query_params={"verbose": "false", "mempool_sequence": "TRUE"}) + assert_equal(resp.read().decode('utf-8').strip(), 'The "mempool_sequence" query parameter must be either "true" or "false".') + # Now mine the transactions newblockhash = self.generate(self.nodes[1], 1) From 1ff5d61dfdaf8987e5619162662e4c760af76a43 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Tue, 4 Oct 2022 09:19:05 -0400 Subject: [PATCH 019/791] doc: add mempool/contents rest verbose and mempool_sequence args --- doc/REST-interface.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/REST-interface.md b/doc/REST-interface.md index 265b74ee9aff7..9173f08efb937 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -123,11 +123,15 @@ Returns various information about the transaction mempool. Only supports JSON as output format. Refer to the `getmempoolinfo` RPC help for details. -`GET /rest/mempool/contents.json` +`GET /rest/mempool/contents.json?verbose=&mempool_sequence=` Returns the transactions in the mempool. Only supports JSON as output format. -Refer to the `getrawmempool` RPC help for details. +Refer to the `getrawmempool` RPC help for details. Defaults to setting +`verbose=true` and `mempool_sequence=false`. + +*Query parameters for `verbose` and `mempool_sequence` available in 25.0 and up.* + Risks ------------- From e43a547a3674a31504a60ede9b4912e014a54139 Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 21 Jul 2022 11:10:47 -0300 Subject: [PATCH 020/791] refactor: wallet, do not translate init arguments names --- src/wallet/spend.cpp | 2 +- src/wallet/wallet.cpp | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 6833f9a095503..36c1a65ea2128 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -849,7 +849,7 @@ static util::Result CreateTransactionInternal( } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - return util::Error{_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.")}; + return util::Error{strprintf(_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable %s."), "-fallbackfee")}; } // Calculate the cost of change diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5889de2e36b60..6ed237329438c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2935,7 +2935,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-fallbackfee")) { std::optional fallback_fee = ParseMoney(args.GetArg("-fallbackfee", "")); if (!fallback_fee) { - error = strprintf(_("Invalid amount for -fallbackfee=: '%s'"), args.GetArg("-fallbackfee", "")); + error = strprintf(_("Invalid amount for %s=: '%s'"), "-fallbackfee", args.GetArg("-fallbackfee", "")); return nullptr; } else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") + @@ -2950,7 +2950,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-discardfee")) { std::optional discard_fee = ParseMoney(args.GetArg("-discardfee", "")); if (!discard_fee) { - error = strprintf(_("Invalid amount for -discardfee=: '%s'"), args.GetArg("-discardfee", "")); + error = strprintf(_("Invalid amount for %s=: '%s'"), "-discardfee", args.GetArg("-discardfee", "")); return nullptr; } else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") + @@ -2972,8 +2972,8 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), - args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least %s)"), + "-paytxfee", args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -2984,12 +2984,12 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri error = AmountErrMsg("maxtxfee", args.GetArg("-maxtxfee", "")); return nullptr; } else if (max_fee.value() > HIGH_MAX_TX_FEE) { - warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); + warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee")); } if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), + "-maxtxfee", args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } From 0bd73e2c453e8a88312edf43d184c33109f12ad6 Mon Sep 17 00:00:00 2001 From: klementtan Date: Sat, 30 Oct 2021 22:22:40 +0800 Subject: [PATCH 021/791] util: Add -shutdownnotify option. --- src/init.cpp | 17 +++++++++++++++++ test/functional/feature_notifications.py | 10 +++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 8ffab64622673..38713702d2ff3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -189,8 +189,24 @@ static fs::path GetPidFile(const ArgsManager& args) // shutdown thing. // +#if HAVE_SYSTEM +static void ShutdownNotify(const ArgsManager& args) +{ + std::vector threads; + for (const auto& cmd : args.GetArgs("-shutdownnotify")) { + threads.emplace_back(runCommand, cmd); + } + for (auto& t : threads) { + t.join(); + } +} +#endif + void Interrupt(NodeContext& node) { +#if HAVE_SYSTEM + ShutdownNotify(*node.args); +#endif InterruptHTTPServer(); InterruptHTTPRPC(); InterruptRPC(); @@ -439,6 +455,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-settings=", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #if HAVE_SYSTEM argsman.AddArg("-startupnotify=", "Execute command on startup.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-shutdownnotify=", "Execute command immediately before beginning shutdown. The need for shutdown may be urgent, so be careful not to delay it long (if the command doesn't require interaction with the server, consider having it fork into the background).", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif #ifndef WIN32 argsman.AddArg("-sysperms", "Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index e038afa1adedb..58a1c40bae22d 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -28,7 +28,7 @@ def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True # The experimental syscall sandbox feature (-sandbox) is not compatible with -alertnotify, - # -blocknotify or -walletnotify (which all invoke execve). + # -blocknotify, -walletnotify or -shutdownnotify (which all invoke execve). self.disable_syscall_sandbox = True def setup_network(self): @@ -36,14 +36,18 @@ def setup_network(self): self.alertnotify_dir = os.path.join(self.options.tmpdir, "alertnotify") self.blocknotify_dir = os.path.join(self.options.tmpdir, "blocknotify") self.walletnotify_dir = os.path.join(self.options.tmpdir, "walletnotify") + self.shutdownnotify_dir = os.path.join(self.options.tmpdir, "shutdownnotify") + self.shutdownnotify_file = os.path.join(self.shutdownnotify_dir, "shutdownnotify.txt") os.mkdir(self.alertnotify_dir) os.mkdir(self.blocknotify_dir) os.mkdir(self.walletnotify_dir) + os.mkdir(self.shutdownnotify_dir) # -alertnotify and -blocknotify on node0, walletnotify on node1 self.extra_args = [[ f"-alertnotify=echo > {os.path.join(self.alertnotify_dir, '%s')}", f"-blocknotify=echo > {os.path.join(self.blocknotify_dir, '%s')}", + f"-shutdownnotify=echo > {self.shutdownnotify_file}", ], [ f"-walletnotify=echo %h_%b > {os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))}", ]] @@ -159,6 +163,10 @@ def run_test(self): # TODO: add test for `-alertnotify` large fork notifications + self.log.info("test -shutdownnotify") + self.stop_nodes() + self.wait_until(lambda: os.path.isfile(self.shutdownnotify_file), timeout=10) + def expect_wallet_notify(self, tx_details): self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_details), timeout=10) # Should have no more and no less files than expected From d96d97ad30c4a079450bc2bf02e3e2a45f7efd2d Mon Sep 17 00:00:00 2001 From: klementtan Date: Thu, 17 Mar 2022 01:03:51 +0800 Subject: [PATCH 022/791] doc: Add release note for shutdownnotify. --- doc/release-notes-23395.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/release-notes-23395.md diff --git a/doc/release-notes-23395.md b/doc/release-notes-23395.md new file mode 100644 index 0000000000000..b9d7d9409c263 --- /dev/null +++ b/doc/release-notes-23395.md @@ -0,0 +1,8 @@ +Notable changes +=============== + +New settings +------------ + +- The `shutdownnotify` option is used to specify a command to execute synchronously +before Bitcoin Core has begun its shutdown sequence. (#23395) From 4163093d6374e865dc57e33dbea0e7e359062e0a Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 28 Jun 2022 16:39:15 +0200 Subject: [PATCH 023/791] wallet: use Mutex for g_sqlite_mutex instead of GlobalMutex Using `Mutex` provides stronger guarantee than `GlobalMutex` wrt Clang's thread safety analysis. Thus it is better to reduce the usage of `GlobalMutex` in favor of `Mutex`. Using `Mutex` for `g_sqlite_mutex` is ok because its usage is limited in `wallet/sqlite.cpp` and it does not require propagating the negative annotations to not relevant code. --- src/wallet/sqlite.cpp | 8 +++++--- src/wallet/sqlite.h | 12 +++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 053fb8f9834bf..39158407092b8 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -23,9 +23,6 @@ namespace wallet { static constexpr int32_t WALLET_SCHEMA_VERSION = 0; -static GlobalMutex g_sqlite_mutex; -static int g_sqlite_count GUARDED_BY(g_sqlite_mutex) = 0; - static void ErrorLogCallback(void* arg, int code, const char* msg) { // From sqlite3_config() documentation for the SQLITE_CONFIG_LOG option: @@ -83,6 +80,9 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va } } +Mutex SQLiteDatabase::g_sqlite_mutex; +int SQLiteDatabase::g_sqlite_count = 0; + SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock) : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync) { @@ -146,6 +146,8 @@ SQLiteDatabase::~SQLiteDatabase() void SQLiteDatabase::Cleanup() noexcept { + AssertLockNotHeld(g_sqlite_mutex); + Close(); LOCK(g_sqlite_mutex); diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 47b7ebb0ecde0..d7135679ff8af 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_WALLET_SQLITE_H #define BITCOIN_WALLET_SQLITE_H +#include #include #include @@ -63,7 +64,16 @@ class SQLiteDatabase : public WalletDatabase const std::string m_file_path; - void Cleanup() noexcept; + /** + * This mutex protects SQLite initialization and shutdown. + * sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is). + * Concurrent threads that execute SQLiteDatabase::SQLiteDatabase() should have just one + * of them do the init and the rest wait for it to complete before all can proceed. + */ + static Mutex g_sqlite_mutex; + static int g_sqlite_count GUARDED_BY(g_sqlite_mutex); + + void Cleanup() noexcept EXCLUSIVE_LOCKS_REQUIRED(!g_sqlite_mutex); public: SQLiteDatabase() = delete; From 2555a3950f0304b7af7609c1e6c696993c50ac72 Mon Sep 17 00:00:00 2001 From: Dhruv Mehta <856960+dhruv@users.noreply.github.com> Date: Fri, 25 Sep 2020 12:33:47 -0700 Subject: [PATCH 024/791] p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified --- src/init.cpp | 7 ++++++ src/net.cpp | 8 ++++--- test/functional/feature_config_args.py | 32 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1b3162bf39899..d6207f31de740 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1799,6 +1799,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (connect.size() != 1 || connect[0] != "0") { connOptions.m_specified_outgoing = connect; } + if (!connOptions.m_specified_outgoing.empty() && !connOptions.vSeedNodes.empty()) { + LogPrintf("-seednode is ignored when -connect is used\n"); + } + + if (args.IsArgSet("-dnsseed") && args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED) && args.IsArgSet("-proxy")) { + LogPrintf("-dnsseed is ignored when -connect is used and -proxy is specified\n"); + } } const std::string& i2psam_arg = args.GetArg("-i2psam", ""); diff --git a/src/net.cpp b/src/net.cpp index 3c28b9eddf9c2..e7ce2e014b0f2 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1465,6 +1465,8 @@ void CConnman::ThreadDNSAddressSeed() } LogPrintf("Loading addresses from DNS seed %s\n", seed); + // If -proxy is in use, we make an ADDR_FETCH connection to the DNS resolved peer address + // for the base dns seed domain in chainparams if (HaveNameProxy()) { AddAddrFetch(seed); } else { @@ -1486,8 +1488,9 @@ void CConnman::ThreadDNSAddressSeed() } addrman.Add(vAdd, resolveSource); } else { - // We now avoid directly using results from DNS Seeds which do not support service bit filtering, - // instead using them as a addrfetch to get nodes with our desired service bits. + // If the seed does not support a subdomain with our desired service bits, + // we make an ADDR_FETCH connection to the DNS resolved peer address for the + // base dns seed domain in chainparams AddAddrFetch(seed); } } @@ -1583,7 +1586,6 @@ void CConnman::ThreadOpenConnections(const std::vector connect) { for (int64_t nLoop = 0;; nLoop++) { - ProcessAddrFetch(); for (const std::string& strAddr : connect) { CAddress addr(CService(), NODE_NONE); diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index eb31bca29aeb7..d18e4af1ffe53 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -224,11 +224,43 @@ def test_seed_peers(self): ]): self.nodes[0].setmocktime(start + 65) + def test_connect_with_seednode(self): + self.log.info('Test -connect with -seednode') + seednode_ignored = ['-seednode is ignored when -connect is used\n'] + dnsseed_ignored = ['-dnsseed is ignored when -connect is used and -proxy is specified\n'] + addcon_thread_started = ['addcon thread start\n'] + self.stop_node(0) + + # When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode) + # nodes is irrelevant and -seednode is ignored. + with self.nodes[0].assert_debug_log(expected_msgs=seednode_ignored): + self.start_node(0, extra_args=['-connect=fakeaddress1', '-seednode=fakeaddress2']) + + # With -proxy, an ADDR_FETCH connection is made to a peer that the dns seed resolves to. + # ADDR_FETCH connections are not used when -connect is used. + with self.nodes[0].assert_debug_log(expected_msgs=dnsseed_ignored): + self.restart_node(0, extra_args=['-connect=fakeaddress1', '-dnsseed=1', '-proxy=1.2.3.4']) + + # If the user did not disable -dnsseed, but it was soft-disabled because they provided -connect, + # they shouldn't see a warning about -dnsseed being ignored. + with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started, + unexpected_msgs=dnsseed_ignored): + self.restart_node(0, extra_args=['-connect=fakeaddress1', '-proxy=1.2.3.4']) + + # We have to supply expected_msgs as it's a required argument + # The expected_msg must be something we are confident will be logged after the unexpected_msg + # These cases test for -connect being supplied but only to disable it + for connect_arg in ['-connect=0', '-noconnect']: + with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started, + unexpected_msgs=seednode_ignored): + self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2']) + def run_test(self): self.test_log_buffer() self.test_args_log() self.test_seed_peers() self.test_networkactive() + self.test_connect_with_seednode() self.test_config_file_parser() self.test_invalid_command_line_options() From 04609284ad5e0b72651f2d4b43263461ada40816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A8le=20Oul=C3=A8s?= Date: Sat, 1 Oct 2022 23:52:17 +0200 Subject: [PATCH 025/791] rpc: Improve error when wallet is already loaded --- src/wallet/rpc/wallet.cpp | 8 ++++++++ test/functional/wallet_multiwallet.py | 8 ++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 675c4a759d00b..0f1635fac9099 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -226,6 +226,14 @@ static RPCHelpMan loadwallet() bilingual_str error; std::vector warnings; std::optional load_on_start = request.params[1].isNull() ? std::nullopt : std::optional(request.params[1].get_bool()); + + { + LOCK(context.wallets_mutex); + if (std::any_of(context.wallets.begin(), context.wallets.end(), [&name](const auto& wallet) { return wallet->GetName() == name; })) { + throw JSONRPCError(RPC_WALLET_ALREADY_LOADED, "Wallet \"" + name + "\" is already loaded."); + } + } + std::shared_ptr const wallet = LoadWallet(context, name, load_on_start, options, status, error, warnings); HandleWalletError(wallet, status, error); diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 1c890d7207b82..73acf94329967 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -302,12 +302,8 @@ def wallet_file(name): assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Path does not exist.".format(path), self.nodes[0].loadwallet, 'wallets') # Fail to load duplicate wallets - path = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets", "w1", "wallet.dat") - if self.options.descriptors: - assert_raises_rpc_error(-4, f"Wallet file verification failed. SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of {self.config['environment']['PACKAGE_NAME']}?", self.nodes[0].loadwallet, wallet_names[0]) - else: - assert_raises_rpc_error(-35, "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format(path), self.nodes[0].loadwallet, wallet_names[0]) - + assert_raises_rpc_error(-35, "Wallet \"w1\" is already loaded.", self.nodes[0].loadwallet, wallet_names[0]) + if not self.options.descriptors: # This tests the default wallet that BDB makes, so SQLite wallet doesn't need to test this # Fail to load duplicate wallets by different ways (directory and filepath) path = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets", "wallet.dat") From fa579f306363ed03c1138121415b67b9b36b4d53 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 6 Jul 2022 17:22:31 +0200 Subject: [PATCH 026/791] refactor: Pass reference to last header, not pointer It is never a nullptr, otherwise an assertion would fire in UpdatePeerStateForReceivedHeaders. Passing a reference makes the code easier to read and less brittle. --- src/net_processing.cpp | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 34b4840eb127c..68216a2cfe1aa 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -657,9 +657,9 @@ class PeerManagerImpl final : public PeerManager */ bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ - void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast); + void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header); /** Update peer state based on received headers message */ - void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers); + void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers); void SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlock& block, const BlockTransactionsRequest& req); @@ -2605,22 +2605,21 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& loc } /* - * Given a new headers tip ending in pindexLast, potentially request blocks towards that tip. + * Given a new headers tip ending in last_header, potentially request blocks towards that tip. * We require that the given tip have at least as much work as our tip, and for * our current tip to be "close to synced" (see CanDirectFetch()). */ -void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast) +void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header) { const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); - if (CanDirectFetch() && pindexLast->IsValid(BLOCK_VALID_TREE) && m_chainman.ActiveChain().Tip()->nChainWork <= pindexLast->nChainWork) { - + if (CanDirectFetch() && last_header.IsValid(BLOCK_VALID_TREE) && m_chainman.ActiveChain().Tip()->nChainWork <= last_header.nChainWork) { std::vector vToFetch; - const CBlockIndex *pindexWalk = pindexLast; - // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. + const CBlockIndex* pindexWalk{&last_header}; + // Calculate all the blocks we'd need to switch to last_header, up to a limit. while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && @@ -2636,8 +2635,8 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c // direct fetch and rely on parallel download instead. if (!m_chainman.ActiveChain().Contains(pindexWalk)) { LogPrint(BCLog::NET, "Large reorg, won't direct fetch to %s (%d)\n", - pindexLast->GetBlockHash().ToString(), - pindexLast->nHeight); + last_header.GetBlockHash().ToString(), + last_header.nHeight); } else { std::vector vGetData; // Download as much as possible, from earliest to latest. @@ -2654,14 +2653,15 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c } if (vGetData.size() > 1) { LogPrint(BCLog::NET, "Downloading blocks toward %s (%d) via headers direct fetch\n", - pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); + last_header.GetBlockHash().ToString(), + last_header.nHeight); } if (vGetData.size() > 0) { if (!m_ignore_incoming_txs && nodestate->m_provides_cmpctblocks && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && - pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + last_header.pprev->IsValid(BLOCK_VALID_CHAIN)) { // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } @@ -2672,12 +2672,12 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c } /** - * Given receipt of headers from a peer ending in pindexLast, along with + * Given receipt of headers from a peer ending in last_header, along with * whether that header was new and whether the headers message was full, * update the state we keep for the peer. */ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, - const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers) + const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) { LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); @@ -2686,14 +2686,13 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, } nodestate->nUnconnectingHeaders = 0; - assert(pindexLast); - UpdateBlockAvailability(pfrom.GetId(), pindexLast->GetBlockHash()); + UpdateBlockAvailability(pfrom.GetId(), last_header.GetBlockHash()); // From here, pindexBestKnownBlock should be guaranteed to be non-null, // because it is set in UpdateBlockAvailability. Some nullptr checks // are still present, however, as belt-and-suspenders. - if (received_new_header && pindexLast->nChainWork > m_chainman.ActiveChain().Tip()->nChainWork) { + if (received_new_header && last_header.nChainWork > m_chainman.ActiveChain().Tip()->nChainWork) { nodestate->m_last_block_announcement = GetTime(); } @@ -2859,7 +2858,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } } - Assume(pindexLast); + assert(pindexLast); // Consider fetching more headers if we are not using our headers-sync mechanism. if (nCount == MAX_HEADERS_RESULTS && !have_headers_sync) { @@ -2870,10 +2869,10 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, } } - UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); + UpdatePeerStateForReceivedHeaders(pfrom, *pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); // Consider immediately downloading blocks. - HeadersDirectFetchBlocks(pfrom, peer, pindexLast); + HeadersDirectFetchBlocks(pfrom, peer, *pindexLast); return; } From 0565951f34e6d155dc825964c5d8b1dd00931682 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 23 Aug 2022 13:47:04 -0400 Subject: [PATCH 027/791] p2p: Make block stalling timeout adaptive This makes the stalling detection mechanism (previously a fixed timeout of 2s) adaptive: If we disconnect a peer for stalling, double the timeout for the next peer - and let it slowly relax back to its default value each time the tip advances. (Idea by Pieter Wuille) This makes situations more unlikely in which we'd keep on disconnecting many of our peers for stalling, even though our own bandwidth is insufficient to download a block in 2 seconds. Co-authored-by: Vasil Dimov --- src/net_processing.cpp | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b39412158f84e..10311a10be1c6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -110,8 +110,11 @@ static constexpr auto GETDATA_TX_INTERVAL{60s}; static const unsigned int MAX_GETDATA_SZ = 1000; /** Number of blocks that can be requested at any given time from a single peer. */ static const int MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16; -/** Time during which a peer must stall block download progress before being disconnected. */ -static constexpr auto BLOCK_STALLING_TIMEOUT{2s}; +/** Default time during which a peer must stall block download progress before being disconnected. + * the actual timeout is increased temporarily if peers are disconnected for hitting the timeout */ +static constexpr auto BLOCK_STALLING_TIMEOUT_DEFAULT{2s}; +/** Maximum timeout for stalling block download. */ +static constexpr auto BLOCK_STALLING_TIMEOUT_MAX{64s}; /** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends * less than this number, we reached its tip. Changing this value is a protocol upgrade. */ static const unsigned int MAX_HEADERS_RESULTS = 2000; @@ -705,6 +708,9 @@ class PeerManagerImpl final : public PeerManager /** Number of preferable block download peers. */ int m_num_preferred_download_peers GUARDED_BY(cs_main){0}; + /** Stalling timeout for blocks in IBD */ + std::atomic m_block_stalling_timeout{BLOCK_STALLING_TIMEOUT_DEFAULT}; + bool AlreadyHaveTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_recent_confirmed_transactions_mutex); @@ -1700,7 +1706,8 @@ void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler) /** * Evict orphan txn pool entries based on a newly connected * block, remember the recently confirmed transactions, and delete tracked - * announcements for them. Also save the time of the last tip update. + * announcements for them. Also save the time of the last tip update and + * possibly reduce dynamic block stalling timeout. */ void PeerManagerImpl::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) { @@ -1723,6 +1730,16 @@ void PeerManagerImpl::BlockConnected(const std::shared_ptr& pblock m_txrequest.ForgetTxHash(ptx->GetWitnessHash()); } } + + // In case the dynamic timeout was doubled once or more, reduce it slowly back to its default value + auto stalling_timeout = m_block_stalling_timeout.load(); + Assume(stalling_timeout >= BLOCK_STALLING_TIMEOUT_DEFAULT); + if (stalling_timeout != BLOCK_STALLING_TIMEOUT_DEFAULT) { + const auto new_timeout = std::max(std::chrono::duration_cast(stalling_timeout * 0.85), BLOCK_STALLING_TIMEOUT_DEFAULT); + if (m_block_stalling_timeout.compare_exchange_strong(stalling_timeout, new_timeout)) { + LogPrint(BCLog::NET, "Decreased stalling timeout to %d seconds\n", new_timeout.count()); + } + } } void PeerManagerImpl::BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) @@ -5225,12 +5242,19 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); // Detect whether we're stalling - if (state.m_stalling_since.count() && state.m_stalling_since < current_time - BLOCK_STALLING_TIMEOUT) { + auto stalling_timeout = m_block_stalling_timeout.load(); + if (state.m_stalling_since.count() && state.m_stalling_since < current_time - stalling_timeout) { // Stalling only triggers when the block download window cannot move. During normal steady state, // the download window should be much larger than the to-be-downloaded set of blocks, so disconnection // should only happen during initial block download. LogPrintf("Peer=%d is stalling block download, disconnecting\n", pto->GetId()); pto->fDisconnect = true; + // Increase timeout for the next peer so that we don't disconnect multiple peers if our own + // bandwidth is insufficient. + const auto new_timeout = std::min(2 * stalling_timeout, BLOCK_STALLING_TIMEOUT_MAX); + if (stalling_timeout != new_timeout && m_block_stalling_timeout.compare_exchange_strong(stalling_timeout, new_timeout)) { + LogPrint(BCLog::NET, "Increased stalling timeout temporarily to %d seconds\n", m_block_stalling_timeout.load().count()); + } return true; } // In case there is a block that has been in flight from this peer for block_interval * (1 + 0.5 * N) From 5299cfe371ddad806b1c4538d17cde069e25b1c1 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 27 Oct 2022 12:54:09 +0100 Subject: [PATCH 028/791] qt: Delete splash screen widget explicitly This ensures that during shutdown, including failed initialization, the `SplashScreen::m_connected_wallet_handlers` is deleted before the wallet context is. --- src/qt/bitcoin.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index cc01e4d54afd4..24af58a731266 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -311,11 +311,8 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) { assert(!m_splash); m_splash = new SplashScreen(networkStyle); - // We don't hold a direct pointer to the splash screen after creation, but the splash - // screen will take care of deleting itself when finish() happens. m_splash->show(); connect(this, &BitcoinApplication::splashFinished, m_splash, &SplashScreen::finish); - connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close); } void BitcoinApplication::createNode(interfaces::Init& init) @@ -373,6 +370,9 @@ void BitcoinApplication::requestShutdown() w->hide(); } + delete m_splash; + m_splash = nullptr; + // Show a simple window indicating shutdown status // Do this first as some of the steps may take some time below, // for example the RPC console may still be executing a command. @@ -412,10 +412,13 @@ void BitcoinApplication::requestShutdown() void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info) { qDebug() << __func__ << ": Initialization result: " << success; + // Set exit result. returnValue = success ? EXIT_SUCCESS : EXIT_FAILURE; - if(success) - { + if(success) { + delete m_splash; + m_splash = nullptr; + // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete qInfo() << "Platform customization:" << platformStyle->getName(); clientModel = new ClientModel(node(), optionsModel); @@ -438,7 +441,6 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead } else { window->showMinimized(); } - Q_EMIT splashFinished(); Q_EMIT windowShown(window); #ifdef ENABLE_WALLET @@ -455,7 +457,6 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead #endif pollShutdownTimer->start(SHUTDOWN_POLLING_DELAY); } else { - Q_EMIT splashFinished(); // Make sure splash screen doesn't stick around during shutdown requestShutdown(); } } From 10811afff40efed1fda7eecab89884eaadd7146c Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 27 Oct 2022 12:54:23 +0100 Subject: [PATCH 029/791] qt: Drop no longer used `BitcoinApplication::splashFinished()` signal --- src/qt/bitcoin.cpp | 1 - src/qt/bitcoin.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 24af58a731266..1bd7795a38162 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -312,7 +312,6 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) assert(!m_splash); m_splash = new SplashScreen(networkStyle); m_splash->show(); - connect(this, &BitcoinApplication::splashFinished, m_splash, &SplashScreen::finish); } void BitcoinApplication::createNode(interfaces::Init& init) diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 9ad37ca6c978d..4f9eed5f49618 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -89,7 +89,6 @@ public Q_SLOTS: Q_SIGNALS: void requestedInitialize(); void requestedShutdown(); - void splashFinished(); void windowShown(BitcoinGUI* window); protected: From 1b228497fa729c512a15bdfa80f61a610abfe8a5 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 27 Oct 2022 12:54:34 +0100 Subject: [PATCH 030/791] qt: Drop no longer used `SplashScreen::finish()` slot --- src/qt/splashscreen.cpp | 10 ---------- src/qt/splashscreen.h | 3 --- 2 files changed, 13 deletions(-) diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index bf04a6dd5f77d..e1692aeec79f0 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -161,16 +161,6 @@ bool SplashScreen::eventFilter(QObject * obj, QEvent * ev) { return QObject::eventFilter(obj, ev); } -void SplashScreen::finish() -{ - /* If the window is minimized, hide() will be ignored. */ - /* Make sure we de-minimize the splashscreen window before hiding */ - if (isMinimized()) - showNormal(); - hide(); - deleteLater(); // No more need for this -} - static void InitMessage(SplashScreen *splash, const std::string &message) { bool invoked = QMetaObject::invokeMethod(splash, "showMessage", diff --git a/src/qt/splashscreen.h b/src/qt/splashscreen.h index c14fc521a7be2..0f7fb6bef867d 100644 --- a/src/qt/splashscreen.h +++ b/src/qt/splashscreen.h @@ -37,9 +37,6 @@ class SplashScreen : public QWidget void closeEvent(QCloseEvent *event) override; public Q_SLOTS: - /** Hide the splash screen window and schedule the splash screen object for deletion */ - void finish(); - /** Show message and progress */ void showMessage(const QString &message, int alignment, const QColor &color); From 39b93649c4b98cd82c64b957fd9f6a6fd3c2a359 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 13 Sep 2022 11:07:55 -0400 Subject: [PATCH 031/791] test: add functional test for IBD stalling logic --- test/functional/p2p_ibd_stalling.py | 164 ++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 165 insertions(+) create mode 100755 test/functional/p2p_ibd_stalling.py diff --git a/test/functional/p2p_ibd_stalling.py b/test/functional/p2p_ibd_stalling.py new file mode 100755 index 0000000000000..9bd07be7b910f --- /dev/null +++ b/test/functional/p2p_ibd_stalling.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022- The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test stalling logic during IBD +""" + +import time + +from test_framework.blocktools import ( + create_block, + create_coinbase +) +from test_framework.messages import ( + MSG_BLOCK, + MSG_TYPE_MASK, +) +from test_framework.p2p import ( + CBlockHeader, + msg_block, + msg_headers, + P2PDataStore, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + + +class P2PStaller(P2PDataStore): + def __init__(self, stall_block): + self.stall_block = stall_block + super().__init__() + + def on_getdata(self, message): + for inv in message.inv: + self.getdata_requests.append(inv.hash) + if (inv.type & MSG_TYPE_MASK) == MSG_BLOCK: + if (inv.hash != self.stall_block): + self.send_message(msg_block(self.block_store[inv.hash])) + + def on_getheaders(self, message): + pass + + +class P2PIBDStallingTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def run_test(self): + NUM_BLOCKS = 1025 + NUM_PEERS = 4 + node = self.nodes[0] + tip = int(node.getbestblockhash(), 16) + blocks = [] + height = 1 + block_time = node.getblock(node.getbestblockhash())['time'] + 1 + self.log.info("Prepare blocks without sending them to the node") + block_dict = {} + for _ in range(NUM_BLOCKS): + blocks.append(create_block(tip, create_coinbase(height), block_time)) + blocks[-1].solve() + tip = blocks[-1].sha256 + block_time += 1 + height += 1 + block_dict[blocks[-1].sha256] = blocks[-1] + stall_block = blocks[0].sha256 + + headers_message = msg_headers() + headers_message.headers = [CBlockHeader(b) for b in blocks[:NUM_BLOCKS-1]] + peers = [] + + self.log.info("Check that a staller does not get disconnected if the 1024 block lookahead buffer is filled") + for id in range(NUM_PEERS): + peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_block), p2p_idx=id, connection_type="outbound-full-relay")) + peers[-1].block_store = block_dict + peers[-1].send_message(headers_message) + + # Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc + # returning the number of downloaded (but not connected) blocks. + self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761) + + self.all_sync_send_with_ping(peers) + # If there was a peer marked for stalling, it would get disconnected + self.mocktime = int(time.time()) + 3 + node.setmocktime(self.mocktime) + self.all_sync_send_with_ping(peers) + assert_equal(node.num_test_p2p_connections(), NUM_PEERS) + + self.log.info("Check that increasing the window beyond 1024 blocks triggers stalling logic") + headers_message.headers = [CBlockHeader(b) for b in blocks] + with node.assert_debug_log(expected_msgs=['Stall started']): + for p in peers: + p.send_message(headers_message) + self.all_sync_send_with_ping(peers) + + self.log.info("Check that the stalling peer is disconnected after 2 seconds") + self.mocktime += 3 + node.setmocktime(self.mocktime) + peers[0].wait_for_disconnect() + assert_equal(node.num_test_p2p_connections(), NUM_PEERS - 1) + self.wait_until(lambda: self.is_block_requested(peers, stall_block)) + # Make sure that SendMessages() is invoked, which assigns the missing block + # to another peer and starts the stalling logic for them + self.all_sync_send_with_ping(peers) + + self.log.info("Check that the stalling timeout gets doubled to 4 seconds for the next staller") + # No disconnect after just 3 seconds + self.mocktime += 3 + node.setmocktime(self.mocktime) + self.all_sync_send_with_ping(peers) + assert_equal(node.num_test_p2p_connections(), NUM_PEERS - 1) + + self.mocktime += 2 + node.setmocktime(self.mocktime) + self.wait_until(lambda: node.num_test_p2p_connections() == NUM_PEERS - 2) + self.wait_until(lambda: self.is_block_requested(peers, stall_block)) + self.all_sync_send_with_ping(peers) + + self.log.info("Check that the stalling timeout gets doubled to 8 seconds for the next staller") + # No disconnect after just 7 seconds + self.mocktime += 7 + node.setmocktime(self.mocktime) + self.all_sync_send_with_ping(peers) + assert_equal(node.num_test_p2p_connections(), NUM_PEERS - 2) + + self.mocktime += 2 + node.setmocktime(self.mocktime) + self.wait_until(lambda: node.num_test_p2p_connections() == NUM_PEERS - 3) + self.wait_until(lambda: self.is_block_requested(peers, stall_block)) + self.all_sync_send_with_ping(peers) + + self.log.info("Provide the withheld block and check that stalling timeout gets reduced back to 2 seconds") + with node.assert_debug_log(expected_msgs=['Decreased stalling timeout to 2 seconds']): + for p in peers: + if p.is_connected and (stall_block in p.getdata_requests): + p.send_message(msg_block(block_dict[stall_block])) + + self.log.info("Check that all outstanding blocks get connected") + self.wait_until(lambda: node.getblockcount() == NUM_BLOCKS) + + def total_bytes_recv_for_blocks(self): + total = 0 + for info in self.nodes[0].getpeerinfo(): + if ("block" in info["bytesrecv_per_msg"].keys()): + total += info["bytesrecv_per_msg"]["block"] + return total + + def all_sync_send_with_ping(self, peers): + for p in peers: + if p.is_connected: + p.sync_send_with_ping() + + def is_block_requested(self, peers, hash): + for p in peers: + if p.is_connected and (hash in p.getdata_requests): + return True + return False + + +if __name__ == '__main__': + P2PIBDStallingTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 37d0c4f87e27f..05de01cfaecbf 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -242,6 +242,7 @@ 'wallet_importprunedfunds.py --descriptors', 'p2p_leak_tx.py', 'p2p_eviction.py', + 'p2p_ibd_stalling.py', 'wallet_signmessagewithaddress.py', 'rpc_signmessagewithprivkey.py', 'rpc_generate.py', From 1914e470e327091c625f627b4447beb336f7fe5a Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 29 Oct 2022 15:51:10 +0100 Subject: [PATCH 032/791] build: copy config.{guess,sub} post autogen in zmq package Otherwise our config.guess and config.sub will be copied over. This problem has been masked by the fact that modern systems ship with versions that recognise all the triplets we use (namely arm64-apple-darwin). However building on ubuntu 20.04 surfaces the issue. Fixes #26420. --- depends/packages/zeromq.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index 267ed11253186..d7152327934fb 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -20,12 +20,12 @@ endef define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/remove_libstd_link.patch && \ - patch -p1 < $($(package)_patch_dir)/netbsd_kevent_void.patch && \ - cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub config + patch -p1 < $($(package)_patch_dir)/netbsd_kevent_void.patch endef define $(package)_config_cmds ./autogen.sh && \ + cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub config && \ $($(package)_autoconf) endef From f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 Mon Sep 17 00:00:00 2001 From: Douglas Chimento Date: Sun, 21 Nov 2021 11:17:59 +0200 Subject: [PATCH 033/791] rpc: Return fee and prevout(s) to getrawtransaction * Add optional fee response in BTC to getrawtransaction * Add optional prevout(s) response to getrawtransaction showing utxos being spent * Add getrawtransaction_verbosity functional test to validate fields --- src/rpc/client.cpp | 1 + src/rpc/rawtransaction.cpp | 112 ++++++++++++++++++++------ test/functional/rpc_rawtransaction.py | 81 +++++++++++++++++-- 3 files changed, 164 insertions(+), 30 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8688263ef547e..0398db964a428 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -102,6 +102,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getchaintxstats", 0, "nblocks" }, { "gettransaction", 1, "include_watchonly" }, { "gettransaction", 2, "verbose" }, + { "getrawtransaction", 1, "verbosity" }, { "getrawtransaction", 1, "verbose" }, { "createrawtransaction", 0, "inputs" }, { "createrawtransaction", 1, "outputs" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d654de18626f1..a83bf9a03ff40 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -32,6 +32,7 @@ #include