aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-02-07Merge bitcoin/bitcoin#29363: test: Fix CPartialMerkleTree.nTransactions ↵fanquake
signedness facafa90f7a1eee452f9baf8a1b65a2edac0982b test: Fix CPartialMerkleTree.nTransactions signedness (MarcoFalke) Pull request description: It is unsigned in Bitcoin Core, so the tests should match it: https://github.com/bitcoin/bitcoin/blob/aa9231fafe45513134ec8953a217cda07446fae8/src/merkleblock.h#L59 Large positive values, or "negative" values, are rejected anyway, but it still seems fine to fix this. The bug was introduced when the code was written in d280617bf569f84457eaea546541dc74c67cd1e4. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: theStack: LGTM ACK facafa90f7a1eee452f9baf8a1b65a2edac0982b Empact: ACK facafa90f7a1eee452f9baf8a1b65a2edac0982b Tree-SHA512: 35ac11bb5382dffe132bfae6097efc343ef6c06b1b4b1545130ca27b228ca6894679004862fee921b095172abaddbef5972c24d9bc195ce970f35643bd4a0f09
2024-02-06Merge bitcoin/bitcoin#29388: fuzz: remove unused `args` and `context` from ↵Ryan Ofsky
`FuzzedWallet` b14298c5bca395e5ed6a27fe1758c0d1f4b824ac fuzz: remove unused `args` and `context` from `FuzzedWallet` (brunoerg) Pull request description: `ArgsManager args` and `WalletContext context` were previously used to create the wallet into `FuzzedWallet`. After fa15861763df71e788849b587883b3c16bb12229, they are not used anymore. This PR removes them. ACKs for top commit: maflcko: lgtm ACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac epiccurious: utACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac ryanofsky: Code review ACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac Tree-SHA512: 164e6a66ba05e11176a0cf68db6257f0ac07459cf7aa01ec4302b303c156c205a68128373a0b8daba0a6dfbff990af7fa14465a6341a296312fb20ea778c7a8c
2024-02-06Merge bitcoin/bitcoin#28833: wallet: refactor: remove unused `SignatureData` ↵Ava Chow
instances in spkm's `FillPSBT` methods e2ad343f69af4f924b22dccf94a52b6431ef6e3c wallet: remove unused `SignatureData` instances in spkm's `FillPSBT` methods (Sebastian Falbesoner) Pull request description: These are filled with signature data from a PSBT input, but not used anywhere after, hence they can be removed. Note that the same code is in the `SignPSBTInput` function where the `sigdata` result is indeed used. ACKs for top commit: achow101: ACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c brunoerg: crACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c Tree-SHA512: f0cabcc000bcea6bc7d7ec9d3be2e2a8accbdbffbe35252250ea2305b65a5813fde2b8096fbdd2c7cccdf417ea285183dc325fc2d210d88bce62978ce642930e
2024-02-06Merge bitcoin/bitcoin#29375: wallet: remove unused 'accept_no_keys' arg from ↵Ava Chow
decryption process 2bb25ce5023c4d56c8b11e9c75f9f8bd69894452 wallet: remove unused 'accept_no_keys' arg from decryption process (furszy) Pull request description: Found it while reviewing other PR. Couldn't contain myself from cleaning it up. The wallet decryption process (`CheckDecryptionKey()` and `Unlock()`) contains an arg 'accept_no_keys,' introduced in #13926, that has never been used. Additionally, this also removes the unimplemented `SplitWalletPath` function. ACKs for top commit: delta1: ACK 2bb25ce5023c4d56c8b11e9c75f9f8bd69894452 epiccurious: utACK 2bb25ce5023c4d56c8b11e9c75f9f8bd69894452. achow101: ACK 2bb25ce5023c4d56c8b11e9c75f9f8bd69894452 theStack: Code-review ACK 2bb25ce5023c4d56c8b11e9c75f9f8bd69894452 Tree-SHA512: e0537c994be19ca0032551d8a64cf1755c8997e04d21ee0522b31de26ad90b9eb02a8b415448257b60bced484b9d2a23b37586e12dc5ff6e35bdd8ff2165c6bf
2024-02-06Merge bitcoin/bitcoin#29356: test: make v2transport arg in addconnection ↵glozow
mandatory and few cleanups e7fd70f4b6b163f4ad5b25b4da7fa79899245235 [test] make v2transport arg in addconnection mandatory and few cleanups (stratospher) Pull request description: - make `v2transport` argument in `addconnection` regression-testing only RPC mandatory. https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1470738750 - previously it was an optional arg with default `false` value. - only place this RPC is used is in the [functional tests](https://github.com/bitcoin/bitcoin/blob/11b436a66af3ceaebb0f907878715f331516a0bc/test/functional/test_framework/test_node.py#L742) where we always pass the appropriate `v2transport` option to the RPC anyways. (and that too just for python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) interactions) - rename `v2_handshake()` to `_on_data_v2_handshake()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466958424 - more compact return statement in `wait_for_reconnect()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466979708 - assertion to check that empty version packets are received from `TestNode`. ACKs for top commit: glozow: ACK e7fd70f4b6 theStack: Code-review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 mzumsande: Code Review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 Tree-SHA512: e66e29baccd91e1e4398b91f7d45c5fc7c2841d77d8a6178734586017bf2be63496721649da91848dec71da605ee31664352407d5bb896e624cc693767c61a1f
2024-02-06Merge bitcoin/bitcoin#29353: test: p2p: adhere to typical VERSION message ↵glozow
protocol flow c340503b67585b631909584f1acaa327f5af25d3 test: p2p: adhere to typical VERSION message protocol flow (Sebastian Falbesoner) 7ddfc28309e6b70c273112a93f01e518409ceaa0 test: p2p: process post-v2-handshake data immediately (Sebastian Falbesoner) b198b9c2ce28463f7c6a2b4cf08d64138c8509ee test: p2p: introduce helper for sending prepared VERSION message (Sebastian Falbesoner) Pull request description: This PR addresses a quirk in the test framework's p2p implementation regarding the version handshake protocol: Currently, the VERSION message is sent immediately after an inbound connection (i.e. TestNode outbound connection) is made. This doesn't follow the usual protocol flow where the initiator sends a version first, the responder processes that and only then responds with its own version message. Change that accordingly by only sending immediate VERSION message for outbound connections (or after v2 handshake for v2 connections, respectively), and sending out VERSION message as response for incoming VERSION messages (i.e. in the function `on_version`) for inbound connections. I first stumbled upon this issue through reading comment https://mirror.b10c.me/bitcoin-bitcoin/24748/#discussion_r1465420112 (see last paragraph) and recently again in the course of working on a v2-followup for #29279, where this causes issues for TestNode outbound connections that disconnect *before* sending out their own version message. Note that these changes lead to slightly more code in some functional tests that override the `on_version` method, as the version reply has to be sent explicitly now, but I think is less confusing and reflects better what is actually happening. ACKs for top commit: epiccurious: utACK c340503b67585b631909584f1acaa327f5af25d3 stratospher: tested ACK c340503b67585b631909584f1acaa327f5af25d3. very useful to have since we'd want real node behaviour! mzumsande: ACK c340503b67585b631909584f1acaa327f5af25d3 sr-gi: tACK https://github.com/bitcoin/bitcoin/commit/c340503b67585b631909584f1acaa327f5af25d3 Tree-SHA512: 63eac287d3e1c87a01852bfd9f0530363354bbb642280298673b9c8817056356373adf348955c4e92af95c7c6efa8cc515cee2892e9f077bfbe1bce8e97ad082
2024-02-05fuzz: remove unused `args` and `context` from `FuzzedWallet`brunoerg
2024-02-05Merge bitcoin/bitcoin#29254: log: Don't use scientific notation in log messagesglozow
c819a83b4d83413a31323c1304664ee4c228ca29 Don't use scientific notation in log messages (Kristaps Kaupe) Pull request description: Don't see any benefits here, only harder to read for most of the users. Before: ``` 2024-01-16T13:11:36Z Dumped mempool: 8.165e-06s to copy, 0.00224268s to dump ``` After: ``` 2024-01-16T13:11:36Z Dumped mempool: 0.000s to copy, 0.002s to dump ``` ACKs for top commit: kristapsk: > > > > lgtm ACK [c819a83](https://github.com/bitcoin/bitcoin/commit/c819a83b4d83413a31323c1304664ee4c228ca29). can you update the PR description? glozow: lgtm ACK c819a83b4d83413a31323c1304664ee4c228ca29. can you update the PR description? Tree-SHA512: 0972e0a05934e1b014fdeca0c235065aa017ba9abf74b3018f514e4d8022ef02b7f042a07d3675144b51449492468aea6b5b0183233ad7f1bab887d18e3d06af
2024-02-05Merge bitcoin/bitcoin#29354: test: Assumeutxo with more than just coinbase ↵glozow
transactions fa5cd66f0a47d1b759c93d01524ee4558432c0cc test: Assumeutxo with more than just coinbase transactions (MarcoFalke) Pull request description: Currently the AU tests only check that loading a txout set with only coinbase outputs works. Fix that by adding other transactions. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/29354/commits/fa5cd66f0a47d1b759c93d01524ee4558432c0cc glozow: concept ACK fa5cd66f0a47d1b759c93d01524ee4558432c0cc Tree-SHA512: e090c41f73490ad72e36c478405bfd0716d46fbf5a131415095999da6503094a86689a179a84addae3562b760df64cdb67488f81692178c8ca8bf771b1e931ff
2024-02-03wallet: remove unused 'accept_no_keys' arg from decryption processfurszy
The wallet decryption process (CheckDecryptionKey() and Unlock()) contains an arg 'accept_no_keys,' introduced in #13926, that has never been used. Additionally, this also removes the unimplemented SplitWalletPath function.
2024-02-02Merge bitcoin/bitcoin#28868: wallet: Fix migration of wallets with txs that ↵Ryan Ofsky
have both spendable and watchonly outputs 4da76ca24725eb9ba8122317e04a6e1ee14ac846 test: Test migration of tx with both spendable and watchonly (Ava Chow) c62a8d03a862fb124b4f4b88efd61978e46605f8 wallet: Keep txs that belong to both watchonly and migrated wallets (Ava Chow) 71cb28ea8cb579ac04cefc47a57557c94080d1af test: Make sure that migration test does not rescan on reloading (Ava Chow) 78ba0e6748d2a519a96c41dea851e7c43b82f251 wallet: Reload the wallet if migration exited early (Ava Chow) 9332c7edda79a39bb729b71b6f8db6a9d37343bb wallet: Write bestblock to watchonly and solvable wallets (Ava Chow) Pull request description: A transaction does not necessarily have to belong to either the migrated wallet (with the private keys) and the watchonly wallet (with watchonly things), it could have multiple outputs with each isminetype. So we should be putting such transactions in one or the other wallet, but rather putting it in both. I've added a test for this behavior, however the test also revealed a few other issues. Notably, it revealed that `migratewallet` would have the watchonly wallet rescan from genesis when it is reloaded at the end of migration. This could be a cause for migration appearing to be very slow. This is resolved by first writing best block records to the watchonly and solvable wallets, as well as updating the test to make sure that rescans don't happen. The change to avoid rescans also found an issue where some of our early exits would result in unloading the wallet even though nothing happened. So there is also a commit to reload the wallet for such early exits. ACKs for top commit: ryanofsky: Code review ACK 4da76ca24725eb9ba8122317e04a6e1ee14ac846. This looks great. The code is actually cleaner than before, two bugs are fixed, and the test checking for rescanning is pretty clever and broadens test coverage. furszy: Code review ACK 4da76ca2 Tree-SHA512: 5fc210cff16ca6720d7b2d0616d7e3f295c974147854abc704cf99a3bfaad17572ada084859e7a1b1ca94da647ad130303219678f429b7995f85e040236db35c
2024-02-02Merge bitcoin/bitcoin#29367: wallet: Set descriptors flag after migrating ↵Ryan Ofsky
blank wallets 3904123da954f9ebd905de4129aec9f9bab9efc7 tests: Test that descriptors flag is set for migrated blank wallets (Ava Chow) 072d506240f6c39387b2edd4421818cc914c0912 wallet: Make sure that the descriptors flag is set for blank wallets (Ava Chow) Pull request description: While rebasing #28710 after #28976 was merged, I realized that although blank wallets were being moved to sqlite, `WALLET_FLAG_DESCRIPTORS` was not being set so those blank wallets would still continue to be treated as legacy wallets. To fix that, just set the descriptor flags for blank wallets. Also added a test to catch this. ACKs for top commit: epiccurious: Tested ACK 3904123da954f9ebd905de4129aec9f9bab9efc7. delta1: tested ACK 3904123da954f9ebd905de4129aec9f9bab9efc7 ryanofsky: Code review ACK 3904123da954f9ebd905de4129aec9f9bab9efc7 murchandamus: code review ACK 3904123da954f9ebd905de4129aec9f9bab9efc7 Tree-SHA512: 9f6fe9c1899ca387ab909b1bb6956cd6bc5acbf941686ddc6c061f9b1ceec2cc9d009ff472486fc86e963f6068f0e2f1ae96282e7c630193797a9734c4f424ab
2024-02-02Merge bitcoin/bitcoin#29361: refactor: Fix timedata includesAva Chow
fad0fafd5aca699cfab7673f8eb18211139aeb18 refactor: Fix timedata includes (MarcoFalke) Pull request description: Remove unused includes. Also, fixup comments, see https://github.com/bitcoin/bitcoin/pull/28956/files#r1464827885. Also, add missing includes to `chain.h` while touching it. ACKs for top commit: achow101: ACK fad0fafd5aca699cfab7673f8eb18211139aeb18 dergoegge: utACK fad0fafd5aca699cfab7673f8eb18211139aeb18 stickies-v: ACK fad0fafd5aca699cfab7673f8eb18211139aeb18 Tree-SHA512: 45e86f2eb90f0e37012bd83bf30259719e0e58ede18b31f51ca8a6f6d23e6ca4d060fc0f56f821a711cbdb45792b82cf780f5ae3226680d7a966471990f352bc
2024-02-01tests: Test that descriptors flag is set for migrated blank walletsAva Chow
2024-02-01wallet: Make sure that the descriptors flag is set for blank walletsAva Chow
2024-02-01test: Test migration of tx with both spendable and watchonlyAva Chow
2024-02-01wallet: Keep txs that belong to both watchonly and migrated walletsAva Chow
It is possible for a transaction that has an output that belongs to the mgirated wallet, and another output that belongs to the watchonly wallet. Such transaction should appear in both wallets during migration.
2024-02-01test: Make sure that migration test does not rescan on reloadingAva Chow
We want to make sure that all of the transactions are being copied to the watchonly and solvable wallets as expected. The automatic rescanning behavior can cause us to pass a test by finding the transaction on loading rather than having it be copied as expected.
2024-02-01wallet: Reload the wallet if migration exited earlyAva Chow
Migration will unload loaded wallets prior to beginning. It will then perform some checks which may exit early. Such unloaded wallets should be reloaded prior to exiting.
2024-02-01wallet: Write bestblock to watchonly and solvable walletsAva Chow
When migrating, we should also be writing the bestblock record to the watchonly and solvable wallets to avoid rescanning on the reload as that can be slow.
2024-02-01Merge bitcoin/bitcoin#29189: RFC: Deprecate libconsensusfanquake
25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 libconsensus: deprecate (Cory Fields) Pull request description: This library has existed for nearly 10 years with very little known uptake or impact. It has become a maintenance burden. In several cases it dictates our code/library structure (for example necessitating LIBBITCOIN_CRYPTO_BASE), as well as build-system procedures (building multiple copies of object files especially for the lib). Several discussions have arisen wrt migrating it to CMake and it has become difficult to justify adding more complexity for a library that is virtually unused anyway. See for example the discussions: https://github.com/hebasto/bitcoin/pull/41 https://github.com/bitcoin/bitcoin/pull/29123 And here: https://github.com/bitcoin/bitcoin/pull/29180 Where it is pointed out that the libbitcoinconsensus functions are slower than those the internal bitcoind equivalents due to the missing sha2 implementations. Instead, we (fanquake, hebasto, TheCharlatan, and I) propose simply not migrating it to CMake and letting it end with v27. Any remaining use-cases could be handled in the future by libbitcoinkernel. If there are any users currently using libbitcoinconsensus, please chime in with your use-case! Edit: Corrected final release to be v27. ACKs for top commit: TheCharlatan: ACK 25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 fanquake: ACK 25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 - this library has very little, if any impactful real world usage. It has been entirely broken (on various platforms) for long periods of its existence, where nobody even noticed. Pruning this out to save porting, and starting anew with the kernel, is the sane thing to do. Tree-SHA512: baff2b3c4f76f520c96021035f751fdcb51bedf00e767660249e92a7bc7c5c176786bcf2c4cfe2d2351c200f932b39eb886bcfb22fbec824a41617590d6a1638
2024-02-01Merge bitcoin/bitcoin#29275: refactor: Fix prevector iterator concept issuesfanquake
fad74bbbd0ab4425573f182ebde1b31a99e80547 refactor: Mark prevector iterator with std::contiguous_iterator_tag (MarcoFalke) fab8a01048fc6cfcee7e89884a5af31385189c63 refactor: Fix binary operator+ for prevector iterators (MarcoFalke) fa44a60b2bb5cb91bc411e5b625fc81bd84befff refactor: Fix constness for prevector iterators (MarcoFalke) facaa66b49ef7eeefe1d57c1bfc1bbe5b3011661 refactor: Add missing default constructor to prevector iterators (MarcoFalke) Pull request description: Currently prevector iterators have many issues: * Forward iterators (and stronger) must be default constructible (https://eel.is/c++draft/forward.iterators#1.2). Otherwise, some functions can not be instantiated, like `std::minmax_element`. * Various `const` issues with random access iterators. For example, a `const iterator` is different from a `const_iterator`, because the first one holds a mutable reference and must also return it without `const`. Also, `operator+` must be callable regardless of the iterator object's `const`-ness. * When adding an offset to random access iterators, both `x+n` and `n+x` must be specified, see https://eel.is/c++draft/random.access.iterators#tab:randomaccessiterator Fix all issues. Also, upgrade the `std::random_access_iterator_tag` (C++17) to `std::contiguous_iterator_tag` (C++20) ACKs for top commit: TheCharlatan: ACK fad74bbbd0ab4425573f182ebde1b31a99e80547 stickies-v: ACK fad74bbbd0ab4425573f182ebde1b31a99e80547 willcl-ark: ACK fad74bbbd0ab4425573f182ebde1b31a99e80547 Tree-SHA512: b1ca778a31602af94b323b8feaf993833ec78be09f1d438a68335485a4ba97f52125fdd977ffb9541b89f8d45be0105076aa07b5726936133519aae832556e0b
2024-02-01refactor: Fix timedata includesMarcoFalke
2024-02-01test: p2p: adhere to typical VERSION message protocol flowSebastian Falbesoner
The test framework's p2p implementation currently sends out it's VERSION message immediately after an inbound connection (i.e. TestNode outbound connection) is made. This doesn't follow the usual protocol flow where the initiator sends a version first, and the responders processes that and only then responds with its own version message. Change that accordingly by only sending immediate VERSION message for outbound connections (or after v2 handshake for v2 connections, respectively), and sending out VERSION messages as response for incoming VERSION messages (i.e. in the function `on_version`) for inbound connections. Note that some of the overruled `on_version` methods in functional tests needed to be changed to send the version explicitly.
2024-02-01test: p2p: process post-v2-handshake data immediatelySebastian Falbesoner
In the course of executing the asyncio data reception callback during a v2 handshake, it's possible that the receive buffer already contains data for after the handshake (usually a VERSION message for inbound connections). If we don't process that data immediately, we would do so after the next message is received, but with the adapted protocol flow introduced in the next commit, there is no next message, as the TestNode wouldn't continue until we send back our own version in `on_version`. Fix this by calling `self._on_data` immediately if there's data left in the receive buffer after a completed v2 handshake.
2024-02-01test: p2p: introduce helper for sending prepared VERSION messageSebastian Falbesoner
This deduplicates code for sending out the VERSION message (if available and not sent yet), currently used at three different places: 1) in the `connection_made` asyncio callback (for v1 connections that are not v2 reconnects) 2) at the end of `v2_handshake`, if the v2 handshake succeeded 3) in the `on_version` callback, if a reconnection with v1 happens
2024-02-01test: Fix CPartialMerkleTree.nTransactions signednessMarcoFalke
2024-01-31Merge bitcoin/bitcoin#26859: fuzz: extend ConsumeNetAddr() to return I2P and ↵Ava Chow
CJDNS addresses b851c5385d0a0acec4493be1561cea285065d5dc fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses (Vasil Dimov) Pull request description: In the process of doing so, refactor `ConsumeNetAddr()` to generate the addresses from IPv4, IPv6, Tor, I2P and CJDNS networks in the same way - by preparing some random stream and deserializing from it. Similar code was already found in `RandAddr()`. ACKs for top commit: achow101: ACK b851c5385d0a0acec4493be1561cea285065d5dc mzumsande: ACK b851c5385d0a0acec4493be1561cea285065d5dc brunoerg: utACK b851c5385d0a0acec4493be1561cea285065d5dc Tree-SHA512: 9905acff0e996f30ddac0c14e5ee9e1db926c7751472c06d6441111304242b563f7c942b162b209d80e8fb65a97249792eef9ae0a96100419565bf7f59f59676
2024-01-31Merge bitcoin/bitcoin#29352: test: fix intermittent failure in ↵Ava Chow
p2p_v2_earlykeyresponse 9642aefb81a9c87227eccf9997380024247ed1fc test: fix intermittent failure in p2p_v2_earlykeyresponse (Martin Zumsande) Pull request description: The test fails intermittently, see https://cirrus-ci.com/task/6403578080788480?logs=ci#L3521 and https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1916996716. I think it's because of a race between the python NetworkThread and the actual test, which will both call `initiate_v2_handshake`. I could reproduce it by adding a sleep into `initiate_v2_handshake` after the line `self.sent_garbage = random.randbytes(garbage_len)`. Fix this by waiting for the first `initiate_v2_handshake` to have finished before calling it a second time. ACKs for top commit: stratospher: tested ACK 9642aef. achow101: ACK 9642aefb81a9c87227eccf9997380024247ed1fc theStack: Tested ACK 9642aefb81a9c87227eccf9997380024247ed1fc Tree-SHA512: f728bbceaf816ddefeee4957494ccb608ad4fc912cb5cbf5f2acf09836df969c4e8fa2bb441aadb94fa39b3ffbb005d4132e7b6a5a98d80811810d8bd1d624e3
2024-01-31Merge bitcoin/bitcoin#29301: init: settings, do not load auto-generated ↵Ava Chow
warning msg 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c init: settings, do not load auto-generated warning msg (furszy) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1907071391. The settings warning message is meant to be used only to discourage users from modifying the file manually. Therefore, there is no need to keep it in memory. ACKs for top commit: achow101: ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c ryanofsky: Code review ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c. Seems like a clean, simple fix Tree-SHA512: 3f2bdcf4b4a9cadb396dcff9b43155211eeed018527a07356970a341d139ad18edbd1a4d369377c8907b8ec1f19ee2ab8aacf85a887379e6d57a8a6db2403d51
2024-01-31Merge bitcoin/bitcoin#28976: wallet: Fix migration of blank walletsRyan Ofsky
c11c404281d2d0e22967e30e16c0733db84f4eee tests: Test migration of blank wallets (Andrew Chow) 563b2a60d6a371f26474410397da435547e58786 wallet: Better error message when missing LegacySPKM during migration (Andrew Chow) b1d2c771d43b802db12768e620588ed179e92b29 wallet: Check for descriptors flag before migration (Andrew Chow) 8c127ff1edb6b9a607bf1ad247893347252a38e3 wallet: Skip key and script migration for blank wallets (Andrew Chow) Pull request description: Blank wallets (wallets without any keys are scripts) are being detected as already being descriptor wallets even though they are not. This is because the check for whether a wallet is already a descriptor wallet uses the presence of a `LegacyScriptPubKeyMan` which is only setup when keys or scripts are found. This PR resolves this issue by checking for the descriptor wallet flag instead and subsequently skipping the keys and scripts part of migration for blank wallets. Fixes the issue mentioned in https://github.com/bitcoin/bitcoin/pull/28868#issuecomment-1809641110 ACKs for top commit: furszy: reACK c11c404281d2d0e22967e30e16c0733db84f4eee. CI failure is unrelated. ryanofsky: Code review ACK c11c404281d2d0e22967e30e16c0733db84f4eee Tree-SHA512: 2466fdf1542eb8489c841253191f85dc88365493f0bb3395b67dee3e43709a9993c68b9d7623657b54b779adbe68fc81962d60efef4802c5d461f154167af7f4
2024-01-31Merge bitcoin/bitcoin#28956: Nuke adjusted time from validation (attempt 2)Ava Chow
ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Remove GetAdjustedTime (dergoegge) Pull request description: This picks up parts of #25908. The use of adjusted time is removed from validation code while the warning to users if their clock is out of sync with the rest of the network remains. ACKs for top commit: naumenkogs: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 achow101: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 maflcko: lgtm ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 🤽 stickies-v: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Tree-SHA512: d1f6b9445c236915503fd2ea828f0d3b92285a5dbc677b168453276115e349972edbad37194d8becd9136d8e7219b576af64ec51c72bdb1923e57e405c0483fc
2024-01-31Merge bitcoin/bitcoin#29347: net: enable v2transport by defaultAva Chow
0bef1042ce6c459acb1de965cbccd98867a417f1 net: enable v2transport by default (Pieter Wuille) Pull request description: This enables BIP324's v2 transport by default (see #27634): * Inbound connections will auto-sense whether v1 or v2 is in use. * Automatic outbound connections will use v2 if `NODE_P2P_V2` was set in addr gossip, but retry with v1 if met with immediate failure. * Manual outbound connections will default to v2, but retry with v1 if met with immediate failure. It remains possible to run with `-v2transport=0` to disable all of these, and make all outbound and inbound connections v1. It also remains possible to specify the `v2transport` argument to the `addnode` RPC as `false`, to disable attempting a v2 connection for that particular added node. ACKs for top commit: stratospher: ACK 0bef104. josibake: reACK https://github.com/bitcoin/bitcoin/pull/29347/commits/0bef1042ce6c459acb1de965cbccd98867a417f1 achow101: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 naumenkogs: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 theStack: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 willcl-ark: crACK 0bef1042ce6c459acb1de965cbccd98867a417f1 BrandonOdiwuor: utACK 0bef1042ce6c459acb1de965cbccd98867a417f1 pablomartin4btc: re ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 kristapsk: utACK 0bef1042ce6c459acb1de965cbccd98867a417f1 Tree-SHA512: 3f17a91e318b9304c40c74a7a5b231149f664ae684d13e9739a05be6c05ba9720f3c3c62da6a73ace0ae8ce733f1c8410b211f9fa15694e6a8d28999ab9882d8
2024-01-31Merge bitcoin/bitcoin#29253: wallet: guard against dangling to-be-reverted ↵Ava Chow
db transactions b298242c8d495c36072415e1b95eaa7bf485a38a test: sqlite, add coverage for dangling to-be-reverted db txns (furszy) fc0e747192e98e779c5f31f2df808f62b3fdd071 sqlite: guard against dangling to-be-reverted db transactions (furszy) 472d2ca98170049e0edec830e2d11c5ef23740a4 sqlite: introduce HasActiveTxn method (furszy) dca874e838c2599bd24433675b291168f8e7b055 sqlite: add ability to interrupt statements (furszy) fdf9f66909a354a95f4b7c5f092f0e9fbe1baa7c test: wallet db, exercise deadlock after write failure (furszy) Pull request description: Discovered while was reviewing #29112, specifically https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1821862931. If the db handler that initiated the database transaction is destroyed, the ongoing transaction cannot be left dangling when the db txn fails to abort. It must be forcefully reverted; otherwise, any subsequent db handler executing a write operation will dump the dangling, to-be-reverted transaction data to disk. This not only breaks the isolation property but also results in the improper storage of incomplete information on disk, impacting the wallet consistency. This PR fixes the issue by resetting the db connection, automatically rolling back the transaction (per https://www.sqlite.org/c3ref/close.html) when the handler object is being destroyed and the txn abortion failed. Testing Notes Can verify the failure by reverting the fix e5217fea and running the test. It will fail without e5217fea and pass with it. ACKs for top commit: achow101: ACK b298242c8d495c36072415e1b95eaa7bf485a38a ryanofsky: Code review ACK b298242c8d495c36072415e1b95eaa7bf485a38a. Just fix for exec result codes and comment update since last review. Tree-SHA512: 44ba0323ab21440e79e9d7791bc1c56a8873c8bd3e8f6a85641b91576e1293011fa8032d8ae5b0580f3fb7a949356f7b9676693d7ceffa617aaad9f6569993eb
2024-01-31Don't use scientific notation in log messagesKristaps Kaupe
2024-01-31[test] make v2transport arg in addconnection mandatory and few cleanupsstratospher
`TestNode::add_outbound_p2p_connection()` is the only place where addconnection test-only RPC is used. here, we always pass the appropriate v2transport option to addconnection RPC. currently the v2transport option for addconnection RPC is optional. so simply make the v2transport option mandatory instead.
2024-01-31Merge bitcoin/bitcoin#28170: p2p: adaptive connections services flagsAva Chow
27f260aa6e04f82dad78e9a06d58927546143a27 net: remove now unused global 'g_initial_block_download_completed' (furszy) aff7d92b1500e2478ce36a7e86ae47df47dda178 test: add coverage for peerman adaptive connections service flags (furszy) 6ed53602ac7c565273b5722de167cb2569a0e381 net: peer manager, dynamically adjust desirable services flag (furszy) 9f36e591c551ec2e58a6496334541bfdae8fdfe5 net: move state dependent peer services flags (furszy) f9ac96b8d6f4eba23c88f302b22a2c676e351263 net: decouple state independent service flags from desirable ones (furszy) 97df4e38879d2644aeec34c1eef241fed627333e net: store best block tip time inside PeerManager (furszy) Pull request description: Derived from #28120 discussion. By relocating the peer desirable services flags into the peer manager, we allow the connections acceptance process to handle post-IBD potential stalling scenarios. The peer manager will be able to dynamically adjust the services flags based on the node's proximity to the tip (back and forth). Allowing the node to recover from the following post-IBD scenario: Suppose the node has successfully synced the chain, but later experienced dropped connections and remained inactive for a duration longer than the limited peers threshold (the timeframe within which limited peers can provide blocks). In such cases, upon reconnecting to the network, the node might only establish connections with limited peers, filling up all available outbound slots. Resulting in an inability to synchronize the chain (because limited peers will not provide blocks older than the `NODE_NETWORK_LIMITED_MIN_BLOCKS` threshold). ACKs for top commit: achow101: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 vasild: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 naumenkogs: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 mzumsande: Light Code Review ACK 27f260aa6e04f82dad78e9a06d58927546143a27 andrewtoth: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 Tree-SHA512: 07befb9bcd0b60a4e7c45e4429c02e7b6c66244f0910f4b2ad97c9b98258b6f46c914660a717b5ed4ef4814d0dbfae6e18e6559fe9bec7d0fbc2034109200953
2024-01-31test: fix intermittent failure in p2p_v2_earlykeyresponseMartin Zumsande
This fixes a possible race between the python NetworkThread and the actual test, which will both call initiate_v2_handshake.
2024-01-31test: Assumeutxo with more than just coinbase transactionsMarcoFalke
2024-01-31Merge bitcoin/bitcoin#29343: test: fix wallet_import_rescan unrounded ↵fanquake
minimum amount 26ad2aeb29dd0875e8509917ddaa586997e977d2 test: fix wallet_import_rescan unrounded minimum amount (stickies-v) Pull request description: Addresses https://github.com/bitcoin/bitcoin/pull/29283#discussion_r1468842089. Fixes a `JSONRPCException: Invalid amount (-3)` exception by ensuring the amount sent to `sendtoaddress` is rounded to 8 decimals. See https://cirrus-ci.com/task/5562947183837184?logs=ci#L2559 Note: since `round` can also round down, `min_amount` is not _exactly_ guaranteed, but this is not a problem for the current usage. I've added a docstring to highlight this. ACKs for top commit: sr-gi: ACK [26ad2ae](https://github.com/bitcoin/bitcoin/pull/29343/commits/26ad2aeb29dd0875e8509917ddaa586997e977d2) Tree-SHA512: 82ce16447f30535f17fa73336f7e4f74639e33215a228294b9b8005b8050a760b90a3726de279cce98c7e439f09104172b74072be3a300dbd461bf0c3f54b954
2024-01-30libconsensus: deprecateCory Fields
This library has existed for nearly 10 years with very little known uptake or impact. It has become a maintenance burden. In several cases it dictates our code/library structure (for example necessitating LIBBITCOIN_CRYPTO_BASE), as well as build-system procedures (building multiple copies of object files especially for the lib). Several discussions have arisen wrt migrating it to CMake and it has become difficult to justify adding more complexity for a library that is virtually unused anyway. See for example the discussions: https://github.com/hebasto/bitcoin/pull/41 https://github.com/bitcoin/bitcoin/pull/29123 Instead, we (fanquake, hebasto, TheCharlatan, and I) propose simply not migrating it to CMake and letting it end with v27. Any remaining use-cases could be handled in the future by libbitcoinkernel.
2024-01-30test: sqlite, add coverage for dangling to-be-reverted db txnsfurszy
2024-01-30sqlite: guard against dangling to-be-reverted db transactionsfurszy
If the handler that initiated the database transaction is destroyed, the ongoing transaction cannot be left dangling when the db txn fails to abort. It must be forcefully reversed; otherwise, any subsequent db handler executing a write operation will dump the dangling, to-be-reverted transaction data to disk. This not only breaks the database isolation property but also results in the improper storage of incomplete information on disk, impacting the wallet consistency.
2024-01-30sqlite: introduce HasActiveTxn methodfurszy
Util function to clean up code and let us verify, in the following-up commit, that dangling, to-be-reverted db transactions cannot occur anymore.
2024-01-30sqlite: add ability to interrupt statementsfurszy
By encapsulating sqlite3_exec into its own standalone method and introducing the 'SQliteExecHandler' class, we enable the ability to test db statements execution failures within the unit test framework. This is used in the following-up commit to exercise a deadlock and improve our wallet db error handling code. Moreover, the future encapsulation of other sqlite functions within this class will contribute to minimize the impact of any future API changes.
2024-01-30Merge bitcoin/bitcoin#29308: doc: update `BroadcastTransaction` commentglozow
31cce4a1bdbb48f57996615ee6c686e456cc0bea doc: update `BroadcastTransaction` comment (ismaelsadeeq) Pull request description: `BroadcastTransaction` is also called by `submitpackage` RPC. All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926 It's not maintainable to list all the callers of a function. ACKs for top commit: stickies-v: ACK 31cce4a1bdbb48f57996615ee6c686e456cc0bea kristapsk: ACK 31cce4a1bdbb48f57996615ee6c686e456cc0bea naumenkogs: ACK 31cce4a1bdbb48f57996615ee6c686e456cc0bea Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
2024-01-30Merge bitcoin/bitcoin#29299: validation: fix misleading checkblockindex commentsglozow
9819db4ccaa03519a78d4d9ecce9f89f5be669e5 validation: move nChainTx assert down in CheckBlockIndex (Martin Zumsande) 033477dba65151b1863214606d5a49686ab6f559 doc: fix checkblockindex comments (Martin Zumsande) Pull request description: The two assumptions there were described as test-only, which has led to confusion whether they should exist. However, they are necessary in general, as the changed comment explains - without them, the check would fail everywhere where it is enabled. The second commit moves this assert down to the other checks. Closes #29261 ACKs for top commit: maflcko: ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5 🌦 naumenkogs: ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5 ryanofsky: Code review ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5. Thanks for figuring this issue out and fixing it. Would suggest changing pr name from "improve comments" to "fix misleading comments" since previous comments were wrong about the reasons the conditions are needed. Tree-SHA512: 3f77791253eb0c97f8153dd8ae1c567f43f6387ea7a53efea94817463c672a4e11d548aa7eff62235346ff0713ff4d6fe08f9ec50d0c30a1e6b6d27b9918b419
2024-01-30Merge bitcoin/bitcoin#29067: test: Treat msg_version.relay as unsigned, ↵glozow
Remove `struct` packing in messages.py 55556a64a8e4e6238f990cf66295c3b9594c2c3d test: Remove struct import from messages.py (MarcoFalke) fa3fa86ddaaa2246e873b7a3f19bc589a3f46c11 scripted-diff: test: Use int from_bytes and to_bytes over struct packing (MarcoFalke) fafc0d68eef9c9381b1a3d8e160aad9eeb8540a7 test: Use int from_bytes and to_bytes over struct packing (MarcoFalke) fa3886b7c69cbbe564478f30bb2c35e9e6b1cffa test: Treat msg_version.relay as unsigned (MarcoFalke) Pull request description: `struct` has many issues in messages.py: * For unpacking, it requires to specify the length a second time, even when it is already clear from the `f.read(num_bytes)` context. * For unpacking, it is designed to support a long format string and returning a tuple of many values. However, except for 3 instances in `messages.py`, usually only a single value is unpacked and all those cases require an `[0]` access. * For packing and unpacking of a single value, the format string consists of characters that may be confusing and may need to be looked up in the documentation, as opposed to using easy to understand self-documenting code. I presume the above issues lead to accidentally treat `msg_version.relay` as a "signed bool", which is fine, but confusing. Fix all issues by using the built-in `int` helpers `to_bytes` and `from_bytes` via a scripted diff. Review notes: * `struct.unpack` throws an error if the number of bytes passed is incorrect. `int.from_bytes` doesn't know about "missing" bytes and treats an empty byte array as `int(0)`. "Extraneous" bytes should never happen, because all `read` calls are limited in this file. If it is important to keep this error behavior, a helper `int_from_stream(stream, num_bytes, bytes, byteorder, *, **kwargs)` can be added, which checks the number of bytes read from the stream. * For `struct.pack` and `int.to_bytes` the error behavior is the same, although the error messages are not identical. ACKs for top commit: stickies-v: ACK 55556a64a8e4e6238f990cf66295c3b9594c2c3d theStack: re-ACK 55556a64a8e4e6238f990cf66295c3b9594c2c3d Tree-SHA512: 1cef8cdfd763fb424ed4b8be850a834b83fd0ef47fbea626a29784eb4f4832d44e42c4fe05b298b6070a933ef278b0222289a9955a97c86707e091e20bbb247a
2024-01-29net: enable v2transport by defaultPieter Wuille
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-19 19:19:41 +0000