aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-06-13Fuzz: pass mempool to CheckPackageMempoolAcceptResultGreg Sanders
2024-06-13[test] package rbfglozow
2024-06-13[policy] package rbfSuhas Daftuar
Support package RBF where the conflicting package would result in a mempool cluster of size two, and each of its direct conflicts are also part of an up-to-size-2 mempool cluster. This restricted topology allows for exact calculation of miner scores for each side of the equation, reducing the surface area for new pins, or incentive-incompatible replacements. This allows wallets to create simple CPFP packages that can fee bump other simple CPFP packages. This, leveraged with other restrictions such as V3 transactions, can create pin-resistant applications. Future package RBF relaxations can be considered when appropriate. Co-authored-by: glozow <gloriajzhao@gmail.com> Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2024-06-10PackageV3Checks: Relax assumptionsGreg Sanders
Relax assumptions about in-mempool children of in-mempool parents. With package RBF, we will allow a package of size 2 with conflicts on its parent and reconsider the parent if its fee is insufficient on its own. Consider: TxA (in mempool) <- TxB (in mempool) TxA (in mempool) <- TxB' (in package, conflicts with TxB) <- TxC (in package) If TxB' fails to RBF TxB due to insufficient feerate, the package TxB' + TxC will be considered. PackageV3Checks called on TxB' will see an in-mempool parent TxA, and see the in-mempool child TxB. We cannot assume there is no in-mempool sibling, rather detect it and fail normally. Prior to package RBF, this would have failed on the first conflict in package.
2024-06-10Merge bitcoin/bitcoin#30132: indexes: Don't wipe indexes again when ↵Ryan Ofsky
continuing a prior reindex f68cba29b3be0dec7877022b18a193a3b78c1099 blockman: Replace m_reindexing with m_blockfiles_indexed (Ryan Ofsky) 1b1c6dcca0cc891bd35d29b61628c39098cd94ce test: Add functional test for continuing a reindex (TheCharlatan) 201c1a92824c71ae646d5bba9963871b1d704cc1 indexes: Don't wipe indexes again when already reindexing (TheCharlatan) 804f09dfa116300914e2aeef05ed9710dd504e8c kernel: Add less confusing reindex options (Ryan Ofsky) e17255322378076edce3ef6f06cd36ca58d2e236 validation: Remove needs_init from LoadBlockIndex (TheCharlatan) 533eab7d67d78f217f74909662133086b79ea808 bugfix: Streamline setting reindex option (TheCharlatan) Pull request description: When restarting `bitcoind` during an ongoing reindex without setting the `-reindex` flag again, the block and coins db is left intact, but any data from the optional indexes is discarded. While not a bug per se, wiping the data again is wasteful, both in terms of having to write it again, as well as potentially leading to longer startup times. So keep the index data instead when continuing a prior reindex. Also includes a bugfix and smaller code cleanups around the reindexing code. The bug was introduced in b47bd959207e82555f07e028cc2246943d32d4c3: "kernel: De-globalize fReindex". ACKs for top commit: stickies-v: ACK f68cba29b3be0dec7877022b18a193a3b78c1099 fjahr: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099 furszy: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099 ryanofsky: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code. Tree-SHA512: b252228cc76e9f1eaac56d5bd9e4eac23408e0fc04aeffd97a85417f046229364673ee1ca7410b9b6e7b692b03f13ece17c42a10176da0d7e975a8915deb98ca
2024-06-10Merge bitcoin/bitcoin#30257: build: Remove --enable-gprofmerge-script
fa780e1c25e8e98253e32d93db65f78a0092433f build: Remove --enable-gprof (MarcoFalke) Pull request description: It is unclear what benefit this option has, given that: * `gprof` requires re-compilation (`perf` and other tools can instead be used on existing executables) * `gprof` requires hardening to be disabled * `gprof` doesn't work with `clang` * `perf` is documented in the dev-notes, and test notes, and embedded into the functional test framework; `gprof` isn't * Anyone really wanting to use it could pass the required flags to `./configure` * I couldn't find any mention of the use of `gprof` in the discussions in this repo, apart from the initial pull request adding it (cfaac2a60f3ac63ae8deccb03d88bd559449b78c) * Keeping it means that it needs to be maintained and ported to CMake Fix all issues by removing it. ACKs for top commit: TheCharlatan: ACK fa780e1c25e8e98253e32d93db65f78a0092433f hebasto: ACK fa780e1c25e8e98253e32d93db65f78a0092433f, I have reviewed the code and it looks OK. willcl-ark: crACK fa780e1c25e8e98253e32d93db65f78a0092433f Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c
2024-06-10Merge bitcoin/bitcoin#30227: doc: fixup deps doc after #30198merge-script
e6636ff4ec594a38f2e2c4bda3a9549bbc07118e doc: fixup deps doc after #30198 (fanquake) Pull request description: Forgotten in #30198. Addresses: https://github.com/bitcoin/bitcoin/pull/30198#issuecomment-2148087913. ACKs for top commit: TheCharlatan: ACK e6636ff4ec594a38f2e2c4bda3a9549bbc07118e Tree-SHA512: 138cba946d0482f11b604a8197177e74f4384c1962dee1d50af6baad40ceb9d064a148244d652d8e140f955f95457f7d0ffdb0e179f8622e71c57cb91c44af29
2024-06-10Merge bitcoin/bitcoin#30242: ci: Native Windows CI job cleanupmerge-script
0d3ef83433805d3f367130fd5bd227a8ed5a7ccd ci: Use relative paths in `win64-native` CI job consistently (Hennadii Stepanov) 501aceefcf7536cbdead5bcb53b13f2fec7ab6be ci: Remove no longer needed workaround for GHA Windows images (Hennadii Stepanov) Pull request description: This PR: 1. Removes no longer needed workaround for GHA Windows images. GHA Windows images previously had multiple VC Build Tools installed, which required specifying the `VCPKG_PLATFORM_TOOLSET_VERSION` explicitly to avoid linker errors. This issue has been resolved as per https://github.com/actions/runner-images/issues/9701. 2. Switches all references to temporary files to relative ones for consistency and readability. ACKs for top commit: sipsorcery: ACK 0d3ef83433805d3f367130fd5bd227a8ed5a7ccd. maflcko: ACK 0d3ef83433805d3f367130fd5bd227a8ed5a7ccd Tree-SHA512: e832b87fc6dee1e9d1eb452797f16b732e776c2ecdbe3dc9e64cc48ce9b5b89c569d5b96b999423ae1261ff4bf684b7003af84d8024ef5260682f531c4e8ff5e
2024-06-10Merge bitcoin/bitcoin#30235: build: warn on self-assignmentmerge-script
15796d4b61342f75548b20a18c670ed21d102ba8 build: warn on self-assignment (Cory Fields) 53372f21767be449bb452fc3f5fe7f16286ae371 refactor: disable self-assign warning for tests (Cory Fields) Pull request description: Belt-and suspenders after #30234. Self-assignment should be safe _and_ discouraged. We used to opt out of this warning because something deep in our serialization/byteswapping code could self-assign, but that doesn't appear to be the case anymore. ACKs for top commit: maflcko: ACK 15796d4b61342f75548b20a18c670ed21d102ba8 fanquake: ACK 15796d4b61342f75548b20a18c670ed21d102ba8 - not a huge fan of inline pragma usage, but this seems fine, given it's to work around an already-fixed compiler bug, and we'll only be carrying it for a shortish time in any case. Tree-SHA512: 1f95f7c730b974ad1da55ebd381040bac312f2f380fff9d569ebab91d7c1963592a84d1613d81d96238c6f5a66aa40deebba68a76f6b24b02150d0a77c769654
2024-06-10Merge bitcoin/bitcoin#30253: refactor: performance-for-range-copy in psbt.hmerge-script
fab01b5220c28a334b451ed9625bd3914c48e6af refactor: performance-for-range-copy in psbt.h (MarcoFalke) Pull request description: A copy of the partial signatures is not required before serializing them. For reference, this was found by switching the codebase to `std::span` (not sure why it wasn't found with `Span` though): ``` ./psbt.h:240:23: error: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy,-warnings-as-errors] 240 | for (auto sig_pair : partial_sigs) { | ^ | const & ACKs for top commit: tdb3: ACK fab01b5220c28a334b451ed9625bd3914c48e6af theStack: utACK fab01b5220c28a334b451ed9625bd3914c48e6af Tree-SHA512: b55513d8191118499716684190ee568d171b50ae9171d246ca6e047f0cfd3ec14c9453d721e88af55e47bb41fa66cbafdbfb47bc5f9b8d82789e0a9b634b371b
2024-06-09build: Remove --enable-gprofMarcoFalke
This reverts cfaac2a60f3ac63ae8deccb03d88bd559449b78c
2024-06-09refactor: performance-for-range-copy in psbt.hMarcoFalke
2024-06-08Merge bitcoin/bitcoin#30238: json-rpc 2.0 followups: docs, tests, climerge-script
1f6ab1215bbb1f8a5f1743c3c413b95ad08090df minor: remove unnecessary semicolons from RPC content type examples (Matthew Zipkin) b22529529823c0cb5916ac318c8536e9107b7e78 test: use json-rpc 2.0 in all functional tests by default (Matthew Zipkin) 391843b0297db03d71a8d88ab77609e2ad230bf2 bitcoin-cli: use json-rpc 2.0 (Matthew Zipkin) d39bdf339772166a5545ae811e58b7764af093a8 test: remove unused variable in interface_rpc.py (Matthew Zipkin) 0ead71df8c83a2f9eae1220544ec84dcf38a0326 doc: update and link for JSON-RPC 2.0 (Matthew Zipkin) Pull request description: This is a follow-up to #27101. - Addresses [post-merge comments ](https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606723428) - bitcoin-cli uses JSON-RPC 2.0 - functional tests use JSON-RPC 2.0 by default (exceptions are in the regression tests added by #27101) ACKs for top commit: tdb3: ACK 1f6ab1215bbb1f8a5f1743c3c413b95ad08090df cbergqvist: ACK 1f6ab1215bbb1f8a5f1743c3c413b95ad08090df Tree-SHA512: 49bf14c70464081280216ece538a2f5ec810bac80a86a83ad3284f0f1b017edf755a1a74a45be279effe00218170cafde7c2de58aed07097a95c2c6b837a6b6c
2024-06-08Merge bitcoin/bitcoin#30231: guix: bump time-machine to ↵merge-script
f0bb724211872cd6158fce6162e0b8c73efed126 2599655c1fb8e7d0b8407d2be249977372cb73ff guix: bump time-machine to f0bb724211872cd6158fce6162e0b8c73efed126 (fanquake) Pull request description: Includes: LLVM 18.1.x (#30201) GCC 13.x (#29881) git-minimal 2.41.0 -> 2.45.1 Kernel Headers 6.1.80 -> 6.1.92 moreutils 0.68 -> 0.69 Commits like https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7b0f145802f0c2c785014293d748721678fef824, which should improve the bootstrap situation (#30042). This can somewhat be visualised by comparing the (simplified) dependencies of guix itself, between the two time-machines. Master: ![master_2](https://github.com/bitcoin/bitcoin/assets/863730/714402a2-345e-43c7-974b-5112d03d44c2) PR: ![pr](https://github.com/bitcoin/bitcoin/assets/863730/7079a155-e013-4d59-9ea1-21a64d71e2d8) Note that in the case of this PR, we are better off, no-longer having to build a number of tex packages, ruby, cairo, graphics libs, openssl 1.x etc. ACKs for top commit: TheCharlatan: ACK 2599655c1fb8e7d0b8407d2be249977372cb73ff Tree-SHA512: 9c5675a5d563c17744c89c8a392bc7865aa1c9e71a5e3044c23f31e51458dac28c0c238d2055c86dc732df595dae60bcbc8b85266b23b7991c4c770d56f7d23a
2024-06-07blockman: Replace m_reindexing with m_blockfiles_indexedRyan Ofsky
This is a just a mechanical change, renaming and inverting the meaning of the indexing variable. "m_blockfiles_indexed" is a more straightforward name for this variable because this variable just indicates whether or not <datadir>/blocks/blk?????.dat files have been indexed in the <datadir>/blocks/index LevelDB database. The name "m_reindexing" was more confusing, it could be true even if -reindex was not specified, and false when it was specified. Also, the previous name unnecessarily required thinking about the whole reindexing process just to understand simple checks in validation code about whether blocks were indexed. The motivation for this change is to follow up on previous commits, moving away from having multiple variables called "reindex" internally, and instead naming variables individually after what they do and represent.
2024-06-07test: Add functional test for continuing a reindexTheCharlatan
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2024-06-07indexes: Don't wipe indexes again when already reindexingTheCharlatan
Before this change continuing a reindex without the -reindex flag set would leave the block and coins db intact, but discard the data of the optional indexes. While not a bug per se, wiping the data again is wasteful, both in terms of having to write it again, and potentially leading to longer startup times. When initially running a reindex, both the block index and any further activated indexes are wiped. On an index's Init(), both the best block stored by the index and the chain's tip are null. An index's m_synced member is therefore true. This means that it will process blocks through validation events while the reindex is running. Currently, if the reindex is continued without the user re-specifying the reindex flag, the block index is preserved but further index data is wiped. This leads to the stored best block being null, but the chain tip existing. The m_synced member will be set to false. The index will not process blocks through the validation interface, but instead use the background sync once the reindex is completed. If the index is preserved (this change) after a restart its best block may potentially match the chain tip. The m_synced member will be set to true and the index can process validation events during the rest of the reindex.
2024-06-07kernel: Add less confusing reindex optionsRyan Ofsky
Drop confusing kernel options: BlockManagerOpts::reindex ChainstateLoadOptions::reindex ChainstateLoadOptions::reindex_chainstate Replacing them with more straightforward options: ChainstateLoadOptions::wipe_block_tree_db ChainstateLoadOptions::wipe_chainstate_db Having two options called "reindex" which did slightly different things was needlessly confusing (one option wiped the block tree database, and the other caused block files to be rescanned). Also the previous set of options did not allow rebuilding the block database without also rebuilding the chainstate database, when it should be possible to do those independently.
2024-06-07Merge bitcoin/bitcoin#29496: policy: bump TX_MAX_STANDARD_VERSION to 3Ava Chow
30a01134cdec37e7467fcd6eee8b0ae3890a131c [doc] update bips.md for 431 (glozow) 9dbe6a03f0d6e70ccdf8e8715f888c0c17216bee [test] wallet uses CURRENT_VERSION which is 2 (glozow) 539404fe0fc0346b3aa77c330b38a5a0ad6565b2 [policy] make v3 transactions standard (glozow) 052ede75aff5c9f3a0a422ef413852eabeecc665 [refactor] use TRUC_VERSION in place of 3 (glozow) Pull request description: Make `nVersion=3` (which is currently nonstandard on mainnet) standard. Note that we will treat these transactions as Topologically Restricted Until Confirmation (TRUC). Spec is in BIP 431 and implementation is in #28948, #29306, and #29873 See #27463 for overall project tracking, and #29319 for information about relevance to cluster mempool. ACKs for top commit: sdaftuar: utACK 30a01134c achow101: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c instagibbs: utACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c murchandamus: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c ismaelsadeeq: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c 🛰️ Tree-SHA512: 2a4aec0442c860e792a061d83e36483c1f1b426f946efbdf664c8db97a596e498b535707e1d3a900218429486ea69fd4552e3d476526a6883cbd5556c6534b48
2024-06-07minor: remove unnecessary semicolons from RPC content type examplesMatthew Zipkin
2024-06-07Merge bitcoin/bitcoin#30161: util: add VecDequeglozow
7b8eea067f188c0b0e52ef21b01aedd37667a237 tests: add fuzz tests for VecDeque (Pieter Wuille) 62fd24af6a3fe1569662c2802f59bb68a0172087 util: add VecDeque (Pieter Wuille) Pull request description: Extracted from #30126. This adds a `VecDeque` data type, inspired by `std::deque`, but backed by a single allocated memory region used as a ring buffer instead of a linked list of arrays. This gives better memory locality and less allocation overhead, plus better guarantees (some C++ standard library implementations, though not libstdc++ and libc++, use a separate allocation per element in a deque). It is intended for the candidate set search queue in #30126, but may be useful as a replacement for `std::deque` in other places too. It's not a full drop-in replacement, as I did not add iteration support which is unnecessary for the intended use case, but nothing prevents adding that if needed. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::deque` equivalent operations, both for trivially-copyable/destructible types and others. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30161/commits/7b8eea067f188c0b0e52ef21b01aedd37667a237 cbergqvist: re-ACK 7b8eea067f188c0b0e52ef21b01aedd37667a237 hebasto: re-ACK 7b8eea067f188c0b0e52ef21b01aedd37667a237, I've verified changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2103018546) with glozow: ACK 7b8eea067f Tree-SHA512: 1b62f3ba1a43a1293d8c9de047e2399442e74c46de2df81406151fe27538716ce265f35fb6779ee56d77a39cddf8fb4b4e15bda8f04ebf3b149e2f05fa55cb21
2024-06-07test: use json-rpc 2.0 in all functional tests by defaultMatthew Zipkin
2024-06-07bitcoin-cli: use json-rpc 2.0Matthew Zipkin
2024-06-07test: remove unused variable in interface_rpc.pyMatthew Zipkin
2024-06-07doc: update and link for JSON-RPC 2.0Matthew Zipkin
2024-06-07validation: Remove needs_init from LoadBlockIndexTheCharlatan
It does not control any actual logic and the log message as well as the comment are obsolete, since no database initialization takes place there anymore. Log messages indicating when indexes and chainstate databases are loaded exist in other places.
2024-06-07bugfix: Streamline setting reindex optionTheCharlatan
Reverts a bug introduced in b47bd959207e82555f07e028cc2246943d32d4c3 "kernel: De-globalize fReindex". The change leads to a GUI user being prompted to re-index on a chainstate loading failure more than once as well as the node actually not reindexing if the user chooses to. Fix this by setting the reindexing option instead of the atomic, which can be safely re-used to indicate that a reindex should be attempted. The bug specifically is caused by the chainman, and thus the blockman and its m_reindexing atomic being destroyed on every iteration of the for loop. The reindex option for ChainstateLoadOptions is currently also set in a confusing way. By using the reindex atomic, it is not obvious in which scenario it is true or false. The atomic is controlled by both the user passing the -reindex option, the user chosing to reindex if something went wrong during chainstate loading when running the gui, and by reading the reindexing flag from the block tree database in LoadBlockIndexDB. In practice this read is done through the chainstate module's CompleteChainstateInitialization's call to LoadBlockIndex. Since this is only done after the reindex option is set already, it does not have an effect on it. Make this clear by using the reindex option from the blockman opts which is only controlled by the user.
2024-06-07ci: Use relative paths in `win64-native` CI job consistentlyHennadii Stepanov
This change improves readability. Also the `Tee-Object` cmdlet is used when appropriate.
2024-06-07ci: Remove no longer needed workaround for GHA Windows imagesHennadii Stepanov
GHA Windows images previously had multiple VC Build Tools installed, which required specifying the `VCPKG_PLATFORM_TOOLSET_VERSION` explicitly to avoid linker errors. This issue has been resolved as per https://github.com/actions/runner-images/issues/9701.
2024-06-06Merge bitcoin/bitcoin#29401: test: Remove struct.pack from almost all placesAva Chow
fa52e13ee81fcc7543890dbd6986fcb55168583f test: Remove struct.pack from almost all places (MarcoFalke) fa826db477a981b48bff53021f9695a5f6682dc0 scripted-diff: test: Use int.to_bytes over struct packing (MarcoFalke) faf2a975ad46799d075e3a70c699da0d8182aab9 test: Use int.to_bytes over struct packing (MarcoFalke) faf3cd659a72473a1aa73c4367a145f4ec64f146 test: Normalize struct.pack format (MarcoFalke) Pull request description: `struct.pack` has many issues: * 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. This lead to many test bugs, which weren't hit, which is fine, but still confusing. Ref: https://github.com/bitcoin/bitcoin/pull/29400, https://github.com/bitcoin/bitcoin/pull/29399, https://github.com/bitcoin/bitcoin/pull/29363, fa3886b7c69cbbe564478f30bb2c35e9e6b1cffa, ... Fix all issues by using the built-in `int` helpers `to_bytes` via a scripted diff. Review notes: * For `struct.pack` and `int.to_bytes` the error behavior is the same, although the error messages are not identical. * Two `struct.pack` remain. One for float serialization in a C++ code comment, and one for native serialization. ACKs for top commit: achow101: ACK fa52e13ee81fcc7543890dbd6986fcb55168583f rkrux: tACK [fa52e13](https://github.com/bitcoin/bitcoin/pull/29401/commits/fa52e13ee81fcc7543890dbd6986fcb55168583f) theStack: Code-review ACK fa52e13ee81fcc7543890dbd6986fcb55168583f Tree-SHA512: ee80d935b68ae43d1654b047e84ceb80abbd20306df35cca848b3f7874634b518560ddcbc7e714e2e7a19241e153dee64556dc4701287ae811e26e4f8c57fe3e
2024-06-06tests: add fuzz tests for VecDequePieter Wuille
2024-06-06util: add VecDequePieter Wuille
This is an STL-like container that interface-wise looks like std::deque, but is backed by a (fixed size, with vector-like capacity/reserve) circular buffer.
2024-06-06build: warn on self-assignmentCory Fields
Belt-and suspenders after #30234. Self-assignment should be safe _and_ discouraged. We used to opt out of this warning because something deep in our serialization/byteswapping code could self-assign, but that doesn't appear to be the case anymore.
2024-06-06refactor: disable self-assign warning for testsCory Fields
clang-16 and earlier detect "foo -= foo" and "foo /= foo" as self-assignments.
2024-06-06Merge bitcoin/bitcoin#30228: build: no-longer allow GCC-10 in C++20 checkmerge-script
232928b58a82e3f15307deba1ae921ae2960ccc8 build: no-longer allow GCC-10 in C++20 check (fanquake) Pull request description: Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11. See also: https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1745143612. ACKs for top commit: maflcko: utACK 232928b58a82e3f15307deba1ae921ae2960ccc8 theuni: utACK 232928b58a82e3f15307deba1ae921ae2960ccc8 Tree-SHA512: 10e0adac2dd6e455aaf97ebfe73c7586430349fc27ac435bc6c0d99a4934a380398d14467aacd9cedf371345da291366b3ab2c3be7db5d97e21ad6212b2c7890
2024-06-06Merge bitcoin/bitcoin#30236: build: re-enable deprecated warning copymerge-script
c3a5e8a0639ff2505adb4a4e7776db87d5ebafd3 build: re-enable deprecated warning copy (Cory Fields) Pull request description: Noticed while looking at the `-wno-*` flags in #30235. This was disabled in #18738 due to the combo of old gcc and qt. We no longer support the affected gcc, and the old qt should no longer be relevant to us anyway. See old fixes in: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88136 and https://bugreports.qt.io/browse/QTBUG-75210 and https://codereview.qt-project.org/c/qt/qtbase/+/245434 ACKs for top commit: maflcko: ACK c3a5e8a0639ff2505adb4a4e7776db87d5ebafd3 fanquake: ACK c3a5e8a0639ff2505adb4a4e7776db87d5ebafd3 - this is in `-Wextra` for Clang and GCC. Tree-SHA512: bd008dc50134d15ca3bb0c4f78d910db5b7a0ee98b04c159122a6f13a24b18827806492f053293d9cc1f1528ba60dea6d9ed31a366f63840ccb7c55f002d263b
2024-06-05build: re-enable deprecated warning copyCory Fields
This was disabled in #18738 due to the combo of old gcc and qt, neither of which are relevant to us anymore.
2024-06-05guix: bump time-machine to f0bb724211872cd6158fce6162e0b8c73efed126fanquake
Includes: LLVM 18.1.x (#30201) GCC 13.x (#29881) git-minimal 2.41.0 -> 2.45.1 Kernel Headers 6.1.80 -> 6.1.92 moreutils 0.68 -> 0.69 Commits like https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7b0f145802f0c2c785014293d748721678fef824, which should improve the bootstrap situation (#30042).
2024-06-05Merge bitcoin/bitcoin#30185: guix: show `*_FLAGS` variables in pre-build outputmerge-script
5f2c1d84e37697f4f8a20e3c12f37bba71b3c2a6 guix: show *_FLAGS variables in pre-build output (fanquake) Pull request description: For example: ```bash # ADDITIONAL_GUIX_COMMON_FLAGS set in the ENV ADDITIONAL_GUIX_ENVIRONMENT_FLAGS="--emulate-fhs" ./contrib/guix/guix-build <snip> INFO: Building f751991 for platform triple x86_64-linux-gnu: ...using reference timestamp: 1716905119 ...running at most 10 jobs ...from worktree directory: '/bitcoin' ...bind-mounted in container to: '/bitcoin' ...in build directory: '/bitcoin/guix-build-f75199182133/distsrc-f75199182133-x86_64-linux-gnu' ...bind-mounted in container to: '/distsrc-base/distsrc-f75199182133-x86_64-linux-gnu' ...outputting in: '/bitcoin/guix-build-f75199182133/output/x86_64-linux-gnu' ...bind-mounted in container to: '/outdir-base/x86_64-linux-gnu' ADDITIONAL FLAGS (if set) ADDITIONAL_GUIX_COMMON_FLAGS: --no-substitutes ADDITIONAL_GUIX_ENVIRONMENT_FLAGS: --emulate-fhs ADDITIONAL_GUIX_TIMEMACHINE_FLAGS: ``` ACKs for top commit: hebasto: ACK 5f2c1d84e37697f4f8a20e3c12f37bba71b3c2a6. Tree-SHA512: 85a6d508499b4ec1d6166343a1707b682d327b2fcfb2fb438571894478aac0062d21e1239b5092091ff98711c5c747151973c4f325a7a7c447d0e807166fcb07
2024-06-05Merge bitcoin/bitcoin#30174: test: Set mocktime in p2p_disconnect_ban.py to ↵merge-script
avoid intermittent test failure 4444de152f01368e603f2b089679a86eae02e34a test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure (MarcoFalke) fa6aa4027cecd819c1210d6959af364d5bf9f608 test: Fix typos and use names args (MarcoFalke) Pull request description: Otherwise, the test may fail on slow hardware when running in valgrind. Also, use named args for the absolute timepoint, while touching this file. ACKs for top commit: tdb3: ACK for 4444de152f01368e603f2b089679a86eae02e34a AngusP: re-ACK 4444de152f01368e603f2b089679a86eae02e34a Tree-SHA512: 660269c8dd18887d69b284f38656899caf028159ce83ddf921f3e9c080a5d0e663989f0e42b4baf4c4939f20f34da0e7e844dff9b7c91d0cab570c60958bd0e1
2024-06-05build: no-longer allow GCC-10 in C++20 checkfanquake
Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11. See also: https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1745143612.
2024-06-05doc: fixup deps doc after #30198fanquake
2024-06-04Merge bitcoin/bitcoin#30218: refactor: remove unused `CKey::Negate` methodAva Chow
8801e319d51209fe3a3b06e2aab5f96ceead290d refactor: remove unused `CKey::Negate` method (Sebastian Falbesoner) Pull request description: This method was introduced as a pre-requirement for the v2 transport protocol back then (see PR #14047, commit 463921bb), when it was still BIP151. With the replacement BIP324, this is not needed anymore, and it's also unlikely that for any other proposal we'd ever need to negate private keys at this abstraction level. I'd argue that this operation is usually something that should happen within a secp256k1 module (like e.g. done in MuSig2, Silent Payments...). (If there is really demand in the future, it's also trivial to reintroduce the method.) ACKs for top commit: laanwj: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d sipa: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d achow101: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d Tree-SHA512: 7bc1566399635c5c6e4940a2724c865d5443eb190024379099330c023c516f1e4f423ed9e8c42bc93413b723a5464ec79d3f879f58c0e598fe24f495238df4ec
2024-06-04Merge bitcoin/bitcoin#29998: functional test: ensure confirmed utxo being ↵Ava Chow
sourced for 2nd chain 07aba8dd215b23b06853b1a9fe04ac8b08f62932 functional test: ensure confirmed utxo being sourced for 2nd chain (Greg Sanders) Pull request description: The test could fail/stop testing what we want if non-confirmed utxos become sourced through some internal change to `MiniWallet`; better to just fetch confirmed explicitly. ACKs for top commit: achow101: ACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932 ismaelsadeeq: utACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932 theStack: ACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932 Tree-SHA512: 66795fdf881139ed91bde0f8239a46bd9bc70bb311fa97c0e2b5537e1fd2a1fd36bf3a225fc77b9695deb835a9d6d29879aa1e05ea5054b9a33a400e199da014
2024-06-04Merge bitcoin/bitcoin#28307: rpc, wallet: fix incorrect segwit redeem script ↵Ava Chow
size limit 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 test: addmultisigaddress, coverage for script size limits (furszy) 53302a09817e5b799d345dfea432546a55a9d727 bugfix: addmultisigaddress, add unsupported operation for redeem scripts over 520 bytes (furszy) 9be6065cc03f2408f290a332b203eef9c9cebf24 test: coverage for 16-20 segwit multisig scripts (furszy) 9d9a91c4ea6b3bb32ef4131bca86f1d6683fc901 rpc: bugfix, incorrect segwit redeem script size used in signrawtransactionwithkey (furszy) 0c9fedfc45fa7cbd6801ca5fd756863ec9a6911c fix incorrect multisig redeem script size limit for segwit (furszy) f7a173b5785cda460470df9a74a0e0f94d7f9a18 test: rpc_createmultisig, decouple 'test_sortedmulti_descriptors_bip67' (furszy) 4f33dbd8f8c0e29f37b04e6af6d2c7905ecceaf6 test: rpc_createmultisig, decouple 'test_mixing_uncompressed_and_compressed_keys' (furszy) 25a81705d376e8c96dad45436ae3fca975b3daf5 test: rpc_createmultisig, remove unnecessary checkbalances() (furszy) b5a328943362cfac6e90fd4e1b167c357d53b7d4 test: refactor, multiple cleanups in rpc_createmultisig.py (furszy) 3635d432681847313c098f9827483372a840e70f test: rpc_createmultisig, remove manual wallet initialization (furszy) Pull request description: Fixing https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1674830104 and more. Currently, redeem scripts longer than 520 bytes, which are technically valid under segwit rules, have flaws in the following processes: 1) The multisig creation process fails to deduce the output descriptor, resulting in the generation of an incorrect descriptor. Additionally, the accompanying user warning is also inaccurate. 2) The `signrawtransactionwithkey` RPC command fail to sign them. 3) The legacy wallet `addmultisigaddress` wrongly discards them. The issue arises because most of these flows are utilizing the legacy spkm keystore, which imposes the [p2sh max redeem script size rule](https://github.com/bitcoin/bitcoin/blob/ded687334031f4790ef6a36b999fb30a79dcf7b3/src/script/signingprovider.cpp#L160) on all scripts. Which blocks segwit redeem scripts longer than the max element size in all the previously mentioned processes (`createmultisig`, `addmultisigaddress`, and `signrawtransactionwithkey`). This PR fixes the problem, enabling the creation of multisig output descriptors involving more than 15 keys and allowing the signing of these scripts, along with other post-segwit redeem scripts that surpass the 520-byte p2sh limit. Important note: Instead of adding support for these longer redeem scripts in the legacy wallet, an "unsupported operation" error has been added. The reasons behind this decision are: 1) The introduction of this feature brings about a compatibility-breaking change that requires downgrade protection; older wallets would be unable to interact with these "new" legacy wallets. 2) Considering the ongoing deprecation of the legacy spkm, this issue provides another compelling reason to transition towards descriptors. Testing notes: To easily verify each of the fixes, I decoupled the tests into standalone commits. So they can be cherry-picked on top of master. Where `rpc_createmultisig.py` (with and without the `--legacy-wallet` arg) will fail without the bugs fixes commits. Extra note: The initial commits improves the `rpc_createmultisig.py` test in many ways. I found this test very antiquated, screaming for an update and cleanup. ACKs for top commit: pinheadmz: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 theStack: Code-review ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 achow101: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 Tree-SHA512: 71794533cbd46b3a1079fb4e9d190d3ea3b615de0cbfa443466e14f05e4616ca90e12ce2bf07113515ea8113e64a560ad572bb9ea9d4835b6fb67b6ae596167f
2024-06-04Merge bitcoin/bitcoin#28074: fuzz: wallet, add target for `Crypter`Ava Chow
d7290d662f494503f28e087dd728b492c0bb2c5f fuzz: wallet, add target for Crypter (Ayush Singh) Pull request description: This PR adds fuzz coverage for `wallet/crypter`. Motivation: Issue [27272](https://github.com/bitcoin/bitcoin/issues/27272#issue-1628327906) I ran this for a long time with Sanitizers on and had no crashes; the average `exec/sec` also looks good to me. However, I would really appreciate it if some of the reviewers could try it on their machines too, and give their feedback. ACKs for top commit: maflcko: utACK d7290d662f494503f28e087dd728b492c0bb2c5f achow101: ACK d7290d662f494503f28e087dd728b492c0bb2c5f brunoerg: utACK d7290d662f494503f28e087dd728b492c0bb2c5f Tree-SHA512: f5c496cabdd3263a7e1ad49eeff702725336f76bf19a82e5dbbead082e990889dd43c851d0d2d6ab740f44b8ec2aa06defd9ff6b02be68b5f8b4eaf963f88599
2024-06-04Merge bitcoin/bitcoin#30047: refactor: Model the bech32 charlimit as an EnumAva Chow
7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd refactor: replace hardcoded numbers (Lőrinc) 5676aec1e1a6d2c6fd3099e120e263a0a7def089 refactor: Model the bech32 charlimit as an Enum (josibake) Pull request description: Broken out from #28122 --- Bech32(m) was defined with a 90 character limit so that certain guarantees for error detection could be made for segwit addresses (see https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#checksum-design). However, there is nothing about the encoding scheme itself that requires a limit of 90 and in practice bech32(m) is being used without the 90 char limit (e.g. lightning invoices, silent payments). Further, increasing the character limit doesn't do away with error detection, it simply changes the guarantee. The primary motivation for this change is for being able to parse BIP352 v0 silent payment addresses (see https://github.com/bitcoin/bitcoin/pull/28122/commits/622c7a98b9f08177a3cfb601306daabb101af1fd), which require up to 118 characters. In addition to BIP352, modeling the character limit as an enum allows us to easily support new address types that use bech32m and specify their own character limit. ACKs for top commit: paplorinc: re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd achow101: ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd theuni: utACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd Tree-SHA512: 9c793d657448c1f795093b9f7d4d6dfa431598f48d54e1c899a69fb2f43aeb68b40ca2ff08864eefeeb6627d4171877234b5df0056ff2a2b84415bc3558bd280
2024-06-04Merge bitcoin/bitcoin#29997: rpc: Remove index-based Arg accessorAva Chow
fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 rpc: Remove index-based Arg accessor (MarcoFalke) Pull request description: The index-based Arg accessor is redundant with the name-based one. It does not provide any benefit to the code reader, or otherwise, so remove it. ACKs for top commit: stickies-v: re-ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54, addressed doc nits achow101: ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 ryanofsky: Code review ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54. One changes since last review are some documentation improvements Tree-SHA512: f9da1c049dbf38c3b47a8caf8d24d195c2d4b88c7ec45a9ccfb78f1e39f29cb86869f84b308f6e49856b074c06604ab634c90eb89c9c93d2a8169e070aa1bd40
2024-06-04Merge bitcoin/bitcoin#29428: test: Assumeutxo: snapshots with less work ↵Ava Chow
should not be loaded df6dc2aaaeffc664006b86ee8c8797dc484ec40e test: Assumeutxo: snapshots with less work should not be loaded (Hernan Marino) Pull request description: This PR adds a test which checks that snapshots with less accumulated work than the node's active chain, should not be loaded and return with an error. Although in a different context of discussion the missing test was detect in a thread in https://github.com/bitcoin/bitcoin/pull/29394 (see https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1484122214) ACKs for top commit: maflcko: utACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e kevkevinpal: utACK [df6dc2a](https://github.com/bitcoin/bitcoin/pull/29428/commits/df6dc2aaaeffc664006b86ee8c8797dc484ec40e) achow101: ACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e alfonsoromanz: Re ACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e. Make is successful and the test passes. Tree-SHA512: 07a394b4b288cc8ad3f66ed4e70dcda468db18113e9442eb7215cf491768432d55efaaa5b79d633094917e05475a30f0c5e4f64f8f2da293ba306891b4485560
2024-06-04Merge bitcoin/bitcoin#30154: doc: update mention of generating bitcoin.confAva Chow
9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 Link to gen-bitcoin-conf.sh instead of bitcoin.conf placeholder (Epic Curious) Pull request description: Closes #30153. This PR updates `doc/init.md` to mention generating an example bitcoin.conf instead of referencing the placeholder `share/examples/bitcoin.conf`. Also changes the code-formatted text to a markdown link. ## Background - Two years ago, `share/examples/bitcoin.conf` was replaced with [a placeholder file](https://github.com/bitcoin/bitcoin/commit/b483084d866c16d97a34251ae652bac94f85f61d). To see an example `bitcoin.conf`, the user now runs the `contrib/devtools/gen-bitcoin-conf.sh` script, which replaces the placeholder file with the parsed contents of `bitcoind --help`. - The instructions in `init.md` about an example `bitcoin.conf` haven't changed significantly since they were [added almost 10 years ago](https://github.com/bitcoin/bitcoin/blame/234bfbf6a5fcba37e510e9cb6c1f2a629cd0290e/doc/init.md#L39). They should be updated to improve clarity. ACKs for top commit: edilmedeiros: ACK 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 kevkevinpal: reACK [9013e2b](https://github.com/bitcoin/bitcoin/pull/30154/commits/9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2) achow101: ACK 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 stickies-v: ACK 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 tdb3: ACK for 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 Tree-SHA512: 5ac5ad672ad181d574e19e29c3727fb9e5373282444fae09b42d113d5c8915cb2829d496212638cdc4b70540b7e1794a751fcdc9539f956a594cddd70c8fd747
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 05:09:54 +0000