aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-03-11Merge bitcoin/bitcoin#26564: test: test_bitcoin: allow -testdatadir=<datadir>Ava Chow
d27e2d87b95b7982c05b4c88e463cc9626ab9f0a test: test_bitcoin: allow -testdatadir=<datadir> (Larry Ruane) Pull request description: This backward-compatible change would help with code review, testing, and debugging. When `test_bitcoin` runs, it creates a working or data directory within `/tmp/test_common_Bitcoin\ Core/`, named as a long random (hex) string. This small patch does three things: - If the (new) argument `-testdatadir=<datadir>` is given, use `<datadir>/test_temp/<test-name>/datadir` as the working directory - When the test starts, remove `<datadir>/test_temp/<test-name>/datadir` if it exists from an earlier run (currently, it's presumed not to exist due to the long random string) - Don't delete the working directory at the end of the test if a custom data directory is being used Example usage, which will remove, create, use `/somewhere/test_temp/getarg_tests/boolarg`, and leave it afterward: ``` $ test_bitcoin --run_test=getarg_tests/boolarg -- -testdatadir=/somewhere Running 1 test case... Test directory (will not be deleted): "/somewhere/test_temp/getarg_tests/boolarg/datadir" *** No errors detected $ ls -l /somewhere/test_temp/getarg_tests/boolarg/datadir total 8 drwxrwxr-x 2 larry larry 4096 Feb 22 10:28 blocks -rw-rw-r-- 1 larry larry 1273 Feb 22 10:28 debug.log ``` (A relative pathname also works.) This change affects only `test_bitcoin`; it could also be applied to `test_bitcoin-qt` but that's slightly more involved so I'm skipping that for now. The rationale for this change is that, when running the test using the debugger, it's often useful to watch `debug.log` as the test runs and inspect some of the other files (I've looked at the generated `blknnnn.dat` files for example). Currently, that requires figuring out where the test's working directory is since it changes on every test run. Tests can be run with `-printtoconsole=1` to show debug logging to the terminal, but it's nice to keep `debug.log` continuously open in an editor, for example. Even if not using a debugger, it's sometimes helpful to see `debug.log` and other artifacts after the test completes. Similar functionality is already possible with the functional tests using the `--tmpdir=` and `--nocleanup` arguments. ACKs for top commit: davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/26564/commits/d27e2d87b95b7982c05b4c88e463cc9626ab9f0a tdb3: re-ACK for d27e2d87b95b7982c05b4c88e463cc9626ab9f0a achow101: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a cbergqvist: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a! (Already did some testing with `fs::remove()` to make sure it was compatible with the `util::Lock/UnlockDirectory` implementation). marcofleon: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a. I ran all the tests with my previous open file limit and no errors were detected. Also ran some individual tests with no, relative, and absolute paths and everything looks good. furszy: ACK d27e2d8 Tree-SHA512: a8f535f34a48b6699cb440f97f5562ec643f3bfba4ea685768980b871fc8b6e1135f70fc05dbe19aa2c8bacb1ddeaff212d63473605a7422ff76332b3a6b1f68
2024-03-08Merge bitcoin/bitcoin#29569: Rename CalculateHeadersWork to ↡Ava Chow
CalculateClaimedHeadersWork eb7cc9fd2140df77acc6eb42004cf45b260bc629 Rename CalculateHeadersWork to CalculateClaimedHeadersWork (Greg Sanders) Pull request description: And clean up some comments. Confusion about what this is doing seems to be a running theme: https://github.com/bitcoin/bitcoin/pull/29549#discussion_r1511113344 https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141510303 ACKs for top commit: achow101: ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629 pablomartin4btc: ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629 0xB10C: ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629 dergoegge: ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629 BrandonOdiwuor: ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629 Tree-SHA512: 6ccbc5e417155516487bb220753d189b5341dec05366db88a3fa5b1932eace21fbfaf23408c639bb54b36169a8d0a7536a1ee5e63b4ce5a3b70f2ff8407b6e07
2024-03-08Merge bitcoin/bitcoin#29393: i2p: log connection was refused due to ↡Ava Chow
arbitrary port 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 i2p: log connection was refused due to arbitrary port (brunoerg) Pull request description: For I2P, we do not try to connect if port is != 0. However, we do not have anything that indicates it or any error when trying to connect with port != 0. This PR adds a log for it. Also, it improves the functional test. With this log we can ensure the reason we won't connect is the port, in the current test, we cannot ensure it. ACKs for top commit: jonatack: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 epiccurious: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796. achow101: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 kristapsk: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 vasild: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 Tree-SHA512: 027245afa771c9295fff0bfd17c251dca4a9f4c739e5773922de3c030a65ef05d96291edcbdeeaa50ba3add61f75f28d8c00be503e03fc33d3491d1956fc549f
2024-03-08Merge bitcoin/bitcoin#28960: kernel: Remove dependency on CSchedulerAva Chow
d5228efb5391b31a9a0673019e43e7fa2cd4ac07 kernel: Remove dependency on CScheduler (TheCharlatan) 06069b3913dda048f5d640a662b0852f86346ace scripted-diff: Rename MainSignals to ValidationSignals (TheCharlatan) 0d6d2b650d1017691f48c9109a6cd020ab46aa73 scripted-diff: Rename SingleThreadedSchedulerClient to SerialTaskRunner (TheCharlatan) 4abde2c4e3fd9b66394b79874583bdc2a9132c36 [refactor] Make MainSignals RAII styled (TheCharlatan) 84f5c135b8118cbe15b8bfb4db80d61237987f64 refactor: De-globalize g_signals (TheCharlatan) 473dd4b97ae40e43e1a1a97fdbeb40be4855e9bc [refactor] Prepare for g_signals de-globalization (TheCharlatan) 3fba3d5deec6d7bae33823b8da7682f9b03d9deb [refactor] Make signals optional in mempool and chainman (TheCharlatan) Pull request description: By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design. Removing `CScheduler` also allows removing the thread and exception modules from the kernel library. To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices. Renames `CMainSignals` to `ValidationSignals`, which more accurately describes its purpose and scope. Also make the `ValidationSignals` in the `ChainstateManager` and CTxMemPool` optional. This could be useful in the future for using or testing these classes without having to instantiate any form of signal handling. --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It improves the kernel API and removes two modules from the kernel library. ACKs for top commit: maflcko: re-ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 πŸŒ„ ryanofsky: Code review ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07. Just comment change since last review. vasild: ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 furszy: diff ACK d5228ef Tree-SHA512: e93a5f10eb6182effb84bb981859a7ce750e466efd8171045d8d9e7fe46e4065631d9f6f533c5967c4d34c9bb7d7a67e9f4593bd4c5b30cd7b3bbad7be7b331b
2024-03-08wallet: default wallet migration, modify inconvenient backup filenamefurszy
On default legacy wallets, the backup filename starts with an "-" due to the wallet name being empty. This is inconvenient for systems who treat what follows the initial "-" character as flags.
2024-03-08Reserve memory for ToLower/ToUpper conversionsLΕ‘rinc
2024-03-07test: test_bitcoin: allow -testdatadir=<datadir>Larry Ruane
Specifying this argument overrides the path location for test_bitcoin; it becomes <datadir>/test_common_Bitcoin Core/<testname>/datadir. Also, this directory isn't removed after the test completes. This can make it easier for developers to study the results of a test (see the state of the data directory after the test runs), and also (for example) have an editor open on debug.log to monitor it across multiple test runs instead of having to re-open a different pathname each time. Example usage (note the "--" is needed): test_bitcoin --run_test=getarg_tests/boolarg -- \ -testdatadir=/somewhere/mydatadir This will create (if necessary) and use the data directory: /somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/boolarg/datadir Co-authored-by: furszy <mfurszy@protonmail.com>
2024-03-07build: Add missed definition for `AM_OBJCXXFLAGS`Hennadii Stepanov
2024-03-07Merge bitcoin-core/gui#803: test: Set organization nameHennadii Stepanov
0dcbad341b0a8420a899c6dce0db56dd0deaa036 qt, test: Clean settings after tests (Hennadii Stepanov) 49cf63522e202caf326bad161ff6fa05d1076566 qt, test: Set organization name (Hennadii Stepanov) Pull request description: From Qt [docs](https://doc.qt.io/qt-5/qsettings.html#QSettings-4): > If [`QCoreApplication::setOrganizationName()`](https://doc.qt.io/qt-5/qcoreapplication.html#organizationName-prop) and [`QCoreApplication::setApplicationName()`](https://doc.qt.io/qt-5/qcoreapplication.html#applicationName-prop) has not been previously called, the `QSettings` object will not be able to read or write any settings, and [`status()`](https://doc.qt.io/qt-5/qsettings.html#status) will return [`AccessError`](https://doc.qt.io/qt-5/qsettings.html#Status-enum). Fixes https://github.com/bitcoin-core/gui/issues/799. ACKs for top commit: pablomartin4btc: utACK 0dcbad341b0a8420a899c6dce0db56dd0deaa036 Tree-SHA512: d5ac160f17cc358f0c1b89097193cd5adfd25f5531955c211f3e0994fc084e0a2b8d3aeddebe38f3a8ab5333edef5aa92b18915885c9e58b33f2e5786f31c600
2024-03-06Merge bitcoin/bitcoin#29561: Fixed seeds update for 27.0fanquake
7ab54397f8b818b0474d4e9d4f5db45abb6fc249 seeds: Update testnet seeds (Ava Chow) 34a233b6d875976f354094f67b363a4d7b63ad2f seeds: Update mainnet seeds (Ava Chow) 9701bc435fe16fb7c285c682d87c44972f1c23b9 makeseeds: Check i2p seeds too (Ava Chow) a8ec9eede4c745c6b6fd76974816ffad8034129a makeseeds: Update PATTERN_AGENT (Ava Chow) Pull request description: The ipv4 and ipv6 seeds are updated from sipa's crawler, as outlined in contrib/seeds/README.md. The onion and i2p seeds are pulled from my node's addrman using `getrawaddrman` and then a connection was made to each node to retrieve the current service flags, block height, and user agent string before filtering through makeseeds.py. The CJDNS nodes were not updated as my node is not connected to that network. makeseeds.py is also updated for more recent user agent strings as well as being able to handle i2p addresses. Also updated the testnet seeds. ACKs for top commit: fanquake: ACK 7ab54397f8b818b0474d4e9d4f5db45abb6fc249 Tree-SHA512: 5edba63d51116e5d9a8ae23561ba5a311f4df88c555c60b2d7a6066e63f8cdfd256be7dac9acea4b370879d0d3c3a4b55328c15de4284b5f0d86e6cac2e5ba9b
2024-03-05fuzz: restrict fopencookie usage to Linux & FreeBSDfanquake
Should fix the GCC compilation portion of #29517: https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-1973573314. See also: https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html but note that FreeBSD has supported this function since 11.x.
2024-03-05Merge bitcoin/bitcoin#29547: kernel: chainparams updates for 27.xfanquake
d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 kernel: chainparams updates for 27.x (fanquake) 1611aa1789dc1dcb445bc1654ac4bec841345a20 kernel: update chainTxData for 27.x (fanquake) af78d31e710f61601dc2b21014cb4250ddea85ad kernel: update nMinimumChainWork & defaultAssumeValid for 27.x (fanquake) Pull request description: Update chainparams pre `27.x` branch off. Note: Remember that some variance is expected in the m_assumed_* sizes. ACKs for top commit: Sjors: ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 glozow: ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 (checked mainnet locally, checked testnet/signet on block explorers and sanity checked the numbers) instagibbs: ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 Tree-SHA512: 6ce65b964334b9d15fff4aa1af6d26fb3ef4eab50b8237fc2cda180230ae724a99d13c9f6b3c58105548d3520c0ca0810f354736132d11793d6c91ad3eeac4c7
2024-03-05Rename CalculateHeadersWork to CalculateClaimedHeadersWorkGreg Sanders
2024-03-05qt, test: Clean settings after testsHennadii Stepanov
A test suite should not leave any artifacts except for those explicitly expected. This change is easy to review with `git diff --ignore-all-space` command.
2024-03-05qt, test: Set organization nameHennadii Stepanov
If `setOrganizationName()` and `setApplicationName()` has not been previously called, the `QSettings` object will not be able to read or write any settings.
2024-03-04seeds: Update testnet seedsAva Chow
2024-03-04seeds: Update mainnet seedsAva Chow
2024-03-04Merge bitcoin/bitcoin#29546: qt: 27.0 translations updatefanquake
632b69f79bb83d2313df7d76667763fbb590136b qt: 27.0 translations update (Hennadii Stepanov) Pull request description: This PR pulls the recent translations from the [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool. ACKs for top commit: stickies-v: ACK 632b69f79bb83d2313df7d76667763fbb590136b , getting a zero-diff when running `update-translations.py` on fce53f132e1b3f2c8bf1530dca18f3da136f08ab Tree-SHA512: 1e2823451e9192e60dec9d50e801fca4cdc621e6acabdc15dbd88cab1624e05bd56de9ac818a1ff91002d62e24c0bab0ef1eaad3fd3cc6ef6cd044989d39734f
2024-03-04kernel: chainparams updates for 27.xfanquake
2024-03-04kernel: update chainTxData for 27.xfanquake
2024-03-04kernel: update nMinimumChainWork & defaultAssumeValid for 27.xfanquake
2024-03-04Merge bitcoin-core/gui#801: Fix nullptr clientModel access during shutdownHennadii Stepanov
b7aa717cdd3f6af266c244fec6d775e917cf8d0c refactor: gui, simplify boost signals disconnection (furszy) f3a612f9016fe1f59c73d6059274bea8025b8940 gui: guard accessing a nullptr 'clientModel' (furszy) Pull request description: Fixing #800. During shutdown, already queue events dispatched from the backend such 'numConnectionsChanged' and 0networkActiveChanged' could try to access the clientModel object, which might not exist because we manually delete it inside 'BitcoinApplication::requestShutdown()'. This happen because boost does not clears the queued events when they arise concurrently with the signal disconnection (see https://www.boost.org/doc/libs/1_55_0/doc/html/signals2/thread-safety.html). From the docs: 1) "Note that since we unlock the connection's mutex before executing its associated slot, it is possible a slot will still be executing after it has been disconnected by a [connection::disconnect](https://www.boost.org/doc/libs/1_55_0/doc/html/boost/signals2/connection.html#idp89761576-bb)(), if the disconnect was called concurrently with signal invocation." 2) "The fact that concurrent signal invocations use the same combiner object means you need to insure any custom combiner you write is thread-safe" So, we need to guard `clientModel` before accessing it at the handler side. ACKs for top commit: hebasto: re-ACK b7aa717cdd3f6af266c244fec6d775e917cf8d0c Tree-SHA512: f1a21d69248628f6a13556a9438c9e4ea9f0a3678aab09ddfe836e78e4eee405a6730d37d39f1445068ada3a110b655b619cf0e090fc2d0cdf99bed061364aeb
2024-03-04Merge bitcoin/bitcoin#29524: p2p: Don't consider blocks mutated if they ↡fanquake
don't connect to known prev block a1fbde0ef7cf6c94d4a5181f8ceb327096713160 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders) Pull request description: Followup to https://github.com/bitcoin/bitcoin/pull/29412 to revert some of the behavior change that was likely unintentional. Based on comments from https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1507499192 ACKs for top commit: 0xB10C: utACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 dergoegge: Code review ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 Sjors: ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 sr-gi: tACK https://github.com/bitcoin/bitcoin/commit/a1fbde0ef7cf6c94d4a5181f8ceb327096713160 Tree-SHA512: be6204c8cc57b271d55c1d02a5c77d03a37738d91cb5ac164f483b0bab3991c24679c5ff02fbaa52bf37ee625874b63f4c9f7b39ad6fd5f3a25386567a0942e4
2024-03-04qt: 27.0 translations updateHennadii Stepanov
2024-03-01doc: add release notes and help text for unix socketsMatthew Zipkin
2024-03-01init: allow UNIX socket path for -proxy and -onionMatthew Zipkin
2024-03-01gui: accomodate unix socket Proxy in updateDefaultProxyNets()Matthew Zipkin
This will require a follow-up to add unix socket options to the GUI
2024-03-01i2p: construct Session with Proxy instead of CServiceMatthew Zipkin
2024-03-01net: split ConnectToSocket() from ConnectDirectly() for unix socketsMatthew Zipkin
2024-03-01proxy: rename randomize_credentials to m_randomize_credentialsMatthew Zipkin
2024-03-01netbase: extend Proxy class to wrap UNIX socket as well as TCPMatthew Zipkin
2024-03-01net: move CreateSock() calls from ConnectNode() to netbase methodsMatthew Zipkin
2024-03-01netbase: allow CreateSock() to create UNIX sockets if supportedMatthew Zipkin
2024-03-01netbase: refactor CreateSock() to accept sa_family_tMatthew Zipkin
Also implement CService::GetSAFamily() to provide sa_family_t
2024-03-01build: move sha256_sse4 into libbitcoin_crypto_basefanquake
Followup to discussion in #29407. Drops LIBBITCOIN_CRYPTO_SSE4.
2024-03-01Merge bitcoin/bitcoin#29263: serialization: c++20 endian/byteswap/clz ↡fanquake
modernization 86b7f28d6c507155a9d3a15487ee883989b88943 serialization: use internal endian conversion functions (Cory Fields) 432b18ca8d0654318a8d882b28b20af2cb2d2e5d serialization: detect byteswap builtins without autoconf tests (Cory Fields) 297367b3bb062c57142747719ac9bf2e12717ce9 crypto: replace CountBits with std::bit_width (Cory Fields) 52f9bba889fd9b50a0543fd9fedc389592cdc7e5 crypto: replace non-standard CLZ builtins with c++20's bit_width (Cory Fields) Pull request description: This replaces #28674, #29036, and #29057. Now ready for testing and review. Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h. I apologize for the size of the last commit, but it's hard to avoid making those changes at once. All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC. Sadly, benchmarking showed that not all compilers are capable of detecting and optimizing byteswap functions, so compiler builtins are instead used where possible. However, they're now detected via macros rather than autoconf checks. This[ matches how libc++ implements std::byteswap for c++23](https://github.com/llvm/llvm-project/blob/main/libcxx/include/__bit/byteswap.h#L26). I suggest we move/rename `compat/endian.h`, but I left that out of this PR to avoid bikeshedding. #29057 pointed out some irregularities in benchmarks. After messing with various compilers and configs for a few weeks with these changes, I'm of the opinion that we can't win on every platform every time, so we should take the code that makes sense going forward. That said, if any real-world slowdowns are caused here, we should obviously investigate. ACKs for top commit: maflcko: ACK 86b7f28d6c507155a9d3a15487ee883989b88943 πŸ“˜ fanquake: ACK 86b7f28d6c507155a9d3a15487ee883989b88943 - we can finish pruning out the __builtin_clz* checks/usage once the minisketch code has been updated. This is more good cleanup pre-CMake & for the kernal. Tree-SHA512: 715a32ec190c70505ffbce70bfe81fc7b6aa33e376b60292e801f60cf17025aabfcab4e8c53ebb2e28ffc5cf4c20b74fe3dd8548371ad772085c13aec8b7970e
2024-03-01[unit test] sibling not returned from SingleV3Checks if 1p2c or 3genglozow
2024-03-01[policy] sibling eviction for v3 transactionsglozow
2024-03-01Merge bitcoin/bitcoin#29495: fuzz: add target for local address stufffanquake
25eab523897e790f4f4d7b49cdbf19d13e3b0fcc fuzz: add target for local addresses (brunoerg) Pull request description: This PR adds fuzz target for local address functions - (`AddLocal`, `RemoveLocal`, `SeenLocal`, `IsLocal`) ACKs for top commit: dergoegge: ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc vasild: ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc Tree-SHA512: 24faaab86dcd8835ba0e2d81fb6322a39a9266c7edf66415dbc4421754054f47efb6e0de4efdc7ea026b0686792658e86a526f7cf27cbc6cf9ed0c4aed376f97
2024-02-29p2p: Don't consider blocks mutated if they don't connect to known prev blockGreg Sanders
2024-02-29Merge bitcoin/bitcoin#29407: build: remove confusing and inconsistent ↡fanquake
disable-asm option f8a06f7a02be83e9b76a1b31f1b66a965dbedfce doc: remove references to disable-asm option now that it's gone (Cory Fields) 376f0f6d0798c10f09266d609afea3ada1b99f9b build: remove confusing and inconsistent disable-asm option (Cory Fields) Pull request description: 1. It didn't actually disable asm usage in our code. Regardless of the setting, asm is used in random.cpp and support/cleanse.cpp. 2. The value wasn't forwarded to libsecp as a user might have reasonably expected. 3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm actually did in practice. If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new configure option that actually does what it says. Additionally, this is one of the last (THE last?) remaining uses of autoconf defines in our crypto code. As such it seems like low-hanging fruit. ACKs for top commit: fanquake: ACK f8a06f7a02be83e9b76a1b31f1b66a965dbedfce Tree-SHA512: 4a99c2130225acbe9dc7399ed572a04ca155cbfa3eef8178a632ba533017d264691e6482cceb1d8f9c5d768619d99a2466dea4b82b27b18b872bceae91b92fbb
2024-02-29Merge bitcoin/bitcoin#29516: test: removes unnecessary check from ↡fanquake
validation_tests 6ee3997d03e456655e3c44abf1e15270c423ed41 test: removes unnecessary check from validation_tests (Sergi Delgado Segura) Pull request description: An unnecessary check was added to the block mutation tests in #29412 where IsBlockMutated is returning true for the invalid reasons: we try to check mutation via transaction duplication, but the merkle root is not updated before the check, therefore the check fails because the provided root and the computed root differ, but not because the block contains the same transaction twice. Notice that a proper check to test the duplication case is added a few lines later, so this check is just meaningless and can be removed. Check https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1506490281 for context. ACKs for top commit: maflcko: ACK 6ee3997d03e456655e3c44abf1e15270c423ed41 dergoegge: utACK 6ee3997d03e456655e3c44abf1e15270c423ed41 BrandonOdiwuor: utACK 6ee3997d03e456655e3c44abf1e15270c423ed41 Tree-SHA512: e4627668091dda5f589e4c15edac39dc84aabc9b34b8f7fadbf512beb7111d5477e1b69567a34b4a657e48ba66dfb864db5ff37c9bbe3ff24cd32931b2dd89e6
2024-02-29build: remove confusing and inconsistent disable-asm optionCory Fields
1. It didn't actually disable asm usage in our code. Regardless of the setting, asm is used in random.cpp and support/cleanse.cpp. 2. The value wasn't forwarded to libsecp as a user might have reasonably expected. 3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm actually did in practice. If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new configure option that actually does what it says.
2024-02-29Merge bitcoin/bitcoin#29510: wallet: `getrawchangeaddress` and ↡Ava Chow
`getnewaddress` failures should not affect keypools for descriptor wallets e073f1dfda7a2a2cb2be9fe2a1d576f122596021 test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6) 367bb7a80cc71130995672c853d4a6e0134721d6 wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6) Pull request description: I think the expected behaviour of `getrawchangeaddress` and `getnewaddress` RPCs is that their failure should not affect keypool in any way. At least that's how legacy wallets work, you can confirm this behaviour by running `wallet_keypool.py --legacy-wallet` on master with e073f1dfda7a2a2cb2be9fe2a1d576f122596021 applied on top. However running `wallet_keypool.py --descriptors` on the same commit results in the following failure: ``` File "/path/to/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/path/to/bitcoin/test/functional/wallet_keypool.py", line 114, in run_test assert_equal(kp_size_before, kp_size_after) File "/path/to/bitcoin/test/functional/test_framework/util.py", line 57, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not([18, 24] == [19, 24]) ``` This happens because we pass `nIndex` (which is a class member) into `GetReservedDestination` and since it's passed by reference we get an updated value back, so `nIndex` won't be equal `-1` anymore, no matter if the function failed or succeeded. This means that `ReturnDestination` (called by dtor of `ReserveDestination`) will try to return something we did not actually reserve. The fix is to simply use a temporary variable instead of a class member and only update `nIndex` when `op_address` actually has value, basically do it the same way we do for other class members (`address` and `fInternal`) already. ACKs for top commit: achow101: ACK e073f1dfda7a2a2cb2be9fe2a1d576f122596021 josibake: ACK https://github.com/bitcoin/bitcoin/pull/29510/commits/e073f1dfda7a2a2cb2be9fe2a1d576f122596021 Tree-SHA512: 1128288a60dd4d8f306ef6f7ac66cdfeae3c9cc35c66ecada2d78fa61ac759f2a757b70fc3976ba8b5081200942b58dfabc184c01ccf911af40ba8c145344651
2024-02-29refactor: gui, simplify boost signals disconnectionfurszy
Preventing dangling signals.
2024-02-29fuzz: add target for local addressesbrunoerg
2024-02-29test: removes unnecessary check from validation_testsSergi Delgado Segura
An unnecessary check was added to the block mutation tests in #29412 where IsBlockMutated is returning true for the invalid reasons: we try to check mutation via transaction duplication, but the merkle root is not updated before the check, therefore the check fails because the provided root and the computed root differ, but not because the block contains the same transaction twice. The check is meaningless so it can be removed.
2024-02-28Merge bitcoin/bitcoin#29412: p2p: Don't process mutated blocksAva Chow
d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc [test] IsBlockMutated unit tests (dergoegge) 1ed2c9829700054526156297552bb47230406098 Add transaction_identifier::size to allow Span conversion (dergoegge) 1ec6bbeb8d27d31647d1433ccb87b362f6d81f90 [validation] Cache merkle root and witness commitment checks (dergoegge) 5bf4f5ba32da4627f152b54d266df9b2aa930457 [test] Add regression test for #27608 (dergoegge) 49257c0304828a185c273fcb99742c54bbef0c8e [net processing] Don't process mutated blocks (dergoegge) 2d8495e0800f5332748cd50eaad801ff77671bba [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge) 66abce1d98115e41f394bc4f4f52341960ddc839 [validation] Introduce IsBlockMutated (dergoegge) e7669e1343aec4298fd30d539847963e6fa5619c [refactor] Cleanup merkle root checks (dergoegge) 95bddb930aa72edd40fdff52cf447202995b0dce [validation] Isolate merkle root checks (dergoegge) Pull request description: This PR proposes to check for mutated blocks early as a defense-in-depth mitigation against attacks leveraging mutated blocks. We introduce `IsBlockMutated` which catches all known forms of block malleation and use it to do an early mutation check whenever we receive a `block` message. We have observed attacks that abused mutated blocks in the past, which could have been prevented by simply not processing mutated blocks (e.g. https://github.com/bitcoin/bitcoin/pull/27608 for which a regression test is included in this PR). ACKs for top commit: achow101: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc maflcko: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc πŸ„ fjahr: Code review ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc sr-gi: Code review ACK https://github.com/bitcoin/bitcoin/commit/d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc Tree-SHA512: 618ff4ea7f168e10f07504d3651290efbb1bb2ab3b838ffff3527c028caf6c52dedad18d04d3dbc627977479710930e200f2dfae18a08f627efe7e64a57e535f
2024-02-28Merge bitcoin/bitcoin#29489: test: Remove Windows-specific code from ↡fanquake
`system_tests/run_command` 51bc1c7126d6e130bc40c529fb71ae6486da0492 test: Remove Windows-specific code from `system_tests/run_command` (Hennadii Stepanov) Pull request description: The removed code has been dead since https://github.com/bitcoin/bitcoin/pull/28967. Required as a precondition for replacing Boost.Process with [cpp-subprocess](https://github.com/bitcoin/bitcoin/pull/28981) to make diff for this code meaningful and reviewable. The plan is to reintroduce Windows-specific code in this test simultaneously with enabling Windows support in cpp-subprocess. ACKs for top commit: Sjors: utACK 51bc1c7126d6e130bc40c529fb71ae6486da0492 theStack: Code-review ACK 51bc1c7126d6e130bc40c529fb71ae6486da0492 Tree-SHA512: 0e3875c4dc20564332555633daf2227223b10dc3d052557635eced2734575d1e0252fb19e46ea6e6c47a15c51c345f70b6d437e33435abcd0e4fcf29edb50887
2024-02-28Merge bitcoin/bitcoin#29493: subtree: update crc32c subtreefanquake
5d45552fd4303f8d668ffbc50cce1053485aeead Squashed 'src/crc32c/' changes from 0bac72c455..b60d2b7334 (fanquake) Pull request description: Update the crc32c subtree. Includes: * https://github.com/bitcoin-core/crc32c-subtree/pull/6 Which fixes #29178. ACKs for top commit: hebasto: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d. theuni: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d dergoegge: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d Tree-SHA512: 2cec81a34ad26bbbc298aea5daffa41e56114d31cc2eb5fe486f46a77c3467bba22bdeca1c52ae97220e119d98818304272fc6337442af55282accabcd4c5833