aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-03-26Merge bitcoin/bitcoin#29695: guix: build GCC with ↵fanquake
--enable-standard-branch-protection 7850c5fe20a034438e00f6c12ce51efc6af3a1aa guix: build GCC with --enable-standard-branch-protection (fanquake) Pull request description: This is one change extracted from #24123 (which now produces fully BTI & PAC enabled bins), which will mean that everything in depends, for Guix builds, is compiled using `-mbranch-protection=standard`. Turning this on by default, is similar to what we already do with `--enable-default-ssp`, `--enable-default-pie` etc. See: https://gcc.gnu.org/install/specific.html#aarch64-x-x > To enable Branch Target Identification Mechanism and Return Address Signing by default at configure time use the `--enable-standard-branch-protection` option. > This is equivalent to having `-mbranch-protection=standard` during compilation. This can be explicitly disabled during compilation by passing the `-mbranch-protection=none` option which turns off all types of branch protections. ACKs for top commit: TheCharlatan: ACK 7850c5fe20a034438e00f6c12ce51efc6af3a1aa Tree-SHA512: 18f898da27021bab502e708ea5fa9b325352f8f6e23d9488a2a0feda87e0af2ac0e4f87b3af9ad6a9a37bbfc99ab0285de4f0bdc174dcd38163d92c122e958e2
2024-03-26Merge bitcoin/bitcoin#29722: 28950 followupsglozow
7b29119d79efbc8c4148f350cc86531fde8b7251 use const ref for client_maxfeerate (Greg Sanders) f10fd07320da302e8d038213c85e2b16e77d5dc2 scripted-diff: Rename max_sane_feerate to client_maxfeerate (Greg Sanders) Pull request description: Follow-ups to https://github.com/bitcoin/bitcoin/pull/28950 ACKs for top commit: glozow: utACK 7b29119d79efbc8c4148f350cc86531fde8b7251 stickies-v: ACK 7b29119d79efbc8c4148f350cc86531fde8b7251 Tree-SHA512: b9e13509c6e9d7c08aa9d4e759f9707004c1c7b8f3e521fe2ec0037160b87c7fb02528966b9f26eaca6291621df9300e84b5aec66dbc2e97d13bf2f3cd7f979c
2024-03-26Merge bitcoin/bitcoin#29242: Mempool util: Add RBF diagram checks for single ↵glozow
chunks against clusters of size 2 72959867784098137a50c34f86deca8235eef4f8 Unit tests for CalculateFeerateDiagramsForRBF (Greg Sanders) b767e6bd47cb0fb8f7aea3fb10c597e59a35bf74 test: unit test for ImprovesFeerateDiagram (Greg Sanders) 7e89b659e1ddd0c04fa2bddba9706b5d1a1daec3 Add fuzz test for FeeFrac (Greg Sanders) 4d6528a3d6bf3821c216c68f99170e2faab5d63c fuzz: fuzz diagram creation and comparison (Greg Sanders) e9c5aeb11d641b8cae373452339760809625021d test: Add tests for CompareFeerateDiagram and CheckConflictTopology (Greg Sanders) 588a98dccc5dbb6e331f28d83a4a10a13d70eb31 fuzz: Add fuzz target for ImprovesFeerateDiagram (Greg Sanders) 2079b80854e2595f6f696e7c13a56c7f2a7da9f4 Implement ImprovesFeerateDiagram (Greg Sanders) 66d966dcfaad3638f84654e710f403cb0a0a2ac7 Add FeeFrac unit tests (Greg Sanders) ce8e22542ed0b4fa5794d3203207146418d59473 Add FeeFrac utils (Greg Sanders) Pull request description: This is a smaller piece of https://github.com/bitcoin/bitcoin/pull/28984 broken off for easier review. Up to date explanation of diagram checks are here: https://delvingbitcoin.org/t/mempool-incentive-compatibility/553 This infrastructure has two near term applications prior to cluster mempool: 1) Limited Package RBF(https://github.com/bitcoin/bitcoin/pull/28984): We want to allow package RBF only when we know it improves the mempool. This narrowly scoped functionality allows use with v3-like topologies, and will be expanded at some point post-cluster mempool when diagram checks can be done efficiently against bounded cluster sizes. 2) Replacement for single tx RBF(in a cluster size of up to two) against conflicts of up to cluster size two. `ImprovesFeerateDiagram` interface will have to change for this use-case, which is a future direction to solve certain pins and improve mempool incentive compatibility: https://delvingbitcoin.org/t/ephemeral-anchors-and-mev/383#diagram-checks-fix-this-3 And longer-term, this would be the proposed way we would compute incentive compatibility for all conflicts, post-cluster mempool. ACKs for top commit: sipa: utACK 72959867784098137a50c34f86deca8235eef4f8 glozow: code review ACK 72959867784098137a50c34f86deca8235eef4f8 murchandamus: utACK 72959867784098137a50c34f86deca8235eef4f8 ismaelsadeeq: Re-ACK https://github.com/bitcoin/bitcoin/commit/72959867784098137a50c34f86deca8235eef4f8 willcl-ark: crACK 72959867784098137a50c34f86deca8235eef4f8 sdaftuar: ACK 72959867784098137a50c34f86deca8235eef4f8 Tree-SHA512: 79593e5a087801c06f06cc8b73aa3e7b96ab938d3b90f5d229c4e4bfca887a77b447605c49aa5eb7ddcead85706c534ac5eb6146ae2396af678f4beaaa5bea8e
2024-03-25Merge bitcoin/bitcoin#28928: test: add coverage for bech32m in ↵Ava Chow
`wallet_keypool_topup` a8bfc3dea1d986b458202bf5e49cf1944392d676 test: add coverage for bech32m in `wallet_keypool_topup` (brunoerg) Pull request description: 0dcac51049cdd924a50d62629757effc8d542046 added coverage for all keypool addresses types in `wallet_keypool_topup` (4y ago). Now we have bech23m, so this PR adds it. ACKs for top commit: achow101: ACK a8bfc3dea1d986b458202bf5e49cf1944392d676 marcofleon: ACK a8bfc3dea1d986b458202bf5e49cf1944392d676. Definitely a more straightfoward addition to the test. Reviewed the code, built the PR branch and ran all functional tests without issues. furszy: utACK a8bfc3dea Tree-SHA512: aa830b723a7a54b23744f9fb3cf5214452c4ffc8e3bbe0e8bd980bdf902e61c3dd2fd57361b82c5c0c5224aa0774158daf34b6b2188edda0a971f82111976051
2024-03-25Merge bitcoin/bitcoin#29706: depends: set two CMake options globallyfanquake
76045bb9d6808931cd0f2933203b5b611e032ec8 depends: always set CMAKE_POSITION_INDEPENDENT_CODE=ON (fanquake) d04623678c70ff58a20fb5c35d33cb8f483f1efb depends: always set CMAKE_INSTALL_LIBDIR=lib/ (fanquake) Pull request description: Set `CMAKE_INSTALL_LIBDIR=lib/` and `CMAKE_POSITION_INDEPENDENT_CODE=ON` globally in depends, rather than per-package. `CMAKE_INSTALL_LIBDIR=lib/` is needed to override the annoying [`GNUInstallDirs`](https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html) `lib` vs `lib64` behaviour, and we always want PIC code. The PIC commit is the counterpart to the same Autotools change in #29488. I'm PRing these commits as I have a CMake branch building on top, and want to avoid adding the same workarounds to every package we are going to touch, but these can go in separately as the build should be tested for existing packages (i.e multiprocess). ACKs for top commit: hebasto: re-ACK 76045bb9d6808931cd0f2933203b5b611e032ec8. theuni: utACK 76045bb9d6808931cd0f2933203b5b611e032ec8. Both changes make sense to me, and both can be overridden if needed, though I can't imagine we'd need to. Tree-SHA512: 655a0b6b7ee5a5820f52e8e919ef03fc216d29f13f3904f72b64ce57436510e073c903039488d5740535c56e1f6221267229238c5231de5f8467d238fd562578
2024-03-25Merge bitcoin/bitcoin#29718: Correct '-dbcache' to '-prune'fanquake
416b9d9427c017fe7eb2975ca76f6a89ba24ab58 correct '-dbcache' to '-prune' (Ben Westgate) Pull request description: This looks like a typo, '-prune' is doubled not the '-dbcache' by my understanding. ACKs for top commit: Sjors: ACK 416b9d9427c017fe7eb2975ca76f6a89ba24ab58 fjahr: ACK 416b9d9427c017fe7eb2975ca76f6a89ba24ab58 Tree-SHA512: d954a8976f6e43e0af9bc2ba89fec1a1ccc90483cc005af3d2810379bd6bea7dbd87766af71c2a45b3d751a3f682f0f67a4525ec350bf6163bc4bb88fab6e4cb
2024-03-25use const ref for client_maxfeerateGreg Sanders
2024-03-25scripted-diff: Rename max_sane_feerate to client_maxfeerateGreg Sanders
-BEGIN VERIFY SCRIPT- git grep -l 'max_sane_feerate' | xargs sed -i 's/max_sane_feerate/client_maxfeerate/g' -END VERIFY SCRIPT-
2024-03-25Merge bitcoin/bitcoin#29660: lint: Fix COMMIT_RANGE issuesfanquake
fa1146d01b148dd60fcada36a3b37ed37532ce2b lint: Fix COMMIT_RANGE issues (MarcoFalke) Pull request description: `COMMIT_RANGE` has problems on forks or local branches: * When `LOCAL_BRANCH` is set, it assumes the presence of a `master` branch, and that the `master` branch is up-to-date. Both of which may be false. (See also discussion in https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1504226422) * When `COMMIT_RANGE` isn't set in `lint-git-commit-check.py`, and `--prev-commits` isn't set either, it has the same (broken) assumptions. Fix all issues by simply assuming a merge commit exists. This allows to drop `LOCAL_BRANCH`. It also allows to drop `SKIP_EMPTY_NOT_A_PR`, because scripts will already skip an empty range. Finally, it allows to drop `--prev-commits n`, because one can simply say `COMMIT_RANGE='HEAD~n..HEAD'` to achieve the same. ACKs for top commit: Sjors: tACK fa1146d01b148dd60fcada36a3b37ed37532ce2b Tree-SHA512: f1477a38267fd4fdb8d396211a5d6bed5f418798c7edaba43487957aaf726cd45244ccf15187b3dd896d398fa1df3fe0a37323e49cf44d60a2018786ed41e5ba
2024-03-25Merge bitcoin/bitcoin#29696: tidy: remove todo, set minimum CMake to 3.22fanquake
11ee058ef5794de5f1b8e89d62bfa69c64693fff tidy: remove C compiler check (fanquake) c3a4ea19715de292517b932d0a3b24ace72e9919 tidy: set CMAKE_CXX_STANDARD to 20 (fanquake) 5b690aeb1583e207b083e83b8d882f7d1c2d2683 tidy: remove terminfo TODO (fanquake) 24410e560ac9add5dbae424964bc96554e6fd1a9 tidy: set minimum CMake to 3.22 (fanquake) Pull request description: See https://github.com/hebasto/bitcoin/pull/123 for the minimum version bump. ACKs for top commit: hebasto: re-ACK 11ee058ef5794de5f1b8e89d62bfa69c64693fff. Tree-SHA512: 94a508ea24bf17919961bbdbc2e9d17658858e179c3b2017d5932557af32530d9d6aab197453aa5444f5478c417129c5a8e39522ff82bafac0d5a6966c7246d3
2024-03-25depends: always set CMAKE_POSITION_INDEPENDENT_CODE=ONfanquake
Rather than potentially having to set this per-package, set it globally, as this should always be what we want. Without doing this, changes in later commits will have to add this per-package. Similar to https://github.com/bitcoin/bitcoin/pull/29488, which is the Autotools equivalent.
2024-03-25depends: always set CMAKE_INSTALL_LIBDIR=lib/fanquake
Rather than setting this per package, set it globally, as this is always what we want. Without doing this, later commit will have to add the same doc + change to more packages.
2024-03-25Merge bitcoin/bitcoin#29488: depends: always configure with `--with-pic`fanquake
e037c4fe0914d8fa9149ce7532c0d70f738e79e9 depends: always configure with --with-pic (fanquake) Pull request description: We currently do this sporadically. Not only amongst packages, but across OS's, i.e sometimes it's done for BSDs/Android, and sometimes not. Configure with `--with-pic` globally instead. I think this generally makes more sense, and should not have any downsides. See related discussion in https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399123100. ACKs for top commit: hebasto: ACK e037c4fe0914d8fa9149ce7532c0d70f738e79e9. Tree-SHA512: efc743ff92f9f99f3ac16514e98363ad395c6f956cd4be7e785b5c573685baf7fcd68c51d6a705ee8761fc676eb045b7e61676595be0eb0f70f34e99174cddc0
2024-03-25Merge bitcoin/bitcoin#29636: test: #29007 follow upsfanquake
9a44a20fb790f3be5d5d5d8f5d0f48aac633b2a4 init: clarify -test error (0xb10c) 3047c3e3a99112c38f118034daa672db70fa4a60 addrman: drop /*deterministic=*/ comment (0xb10c) 89b84ea91ae40876a52879c509c63d0bacbfaade test: check that addrman seeding is successful (0xb10c) Pull request description: A few, small follow-ups to #29007. See commit messages for details. ACKs for top commit: maflcko: lgtm ACK 9a44a20fb790f3be5d5d5d8f5d0f48aac633b2a4 stratospher: tested ACK 9a44a20. mzumsande: Code Review ACK 9a44a20fb790f3be5d5d5d8f5d0f48aac633b2a4 Tree-SHA512: 987245e035da08fa7fe541a1dc3b7c2d90f703a6f9813875048d286335c63ffa5201db702a3f368087c00fa02c3fdafb06cf54dc7a92922749a94588b1500e98
2024-03-25correct '-dbcache' to '-prune'Ben Westgate
2024-03-23init: clarify -test error0xb10c
See https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1469388717
2024-03-23addrman: drop /*deterministic=*/ comment0xb10c
Just having deterministic is enough. See https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1488241966
2024-03-23test: check that addrman seeding is successful0xb10c
The addpeeraddress calls can fail due to collisions. As we are using a deteministic addrman, they won't fail with the current bucket/position calculation. However, if the calculation is changed, they might collide and fail silently causing tests using `seed_addrman()` to fail. Assert that the addpeeraddress calls are successful.
2024-03-22Merge bitcoin/bitcoin#29672: validation: Make translations of fatal errors ↵Ava Chow
consistent 824f47294a309ba8e58ba8d1da0af15d8d828f43 node: Use log levels in noui_ThreadSafeMessageBox (TheCharlatan) ddc7872c08b7ddf9b1e83abdb97c21303f4a9172 node: Make translations of fatal errors consistent (TheCharlatan) Pull request description: The extra `bilingual_str` argument of the fatal error notifications and `node::AbortNode()` is often unused and when used usually contains the same string as the message argument. It also seems to be confusing, since it is not consistently used for errors requiring user action. For example some assumeutxo fatal errors require the user to do something, but are not translated. So simplify the fatal error and abort node interfaces by only passing a translated string. This slightly changes the fatal errors displayed to the user. ACKs for top commit: stickies-v: re-ACK 824f47294a309ba8e58ba8d1da0af15d8d828f43 maflcko: ACK 824f47294a309ba8e58ba8d1da0af15d8d828f43 🔎 achow101: ACK 824f47294a309ba8e58ba8d1da0af15d8d828f43 hebasto: re-ACK 824f47294a309ba8e58ba8d1da0af15d8d828f43. Tree-SHA512: 2868ee7b045fe7f3ac582ce5039141b398480b7627734976201dafaaef7544b8461635a7292fee4a7f32ff1bfc26f9bd4d0c292dca424ba42fb7fc4483d7ce8d
2024-03-22Merge bitcoin/bitcoin#28998: rpc: "addpeeraddress tried" return error on failureAva Chow
99954f914f031c80aa53daa367fc049c4c55bdf3 test: fix test to ensure hidden RPC is present in detailed help (stratospher) 0d01f6f0c6e53c9765f84e0616ab46b83923a6ad test: remove unused mocktime in test_addpeeraddress (0xb10c) 6205466512d4b94d1e507a77ab2151425790d29f rpc: "addpeeraddress tried" return error on failure (0xb10c) Pull request description: When trying to add an address to the IP address manager tried table, it's first added to the new table and then moved to the tried table. Previously, adding a conflicting address to the address manager's tried table with test-only `addpeeraddress tried=true` RPC would return `{ "success": true }`. However, the address would not be added to the tried table, but would remain in the new table. This caused, e.g., issue #28964. This is fixed by new returning `{ "success": false, "error": "..." }` for failed tried table additions. Since the address remaining in the new table can't be removed (the address manager interface does not support removing addresses at the moment and adding this seems to be a bigger effort), an error message is returned. This indicates to a user why the RPC failed and allows accounting for the extra address in the new table. This is done in the functional test for the `getrawaddrman` RPC. Fixes #28964 ACKs for top commit: achow101: ACK 99954f914f031c80aa53daa367fc049c4c55bdf3 stratospher: reACK 99954f9. 🚀 brunoerg: utACK 99954f914f031c80aa53daa367fc049c4c55bdf3 Tree-SHA512: 2f1299410c0582ebc2071271ba789a8abed905f9a510821f77afbcf2a555ec31397578ea55cbcd162fb828be27afedd3246c7b13ad8883f2f745bb8e04364a76
2024-03-22Merge bitcoin/bitcoin#29704: test: make p2p_handshake robust against ↵fanquake
timeoffset warnings 032a59748295859845b2a9181ceb1c4ae70bae5c test: make p2p_handshake robust against timeoffset warnings (stickies-v) Pull request description: The new `p2p_handshake` test requires that limited nodes are not peered with when the node's system time exceeds ~ 24h of the node's chaintip timestamp, as per [`PeerManagerImpl::GetDesirableServiceFlags`](https://github.com/bitcoin/bitcoin/blob/2ffaa927023f5dc2a7b8d6cfeb4f4810e573b18c/src/net_processing.cpp#L1717). By patching this test to modify the timestamp of the chaintip as opposed to mocking the node's system time, we make it resilient to future commits where the node raises a warning if it detects its system time is too much out of sync with its outbound peers. Resolves a silent merge conflict in https://github.com/bitcoin/bitcoin/pull/29623, that is changing the warning behaviour when significant time differences with outbound peers are detected, [failing the test as it's currently in master](https://cirrus-ci.com/task/6553996884705280?logs=ci#L4666). Considerations/alternatives I've thought of: - could add `self.setup_clean_chain = True` to `self.set_test_params()`, to avoid creating a new tip with a (much) older date, but it doesn't seem to matter? - could avoid using `setmocktime` altogether and instead use [`create_block`](https://github.com/bitcoin/bitcoin/blob/2ffaa927023f5dc2a7b8d6cfeb4f4810e573b18c/test/functional/test_framework/blocktools.py#L68) instead, but that seems like it'll be a lot more verbose and I don't think it's worth it? Big thanks to theStack for his time in discussing this with me offline. ACKs for top commit: maflcko: lgtm ACK 032a59748295859845b2a9181ceb1c4ae70bae5c theStack: ACK 032a59748295859845b2a9181ceb1c4ae70bae5c brunoerg: crACK 032a59748295859845b2a9181ceb1c4ae70bae5c BrandonOdiwuor: Code Review ACK 032a59748295859845b2a9181ceb1c4ae70bae5c Tree-SHA512: 407564754a100bc9252f5737182de2e111993944ec9a0463a4a43195ce98cd1119de982c8fe5f7531ddb56603043812bf7bf2163a780d30b6df03a072c3308c3
2024-03-22Merge bitcoin/bitcoin#29647: Avoid divide-by-zero in header sync logs when ↵Ava Chow
NodeClock is behind fa4d98b3c8e63f20c6f366fc9382039ba52614a4 Avoid divide-by-zero in header sync logs when NodeClock is behind (MarcoFalke) fa58550317c633c411009c1cc8fb692e3baf97e8 refactor: Modernize header sync logs (MarcoFalke) Pull request description: The log may be confusing, when the NodeClock is behind the current header tip. Fix it, by assuming the NodeClock is never behind the current header tip. ACKs for top commit: sipa: utACK fa4d98b3c8e63f20c6f366fc9382039ba52614a4 sr-gi: tACK [fa4d98b](https://github.com/bitcoin/bitcoin/pull/29647/commits/fa4d98b3c8e63f20c6f366fc9382039ba52614a4) achow101: ACK fa4d98b3c8e63f20c6f366fc9382039ba52614a4 tdb3: ACK fa4d98b3c8e63f20c6f366fc9382039ba52614a4 Tree-SHA512: 3c5aee4030af387695918c5238012c972ebf850b52e956b5f74590cd7fd4eff0b3e593d411e3eb2a0bb12294af8dc6fbe320f90e4c261399b65a404ff3c3cbd9
2024-03-22test: make p2p_handshake robust against timeoffset warningsstickies-v
The test requires that limited nodes are not peered with when the node's system time exceeds ~ 24h of the node's chaintip timestamp, as per PeerManagerImpl::GetDesirableServiceFlags. By patching this test to modify the timestamp of the chaintip as opposed to mocking the node's system time, we make it resilient to future commits where the node raises a warning if it detects its system time is too much out of sync with its outbound peers. See https://github.com/bitcoin/bitcoin/pull/29623
2024-03-22tidy: remove C compiler checkfanquake
Also requires disabling FFI.
2024-03-22tidy: set CMAKE_CXX_STANDARD to 20fanquake
2024-03-22tidy: remove terminfo TODOfanquake
At the same time, also disable searching for CURL, LibEdit, LibXml2, ZLIB and zstd none of which we use.
2024-03-22tidy: set minimum CMake to 3.22fanquake
Matches https://github.com/hebasto/bitcoin/pull/123. This also also dev/ci only code.
2024-03-22Merge bitcoin/bitcoin#29703: doc: Rename ↵fanquake
`contrib/devtools/bitcoin-tidy/README` to `README.md` 669ea0aa4adb2875a26cd35463d48b857b366a60 doc: Rename `contrib/devtools/bitcoin-tidy/README` to `README.md` (Hennadii Stepanov) Pull request description: This PR fixes the file formatting on the GitHub website. Before: ![image](https://github.com/bitcoin/bitcoin/assets/32963518/e81a61f0-f18c-4917-ae79-d35807e91fa9) After: ![image](https://github.com/bitcoin/bitcoin/assets/32963518/d2a96317-06d6-4185-b0e8-5e62c75f66b5) ACKs for top commit: maflcko: lgtm ACK 669ea0aa4adb2875a26cd35463d48b857b366a60 Tree-SHA512: 3dfa07a482b5447b6f423946cc463648abbedf1012f2da246b8ce16e6ae3895cdbf956fb2db78735f3e0d88473b9b32f226f2557e0004a0adb69b00f2426a8ca
2024-03-22doc: Rename `contrib/devtools/bitcoin-tidy/README` to `README.md`Hennadii Stepanov
This change fixes the file formatting on the GitHub website.
2024-03-21lint: Fix COMMIT_RANGE issuesMarcoFalke
2024-03-21guix: build GCC with --enable-standard-branch-protectionfanquake
To enable Branch Target Identification Mechanism and Return Address Signing by default at configure time use the `--enable-standard-branch-protection` option. This is equivalent to having `-mbranch-protection=standard` during compilation. This can be explicitly disabled during compilation by passing the `-mbranch-protection=none` option which turns off all types of branch protections. See: https://gcc.gnu.org/install/specific.html#aarch64-x-x
2024-03-21node: Use log levels in noui_ThreadSafeMessageBoxTheCharlatan
2024-03-21node: Make translations of fatal errors consistentTheCharlatan
The extra `bilingual_str` argument of the fatal error notifications and `node::AbortNode()` is often unused and when used usually contains the same string as the message argument. It also seems to be confusing, since it is not consistently used for errors requiring user action. For example some assumeutxo fatal errors require the user to do something, but are not translated. So simplify the fatal error and abort node interfaces by only passing a translated string. This slightly changes the fatal errors displayed to the user. Also de-duplicate the abort error log since it is repeated in noui.cpp.
2024-03-21Merge bitcoin/bitcoin#29651: guix: bump time-machine to ↵fanquake
dc4842797bfdc5f9f3f5f725bf189c2b68bd6b5a cf5faf73c99199e7476b8c86358095300544d1bd guix: bump time-machine to dc4842797bfdc5f9f3f5f725bf189c2b68bd6b5a (fanquake) Pull request description: This includes a commit to fix building LLVM 17 on riscv64, see https://git.savannah.gnu.org/cgit/guix.git/commit/?id=4e26331a5ee87928a16888c36d51e270f0f10f90. Followup to discussion in https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1843313196. If you don't have riscv64 hardware, this can be tested with the following: ```bash # observe failure when cross-compiling using our current time-machine guix time-machine --commit=d5ca4d4fd713a9f7e17e074a1e37dda99bbb09fc -- build --target=riscv64-linux-gnu llvm .... riscv64-linux-gnu-ld: CMakeFiles/dsymutil.dir/dsymutil.cpp.o: undefined reference to symbol '__atomic_fetch_and_1@@LIBATOMIC_1.0' riscv64-linux-gnu-ld: /gnu/store/i4ga0pnr1b74bir2bjyp8mcrrbsvk7d3-gcc-cross-riscv64-linux-gnu-11.3.0-lib/riscv64-linux-gnu/lib/libatomic.so.1: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status # build success when using the new time-machine guix time-machine --commit=dc4842797bfdc5f9f3f5f725bf189c2b68bd6b5a -- build --target=riscv64-linux-gnu llvm .... grafting '/gnu/store/7y0j0y8jaz4mjx2nz0y42wdnxxjp6id6-llvm-17.0.6-opt-viewer' -> '/gnu/store/8xvahrrjscbprh6cjj0qp5bm9mm78wwa-llvm-17.0.6-opt-viewer'... grafting '/gnu/store/bjhw648bz7ijd2p9hgzzdbw1q8hpagk8-llvm-17.0.6' -> '/gnu/store/x50qi8i2ywgpx6azv4k55ms0w5xjxxg5-llvm-17.0.6'... successfully built /gnu/store/q9xvk8gzzvb4dxfzf6yi5164zd0d1vj2-llvm-17.0.6.drv ``` Also includes at least: Linux Headers 6.1.67 -> 6.1.80 ACKs for top commit: TheCharlatan: ACK cf5faf73c99199e7476b8c86358095300544d1bd hebasto: ACK cf5faf73c99199e7476b8c86358095300544d1bd, tested on x86_64 hardware as described in the PR description. Tree-SHA512: b49d4f90effeec666b12b5447a24c90315b82675cfc166bc1230ac173134bab6b277fc7e064bbb75e990275165b2b27d88e4ec1cdeea4750541ec6443cb50f41
2024-03-20Merge bitcoin/bitcoin#29671: index: avoid "failed to commit" errors on ↵Ava Chow
initialization f65b0f6401091e4a4ca4c9f4db1cf388f0336bad index: Move last_locator_write_time and logging to end of threadsync loop (Fabian Jahr) Pull request description: In the index sync thread, when initializing an index for the first time, stop callng BaseIndex::Commit when m_best_block_index is null, to avoid a spurious "failed to commit" error from that function. This error started happening in commit https://github.com/bitcoin/bitcoin/commit/7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd from https://github.com/bitcoin/bitcoin/pull/25494 and was reported by pstratem in https://github.com/bitcoin/bitcoin/pull/26903 with an alternate fix. ACKs for top commit: achow101: ACK f65b0f6401091e4a4ca4c9f4db1cf388f0336bad ryanofsky: Code review ACK f65b0f6401091e4a4ca4c9f4db1cf388f0336bad. Just moved log "Syncing" log line since last commit to avoid having to call now() twice. furszy: ACK f65b0f6401091e4a4ca4c9f4db1cf388f0336bad TheCharlatan: ACK f65b0f6401091e4a4ca4c9f4db1cf388f0336bad Tree-SHA512: afa8f05786318d36346d167ff53ea0b3bc8abdb0ad04465d199dc3eb91e9f837369e24fcb7e24b5757b02d698ec504e61da6ac365eaf006c874fc07a424a7e20
2024-03-20Merge bitcoin/bitcoin#29370: assumeutxo: Get rid of faked nTx and nChainTx ↵Ava Chow
values 9d9a7458a2570f7db56ab626b22010591089c312 assumeutxo: Remove BLOCK_ASSUMED_VALID flag (Ryan Ofsky) ef174e9ed21c08f38e5d4b537b6decfd1f646db9 test: assumeutxo snapshot block CheckBlockIndex crash test (Ryan Ofsky) 0391458d767b842a7925785a7053400c0e1cb55a test: assumeutxo stale block CheckBlockIndex crash test (Ryan Ofsky) ef29c8b662309a438121a83f27fd7bdd1779700c assumeutxo: Get rid of faked nTx and nChainTx values (Ryan Ofsky) 9b97d5bbf980d657a277c85d113c2ae3e870e0ec doc: Improve comments describing setBlockIndexCandidates checks (Ryan Ofsky) 0fd915ee6bef63bb360ccc5c039a3c11676c38e3 validation: Check GuessVerificationProgress is not called with disconnected block (Ryan Ofsky) 63e8fc912c21a2f5b47e8eab10fb13c604afed85 ci: add getchaintxstats ubsan suppressions (Ryan Ofsky) f252e687ec94b6ccafb5bc44b7df3daeb473fdea assumeutxo test: Add RPC test for fake nTx and nChainTx values (Ryan Ofsky) Pull request description: The `PopulateAndValidateSnapshot` function introduced in f6e2da5fb7c6406c37612c838c998078ea8d2252 from #19806 has been setting fake `nTx` and `nChainTx` values that can show up in RPC results (https://github.com/bitcoin/bitcoin/issues/29328) and make `CBlockIndex` state hard to reason about, because it is difficult to know whether the values are real or fake. Revert to previous behavior of setting `nTx` and `nChainTx` to 0 when the values are unknown, instead of faking them. Also drop no-longer needed `BLOCK_ASSUMED_VALID` flag. Dropping the faked values also fixes assert failures in the `CheckBlockIndex` `(pindex->nChainTx == pindex->nTx + prev_chain_tx)` check that could happen previously if forked or out-of-order blocks before the snapshot got submitted while the snapshot was being validated. The PR includes two commits adding tests for these failures and describing them in detail. Compatibility note: This change could cause new `-checkblockindex` failures if a snapshot was loaded by a previous version of Bitcoin Core and not fully validated, because fake `nTx` values will have been saved to the block index. It would be pretty easy to avoid these failures by adding some compatibility code to `LoadBlockIndex` and changing `nTx` values from 1 to 0 when they are fake (when `(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS`), but a little simpler not to worry about being compatible in this case. ACKs for top commit: Sjors: re-ACK 9d9a7458a2570f7db56ab626b22010591089c312 achow101: ACK 9d9a7458a2570f7db56ab626b22010591089c312 mzumsande: Tested ACK 9d9a7458a2570f7db56ab626b22010591089c312 maflcko: ACK 9d9a7458a2570f7db56ab626b22010591089c312 🎯 Tree-SHA512: b1e1e2731ec36be30d5f5914042517219378fc31486674030c29d9c7488ed83fb60ba7095600f469dc32f0d8ba79c49ff7706303006507654e1762f26ee416e0
2024-03-20Merge bitcoin/bitcoin#27039: blockstorage: do not flush block to disk if it ↵Ava Chow
is already there dfcef536d0e6c40e98dce35ae7af6e3e4a2595cd blockstorage: do not flush block to disk if it is already there (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/2039 When reindexing from flat-file block storage there is no need to write anything back to disk, since the block data is already there. This PR skips flushing to disk those blocks that already have a known position in the datastore. Skipping this means that users can write-protect the `blk` files on disk which may be useful for security or even safely sharing that data between multiple bitcoind instances. `FindBlockPos()` may also flush the undo data file, but again this is skipped if the corresponding block position is known, like during the initial stage of a reindex when block data is being indexed. Once the block index is complete the validation mechanism will call `ConnectBlock()` which will save undo data at that time. The call stack looks like this: ``` init() ThreadImport() <-- process fReindex flag LoadExternalBlockFile() AcceptBlock() SaveBlockToDisk() FindBlockPos() FlushBlockFile() <-- unnecessary if block is already on disk ``` A larger refactor of this part of the code was started by mzumsande here: https://github.com/mzumsande/bitcoin/tree/202207_refactor_findblockpos including this fix, reviewers can let me know if the changes should be combined. ACKs for top commit: sipa: utACK dfcef536d0e6c40e98dce35ae7af6e3e4a2595cd mzumsande: re-ACK dfcef536d0e6c40e98dce35ae7af6e3e4a2595cd achow101: ACK dfcef536d0e6c40e98dce35ae7af6e3e4a2595cd furszy: Rebase diff ACK dfcef53. Tree-SHA512: 385c5ac1288b325135398d0ddd3ab788fa98cc0ca19bd2474c74039f2ce70d5088c1d1c9d4dd10aefcbd4c757767ec5805d07ba8cee9289a66f96e6f9eaa5279
2024-03-20Merge bitcoin/bitcoin#28955: index: block filters sync, reduce disk read ↵Ava Chow
operations by caching last header 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e refactor: init, simplify index shutdown code (furszy) 0faafb57f8298547949cbc0044ee9e925ed887ba index: decrease ThreadSync cs_main contention (furszy) f1469eb45469672046c5793b44863f606736c853 index: cache last block filter header (furszy) a6756ecdb2f1ac960433412807aa377d1ee80d05 index: blockfilter, decouple header lookup into its own function (furszy) 331f044e3b49223cedd16803d123c0da9d91d6a2 index: blockfilter, decouple Write into its own function (furszy) bcbd7eb8d40fbbd0e58c61acef087d65f2047036 bench: basic block filter index initial sync (furszy) Pull request description: Work decoupled from #26966 per request. The aim is to remove an unnecessary disk read operation that currently takes place with every new arriving block (or scanned block during background sync). Instead of reading the last filter header from disk merely to access its hash for constructing the next filter, this work caches it, occupying just 32 more bytes in memory. Also, reduces `cs_main` lock contention during the index initial sync process. And, simplifies the indexes initialization and shutdown procedure. Testing Note: To compare the changes, added a pretty basic benchmark in the second commit. Alternatively, could also test the changes by timing the block filter sync from scratch on any network; start the node with `-blockfilterindex` and monitor the logs until the syncing process finish. Local Benchmark Results: *Master (c252a0fc0f4dc7d262b971a5e7ff01508159193b): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 132,042,516.60 | 7.57 | 0.3% | 7.79 | `BlockFilterIndexSync` *PR (43a212cfdac6c64e82b601c664443d022f191520): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 126,915,841.60 | 7.88 | 0.6% | 7.51 | `BlockFilterIndexSync` ACKs for top commit: Sjors: re-ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e achow101: ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e TheCharlatan: Re-ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e andrewtoth: ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e Tree-SHA512: 927daadd68f4ee1ca781a89519539b895f5185a76ebaf525fbc246ea8dcf40d44a82def00ac34b188640802844b312270067f1b33e65a2479e06be9169c616de
2024-03-20test: add coverage for bech32m in `wallet_keypool_topup`brunoerg
2024-03-20Merge bitcoin/bitcoin#29676: ci: Bump msan to llvm-18fanquake
faecf3a7e6779c2cacadd91a6eba446431778849 ci: Bump msan to llvm-18 (MarcoFalke) Pull request description: Last one: https://github.com/bitcoin/bitcoin/pull/28476 ACKs for top commit: fanquake: ACK faecf3a7e6779c2cacadd91a6eba446431778849 - There is now a 18.1.2, but given it doesn't fix the instrumenting in libunwind, we don't need that here. I've tested that both jobs are now working on both arches. Tree-SHA512: 489c0b343bdc732687131317a570f3efbb18a3f548736d739da90d1a1e784df1dbb208c2da8a2a7740f27f961a841c477487a14c4d59910368f651225f0779b2
2024-03-19Merge bitcoin/bitcoin#29279: test: p2p: check disconnect due to lack of ↵glozow
desirable service flags 2f23987849758537f76df7374d85a7e87b578b61 test: p2p: check limited peers desirability (depending on best block depth) (Sebastian Falbesoner) c4a67d396d0aa99f658cafe381e39622859eb0be test: p2p: check disconnect due to lack of desirable service flags (Sebastian Falbesoner) 405ac819af1eb0f6cf6d1805cb668f4e8ab4a6f3 test: p2p: support disconnect waiting for `add_outbound_p2p_connection` (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message: https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389 This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here. In lack of finding a proper file where this test would fit in, I created a new one. Happy to take suggestions there. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/2f23987849758537f76df7374d85a7e87b578b61 itornaza: tested ACK 2f23987849758537f76df7374d85a7e87b578b61 fjahr: re-utACK 2f23987849758537f76df7374d85a7e87b578b61 cbergqvist: re ACK 2f23987849758537f76df7374d85a7e87b578b61 stratospher: tested ACK 2f23987. 🚀 Tree-SHA512: cf75d9d4379d0f34fa1e13152e6a8d93cd51b9573466ab3a2fec32dc3e1ac49b174bd1063cae558bc736b111c8a6e7058b1b57a496df56255221bf367d29eb5d
2024-03-19index: Move last_locator_write_time and logging to end of threadsync loopFabian Jahr
This avoids having commit print a needless error message during init. Co-authored-by: furszy <mfurszy@protonmail.com>
2024-03-19Merge bitcoin/bitcoin#29192: Weaken serfloat testsfanquake
6e873df3478f3ab8f67d1b9339c7e990ae90e95b serfloat: improve/simplify tests (Pieter Wuille) b45f1f56582fb3a0d17db5014ac57f1fb40a3611 serfloat: do not test encode(bits)=bits anymore (Pieter Wuille) Pull request description: Closes #28941. Our current tests for serfloat verify two distinct properties: 1. Whether they roundtrip `double`->`uint64_t`->`double` (excluding NaN values) on all systems. 2. Whether on systems with a typical floating point unit that encoding matches the hardware representation, as before v22.0, we would dump the hardware representation directly to disk and we wanted to retain compatibility with that. #28941 seems to show that the second property doesn't always hold, but just for "subnormal" numbers (below $2^{-1021}$). Since we don't care about encoding these numbers, we could exclude such subnormal numbers from the hardware-identical representation test, but this PR goes further and just drops the second property entirely, as I don't think we care about edge-case compatibility with pre-v22.0 code for fee_estimates.dat (the only place it is used). ACKs for top commit: glozow: ACK 6e873df3478f3ab8f67d1b9339c7e990ae90e95b fanquake: ACK 6e873df3478f3ab8f67d1b9339c7e990ae90e95b - It's not as much of a priority, but I think we could still backport this. Tree-SHA512: e18ceee0753a7ee7e999fdfa10b014dc5bb67b6ef79522a0f8c76b889adcfa785772fc26ed7559bcb5a09a9938e243bb54eedd9549bc59080a2c8090155e2267
2024-03-19ci: Bump msan to llvm-18MarcoFalke
2024-03-19test: fix test to ensure hidden RPC is present in detailed helpstratospher
current check to make sure that detailed help for hidden RPC is displayed won't work because the assertion isn't sufficient. Even if unknown RPCs are passed, RPC names would still be present in node.help().
2024-03-19test: remove unused mocktime in test_addpeeraddress0xb10c
Drops the mocktime added in fa4c6836c9366c3cc575cb386a397840d5f1aa57. Setting the mocktime in test_addpeeraddress() isn't needed anymore as it doesn't leak into test_getrawaddrman() anymore (since 2cc8ca19f4185490f30a49516c890b2289fbab71). test_getrawaddrman() clear's the addrman and sets it's own mocktime.
2024-03-19rpc: "addpeeraddress tried" return error on failure0xb10c
When trying to add an address to the IP address manager tried table, it's first added to the new table and then moved to the tried table. Previously, adding a conflicting address to the address manager's tried table with test-only `addpeeraddress tried=true` RPC would return `{ "success": true }`. However, the address would not be added to the tried table, but would remain in the new table. This caused, e.g., issue 28964. This is fixed by returning `{ "success": false, "error": "failed-adding-to-tried" }` for failed tried table additions. Since the address remaining in the new table can't be removed (the address manager interface does not support removing addresses at the moment and adding this seems to be a bigger effort), an error message is returned. This indicates to a user why the RPC failed and allows accounting for the extra address in the new table. Also: To check the number of addresses in each addrman table, the addrman checks were re-run and the log output of this check was asserted. Ideally, logs shouldn't be used as an interface in automated tests. To avoid asserting the logs, use the getaddrmaninfo and getrawaddrman RPCs (which weren't implemented when the test was added). Removing the "getnodeaddress" calls would also remove the addrman checks from the test, which could reduce the test coverage. To avoid this, these are kept.
2024-03-19Merge bitcoin/bitcoin#29094: ci: Better tidy errorsfanquake
fae70ba00da27ca5734c88e9964c872c7faa0f78 ci: Better tidy errors (MarcoFalke) Pull request description: Currently tidy errors are not nice, because the user may have to scroll up to see them in a large block of text. See for example (before) https://github.com/bitcoin/bitcoin/runs/19670551485 Fix that by `tee`ing the output to a file and summarizing the errors in the end again. See for example (after): https://github.com/bitcoin/bitcoin/runs/22647850662 ACKs for top commit: hebasto: ACK fae70ba00da27ca5734c88e9964c872c7faa0f78, logs with errors look cleaner. TheCharlatan: ACK fae70ba00da27ca5734c88e9964c872c7faa0f78 Tree-SHA512: dcaea557fed40089409d16ce2cbaa8a9cfbf047f601d5daadfee0823b0eed7badc12d803addc0b7b6bb3f1eaf5c787fccb2488475d32c4efd80835f386f761dd
2024-03-19Merge bitcoin/bitcoin#29639: test: fix intermittent failures with test=addrmanfanquake
432a542e271f5b6ecb1c6ea4fa9108ad4b3a5a43 test: fix intermittent failures with test=addrman (Martin Zumsande) Pull request description: The `nKey` of the addrman is generated the first time the node is started with an empty `peers.dat`. Therefore, restarting a node or turning it off and on again won't make a previously non-deterministic addrman deterministic. This could lead to intermittent failures in `feature_asmap.py` and `rpc_net.py` Fixes #29634 ACKs for top commit: kevkevinpal: ACK [432a542](https://github.com/bitcoin/bitcoin/pull/29639/commits/432a542e271f5b6ecb1c6ea4fa9108ad4b3a5a43) stratospher: Tested ACK 432a542e271f5b6ecb1c6ea4fa9108ad4b3a5a43. brunoerg: crACK 432a542e271f5b6ecb1c6ea4fa9108ad4b3a5a43 0xB10C: ACK 432a542e271f5b6ecb1c6ea4fa9108ad4b3a5a43 Tree-SHA512: a8e284baeb0be2df7284b8a2792cb9edc9e2d5b877a3b29ab7277ffdb75b17efa58a4d42576441eb493cd518e7c5ffdb05597b27e42b5001cf1a80e78bb04c83
2024-03-19Merge bitcoin/bitcoin#29667: fuzz: actually test garbage >64b in p2p ↵fanquake
transport test 626f8e398e219b84907ccaad036f69177d39284c fuzz: actually test garbage >64b in p2p transport test (Pieter Wuille) Pull request description: This fixes an oversight from #28196: in the `p2p_transport_bidirectional_v2` fuzz test, when the desired garbage length is over 64 bytes, the code would actually use garbage length 0. Fix this. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29667/commits/626f8e398e219b84907ccaad036f69177d39284c brunoerg: crACK 626f8e398e219b84907ccaad036f69177d39284c Tree-SHA512: f6346367adb10464b6c9d20aef43625531d2a4d8110887ad03214b8c1907b83560f2dd5b5415e2180a40b4cd276d51881b32b60c740471b5c6bb218aa19848d8
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-09 10:01:36 +0000