diff options
Diffstat (limited to 'src')
66 files changed, 1105 insertions, 413 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 780f547b4b..e452b42533 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,7 +9,7 @@ print-%: FORCE DIST_SUBDIRS = secp256k1 AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) -AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) +AM_CXXFLAGS = $(CORE_CXXFLAGS) $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) @@ -50,6 +50,10 @@ LIBBITCOIN_WALLET_TOOL=libbitcoin_wallet_tool.a endif LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE) +if USE_ASM +LIBBITCOIN_CRYPTO_SSE4 = crypto/libbitcoin_crypto_sse4.la +LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE4) +endif if ENABLE_SSE41 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41) @@ -541,6 +545,10 @@ libbitcoin_wallet_tool_a_SOURCES = \ # # crypto # + +# crypto_base contains the unspecialized (unoptimized) versions of our +# crypto functions. Functions that require custom compiler flags and/or +# runtime opt-in are omitted. crypto_libbitcoin_crypto_base_la_CPPFLAGS = $(AM_CPPFLAGS) # Specify -static in both CXXFLAGS and LDFLAGS so libtool will only build a @@ -581,9 +589,12 @@ crypto_libbitcoin_crypto_base_la_SOURCES = \ crypto/siphash.cpp \ crypto/siphash.h -if USE_ASM -crypto_libbitcoin_crypto_base_la_SOURCES += crypto/sha256_sse4.cpp -endif +# See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and +# CXXFLAGS above +crypto_libbitcoin_crypto_sse4_la_LDFLAGS = $(AM_LDFLAGS) -static +crypto_libbitcoin_crypto_sse4_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static +crypto_libbitcoin_crypto_sse4_la_CPPFLAGS = $(AM_CPPFLAGS) +crypto_libbitcoin_crypto_sse4_la_SOURCES = crypto/sha256_sse4.cpp # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and # CXXFLAGS above @@ -906,7 +917,7 @@ lib_LTLIBRARIES += $(LIBBITCOINKERNEL) libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS) libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) -libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) +libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) # libbitcoinkernel requires default symbol visibility, explicitly specify that # here so that things still work even when user configures with @@ -1012,7 +1023,7 @@ libbitcoinconsensus_la_SOURCES = support/cleanse.cpp $(crypto_libbitcoin_crypto_ libbitcoinconsensus_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) libbitcoinconsensus_la_LIBADD = $(LIBSECP256K1) -libbitcoinconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL +libbitcoinconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL -DDISABLE_OPTIMIZED_SHA256 libbitcoinconsensus_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) endif @@ -1090,6 +1101,7 @@ ipc/capnp/libbitcoin_ipc_a-protocol.$(OBJEXT): $(libbitcoin_ipc_mpgen_input:=.h) if BUILD_MULTIPROCESS LIBBITCOIN_IPC=libbitcoin_ipc.a libbitcoin_ipc_a_SOURCES = \ + ipc/capnp/common-types.h \ ipc/capnp/context.h \ ipc/capnp/init-types.h \ ipc/capnp/protocol.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 870e49bd75..69d44dbde6 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -117,6 +117,7 @@ BITCOIN_TESTS =\ test/net_tests.cpp \ test/netbase_tests.cpp \ test/orphanage_tests.cpp \ + test/peerman_tests.cpp \ test/pmt_tests.cpp \ test/policy_fee_tests.cpp \ test/policyestimator_tests.cpp \ @@ -216,6 +217,39 @@ BITCOIN_TEST_SUITE += \ wallet/test/init_test_fixture.h endif # ENABLE_WALLET +if BUILD_MULTIPROCESS +# Add boost ipc_tests definition to BITCOIN_TESTS +BITCOIN_TESTS += test/ipc_tests.cpp + +# Build ipc_test code in a separate library so it can be compiled with custom +# LIBMULTIPROCESS_CFLAGS without those flags affecting other tests +LIBBITCOIN_IPC_TEST=libbitcoin_ipc_test.a +EXTRA_LIBRARIES += $(LIBBITCOIN_IPC_TEST) +libbitcoin_ipc_test_a_SOURCES = \ + test/ipc_test.cpp \ + test/ipc_test.h +libbitcoin_ipc_test_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_ipc_test_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) $(LIBMULTIPROCESS_CFLAGS) + +# Generate various .c++/.h files from the ipc_test.capnp file +include $(MPGEN_PREFIX)/include/mpgen.mk +EXTRA_DIST += test/ipc_test.capnp +libbitcoin_ipc_test_mpgen_output = \ + test/ipc_test.capnp.c++ \ + test/ipc_test.capnp.h \ + test/ipc_test.capnp.proxy-client.c++ \ + test/ipc_test.capnp.proxy-server.c++ \ + test/ipc_test.capnp.proxy-types.c++ \ + test/ipc_test.capnp.proxy-types.h \ + test/ipc_test.capnp.proxy.h +nodist_libbitcoin_ipc_test_a_SOURCES = $(libbitcoin_ipc_test_mpgen_output) +CLEANFILES += $(libbitcoin_ipc_test_mpgen_output) +endif + +# Explicitly list dependencies on generated headers as described in +# https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html#Recording-Dependencies-manually +test/libbitcoin_ipc_test_a-ipc_test.$(OBJEXT): test/ipc_test.capnp.h + test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(BOOST_CPPFLAGS) $(EVENT_CFLAGS) test_test_bitcoin_LDADD = $(LIBTEST_UTIL) @@ -223,6 +257,9 @@ if ENABLE_WALLET test_test_bitcoin_LDADD += $(LIBBITCOIN_WALLET) test_test_bitcoin_CPPFLAGS += $(BDB_CPPFLAGS) endif +if BUILD_MULTIPROCESS +test_test_bitcoin_LDADD += $(LIBBITCOIN_IPC_TEST) $(LIBMULTIPROCESS_LIBS) +endif test_test_bitcoin_LDADD += $(LIBBITCOIN_NODE) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) \ $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS) $(MINISKETCH_LIBS) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index ee8b0a44c5..c1a71ed749 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -117,7 +117,6 @@ int main(int argc, char* argv[]) const ChainstateManager::Options chainman_opts{ .chainparams = *chainparams, .datadir = abs_datadir, - .adjusted_time_callback = NodeClock::now, .notifications = *notifications, }; const node::BlockManager::Options blockman_opts{ diff --git a/src/chain.h b/src/chain.h index f9121fb861..fa165a4aa7 100644 --- a/src/chain.h +++ b/src/chain.h @@ -11,15 +11,20 @@ #include <flatfile.h> #include <kernel/cs_main.h> #include <primitives/block.h> +#include <serialize.h> #include <sync.h> #include <uint256.h> #include <util/time.h> +#include <algorithm> +#include <cassert> +#include <cstdint> +#include <string> #include <vector> /** * Maximum amount of time that a block timestamp is allowed to exceed the - * current network-adjusted time before the block will be accepted. + * current time before the block will be accepted. */ static constexpr int64_t MAX_FUTURE_BLOCK_TIME = 2 * 60 * 60; diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 5761e8b321..db1001111a 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -4,6 +4,10 @@ #include <common/settings.h> +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif + #include <tinyformat.h> #include <univalue.h> #include <util/fs.h> @@ -27,6 +31,9 @@ enum class Source { CONFIG_FILE_DEFAULT_SECTION }; +// Json object key for the auto-generated warning comment +const std::string SETTINGS_WARN_MSG_KEY{"_warning_"}; + //! Merge settings from multiple sources in precedence order: //! Forced config > command line > read-write settings file > config file network-specific section > config file default section //! @@ -81,7 +88,9 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va SettingsValue in; if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) { - errors.emplace_back(strprintf("Unable to parse settings file %s", fs::PathToString(path))); + errors.emplace_back(strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))); return false; } @@ -106,6 +115,10 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va break; } } + + // Remove auto-generated warning comment from the accessible settings. + values.erase(SETTINGS_WARN_MSG_KEY); + return errors.empty(); } @@ -114,6 +127,10 @@ bool WriteSettings(const fs::path& path, std::vector<std::string>& errors) { SettingsValue out(SettingsValue::VOBJ); + // Add auto-generated warning comment + out.pushKV(SETTINGS_WARN_MSG_KEY, strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", PACKAGE_NAME)); + // Push settings values for (const auto& value : values) { out.pushKVEnd(value.first, value.second); } diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 5eacaa44e1..11aabeb1da 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -8,14 +8,15 @@ #include <assert.h> #include <string.h> +#if !defined(DISABLE_OPTIMIZED_SHA256) #include <compat/cpuid.h> -#if defined(__linux__) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(__linux__) && defined(ENABLE_ARM_SHANI) #include <sys/auxv.h> #include <asm/hwcap.h> #endif -#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) #include <sys/types.h> #include <sys/sysctl.h> #endif @@ -58,6 +59,7 @@ namespace sha256d64_arm_shani { void Transform_2way(unsigned char* out, const unsigned char* in); } +#endif // DISABLE_OPTIMIZED_SHA256 // Internal implementation code. namespace @@ -567,6 +569,7 @@ bool SelfTest() { return true; } +#if !defined(DISABLE_OPTIMIZED_SHA256) #if defined(USE_ASM) && (defined(__x86_64__) || defined(__amd64__) || defined(__i386__)) /** Check whether the OS has enabled AVX registers. */ bool AVXEnabled() @@ -576,6 +579,7 @@ bool AVXEnabled() return (a & 6) == 6; } #endif +#endif // DISABLE_OPTIMIZED_SHA256 } // namespace @@ -588,6 +592,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64_4way = nullptr; TransformD64_8way = nullptr; +#if !defined(DISABLE_OPTIMIZED_SHA256) #if defined(USE_ASM) && defined(HAVE_GETCPUID) bool have_sse4 = false; bool have_xsave = false; @@ -616,7 +621,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem } } -#if defined(ENABLE_X86_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_X86_SHANI) if (have_x86_shani) { Transform = sha256_x86_shani::Transform; TransformD64 = TransformD64Wrapper<sha256_x86_shani::Transform>; @@ -633,13 +638,13 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64 = TransformD64Wrapper<sha256_sse4::Transform>; ret = "sse4(1way)"; #endif -#if defined(ENABLE_SSE41) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_SSE41) TransformD64_4way = sha256d64_sse41::Transform_4way; ret += ",sse41(4way)"; #endif } -#if defined(ENABLE_AVX2) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_AVX2) if (have_avx2 && have_avx && enabled_avx) { TransformD64_8way = sha256d64_avx2::Transform_8way; ret += ",avx2(8way)"; @@ -647,7 +652,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem #endif #endif // defined(USE_ASM) && defined(HAVE_GETCPUID) -#if defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_ARM_SHANI) bool have_arm_shani = false; if (use_implementation & sha256_implementation::USE_SHANI) { #if defined(__linux__) @@ -679,6 +684,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem ret = "arm_shani(1way,2way)"; } #endif +#endif // DISABLE_OPTIMIZED_SHA256 assert(SelfTest()); return ret; diff --git a/src/headerssync.cpp b/src/headerssync.cpp index 234fc8da60..e14de004f5 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -5,8 +5,8 @@ #include <headerssync.h> #include <logging.h> #include <pow.h> -#include <timedata.h> #include <util/check.h> +#include <util/time.h> #include <util/vector.h> // The two constants below are computed using the simulation script in @@ -41,7 +41,7 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus // exceeds this bound, because it's not possible for a consensus-valid // chain to be longer than this (at the current time -- in the future we // could try again, if necessary, to sync a longer chain). - m_max_commitments = 6*(Ticks<std::chrono::seconds>(GetAdjustedTime() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}}) + MAX_FUTURE_BLOCK_TIME) / HEADER_COMMITMENT_PERIOD; + m_max_commitments = 6*(Ticks<std::chrono::seconds>(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}}) + MAX_FUTURE_BLOCK_TIME) / HEADER_COMMITMENT_PERIOD; LogPrint(BCLog::NET, "Initial headers sync started with peer=%d: height=%i, max_commitments=%i, min_work=%s\n", m_id, m_current_height, m_max_commitments, m_minimum_required_work.ToString()); } diff --git a/src/init.cpp b/src/init.cpp index b825c8ce21..988daefeec 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1448,7 +1448,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ChainstateManager::Options chainman_opts{ .chainparams = chainparams, .datadir = args.GetDataDirNet(), - .adjusted_time_callback = GetAdjustedTime, .notifications = *node.notifications, }; Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction @@ -1743,13 +1742,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 12: start node //// debug print + int64_t best_block_time{}; { LOCK(cs_main); LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); chain_active_height = chainman.ActiveChain().Height(); + best_block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); if (tip_info) { tip_info->block_height = chain_active_height; - tip_info->block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); + tip_info->block_time = best_block_time; tip_info->verification_progress = GuessVerificationProgress(chainman.GetParams().TxData(), chainman.ActiveChain().Tip()); } if (tip_info && chainman.m_best_header) { @@ -1758,7 +1759,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } LogPrintf("nBestHeight = %d\n", chain_active_height); - if (node.peerman) node.peerman->SetBestHeight(chain_active_height); + if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time}); // Map ports with UPnP or NAT-PMP. StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h new file mode 100644 index 0000000000..39e368491b --- /dev/null +++ b/src/ipc/capnp/common-types.h @@ -0,0 +1,108 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_IPC_CAPNP_COMMON_TYPES_H +#define BITCOIN_IPC_CAPNP_COMMON_TYPES_H + +#include <clientversion.h> +#include <streams.h> +#include <univalue.h> + +#include <cstddef> +#include <mp/proxy-types.h> +#include <type_traits> +#include <utility> + +namespace ipc { +namespace capnp { +//! Use SFINAE to define Serializeable<T> trait which is true if type T has a +//! Serialize(stream) method, false otherwise. +template <typename T> +struct Serializable { +private: + template <typename C> + static std::true_type test(decltype(std::declval<C>().Serialize(std::declval<std::nullptr_t&>()))*); + template <typename> + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test<T>(nullptr))::value; +}; + +//! Use SFINAE to define Unserializeable<T> trait which is true if type T has +//! an Unserialize(stream) method, false otherwise. +template <typename T> +struct Unserializable { +private: + template <typename C> + static std::true_type test(decltype(std::declval<C>().Unserialize(std::declval<std::nullptr_t&>()))*); + template <typename> + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test<T>(nullptr))::value; +}; +} // namespace capnp +} // namespace ipc + +//! Functions to serialize / deserialize common bitcoin types. +namespace mp { +//! Overload multiprocess library's CustomBuildField hook to allow any +//! serializable object to be stored in a capnproto Data field or passed to a +//! canproto interface. Use Priority<1> so this hook has medium priority, and +//! higher priority hooks could take precedence over this one. +template <typename LocalType, typename Value, typename Output> +void CustomBuildField( + TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output, + // Enable if serializeable and if LocalType is not cv or reference + // qualified. If LocalType is cv or reference qualified, it is important to + // fall back to lower-priority Priority<0> implementation of this function + // that strips cv references, to prevent this CustomBuildField overload from + // taking precedence over more narrow overloads for specific LocalTypes. + std::enable_if_t<ipc::capnp::Serializable<LocalType>::value && + std::is_same_v<LocalType, std::remove_cv_t<std::remove_reference_t<LocalType>>>>* enable = nullptr) +{ + DataStream stream; + value.Serialize(stream); + auto result = output.init(stream.size()); + memcpy(result.begin(), stream.data(), stream.size()); +} + +//! Overload multiprocess library's CustomReadField hook to allow any object +//! with an Unserialize method to be read from a capnproto Data field or +//! returned from canproto interface. Use Priority<1> so this hook has medium +//! priority, and higher priority hooks could take precedence over this one. +template <typename LocalType, typename Input, typename ReadDest> +decltype(auto) +CustomReadField(TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest, + std::enable_if_t<ipc::capnp::Unserializable<LocalType>::value>* enable = nullptr) +{ + return read_dest.update([&](auto& value) { + if (!input.has()) return; + auto data = input.get(); + SpanReader stream({data.begin(), data.end()}); + value.Unserialize(stream); + }); +} + +template <typename Value, typename Output> +void CustomBuildField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output) +{ + std::string str = value.write(); + auto result = output.init(str.size()); + memcpy(result.begin(), str.data(), str.size()); +} + +template <typename Input, typename ReadDest> +decltype(auto) CustomReadField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Input&& input, + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + auto data = input.get(); + value.read(std::string_view{data.begin(), data.size()}); + }); +} +} // namespace mp + +#endif // BITCOIN_IPC_CAPNP_COMMON_TYPES_H diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 56cb3a63a0..b0e657ba45 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -501,9 +501,9 @@ public: { // For use by test/functional/feature_assumeutxo.py .height = 299, - .hash_serialized = AssumeutxoHash{uint256S("0x61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53")}, - .nChainTx = 300, - .blockhash = uint256S("0x7e0517ef3ea6ecbed9117858e42eedc8eb39e8698a38dcbd1b3962a283233f4c") + .hash_serialized = AssumeutxoHash{uint256S("0xa4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")}, + .nChainTx = 334, + .blockhash = uint256S("0x3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0") }, }; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index ee20eabd79..864aac336e 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -32,7 +32,6 @@ namespace kernel { struct ChainstateManagerOpts { const CChainParams& chainparams; fs::path datadir; - const std::function<NodeClock::time_point()> adjusted_time_callback{nullptr}; std::optional<bool> check_block_index{}; bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; //! If set, it will override the minimum work we will assume exists on some valid chain. diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index cae4b04bf6..57c5168e9f 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -205,7 +205,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock } auto last = SteadyClock::now(); - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + LogPrintf("Dumped mempool: %.3fs to copy, %.3fs to dump\n", Ticks<SecondsDouble>(mid - start), Ticks<SecondsDouble>(last - mid)); } catch (const std::exception& e) { diff --git a/src/net.cpp b/src/net.cpp index 0cc794c2c3..7c82f01d75 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -203,7 +203,7 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn) while (!s.eof()) { CService endpoint; s >> endpoint; - CAddress addr{endpoint, GetDesirableServiceFlags(NODE_NONE)}; + CAddress addr{endpoint, SeedsServiceFlags()}; addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - one_week, -one_week); LogPrint(BCLog::NET, "Added hardcoded seed: %s\n", addr.ToStringAddrPort()); vSeedsOut.push_back(addr); @@ -1827,7 +1827,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, RandAddEvent((uint32_t)id); } -bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type) +bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport = false) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); std::optional<int> max_connections; @@ -1860,7 +1860,7 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ CSemaphoreGrant grant(*semOutbound, true); if (!grant) return false; - OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/false); + OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport); return true; } @@ -2267,7 +2267,7 @@ void CConnman::ThreadDNSAddressSeed() AddAddrFetch(seed); } else { std::vector<CAddress> vAdd; - ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); + constexpr ServiceFlags requiredServiceBits{SeedsServiceFlags()}; std::string host = strprintf("x%x.%s", requiredServiceBits, seed); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { @@ -2638,7 +2638,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) const CAddress addr = m_anchors.back(); m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !g_reachable_nets.Contains(addr) || - !HasAllDesirableServiceFlags(addr.nServices) || + !m_msgproc->HasAllDesirableServiceFlags(addr.nServices) || outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToStringAddrPort()); @@ -2704,7 +2704,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // for non-feelers, require all the services we'll want, // for feelers, only require they be a full node (only because most // SPV clients don't have a good address DB available) - if (!fFeeler && !HasAllDesirableServiceFlags(addr.nServices)) { + if (!fFeeler && !m_msgproc->HasAllDesirableServiceFlags(addr.nServices)) { continue; } else if (fFeeler && !MayHaveUsefulAddressDB(addr.nServices)) { continue; @@ -97,7 +97,7 @@ static constexpr bool DEFAULT_FIXEDSEEDS{true}; static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000; static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000; -static constexpr bool DEFAULT_V2_TRANSPORT{false}; +static constexpr bool DEFAULT_V2_TRANSPORT{true}; typedef int64_t NodeId; @@ -1006,6 +1006,12 @@ public: virtual void FinalizeNode(const CNode& node) = 0; /** + * Callback to determine whether the given set of service flags are sufficient + * for a peer to be "relevant". + */ + virtual bool HasAllDesirableServiceFlags(ServiceFlags services) const = 0; + + /** * Process protocol messages received from a given node * * @param[in] pnode The node which we have received messages from. @@ -1189,13 +1195,14 @@ public: * @param[in] address Address of node to try connecting to * @param[in] conn_type ConnectionType::OUTBOUND, ConnectionType::BLOCK_RELAY, * ConnectionType::ADDR_FETCH or ConnectionType::FEELER + * @param[in] use_v2transport Set to true if node attempts to connect using BIP 324 v2 transport protocol. * @return bool Returns false if there are no available * slots for this connection: * - conn_type not a supported ConnectionType * - Max total outbound connection capacity filled * - Max connection capacity for type is filled */ - bool AddConnection(const std::string& address, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); size_t GetNodeCount(ConnectionDirection) const; uint32_t GetMappedAS(const CNetAddr& addr) const; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 80cf610a0d..c8da927763 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -41,12 +41,12 @@ #include <txrequest.h> #include <util/check.h> #include <util/strencodings.h> +#include <util/time.h> #include <util/trace.h> #include <validation.h> #include <algorithm> #include <atomic> -#include <chrono> #include <future> #include <memory> #include <optional> @@ -133,6 +133,8 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; static const int MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10; /** Minimum blocks required to signal NODE_NETWORK_LIMITED */ static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static const unsigned int NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; /** Average delay between local address broadcasts */ static constexpr auto AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24h}; /** Average delay between peer address broadcasts */ @@ -499,6 +501,7 @@ public: /** Implement NetEventsInterface */ void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); + bool HasAllDesirableServiceFlags(ServiceFlags services) const override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); bool SendMessages(CNode* pto) override @@ -513,12 +516,17 @@ public: bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void SetBestHeight(int height) override { m_best_height = height; }; + void SetBestBlock(int height, std::chrono::seconds time) override + { + m_best_height = height; + m_best_block_time = time; + }; void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; + ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const override; private: /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ @@ -721,6 +729,8 @@ private: /** The height of the best chain */ std::atomic<int> m_best_height{-1}; + /** The time of the best chain tip block */ + std::atomic<std::chrono::seconds> m_best_block_time{0s}; /** Next time to check for stale tip */ std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; @@ -993,6 +1003,12 @@ private: bool CanDirectFetch() EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** + * Estimates the distance, in blocks, between the best-known block and the network chain tip. + * Utilizes the best-block time and the chainparams blocks spacing to approximate it. + */ + int64_t ApproximateBestBlockDepth() const; + + /** * To prevent fingerprinting attacks, only send blocks/headers outside of * the active chain if they are no more than a month older (both in time, * and in best equivalent proof of work) than the best header chain we know @@ -1311,9 +1327,14 @@ bool PeerManagerImpl::TipMayBeStale() return m_last_tip_update.load() < GetTime<std::chrono::seconds>() - std::chrono::seconds{consensusParams.nPowTargetSpacing * 3} && mapBlocksInFlight.empty(); } +int64_t PeerManagerImpl::ApproximateBestBlockDepth() const +{ + return (GetTime<std::chrono::seconds>() - m_best_block_time.load()).count() / m_chainparams.GetConsensus().nPowTargetSpacing; +} + bool PeerManagerImpl::CanDirectFetch() { - return m_chainman.ActiveChain().Tip()->Time() > GetAdjustedTime() - m_chainparams.GetConsensus().PowTargetSpacing() * 20; + return m_chainman.ActiveChain().Tip()->Time() > NodeClock::now() - m_chainparams.GetConsensus().PowTargetSpacing() * 20; } static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -1651,6 +1672,23 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } +bool PeerManagerImpl::HasAllDesirableServiceFlags(ServiceFlags services) const +{ + // Shortcut for (services & GetDesirableServiceFlags(services)) == GetDesirableServiceFlags(services) + return !(GetDesirableServiceFlags(services) & (~services)); +} + +ServiceFlags PeerManagerImpl::GetDesirableServiceFlags(ServiceFlags services) const +{ + if (services & NODE_NETWORK_LIMITED) { + // Limited peers are desirable when we are close to the tip. + if (ApproximateBestBlockDepth() < NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS) { + return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); + } + } + return ServiceFlags(NODE_NETWORK | NODE_WITNESS); +} + PeerRef PeerManagerImpl::GetPeerRef(NodeId id) const { LOCK(m_peer_mutex); @@ -2047,8 +2085,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha */ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - SetBestHeight(pindexNew->nHeight); - SetServiceFlagsIBDCache(!fInitialDownload); + SetBestBlock(pindexNew->nHeight, std::chrono::seconds{pindexNew->GetBlockTime()}); // Don't relay inventory during initial block download. if (fInitialDownload) return; @@ -5555,7 +5592,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!state.fSyncStarted && CanServeBlocks(*peer) && !m_chainman.m_blockman.LoadingBlocks()) { // Only actively request headers from a single peer, unless we're close to today. - if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > GetAdjustedTime() - 24h) { + if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > NodeClock::now() - 24h) { const CBlockIndex* pindexStart = m_chainman.m_best_header; /* If possible, start at the block preceding the currently best known header. This ensures that we always get a @@ -5575,7 +5612,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling // to maintain precision std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} * - Ticks<std::chrono::seconds>(GetAdjustedTime() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing + Ticks<std::chrono::seconds>(NodeClock::now() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing ); nSyncStarted++; } @@ -5879,7 +5916,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && peer->m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (m_chainman.m_best_header->Time() <= GetAdjustedTime() - 24h) { + if (m_chainman.m_best_header->Time() <= NodeClock::now() - 24h) { if (current_time > peer->m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. diff --git a/src/net_processing.h b/src/net_processing.h index afdef00067..f8d7a8f511 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -92,8 +92,8 @@ public: /** Send ping message to all peers */ virtual void SendPings() = 0; - /** Set the best height */ - virtual void SetBestHeight(int height) = 0; + /** Set the height of the best block and its time (seconds since epoch). */ + virtual void SetBestBlock(int height, std::chrono::seconds time) = 0; /* Public for unit testing. */ virtual void UnitTestMisbehaving(NodeId peer_id, int howmuch) = 0; @@ -110,6 +110,29 @@ public: /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; + + /** + * Gets the set of service flags which are "desirable" for a given peer. + * + * These are the flags which are required for a peer to support for them + * to be "interesting" to us, ie for us to wish to use one of our few + * outbound connection slots for or for us to wish to prioritize keeping + * their connection around. + * + * Relevant service flags may be peer- and state-specific in that the + * version of the peer may determine which flags are required (eg in the + * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers + * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which + * case NODE_NETWORK_LIMITED suffices). + * + * Thus, generally, avoid calling with 'services' == NODE_NONE, unless + * state-specific flags must absolutely be avoided. When called with + * 'services' == NODE_NONE, the returned desirable service flags are + * guaranteed to not change dependent on state - ie they are suitable for + * use when describing peers which we know to be desirable, but for which + * we do not have a confirmed set of service flags. + */ + virtual ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const = 0; }; #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/netaddress.h b/src/netaddress.h index 08dd77c0ff..0bbde43dd7 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -261,6 +261,18 @@ public: } } + /** + * BIP155 network ids recognized by this software. + */ + enum BIP155Network : uint8_t { + IPV4 = 1, + IPV6 = 2, + TORV2 = 3, + TORV3 = 4, + I2P = 5, + CJDNS = 6, + }; + friend class CSubNet; private: @@ -283,18 +295,6 @@ private: bool SetI2P(const std::string& addr); /** - * BIP155 network ids recognized by this software. - */ - enum BIP155Network : uint8_t { - IPV4 = 1, - IPV6 = 2, - TORV2 = 3, - TORV3 = 4, - I2P = 5, - CJDNS = 6, - }; - - /** * Size of CNetAddr when serialized as ADDRv1 (pre-BIP155) (in bytes). */ static constexpr size_t V1_SERIALIZATION_SIZE = ADDR_IPV6_SIZE; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index e6164c2e59..c499bbfa6a 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -307,6 +307,7 @@ void BlockManager::FindFilesToPrune( // Distribute our -prune budget over all chainstates. const auto target = std::max( MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / chainman.GetAll().size()); + const uint64_t target_sync_height = chainman.m_best_header->nHeight; if (chain.m_chain.Height() < 0 || target == 0) { return; @@ -329,10 +330,13 @@ void BlockManager::FindFilesToPrune( // On a prune event, the chainstate DB is flushed. // To avoid excessive prune events negating the benefit of high dbcache // values, we should not prune too rapidly. - // So when pruning in IBD, increase the buffer a bit to avoid a re-prune too soon. - if (chainman.IsInitialBlockDownload()) { - // Since this is only relevant during IBD, we use a fixed 10% - nBuffer += target / 10; + // So when pruning in IBD, increase the buffer to avoid a re-prune too soon. + const auto chain_tip_height = chain.m_chain.Height(); + if (chainman.IsInitialBlockDownload() && target_sync_height > (uint64_t)chain_tip_height) { + // Since this is only relevant during IBD, we assume blocks are at least 1 MB on average + static constexpr uint64_t average_block_size = 1000000; /* 1 MB */ + const uint64_t remaining_blocks = target_sync_height - chain_tip_height; + nBuffer += average_block_size * remaining_blocks; } for (int fileNumber = 0; fileNumber < this->MaxBlockfileNum(); fileNumber++) { diff --git a/src/node/miner.cpp b/src/node/miner.cpp index ce5452d1f9..87f40e993f 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -20,8 +20,8 @@ #include <policy/policy.h> #include <pow.h> #include <primitives/transaction.h> -#include <timedata.h> #include <util/moneystr.h> +#include <util/time.h> #include <validation.h> #include <algorithm> @@ -31,7 +31,7 @@ namespace node { int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) { int64_t nOldTime = pblock->nTime; - int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()))}; + int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))}; if (nOldTime < nNewTime) { pblock->nTime = nNewTime; @@ -133,7 +133,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc pblock->nVersion = gArgs.GetIntArg("-blockversion", pblock->nVersion); } - pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()); + pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()); m_lock_time_cutoff = pindexPrev->GetMedianTimePast(); int nPackagesSelected = 0; @@ -171,7 +171,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc BlockValidationState state; if (m_options.test_block_validity && !TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev, - GetAdjustedTime, /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { + /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString())); } const auto time_2{SteadyClock::now()}; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index e8ab2326c1..ef9a30a076 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -32,7 +32,7 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback) { - // BroadcastTransaction can be called by either sendrawtransaction RPC or the wallet. + // BroadcastTransaction can be called by RPC or by the wallet. // chainman, mempool and peerman are initialized before the RPC server and wallet are started // and reset after the RPC sever and wallet are stopped. assert(node.chainman); diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 5440548636..5f1d15c5f2 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -1055,7 +1055,7 @@ void CBlockPolicyEstimator::FlushUnconfirmed() _removeTx(mi->first, false); // this calls erase() on mapMemPoolTxs } const auto endclear{SteadyClock::now()}; - LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear)); + LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %.3fs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear)); } std::chrono::hours CBlockPolicyEstimator::GetFeeEstimatorFileAge() diff --git a/src/prevector.h b/src/prevector.h index bcab1ff00c..4776db789b 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -47,26 +47,28 @@ public: typedef const value_type* const_pointer; class iterator { - T* ptr; + T* ptr{}; public: typedef Diff difference_type; typedef T value_type; typedef T* pointer; typedef T& reference; - typedef std::random_access_iterator_tag iterator_category; + using element_type = T; + using iterator_category = std::contiguous_iterator_tag; + iterator() = default; iterator(T* ptr_) : ptr(ptr_) {} T& operator*() const { return *ptr; } T* operator->() const { return ptr; } - T& operator[](size_type pos) { return ptr[pos]; } - const T& operator[](size_type pos) const { return ptr[pos]; } + T& operator[](size_type pos) const { return ptr[pos]; } iterator& operator++() { ptr++; return *this; } iterator& operator--() { ptr--; return *this; } iterator operator++(int) { iterator copy(*this); ++(*this); return copy; } iterator operator--(int) { iterator copy(*this); --(*this); return copy; } difference_type friend operator-(iterator a, iterator b) { return (&(*a) - &(*b)); } - iterator operator+(size_type n) { return iterator(ptr + n); } + iterator operator+(size_type n) const { return iterator(ptr + n); } + iterator friend operator+(size_type n, iterator x) { return x + n; } iterator& operator+=(size_type n) { ptr += n; return *this; } - iterator operator-(size_type n) { return iterator(ptr - n); } + iterator operator-(size_type n) const { return iterator(ptr - n); } iterator& operator-=(size_type n) { ptr -= n; return *this; } bool operator==(iterator x) const { return ptr == x.ptr; } bool operator!=(iterator x) const { return ptr != x.ptr; } @@ -77,18 +79,17 @@ public: }; class reverse_iterator { - T* ptr; + T* ptr{}; public: typedef Diff difference_type; typedef T value_type; typedef T* pointer; typedef T& reference; typedef std::bidirectional_iterator_tag iterator_category; + reverse_iterator() = default; reverse_iterator(T* ptr_) : ptr(ptr_) {} - T& operator*() { return *ptr; } - const T& operator*() const { return *ptr; } - T* operator->() { return ptr; } - const T* operator->() const { return ptr; } + T& operator*() const { return *ptr; } + T* operator->() const { return ptr; } reverse_iterator& operator--() { ptr++; return *this; } reverse_iterator& operator++() { ptr--; return *this; } reverse_iterator operator++(int) { reverse_iterator copy(*this); ++(*this); return copy; } @@ -98,13 +99,15 @@ public: }; class const_iterator { - const T* ptr; + const T* ptr{}; public: typedef Diff difference_type; typedef const T value_type; typedef const T* pointer; typedef const T& reference; - typedef std::random_access_iterator_tag iterator_category; + using element_type = const T; + using iterator_category = std::contiguous_iterator_tag; + const_iterator() = default; const_iterator(const T* ptr_) : ptr(ptr_) {} const_iterator(iterator x) : ptr(&(*x)) {} const T& operator*() const { return *ptr; } @@ -115,9 +118,10 @@ public: const_iterator operator++(int) { const_iterator copy(*this); ++(*this); return copy; } const_iterator operator--(int) { const_iterator copy(*this); --(*this); return copy; } difference_type friend operator-(const_iterator a, const_iterator b) { return (&(*a) - &(*b)); } - const_iterator operator+(size_type n) { return const_iterator(ptr + n); } + const_iterator operator+(size_type n) const { return const_iterator(ptr + n); } + const_iterator friend operator+(size_type n, const_iterator x) { return x + n; } const_iterator& operator+=(size_type n) { ptr += n; return *this; } - const_iterator operator-(size_type n) { return const_iterator(ptr - n); } + const_iterator operator-(size_type n) const { return const_iterator(ptr - n); } const_iterator& operator-=(size_type n) { ptr -= n; return *this; } bool operator==(const_iterator x) const { return ptr == x.ptr; } bool operator!=(const_iterator x) const { return ptr != x.ptr; } @@ -128,13 +132,14 @@ public: }; class const_reverse_iterator { - const T* ptr; + const T* ptr{}; public: typedef Diff difference_type; typedef const T value_type; typedef const T* pointer; typedef const T& reference; typedef std::bidirectional_iterator_tag iterator_category; + const_reverse_iterator() = default; const_reverse_iterator(const T* ptr_) : ptr(ptr_) {} const_reverse_iterator(reverse_iterator x) : ptr(&(*x)) {} const T& operator*() const { return *ptr; } diff --git a/src/protocol.cpp b/src/protocol.cpp index 27a0a2ffc1..0da160768d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -9,8 +9,6 @@ #include <atomic> -static std::atomic<bool> g_initial_block_download_completed(false); - namespace NetMsgType { const char* VERSION = "version"; const char* VERACK = "verack"; @@ -125,18 +123,6 @@ bool CMessageHeader::IsCommandValid() const return true; } - -ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { - if ((services & NODE_NETWORK_LIMITED) && g_initial_block_download_completed) { - return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); - } - return ServiceFlags(NODE_NETWORK | NODE_WITNESS); -} - -void SetServiceFlagsIBDCache(bool state) { - g_initial_block_download_completed = state; -} - CInv::CInv() { type = 0; diff --git a/src/protocol.h b/src/protocol.h index e405253632..243cd23e6e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -311,43 +311,12 @@ enum ServiceFlags : uint64_t { std::vector<std::string> serviceFlagsToStr(uint64_t flags); /** - * Gets the set of service flags which are "desirable" for a given peer. - * - * These are the flags which are required for a peer to support for them - * to be "interesting" to us, ie for us to wish to use one of our few - * outbound connection slots for or for us to wish to prioritize keeping - * their connection around. - * - * Relevant service flags may be peer- and state-specific in that the - * version of the peer may determine which flags are required (eg in the - * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers - * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which - * case NODE_NETWORK_LIMITED suffices). - * - * Thus, generally, avoid calling with peerServices == NODE_NONE, unless - * state-specific flags must absolutely be avoided. When called with - * peerServices == NODE_NONE, the returned desirable service flags are - * guaranteed to not change dependent on state - ie they are suitable for - * use when describing peers which we know to be desirable, but for which - * we do not have a confirmed set of service flags. - * - * If the NODE_NONE return value is changed, contrib/seeds/makeseeds.py - * should be updated appropriately to filter for the same nodes. - */ -ServiceFlags GetDesirableServiceFlags(ServiceFlags services); - -/** Set the current IBD status in order to figure out the desirable service flags */ -void SetServiceFlagsIBDCache(bool status); - -/** - * A shortcut for (services & GetDesirableServiceFlags(services)) - * == GetDesirableServiceFlags(services), ie determines whether the given - * set of service flags are sufficient for a peer to be "relevant". - */ -static inline bool HasAllDesirableServiceFlags(ServiceFlags services) -{ - return !(GetDesirableServiceFlags(services) & (~services)); -} + * State independent service flags. + * If the return value is changed, contrib/seeds/makeseeds.py + * should be updated appropriately to filter for nodes with + * desired service flags (compatible with our new flags). + */ +constexpr ServiceFlags SeedsServiceFlags() { return ServiceFlags(NODE_NETWORK | NODE_WITNESS); } /** * Checks if a peer with the given service flags may be capable of having a diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index e5a5179d87..7100603616 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -61,7 +61,11 @@ void OptionTests::migrateSettings() QVERIFY(!settings.contains("addrSeparateProxyTor")); std::ifstream file(gArgs.GetDataDirNet() / "settings.json"); + std::string default_warning = strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", + PACKAGE_NAME); QCOMPARE(std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n" + " \"_warning_\": \""+ default_warning+"\",\n" " \"dbcache\": \"600\",\n" " \"listen\": false,\n" " \"onion\": \"onion:234\",\n" diff --git a/src/qt/winshutdownmonitor.h b/src/qt/winshutdownmonitor.h index 78f287637f..060d8546e3 100644 --- a/src/qt/winshutdownmonitor.h +++ b/src/qt/winshutdownmonitor.h @@ -10,7 +10,7 @@ #include <QString> #include <functional> -#include <windef.h> // for HWND +#include <windows.h> #include <QAbstractNativeEventFilter> diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index b0771a75fc..5825efdf82 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -303,6 +303,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmsgtopeer", 0, "peer_id" }, { "stop", 0, "wait" }, { "addnode", 2, "v2transport" }, + { "addconnection", 2, "v2transport" }, }; // clang-format on diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 49b51b83ae..f1abfb6396 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -27,11 +27,11 @@ #include <script/descriptor.h> #include <script/script.h> #include <script/signingprovider.h> -#include <timedata.h> #include <txmempool.h> #include <univalue.h> #include <util/strencodings.h> #include <util/string.h> +#include <util/time.h> #include <util/translation.h> #include <validation.h> #include <validationinterface.h> @@ -383,7 +383,7 @@ static RPCHelpMan generateblock() LOCK(cs_main); BlockValidationState state; - if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), GetAdjustedTime, false, false)) { + if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString())); } } @@ -697,7 +697,7 @@ static RPCHelpMan getblocktemplate() if (block.hashPrevBlock != pindexPrev->GetBlockHash()) return "inconclusive-not-best-prevblk"; BlockValidationState state; - TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, GetAdjustedTime, false, true); + TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, false, true); return BIP22ValidationResult(state); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e53f889897..5e6f42b596 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -371,6 +371,7 @@ static RPCHelpMan addconnection() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."}, {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."}, + {"v2transport", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Attempt to connect using BIP324 v2 transport protocol"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -379,8 +380,8 @@ static RPCHelpMan addconnection() { RPCResult::Type::STR, "connection_type", "Type of connection opened." }, }}, RPCExamples{ - HelpExampleCli("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\"") - + HelpExampleRpc("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\"") + HelpExampleCli("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\" true") + + HelpExampleRpc("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\" true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -402,11 +403,16 @@ static RPCHelpMan addconnection() } else { throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString()); } + bool use_v2transport = self.Arg<bool>(2); NodeContext& node = EnsureAnyNodeContext(request.context); CConnman& connman = EnsureConnman(node); - const bool success = connman.AddConnection(address, conn_type); + if (use_v2transport && !(connman.GetLocalServices() & NODE_P2P_V2)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Adding v2transport connections requires -v2transport init flag to be set."); + } + + const bool success = connman.AddConnection(address, conn_type, use_v2transport); if (!success) { throw JSONRPCError(RPC_CLIENT_NODE_CAPACITY_REACHED, "Error: Already at capacity for specified connection type."); } diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index c471986a44..a9e11622a7 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -70,7 +70,7 @@ void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, std::optio } } -void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) +UniValue NormalizeOutputs(const UniValue& outputs_in) { if (outputs_in.isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null"); @@ -94,11 +94,15 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) } outputs = std::move(outputs_dict); } + return outputs; +} +std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs) +{ // Duplicate checking std::set<CTxDestination> destinations; + std::vector<std::pair<CTxDestination, CAmount>> parsed_outputs; bool has_data{false}; - for (const std::string& name_ : outputs.getKeys()) { if (name_ == "data") { if (has_data) { @@ -106,11 +110,12 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) } has_data = true; std::vector<unsigned char> data = ParseHexV(outputs[name_].getValStr(), "Data"); - - CTxOut out(0, CScript() << OP_RETURN << data); - rawTx.vout.push_back(out); + CTxDestination destination{CNoDestination{CScript() << OP_RETURN << data}}; + CAmount amount{0}; + parsed_outputs.emplace_back(destination, amount); } else { - CTxDestination destination = DecodeDestination(name_); + CTxDestination destination{DecodeDestination(name_)}; + CAmount amount{AmountFromValue(outputs[name_])}; if (!IsValidDestination(destination)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + name_); } @@ -118,13 +123,23 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) if (!destinations.insert(destination).second) { throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_); } + parsed_outputs.emplace_back(destination, amount); + } + } + return parsed_outputs; +} + +void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) +{ + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(outputs_in); - CScript scriptPubKey = GetScriptForDestination(destination); - CAmount nAmount = AmountFromValue(outputs[name_]); + std::vector<std::pair<CTxDestination, CAmount>> parsed_outputs = ParseOutputs(outputs); + for (const auto& [destination, nAmount] : parsed_outputs) { + CScript scriptPubKey = GetScriptForDestination(destination); - CTxOut out(nAmount, scriptPubKey); - rawTx.vout.push_back(out); - } + CTxOut out(nAmount, scriptPubKey); + rawTx.vout.push_back(out); } } diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index a863432b7a..964d0b095b 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_RPC_RAWTRANSACTION_UTIL_H #define BITCOIN_RPC_RAWTRANSACTION_UTIL_H +#include <addresstype.h> +#include <consensus/amount.h> #include <map> #include <string> #include <optional> @@ -38,11 +40,16 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const */ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins); - /** Normalize univalue-represented inputs and add them to the transaction */ void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, bool rbf); -/** Normalize univalue-represented outputs and add them to the transaction */ +/** Normalize univalue-represented outputs */ +UniValue NormalizeOutputs(const UniValue& outputs_in); + +/** Parse normalized outputs into destination, amount tuples */ +std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs); + +/** Normalize, parse, and add outputs to the transaction */ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in); /** Create a transaction from univalue parameters */ diff --git a/src/test/.gitignore b/src/test/.gitignore new file mode 100644 index 0000000000..036df1430c --- /dev/null +++ b/src/test/.gitignore @@ -0,0 +1,2 @@ +# capnp generated files +*.capnp.* diff --git a/src/test/data/tx_valid.json b/src/test/data/tx_valid.json index b874f6f26c..70df0d0f69 100644 --- a/src/test/data/tx_valid.json +++ b/src/test/data/tx_valid.json @@ -319,6 +319,10 @@ [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "HASH160 0x14 0x7c17aff532f22beb54069942f9bf567a66133eaf EQUAL"]], "0200000001000100000000000000000000000000000000000000000000000000000000000000000000030251b2010000000100000000000000000000000000", "NONE"], +["Valid CHECKSEQUENCEVERIFY even with negative tx version number"], +[[["0000000000000000000000000000000000000000000000000000000000000100", 0, "HASH160 0x14 0x7c17aff532f22beb54069942f9bf567a66133eaf EQUAL"]], +"ffffffff01000100000000000000000000000000000000000000000000000000000000000000000000030251b2010000000100000000000000000000000000", "NONE"], + ["Valid P2WPKH (Private key of segwit tests is L5AQtV2HDm4xGsseLokK2VAT2EtYKcTm3c7HwqnJBFt9LdaQULsM)"], [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "0x00 0x14 0x4c9c3dfac4207d5d8cb89df5722cb3d712385e3f", 1000]], "0100000000010100010000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000001976a9144c9c3dfac4207d5d8cb89df5722cb3d712385e3f88ac02483045022100cfb07164b36ba64c1b1e8c7720a56ad64d96f6ef332d3d37f9cb3c96477dc44502200a464cd7a9cf94cd70f66ce4f4f0625ef650052c7afcfe29d7d7e01830ff91ed012103596d3451025c19dbbdeb932d6bf8bfb4ad499b95b6f88db8899efac102e5fc7100000000", "NONE"], diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index ece396aadf..8a54cc656d 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -64,26 +64,13 @@ FUZZ_TARGET(data_stream_addr_man, .init = initialize_addrman) CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context) { CNetAddr addr; - if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { - addr = ConsumeNetAddr(fuzzed_data_provider); - } else { - // The networks [1..6] correspond to CNetAddr::BIP155Network (private). - static const std::map<uint8_t, uint8_t> net_len_map = {{1, ADDR_IPV4_SIZE}, - {2, ADDR_IPV6_SIZE}, - {4, ADDR_TORV3_SIZE}, - {5, ADDR_I2P_SIZE}, - {6, ADDR_CJDNS_SIZE}}; - uint8_t net = fast_random_context.randrange(5) + 1; // [1..5] - if (net == 3) { - net = 6; + assert(!addr.IsValid()); + for (size_t i = 0; i < 8 && !addr.IsValid(); ++i) { + if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(fuzzed_data_provider); + } else { + addr = ConsumeNetAddr(fuzzed_data_provider, &fast_random_context); } - - DataStream s{}; - - s << net; - s << fast_random_context.randbytes(net_len_map.at(net)); - - s >> CAddress::V2_NETWORK(addr); } // Return a dummy IPv4 5.5.5.5 if we generated an invalid address. diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 4a040c56de..b26151f63c 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -70,11 +70,13 @@ FUZZ_TARGET(banman, .init = initialize_banman) fuzzed_data_provider, [&] { CNetAddr net_addr{ConsumeNetAddr(fuzzed_data_provider)}; - const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)}; - if (addr.has_value() && addr->IsValid()) { - net_addr = *addr; - } else { - contains_invalid = true; + if (!net_addr.IsCJDNS() || !net_addr.IsValid()) { + const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)}; + if (addr.has_value() && addr->IsValid()) { + net_addr = *addr; + } else { + contains_invalid = true; + } } ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); }, diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 9b97958a5d..2577f9e97a 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -213,7 +213,6 @@ FUZZ_TARGET(integer, .init = initialize_integer) { const ServiceFlags service_flags = (ServiceFlags)u64; - (void)HasAllDesirableServiceFlags(service_flags); (void)MayHaveUsefulAddressDB(service_flags); } diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 5141d3362d..4803cdccad 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -26,6 +26,12 @@ FUZZ_TARGET(netaddress) if (net_addr.GetNetwork() == Network::NET_ONION) { assert(net_addr.IsTor()); } + if (net_addr.GetNetwork() == Network::NET_I2P) { + assert(net_addr.IsI2P()); + } + if (net_addr.GetNetwork() == Network::NET_CJDNS) { + assert(net_addr.IsCJDNS()); + } if (net_addr.GetNetwork() == Network::NET_INTERNAL) { assert(net_addr.IsInternal()); } @@ -69,6 +75,12 @@ FUZZ_TARGET(netaddress) if (net_addr.IsTor()) { assert(net_addr.GetNetwork() == Network::NET_ONION); } + if (net_addr.IsI2P()) { + assert(net_addr.GetNetwork() == Network::NET_I2P); + } + if (net_addr.IsCJDNS()) { + assert(net_addr.GetNetwork() == Network::NET_CJDNS); + } (void)net_addr.IsValid(); (void)net_addr.ToStringAddr(); diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp index eb0f14ede0..99151bb84d 100644 --- a/src/test/fuzz/util/net.cpp +++ b/src/test/fuzz/util/net.cpp @@ -25,33 +25,63 @@ class CNode; -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext* rand) noexcept { - const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); - CNetAddr net_addr; - if (network == Network::NET_IPV4) { - in_addr v4_addr = {}; - v4_addr.s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); - net_addr = CNetAddr{v4_addr}; - } else if (network == Network::NET_IPV6) { - if (fuzzed_data_provider.remaining_bytes() >= 16) { - in6_addr v6_addr = {}; - auto addr_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(16); - if (addr_bytes[0] == CJDNS_PREFIX) { // Avoid generating IPv6 addresses that look like CJDNS. - addr_bytes[0] = 0x55; // Just an arbitrary number, anything != CJDNS_PREFIX would do. - } - memcpy(v6_addr.s6_addr, addr_bytes.data(), 16); - net_addr = CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; + struct NetAux { + Network net; + CNetAddr::BIP155Network bip155; + size_t len; + }; + + static constexpr std::array<NetAux, 6> nets{ + NetAux{.net = Network::NET_IPV4, .bip155 = CNetAddr::BIP155Network::IPV4, .len = ADDR_IPV4_SIZE}, + NetAux{.net = Network::NET_IPV6, .bip155 = CNetAddr::BIP155Network::IPV6, .len = ADDR_IPV6_SIZE}, + NetAux{.net = Network::NET_ONION, .bip155 = CNetAddr::BIP155Network::TORV3, .len = ADDR_TORV3_SIZE}, + NetAux{.net = Network::NET_I2P, .bip155 = CNetAddr::BIP155Network::I2P, .len = ADDR_I2P_SIZE}, + NetAux{.net = Network::NET_CJDNS, .bip155 = CNetAddr::BIP155Network::CJDNS, .len = ADDR_CJDNS_SIZE}, + NetAux{.net = Network::NET_INTERNAL, .bip155 = CNetAddr::BIP155Network{0}, .len = 0}, + }; + + const size_t nets_index{rand == nullptr + ? fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, nets.size() - 1) + : static_cast<size_t>(rand->randrange(nets.size()))}; + + const auto& aux = nets[nets_index]; + + CNetAddr addr; + + if (aux.net == Network::NET_INTERNAL) { + if (rand == nullptr) { + addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); + } else { + const auto v = rand->randbytes(32); + addr.SetInternal(std::string{v.begin(), v.end()}); } - } else if (network == Network::NET_INTERNAL) { - net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); - } else if (network == Network::NET_ONION) { - auto pub_key{fuzzed_data_provider.ConsumeBytes<uint8_t>(ADDR_TORV3_SIZE)}; - pub_key.resize(ADDR_TORV3_SIZE); - const bool ok{net_addr.SetSpecial(OnionToString(pub_key))}; - assert(ok); + return addr; + } + + DataStream s; + + s << static_cast<uint8_t>(aux.bip155); + + std::vector<uint8_t> addr_bytes; + if (rand == nullptr) { + addr_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(aux.len); + addr_bytes.resize(aux.len); + } else { + addr_bytes = rand->randbytes(aux.len); } - return net_addr; + if (aux.net == NET_IPV6 && addr_bytes[0] == CJDNS_PREFIX) { // Avoid generating IPv6 addresses that look like CJDNS. + addr_bytes[0] = 0x55; // Just an arbitrary number, anything != CJDNS_PREFIX would do. + } + if (aux.net == NET_CJDNS) { // Avoid generating CJDNS addresses that don't start with CJDNS_PREFIX because those are !IsValid(). + addr_bytes[0] = CJDNS_PREFIX; + } + s << addr_bytes; + + s >> CAddress::V2_NETWORK(addr); + + return addr; } CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index 47e4a2fac0..a6c9e23f2e 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -24,7 +24,15 @@ #include <optional> #include <string> -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept; +/** + * Create a CNetAddr. It may have `addr.IsValid() == false`. + * @param[in,out] fuzzed_data_provider Take data for the address from this, if `rand` is `nullptr`. + * @param[in,out] rand If not nullptr, take data from it instead of from `fuzzed_data_provider`. + * Prefer generating addresses using `fuzzed_data_provider` because it is not uniform. Only use + * `rand` if `fuzzed_data_provider` is exhausted or its data is needed for other things. + * @return a "random" network address. + */ +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext* rand = nullptr) noexcept; class FuzzedSock : public Sock { diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp new file mode 100644 index 0000000000..55a3dc2683 --- /dev/null +++ b/src/test/ipc_test.capnp @@ -0,0 +1,18 @@ +# Copyright (c) 2023 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +@0xd71b0fc8727fdf83; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("gen"); + +using Proxy = import "/mp/proxy.capnp"; +$Proxy.include("test/ipc_test.h"); +$Proxy.includeTypes("ipc/capnp/common-types.h"); + +interface FooInterface $Proxy.wrap("FooImplementation") { + add @0 (a :Int32, b :Int32) -> (result :Int32); + passOutPoint @1 (arg :Data) -> (result :Data); + passUniValue @2 (arg :Text) -> (result :Text); +} diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp new file mode 100644 index 0000000000..ce4edaceb0 --- /dev/null +++ b/src/test/ipc_test.cpp @@ -0,0 +1,67 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <logging.h> +#include <mp/proxy-types.h> +#include <test/ipc_test.capnp.h> +#include <test/ipc_test.capnp.proxy.h> +#include <test/ipc_test.h> + +#include <future> +#include <kj/common.h> +#include <kj/memory.h> +#include <kj/test.h> + +#include <boost/test/unit_test.hpp> + +//! Unit test that tests execution of IPC calls without actually creating a +//! separate process. This test is primarily intended to verify behavior of type +//! conversion code that converts C++ objects to Cap'n Proto messages and vice +//! versa. +//! +//! The test creates a thread which creates a FooImplementation object (defined +//! in ipc_test.h) and a two-way pipe accepting IPC requests which call methods +//! on the object through FooInterface (defined in ipc_test.capnp). +void IpcTest() +{ + // Setup: create FooImplemention object and listen for FooInterface requests + std::promise<std::unique_ptr<mp::ProxyClient<gen::FooInterface>>> foo_promise; + std::function<void()> disconnect_client; + std::thread thread([&]() { + mp::EventLoop loop("IpcTest", [](bool raise, const std::string& log) { LogPrintf("LOG%i: %s\n", raise, log); }); + auto pipe = loop.m_io_context.provider->newTwoWayPipe(); + + auto connection_client = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[0])); + auto foo_client = std::make_unique<mp::ProxyClient<gen::FooInterface>>( + connection_client->m_rpc_system.bootstrap(mp::ServerVatId().vat_id).castAs<gen::FooInterface>(), + connection_client.get(), /* destroy_connection= */ false); + foo_promise.set_value(std::move(foo_client)); + disconnect_client = [&] { loop.sync([&] { connection_client.reset(); }); }; + + auto connection_server = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) { + auto foo_server = kj::heap<mp::ProxyServer<gen::FooInterface>>(std::make_shared<FooImplementation>(), connection); + return capnp::Capability::Client(kj::mv(foo_server)); + }); + connection_server->onDisconnect([&] { connection_server.reset(); }); + loop.loop(); + }); + std::unique_ptr<mp::ProxyClient<gen::FooInterface>> foo{foo_promise.get_future().get()}; + + // Test: make sure arguments were sent and return value is received + BOOST_CHECK_EQUAL(foo->add(1, 2), 3); + + COutPoint txout1{Txid::FromUint256(uint256{100}), 200}; + COutPoint txout2{foo->passOutPoint(txout1)}; + BOOST_CHECK(txout1 == txout2); + + UniValue uni1{UniValue::VOBJ}; + uni1.pushKV("i", 1); + uni1.pushKV("s", "two"); + UniValue uni2{foo->passUniValue(uni1)}; + BOOST_CHECK_EQUAL(uni1.write(), uni2.write()); + + // Test cleanup: disconnect pipe and join thread + disconnect_client(); + thread.join(); +} diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h new file mode 100644 index 0000000000..bcfcc2125c --- /dev/null +++ b/src/test/ipc_test.h @@ -0,0 +1,21 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_IPC_TEST_H +#define BITCOIN_TEST_IPC_TEST_H + +#include <primitives/transaction.h> +#include <univalue.h> + +class FooImplementation +{ +public: + int add(int a, int b) { return a + b; } + COutPoint passOutPoint(COutPoint o) { return o; } + UniValue passUniValue(UniValue v) { return v; } +}; + +void IpcTest(); + +#endif // BITCOIN_TEST_IPC_TEST_H diff --git a/src/test/ipc_tests.cpp b/src/test/ipc_tests.cpp new file mode 100644 index 0000000000..6e144b0f41 --- /dev/null +++ b/src/test/ipc_tests.cpp @@ -0,0 +1,13 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <test/ipc_test.h> +#include <boost/test/unit_test.hpp> + +BOOST_AUTO_TEST_SUITE(ipc_tests) +BOOST_AUTO_TEST_CASE(ipc_tests) +{ + IpcTest(); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 342d2514ed..d50af4c175 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -12,7 +12,6 @@ #include <policy/policy.h> #include <test/util/random.h> #include <test/util/txmempool.h> -#include <timedata.h> #include <txmempool.h> #include <uint256.h> #include <util/strencodings.h> diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp new file mode 100644 index 0000000000..2c79329385 --- /dev/null +++ b/src/test/peerman_tests.cpp @@ -0,0 +1,76 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include <chainparams.h> +#include <node/miner.h> +#include <net_processing.h> +#include <pow.h> +#include <test/util/setup_common.h> +#include <validation.h> + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(peerman_tests, RegTestingSetup) + +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static constexpr int64_t NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; + +static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_time) +{ + auto curr_time = GetTime<std::chrono::seconds>(); + SetMockTime(block_time); // update time so the block is created with it + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr}.CreateNewBlock(CScript() << OP_TRUE)->block; + while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; + block.fChecked = true; // little speedup + SetMockTime(curr_time); // process block at current time + Assert(node.chainman->ProcessNewBlock(std::make_shared<const CBlock>(block), /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)); + SyncWithValidationInterfaceQueue(); // drain events queue +} + +// Verifying when network-limited peer connections are desirable based on the node's proximity to the tip +BOOST_AUTO_TEST_CASE(connections_desirable_service_flags) +{ + std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto consensus = m_node.chainman->GetParams().GetConsensus(); + + // Check we start connecting to full nodes + ServiceFlags peer_flags{NODE_WITNESS | NODE_NETWORK_LIMITED}; + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Make peerman aware of the initial best block and verify we accept limited peers when we start close to the tip time. + auto tip = WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Tip()); + uint64_t tip_block_time = tip->GetBlockTime(); + int tip_block_height = tip->nHeight; + peerman->SetBestBlock(tip_block_height, std::chrono::seconds{tip_block_time}); + + SetMockTime(tip_block_time + 1); // Set node time to tip time + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we don't disallow limited peers connections when we are behind but still recoverable (below the connection safety window) + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS - 1)}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we disallow limited peers connections when we are further than the limited peers safety window + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * 2}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // By now, we tested that the connections desirable services flags change based on the node's time proximity to the tip. + // Now, perform the same tests for when the node receives a block. + RegisterValidationInterface(peerman.get()); + + // First, verify a block in the past doesn't enable limited peers connections + // At this point, our time is (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1) * 10 minutes ahead the tip's time. + mineBlock(m_node, /*block_time=*/std::chrono::seconds{tip_block_time + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Verify a block close to the tip enables limited peers connections + mineBlock(m_node, /*block_time=*/GetTime<std::chrono::seconds>()); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Lastly, verify the stale tip checks can disallow limited peers connections after not receiving blocks for a prolonged period. + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index fa8ceb8dd6..41190b3579 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -99,7 +99,9 @@ BOOST_AUTO_TEST_CASE(ReadWrite) // Check invalid json not allowed WriteText(path, R"(invalid json)"); BOOST_CHECK(!common::ReadSettings(path, values, errors)); - std::vector<std::string> fail_parse = {strprintf("Unable to parse settings file %s", fs::PathToString(path))}; + std::vector<std::string> fail_parse = {strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end()); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 8789e86196..9f587d7ec0 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -44,7 +44,6 @@ #include <test/util/net.h> #include <test/util/random.h> #include <test/util/txmempool.h> -#include <timedata.h> #include <txdb.h> #include <txmempool.h> #include <util/chaintype.h> @@ -184,7 +183,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, .datadir = m_args.GetDataDirNet(), - .adjusted_time_callback = GetAdjustedTime, .check_block_index = true, .notifications = *m_node.notifications, .worker_threads_num = 2, diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 368ba8bee4..a33e71d50e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -15,7 +15,6 @@ #include <test/util/random.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <timedata.h> #include <uint256.h> #include <validation.h> #include <validationinterface.h> @@ -383,7 +382,6 @@ struct SnapshotTestSetup : TestChain100Setup { const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, - .adjusted_time_callback = GetAdjustedTime, .notifications = *m_node.notifications, }; const BlockManager::Options blockman_opts{ diff --git a/src/timedata.cpp b/src/timedata.cpp index 15ca90ee6a..d948de976f 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -33,11 +33,6 @@ int64_t GetTimeOffset() return nTimeOffset; } -NodeClock::time_point GetAdjustedTime() -{ - return NodeClock::now() + std::chrono::seconds{GetTimeOffset()}; -} - #define BITCOIN_TIMEDATA_MAX_SAMPLES 200 static std::set<CNetAddr> g_sources; diff --git a/src/timedata.h b/src/timedata.h index c6c36d9a39..3e76f80452 100644 --- a/src/timedata.h +++ b/src/timedata.h @@ -5,11 +5,8 @@ #ifndef BITCOIN_TIMEDATA_H #define BITCOIN_TIMEDATA_H -#include <util/time.h> - #include <algorithm> #include <cassert> -#include <chrono> #include <cstdint> #include <vector> @@ -75,11 +72,10 @@ public: /** Functions to keep track of adjusted P2P time */ int64_t GetTimeOffset(); -NodeClock::time_point GetAdjustedTime(); void AddTimeData(const CNetAddr& ip, int64_t nTime); /** - * Reset the internal state of GetTimeOffset(), GetAdjustedTime() and AddTimeData(). + * Reset the internal state of GetTimeOffset() and AddTimeData(). */ void TestOnlyResetTimeData(); diff --git a/src/validation.cpp b/src/validation.cpp index 8c583c586c..caa4ff3115 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2188,7 +2188,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Also, currently the rule against blocks more than 2 hours in the future // is enforced in ContextualCheckBlockHeader(); we wouldn't want to // re-enforce that rule here (at least until we make it impossible for - // m_adjusted_time_callback() to go backward). + // the clock to go backward). if (!CheckBlock(block, state, params.GetConsensus(), !fJustCheck, !fJustCheck)) { if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) { // We don't write down blocks to disk if they may have been @@ -3777,7 +3777,7 @@ arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers) * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, NodeClock::time_point now) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); assert(pindexPrev != nullptr); @@ -3805,7 +3805,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", "block's timestamp is too early"); // Check timestamp - if (block.Time() > now + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) { + if (block.Time() > NodeClock::now() + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) { return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", "block timestamp too far in the future"); } @@ -3945,7 +3945,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "header %s has prev block invalid: %s\n", hash.ToString(), block.hashPrevBlock.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } - if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, m_options.adjusted_time_callback())) { + if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev)) { LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -4230,7 +4230,6 @@ bool TestBlockValidity(BlockValidationState& state, Chainstate& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function<NodeClock::time_point()>& adjusted_time_callback, bool fCheckPOW, bool fCheckMerkleRoot) { @@ -4244,7 +4243,7 @@ bool TestBlockValidity(BlockValidationState& state, indexDummy.phashBlock = &block_hash; // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, adjusted_time_callback())) + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev)) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); @@ -4862,16 +4861,6 @@ void ChainstateManager::CheckBlockIndex() CBlockIndex* pindexFirstAssumeValid = nullptr; // Oldest ancestor of pindex which has BLOCK_ASSUMED_VALID while (pindex != nullptr) { nNodes++; - // Make sure nChainTx sum is correctly computed. - unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; - assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) - // For testing, allow transaction counts to be completely unset. - || (pindex->nChainTx == 0 && pindex->nTx == 0) - // For testing, allow this nChainTx to be unset if previous is also unset. - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) - // Transaction counts prior to snapshot are unknown. - || pindex->IsAssumedValid()); - if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) { @@ -4954,6 +4943,15 @@ void ChainstateManager::CheckBlockIndex() // Checks for not-invalid blocks. assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. } + // Make sure nChainTx sum is correctly computed. + unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; + assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) + // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. + || (pindex->nChainTx == 0 && pindex->nTx == 0) + // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) + || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) + // Transaction counts prior to snapshot are unknown. + || pindex->IsAssumedValid()); // Chainstate-specific checks on setBlockIndexCandidates for (auto c : GetAll()) { if (c->m_chain.Tip() == nullptr) continue; @@ -5781,7 +5779,6 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) if (!opts.check_block_index.has_value()) opts.check_block_index = opts.chainparams.DefaultConsistencyChecks(); if (!opts.minimum_chain_work.has_value()) opts.minimum_chain_work = UintToArith256(opts.chainparams.GetConsensus().nMinimumChainWork); if (!opts.assumed_valid_block.has_value()) opts.assumed_valid_block = opts.chainparams.GetConsensus().defaultAssumeValid; - Assert(opts.adjusted_time_callback); return std::move(opts); } diff --git a/src/validation.h b/src/validation.h index 093cecfcd1..fd9b53df8f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -377,7 +377,6 @@ bool TestBlockValidity(BlockValidationState& state, Chainstate& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function<NodeClock::time_point()>& adjusted_time_callback, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/wallet/db.h b/src/wallet/db.h index c263f54144..084fcadc24 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -20,7 +20,6 @@ class ArgsManager; struct bilingual_str; namespace wallet { -void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); class DatabaseCursor { diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 5a13b5ac8e..6060f017ce 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -24,34 +24,15 @@ namespace wallet { -static void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient>& recipients) +std::vector<CRecipient> CreateRecipients(const std::vector<std::pair<CTxDestination, CAmount>>& outputs, const std::set<int>& subtract_fee_outputs) { - std::set<CTxDestination> destinations; - int i = 0; - for (const std::string& address: address_amounts.getKeys()) { - CTxDestination dest = DecodeDestination(address); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + address); - } - - if (destinations.count(dest)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address); - } - destinations.insert(dest); - - CAmount amount = AmountFromValue(address_amounts[i++]); - - bool subtract_fee = false; - for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) { - const UniValue& addr = subtract_fee_outputs[idx]; - if (addr.get_str() == address) { - subtract_fee = true; - } - } - - CRecipient recipient = {dest, amount, subtract_fee}; + std::vector<CRecipient> recipients; + for (size_t i = 0; i < outputs.size(); ++i) { + const auto& [destination, amount] = outputs.at(i); + CRecipient recipient{destination, amount, subtract_fee_outputs.contains(i)}; recipients.push_back(recipient); } + return recipients; } static void InterpretFeeEstimationInstructions(const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, UniValue& options) @@ -76,6 +57,37 @@ static void InterpretFeeEstimationInstructions(const UniValue& conf_target, cons } } +std::set<int> InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_instructions, const std::vector<std::string>& destinations) +{ + std::set<int> sffo_set; + if (sffo_instructions.isNull()) return sffo_set; + if (sffo_instructions.isBool()) { + if (sffo_instructions.get_bool()) sffo_set.insert(0); + return sffo_set; + } + for (const auto& sffo : sffo_instructions.getValues()) { + if (sffo.isStr()) { + for (size_t i = 0; i < destinations.size(); ++i) { + if (sffo.get_str() == destinations.at(i)) { + sffo_set.insert(i); + break; + } + } + } + if (sffo.isNum()) { + int pos = sffo.getInt<int>(); + if (sffo_set.contains(pos)) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos)); + if (pos < 0) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos)); + if (pos >= int(destinations.size())) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos)); + sffo_set.insert(pos); + } + } + return sffo_set; +} + static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const UniValue& options, const CMutableTransaction& rawTx) { // Make a blank psbt @@ -275,11 +287,6 @@ RPCHelpMan sendtoaddress() if (!request.params[3].isNull() && !request.params[3].get_str().empty()) mapValue["to"] = request.params[3].get_str(); - bool fSubtractFeeFromAmount = false; - if (!request.params[4].isNull()) { - fSubtractFeeFromAmount = request.params[4].get_bool(); - } - CCoinControl coin_control; if (!request.params[5].isNull()) { coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); @@ -296,13 +303,10 @@ RPCHelpMan sendtoaddress() UniValue address_amounts(UniValue::VOBJ); const std::string address = request.params[0].get_str(); address_amounts.pushKV(address, request.params[1]); - UniValue subtractFeeFromAmount(UniValue::VARR); - if (fSubtractFeeFromAmount) { - subtractFeeFromAmount.push_back(address); - } - - std::vector<CRecipient> recipients; - ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(address_amounts), + InterpretSubtractFeeFromOutputInstructions(request.params[4], address_amounts.getKeys()) + ); const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose); @@ -386,10 +390,6 @@ RPCHelpMan sendmany() if (!request.params[3].isNull() && !request.params[3].get_str().empty()) mapValue["comment"] = request.params[3].get_str(); - UniValue subtractFeeFromAmount(UniValue::VARR); - if (!request.params[4].isNull()) - subtractFeeFromAmount = request.params[4].get_array(); - CCoinControl coin_control; if (!request.params[5].isNull()) { coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); @@ -397,8 +397,10 @@ RPCHelpMan sendmany() SetFeeEstimateMode(*pwallet, coin_control, /*conf_target=*/request.params[6], /*estimate_mode=*/request.params[7], /*fee_rate=*/request.params[8], /*override_min_fee=*/false); - std::vector<CRecipient> recipients; - ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(sendTo), + InterpretSubtractFeeFromOutputInstructions(request.params[4], sendTo.getKeys()) + ); const bool verbose{request.params[9].isNull() ? false : request.params[9].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, std::move(mapValue), verbose); @@ -488,17 +490,17 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true) return args; } -CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) +CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& recipients, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) { + // We want to make sure tx.vout is not used now that we are passing outputs as a vector of recipients. + // This sets us up to remove tx completely in a future PR in favor of passing the inputs directly. + CHECK_NONFATAL(tx.vout.empty()); // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); std::optional<unsigned int> change_position; bool lockUnspents = false; - UniValue subtractFeeFromOutputs; - std::set<int> setSubtractFeeFromOutputs; - if (!options.isNull()) { if (options.type() == UniValue::VBOOL) { // backward compatibility bool only fallback @@ -553,7 +555,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact if (options.exists("changePosition") || options.exists("change_position")) { int pos = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt<int>(); - if (pos < 0 || (unsigned int)pos > tx.vout.size()) { + if (pos < 0 || (unsigned int)pos > recipients.size()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds"); } change_position = (unsigned int)pos; @@ -595,9 +597,6 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact coinControl.fOverrideFeeRate = true; } - if (options.exists("subtractFeeFromOutputs") || options.exists("subtract_fee_from_outputs") ) - subtractFeeFromOutputs = (options.exists("subtract_fee_from_outputs") ? options["subtract_fee_from_outputs"] : options["subtractFeeFromOutputs"]).get_array(); - if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } @@ -703,21 +702,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact } } - if (tx.vout.size() == 0) + if (recipients.empty()) throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); - for (unsigned int idx = 0; idx < subtractFeeFromOutputs.size(); idx++) { - int pos = subtractFeeFromOutputs[idx].getInt<int>(); - if (setSubtractFeeFromOutputs.count(pos)) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos)); - if (pos < 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos)); - if (pos >= int(tx.vout.size())) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos)); - setSubtractFeeFromOutputs.insert(pos); - } - - auto txr = FundTransaction(wallet, tx, change_position, lockUnspents, setSubtractFeeFromOutputs, coinControl); + auto txr = FundTransaction(wallet, tx, recipients, change_position, lockUnspents, coinControl); if (!txr) { throw JSONRPCError(RPC_WALLET_ERROR, ErrorString(txr).original); } @@ -843,11 +831,25 @@ RPCHelpMan fundrawtransaction() if (!DecodeHexTx(tx, request.params[0].get_str(), try_no_witness, try_witness)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); } - + UniValue options = request.params[1]; + std::vector<std::pair<CTxDestination, CAmount>> destinations; + for (const auto& tx_out : tx.vout) { + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + destinations.emplace_back(dest, tx_out.nValue); + } + std::vector<std::string> dummy(destinations.size(), "dummy"); + std::vector<CRecipient> recipients = CreateRecipients( + destinations, + InterpretSubtractFeeFromOutputInstructions(options["subtractFeeFromOutputs"], dummy) + ); CCoinControl coin_control; // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_allow_other_inputs = true; - auto txr = FundTransaction(*pwallet, tx, request.params[1], coin_control, /*override_min_fee=*/true); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + tx.vout.clear(); + auto txr = FundTransaction(*pwallet, tx, recipients, options, coin_control, /*override_min_fee=*/true); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(*txr.tx)); @@ -1275,13 +1277,22 @@ RPCHelpMan send() bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(request.params[0]); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(outputs), + InterpretSubtractFeeFromOutputInstructions(options["subtract_fee_from_outputs"], outputs.getKeys()) + ); CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(options["inputs"], options); - auto txr = FundTransaction(*pwallet, rawTx, options, coin_control, /*override_min_fee=*/false); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + rawTx.vout.clear(); + auto txr = FundTransaction(*pwallet, rawTx, recipients, options, coin_control, /*override_min_fee=*/false); return FinishTransaction(pwallet, options, CMutableTransaction(*txr.tx)); } @@ -1711,12 +1722,21 @@ RPCHelpMan walletcreatefundedpsbt() const UniValue &replaceable_arg = options["replaceable"]; const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()}; CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(request.params[1]); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(outputs), + InterpretSubtractFeeFromOutputInstructions(options["subtractFeeFromOutputs"], outputs.getKeys()) + ); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(request.params[0], options); - auto txr = FundTransaction(wallet, rawTx, options, coin_control, /*override_min_fee=*/true); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + rawTx.vout.clear(); + auto txr = FundTransaction(wallet, rawTx, recipients, options, coin_control, /*override_min_fee=*/true); // Make a blank psbt PartiallySignedTransaction psbtx(CMutableTransaction(*txr.tx)); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 0e30d3e0fe..e6c021d426 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -415,8 +415,8 @@ static std::vector<RPCResult> TransactionDescriptionString() { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, }}, - {RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "The txid if this tx was replaced."}, - {RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "The txid if the tx replaces one."}, + {RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "Only if 'category' is 'send'. The txid if this tx was replaced."}, + {RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "Only if 'category' is 'send'. The txid if this tx replaces another."}, {RPCResult::Type::STR, "to", /*optional=*/true, "If a comment to is associated with the transaction."}, {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b7420d5fb7..e07771b17f 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -225,7 +225,7 @@ isminetype LegacyScriptPubKeyMan::IsMine(const CScript& script) const assert(false); } -bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys) +bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key) { { LOCK(cs_KeyStore); @@ -258,7 +258,7 @@ bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } - if (keyFail || (!keyPass && !accept_no_keys)) + if (keyFail || !keyPass) return false; fDecryptionThoroughlyChecked = true; } @@ -809,7 +809,9 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu std::vector<unsigned char> vchCryptedSecret; CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; - if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, vchSecret, pubkey.GetHash(), vchCryptedSecret); + })) { return false; } @@ -995,7 +997,9 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const { const CPubKey &vchPubKey = (*mi).second.first; const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second; - return DecryptKey(m_storage.GetEncryptionKey(), vchCryptedSecret, vchPubKey, keyOut); + return m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, vchCryptedSecret, vchPubKey, keyOut); + }); } return false; } @@ -2043,7 +2047,7 @@ isminetype DescriptorScriptPubKeyMan::IsMine(const CScript& script) const return ISMINE_NO; } -bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys) +bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key) { LOCK(cs_desc_man); if (!m_map_keys.empty()) { @@ -2068,7 +2072,7 @@ bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } - if (keyFail || (!keyPass && !accept_no_keys)) { + if (keyFail || !keyPass) { return false; } m_decryption_thoroughly_checked = true; @@ -2126,7 +2130,9 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const const CPubKey& pubkey = key_pair.second.first; const std::vector<unsigned char>& crypted_secret = key_pair.second.second; CKey key; - DecryptKey(m_storage.GetEncryptionKey(), crypted_secret, pubkey, key); + m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, crypted_secret, pubkey, key); + }); keys[pubkey.GetID()] = key; } return keys; @@ -2260,7 +2266,9 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const std::vector<unsigned char> crypted_secret; CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; - if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, secret, pubkey.GetHash(), crypted_secret); + })) { return false; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 449a75eb6b..1c99e5ffcd 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -22,6 +22,7 @@ #include <boost/signals2/signal.hpp> +#include <functional> #include <optional> #include <unordered_map> @@ -46,7 +47,8 @@ public: virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; virtual bool CanSupportFeature(enum WalletFeature) const = 0; virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr) = 0; - virtual const CKeyingMaterial& GetEncryptionKey() const = 0; + //! Pass the encryption key to cb(). + virtual bool WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const = 0; virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked() const = 0; }; @@ -177,7 +179,7 @@ public: virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. - virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } + virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } virtual util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; } @@ -377,7 +379,7 @@ public: util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; - bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; + bool CheckDecryptionKey(const CKeyingMaterial& master_key) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; @@ -608,7 +610,7 @@ public: util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; - bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; + bool CheckDecryptionKey(const CKeyingMaterial& master_key) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index b51cd6332f..e229962ca5 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1127,7 +1127,12 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( return util::Error{err.empty() ?_("Insufficient funds") : err}; } const SelectionResult& result = *select_coins_res; - TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result.GetAlgo()).c_str(), result.GetTarget(), result.GetWaste(), result.GetSelectedValue()); + TRACE5(coin_selection, selected_coins, + wallet.GetName().c_str(), + GetAlgorithmName(result.GetAlgo()).c_str(), + result.GetTarget(), + result.GetWaste(), + result.GetSelectedValue()); const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); if (change_amount > 0) { @@ -1336,8 +1341,11 @@ util::Result<CreatedTransactionResult> CreateTransaction( LOCK(wallet.cs_wallet); auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res), - res ? res->fee : 0, res && res->change_pos.has_value() ? *res->change_pos : 0); + TRACE4(coin_selection, normal_create_tx_internal, + wallet.GetName().c_str(), + bool(res), + res ? res->fee : 0, + res && res->change_pos.has_value() ? int32_t(*res->change_pos) : -1); if (!res) return res; const auto& txr_ungrouped = *res; // try with avoidpartialspends unless it's enabled already @@ -1354,8 +1362,12 @@ util::Result<CreatedTransactionResult> CreateTransaction( auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; - TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), - txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() && txr_grouped->change_pos.has_value() ? *txr_grouped->change_pos : 0); + TRACE5(coin_selection, aps_create_tx_internal, + wallet.GetName().c_str(), + use_aps, + txr_grouped.has_value(), + txr_grouped.has_value() ? txr_grouped->fee : 0, + txr_grouped.has_value() && txr_grouped->change_pos.has_value() ? int32_t(*txr_grouped->change_pos) : -1); if (txr_grouped) { wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); @@ -1365,18 +1377,11 @@ util::Result<CreatedTransactionResult> CreateTransaction( return res; } -util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl) +util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& vecSend, std::optional<unsigned int> change_pos, bool lockUnspents, CCoinControl coinControl) { - std::vector<CRecipient> vecSend; - - // Turn the txout set into a CRecipient vector. - for (size_t idx = 0; idx < tx.vout.size(); idx++) { - const CTxOut& txOut = tx.vout[idx]; - CTxDestination dest; - ExtractDestination(txOut.scriptPubKey, dest); - CRecipient recipient = {dest, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1}; - vecSend.push_back(recipient); - } + // We want to make sure tx.vout is not used now that we are passing outputs as a vector of recipients. + // This sets us up to remove tx completely in a future PR in favor of passing the inputs directly. + assert(tx.vout.empty()); // Set the user desired locktime coinControl.m_locktime = tx.nLockTime; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 3bd37cfd0e..62a7b4e4c8 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -224,7 +224,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const * Insert additional inputs into the transaction by * calling CreateTransaction(); */ -util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl); +util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& recipients, std::optional<unsigned int> change_pos, bool lockUnspents, CCoinControl); } // namespace wallet #endif // BITCOIN_WALLET_SPEND_H diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index db9989163d..cff3628049 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -377,6 +377,17 @@ void SQLiteDatabase::Close() m_db = nullptr; } +bool SQLiteDatabase::HasActiveTxn() +{ + // 'sqlite3_get_autocommit' returns true by default, and false if a transaction has begun and not been committed or rolled back. + return m_db && sqlite3_get_autocommit(m_db) == 0; +} + +int SQliteExecHandler::Exec(SQLiteDatabase& database, const std::string& statement) +{ + return sqlite3_exec(database.m_db, statement.data(), nullptr, nullptr, nullptr); +} + std::unique_ptr<DatabaseBatch> SQLiteDatabase::MakeBatch(bool flush_on_close) { // We ignore flush_on_close because we don't do manual flushing for SQLite @@ -394,12 +405,18 @@ SQLiteBatch::SQLiteBatch(SQLiteDatabase& database) void SQLiteBatch::Close() { - // If m_db is in a transaction (i.e. not in autocommit mode), then abort the transaction in progress - if (m_database.m_db && sqlite3_get_autocommit(m_database.m_db) == 0) { + bool force_conn_refresh = false; + + // If we began a transaction, and it wasn't committed, abort the transaction in progress + if (m_database.HasActiveTxn()) { if (TxnAbort()) { LogPrintf("SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted\n"); } else { - LogPrintf("SQLiteBatch: Batch closed and failed to abort transaction\n"); + // If transaction cannot be aborted, it means there is a bug or there has been data corruption. Try to recover in this case + // by closing and reopening the database. Closing the database should also ensure that any changes made since the transaction + // was opened will be rolled back and future transactions can succeed without committing old data. + force_conn_refresh = true; + LogPrintf("SQLiteBatch: Batch closed and failed to abort transaction, resetting db connection..\n"); } } @@ -420,6 +437,17 @@ void SQLiteBatch::Close() } *stmt_prepared = nullptr; } + + if (force_conn_refresh) { + m_database.Close(); + try { + m_database.Open(); + } catch (const std::runtime_error&) { + // If open fails, cleanup this object and rethrow the exception + m_database.Close(); + throw; + } + } } bool SQLiteBatch::ReadKey(DataStream&& key, DataStream& value) @@ -606,8 +634,8 @@ std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewPrefixCursor(Span<const std:: bool SQLiteBatch::TxnBegin() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) == 0) return false; - int res = sqlite3_exec(m_database.m_db, "BEGIN TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.m_db || m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "BEGIN TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to begin the transaction\n"); } @@ -616,8 +644,8 @@ bool SQLiteBatch::TxnBegin() bool SQLiteBatch::TxnCommit() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) != 0) return false; - int res = sqlite3_exec(m_database.m_db, "COMMIT TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "COMMIT TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to commit the transaction\n"); } @@ -626,8 +654,8 @@ bool SQLiteBatch::TxnCommit() bool SQLiteBatch::TxnAbort() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) != 0) return false; - int res = sqlite3_exec(m_database.m_db, "ROLLBACK TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "ROLLBACK TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to abort the transaction\n"); } diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index f1ce0567e1..ad91be1064 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -36,11 +36,21 @@ public: Status Next(DataStream& key, DataStream& value) override; }; +/** Class responsible for executing SQL statements in SQLite databases. + * Methods are virtual so they can be overridden by unit tests testing unusual database conditions. */ +class SQliteExecHandler +{ +public: + virtual ~SQliteExecHandler() {} + virtual int Exec(SQLiteDatabase& database, const std::string& statement); +}; + /** RAII class that provides access to a WalletDatabase */ class SQLiteBatch : public DatabaseBatch { private: SQLiteDatabase& m_database; + std::unique_ptr<SQliteExecHandler> m_exec_handler{std::make_unique<SQliteExecHandler>()}; sqlite3_stmt* m_read_stmt{nullptr}; sqlite3_stmt* m_insert_stmt{nullptr}; @@ -61,6 +71,8 @@ public: explicit SQLiteBatch(SQLiteDatabase& database); ~SQLiteBatch() override { Close(); } + void SetExecHandler(std::unique_ptr<SQliteExecHandler>&& handler) { m_exec_handler = std::move(handler); } + /* No-op. See comment on SQLiteDatabase::Flush */ void Flush() override {} @@ -142,6 +154,9 @@ public: /** Make a SQLiteBatch connected to this database */ std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override; + /** Return true if there is an on-going txn in this connection */ + bool HasActiveTxn(); + sqlite3* m_db{nullptr}; bool m_use_unsafe_sync; }; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index d341e84d9b..7e6219378f 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -205,5 +205,82 @@ BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) } } +BOOST_AUTO_TEST_CASE(db_availability_after_write_error) +{ + // Ensures the database remains accessible without deadlocking after a write error. + // To simulate the behavior, record overwrites are disallowed, and the test verifies + // that the database remains active after failing to store an existing record. + for (const auto& database : TestDatabases(m_path_root)) { + // Write original record + std::unique_ptr<DatabaseBatch> batch = database->MakeBatch(); + std::string key = "key"; + std::string value = "value"; + std::string value2 = "value_2"; + BOOST_CHECK(batch->Write(key, value)); + // Attempt to overwrite the record (expect failure) + BOOST_CHECK(!batch->Write(key, value2, /*fOverwrite=*/false)); + // Successfully overwrite the record + BOOST_CHECK(batch->Write(key, value2, /*fOverwrite=*/true)); + // Sanity-check; read and verify the overwritten value + std::string read_value; + BOOST_CHECK(batch->Read(key, read_value)); + BOOST_CHECK_EQUAL(read_value, value2); + } +} + +#ifdef USE_SQLITE + +// Test-only statement execution error +constexpr int TEST_SQLITE_ERROR = -999; + +class DbExecBlocker : public SQliteExecHandler +{ +private: + SQliteExecHandler m_base_exec; + std::set<std::string> m_blocked_statements; +public: + DbExecBlocker(std::set<std::string> blocked_statements) : m_blocked_statements(blocked_statements) {} + int Exec(SQLiteDatabase& database, const std::string& statement) override { + if (m_blocked_statements.contains(statement)) return TEST_SQLITE_ERROR; + return m_base_exec.Exec(database, statement); + } +}; + +BOOST_AUTO_TEST_CASE(txn_close_failure_dangling_txn) +{ + // Verifies that there is no active dangling, to-be-reversed db txn + // after the batch object that initiated it is destroyed. + DatabaseOptions options; + DatabaseStatus status; + bilingual_str error; + std::unique_ptr<SQLiteDatabase> database = MakeSQLiteDatabase(m_path_root / "sqlite", options, status, error); + + std::string key = "key"; + std::string value = "value"; + + std::unique_ptr<SQLiteBatch> batch = std::make_unique<SQLiteBatch>(*database); + BOOST_CHECK(batch->TxnBegin()); + BOOST_CHECK(batch->Write(key, value)); + // Set a handler to prevent txn abortion during destruction. + // Mimicking a db statement execution failure. + batch->SetExecHandler(std::make_unique<DbExecBlocker>(std::set<std::string>{"ROLLBACK TRANSACTION"})); + // Destroy batch + batch.reset(); + + // Ensure there is no dangling, to-be-reversed db txn + BOOST_CHECK(!database->HasActiveTxn()); + + // And, just as a sanity check; verify that new batchs only write what they suppose to write + // and nothing else. + std::string key2 = "key2"; + std::unique_ptr<SQLiteBatch> batch2 = std::make_unique<SQLiteBatch>(*database); + BOOST_CHECK(batch2->Write(key2, value)); + // The first key must not exist + BOOST_CHECK(!batch2->Exists(key)); +} + +#endif // USE_SQLITE + + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 203ab5f606..376060421c 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -132,6 +132,14 @@ struct FuzzedWallet { } } } + std::vector<CRecipient> recipients; + for (size_t idx = 0; idx < tx.vout.size(); idx++) { + const CTxOut& tx_out = tx.vout[idx]; + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + CRecipient recipient = {dest, tx_out.nValue, subtract_fee_from_outputs.count(idx) == 1}; + recipients.push_back(recipient); + } CCoinControl coin_control; coin_control.m_allow_other_inputs = fuzzed_data_provider.ConsumeBool(); CallOneOf( @@ -158,7 +166,10 @@ struct FuzzedWallet { int change_position{fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, tx.vout.size() - 1)}; bilingual_str error; - (void)FundTransaction(*wallet, tx, change_position, /*lockUnspents=*/false, subtract_fee_from_outputs, coin_control); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + tx.vout.clear(); + (void)FundTransaction(*wallet, tx, recipients, change_position, /*lockUnspents=*/false, coin_control); } }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e03f5532fc..d03a3e979f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -555,7 +555,7 @@ void CWallet::UpgradeDescriptorCache() SetWalletFlag(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED); } -bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys) +bool CWallet::Unlock(const SecureString& strWalletPassphrase) { CCrypter crypter; CKeyingMaterial _vMasterKey; @@ -568,7 +568,7 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool accept_no_key return false; if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, _vMasterKey)) continue; // try another master key - if (Unlock(_vMasterKey, accept_no_keys)) { + if (Unlock(_vMasterKey)) { // Now that we've unlocked, upgrade the key metadata UpgradeKeyMetadata(); // Now that we've unlocked, upgrade the descriptor cache @@ -3374,12 +3374,12 @@ bool CWallet::Lock() return true; } -bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) +bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn) { { LOCK(cs_wallet); for (const auto& spk_man_pair : m_spk_managers) { - if (!spk_man_pair.second->CheckDecryptionKey(vMasterKeyIn, accept_no_keys)) { + if (!spk_man_pair.second->CheckDecryptionKey(vMasterKeyIn)) { return false; } } @@ -3513,9 +3513,10 @@ void CWallet::SetupLegacyScriptPubKeyMan() AddScriptPubKeyMan(id, std::move(spk_manager)); } -const CKeyingMaterial& CWallet::GetEncryptionKey() const +bool CWallet::WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const { - return vMasterKey; + LOCK(cs_wallet); + return cb(vMasterKey); } bool CWallet::HasEncryptionKeys() const @@ -3863,7 +3864,11 @@ std::optional<MigrationData> CWallet::GetDescriptorsForLegacy(bilingual_str& err AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - assert(legacy_spkm); + if (!Assume(legacy_spkm)) { + // This shouldn't happen + error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); + return std::nullopt; + } std::optional<MigrationData> res = legacy_spkm->MigrateToDescriptor(); if (res == std::nullopt) { @@ -3878,8 +3883,9 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - if (!legacy_spkm) { - error = _("Error: This wallet is already a descriptor wallet"); + if (!Assume(legacy_spkm)) { + // This shouldn't happen + error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); return false; } @@ -3921,6 +3927,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) } } + // Get best block locator so that we can copy it to the watchonly and solvables + CBlockLocator best_block_locator; + if (!WalletBatch(GetDatabase()).ReadBestBlock(best_block_locator)) { + error = _("Error: Unable to read wallet's best block locator record"); + return false; + } + // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. // We need to go through these in the tx insertion order so that lookups to spends works. std::vector<uint256> txids_to_delete; @@ -3931,32 +3944,47 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) LOCK(data.watchonly_wallet->cs_wallet); data.watchonly_wallet->nOrderPosNext = nOrderPosNext; watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext); + // Write the best block locator to avoid rescanning on reload + if (!watchonly_batch->WriteBestBlock(best_block_locator)) { + error = _("Error: Unable to write watchonly wallet best block locator record"); + return false; + } + } + if (data.solvable_wallet) { + // Write the best block locator to avoid rescanning on reload + if (!WalletBatch(data.solvable_wallet->GetDatabase()).WriteBestBlock(best_block_locator)) { + error = _("Error: Unable to write solvable wallet best block locator record"); + return false; + } } for (const auto& [_pos, wtx] : wtxOrdered) { - if (!IsMine(*wtx->tx) && !IsFromMe(*wtx->tx)) { - // Check it is the watchonly wallet's - // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for - if (data.watchonly_wallet) { - LOCK(data.watchonly_wallet->cs_wallet); - if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) { - // Add to watchonly wallet - const uint256& hash = wtx->GetHash(); - const CWalletTx& to_copy_wtx = *wtx; - if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) { - if (!new_tx) return false; - ins_wtx.SetTx(to_copy_wtx.tx); - ins_wtx.CopyFrom(to_copy_wtx); - return true; - })) { - error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); - return false; - } - watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); - // Mark as to remove from this wallet + // Check it is the watchonly wallet's + // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for + bool is_mine = IsMine(*wtx->tx) || IsFromMe(*wtx->tx); + if (data.watchonly_wallet) { + LOCK(data.watchonly_wallet->cs_wallet); + if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) { + // Add to watchonly wallet + const uint256& hash = wtx->GetHash(); + const CWalletTx& to_copy_wtx = *wtx; + if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) { + if (!new_tx) return false; + ins_wtx.SetTx(to_copy_wtx.tx); + ins_wtx.CopyFrom(to_copy_wtx); + return true; + })) { + error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); + return false; + } + watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); + // Mark as to remove from the migrated wallet only if it does not also belong to it + if (!is_mine) { txids_to_delete.push_back(hash); - continue; } + continue; } + } + if (!is_mine) { // Both not ours and not in the watchonly wallet error = strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex()); return false; @@ -4188,11 +4216,13 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle std::vector<bilingual_str> warnings; // If the wallet is still loaded, unload it so that nothing else tries to use it while we're changing it + bool was_loaded = false; if (auto wallet = GetWallet(context, wallet_name)) { if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) { return util::Error{_("Unable to unload the wallet before migrating")}; } UnloadWallet(std::move(wallet)); + was_loaded = true; } // Load the wallet but only in the context of this function. @@ -4213,8 +4243,20 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } + // Helper to reload as normal for some of our exit scenarios + const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) { + assert(to_reload.use_count() == 1); + std::string name = to_reload->GetName(); + to_reload.reset(); + to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); + return to_reload != nullptr; + }; + // Before anything else, check if there is something to migrate. - if (!local_wallet->GetLegacyScriptPubKeyMan()) { + if (local_wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + if (was_loaded) { + reload_wallet(local_wallet); + } return util::Error{_("Error: This wallet is already a descriptor wallet")}; } @@ -4223,32 +4265,44 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { + if (was_loaded) { + reload_wallet(local_wallet); + } return util::Error{_("Error: Unable to make a backup of your wallet")}; } res.backup_path = backup_path; bool success = false; - { - LOCK(local_wallet->cs_wallet); - // Unlock the wallet if needed - if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { - if (passphrase.find('\0') == std::string::npos) { - return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; - } else { - return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. " - "The passphrase contains a null character (ie - a zero byte). " - "If this passphrase was set with a version of this software prior to 25.0, " - "please try again with only the characters up to — but not including — " - "the first null character.")}; - } + // Unlock the wallet if needed + if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { + if (was_loaded) { + reload_wallet(local_wallet); + } + if (passphrase.find('\0') == std::string::npos) { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; + } else { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. " + "The passphrase contains a null character (ie - a zero byte). " + "If this passphrase was set with a version of this software prior to 25.0, " + "please try again with only the characters up to — but not including — " + "the first null character.")}; } + } + { + LOCK(local_wallet->cs_wallet); // First change to using SQLite if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; - // Do the migration, and cleanup if it fails - success = DoMigration(*local_wallet, context, error, res); + // Do the migration of keys and scripts for non-blank wallets, and cleanup if it fails + success = local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); + if (!success) { + success = DoMigration(*local_wallet, context, error, res); + } else { + // Make sure that descriptors flag is actually set + local_wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + } } // In case of reloading failure, we need to remember the wallet dirs to remove @@ -4258,24 +4312,19 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle std::set<fs::path> wallet_dirs; if (success) { // Migration successful, unload all wallets locally, then reload them. - const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) { - assert(to_reload.use_count() == 1); - std::string name = to_reload->GetName(); - wallet_dirs.insert(fs::PathFromString(to_reload->GetDatabase().Filename()).parent_path()); - to_reload.reset(); - to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); - return to_reload != nullptr; - }; // Reload the main wallet + wallet_dirs.insert(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(local_wallet); res.wallet = local_wallet; res.wallet_name = wallet_name; if (success && res.watchonly_wallet) { // Reload watchonly + wallet_dirs.insert(fs::PathFromString(res.watchonly_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(res.watchonly_wallet); } if (success && res.solvables_wallet) { // Reload solvables + wallet_dirs.insert(fs::PathFromString(res.solvables_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(res.solvables_wallet); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 487921443f..3c7b0ea490 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -302,7 +302,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati private: CKeyingMaterial vMasterKey GUARDED_BY(cs_wallet); - bool Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys = false); + bool Unlock(const CKeyingMaterial& vMasterKeyIn); std::atomic<bool> fAbortRescan{false}; std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver @@ -578,7 +578,7 @@ public: // Used to prevent deleting the passphrase from memory when it is still in use. RecursiveMutex m_relock_mutex; - bool Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys = false); + bool Unlock(const SecureString& strWalletPassphrase); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); @@ -962,7 +962,8 @@ public: //! Make a LegacyScriptPubKeyMan and set it for all types, internal, and external. void SetupLegacyScriptPubKeyMan(); - const CKeyingMaterial& GetEncryptionKey() const override; + bool WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const override; + bool HasEncryptionKeys() const override; /** Get last block processed height */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9820c7c0ee..f3dd5b328e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1498,20 +1498,26 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas if (format == DatabaseFormat::SQLITE) { #ifdef USE_SQLITE - return MakeSQLiteDatabase(path, options, status, error); -#else - error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path))); - status = DatabaseStatus::FAILED_BAD_FORMAT; - return nullptr; + if constexpr (true) { + return MakeSQLiteDatabase(path, options, status, error); + } else #endif + { + error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path))); + status = DatabaseStatus::FAILED_BAD_FORMAT; + return nullptr; + } } #ifdef USE_BDB - return MakeBerkeleyDatabase(path, options, status, error); -#else - error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path))); - status = DatabaseStatus::FAILED_BAD_FORMAT; - return nullptr; + if constexpr (true) { + return MakeBerkeleyDatabase(path, options, status, error); + } else #endif + { + error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path))); + status = DatabaseStatus::FAILED_BAD_FORMAT; + return nullptr; + } } } // namespace wallet |