94b0adcc37 rpc, refactor: Prevent potential race conditions in dumptxoutset (Fabian Jahr)
e868a6e070 doc: Improve assumeutxo guide and add more docs/comments (Fabian Jahr)
b29c21fc92 assumeutxo: Remove devtools/utxo_snapshot.sh (Fabian Jahr)
20a1c77aa7 contrib: Remove test_utxo_snapshots.sh (Fabian Jahr)
8426850352 test: Test for dumptxoutset at specific height (Fabian Jahr)
993cafe7e4 RPC: Add type parameter to dumptxoutset (Fabian Jahr)
fccf4f91d2 RPC: Extract ReconsiderBlock helper (Fabian Jahr)
446ce51c21 RPC: Extract InvalidateBlock helper (Fabian Jahr)
Pull request description:
This adds a height parameter to the `dumptxoutset` RPC. This internalizes the workflow that was previously done by scripts: roll back the chain to the height we actually want the snapshot from, create the snapshot, roll forward to the real tip again.
The nice thing about internalizing this functionality is that we can write tests for the code and it gives us more options to make the functionality robust. The shell scripts we have so far will be more cumbersome to maintain in the long run, especially since we will only notice later when we have broken them. I think it's safe to remove these `test_utxo_snapshots.sh` as well when we have this option in `dumptxoutset` because we have also added some good additional functional test coverage for this functionality.
ACKs for top commit:
Sjors:
re-utACK 94b0adcc37
achow101:
ACK 94b0adcc37
mzumsande:
ACK 94b0adcc37
pablomartin4btc:
re-ACK 94b0adcc37
Tree-SHA512: a4c9af5f687d1ca7bfb579a36f363882823386b5fa80c05de531b05a2782b5da6ff5baf3ada4bca8f32f63975d86f1948175abed9affe51fc958472b5f838dab
When the tracepoint was introduced in 8f37f5c2a5,
the connect_block duration was passed in microseconds `µs`.
By starting to use steady clock in fabf1cdb20
this changed to nanoseconds `ns`. As the test only checked if the
duration value is `> 0` as a plausibility check, this went unnoticed.
I detected this when setting up monitoring for block validation time
as part of the Great Consensus Cleanup Revival discussion.
This change casts the duration explicitly to nanoseconds (as it has been
nanoseconds for the last three releases; switching back now would 'break'
the broken API again; there don't seem to be many users affected), updates
the documentation and adds a check for an upper bound to the tracepoint
interface tests. The upper bound is quite lax as mining the block takes
much longer than connecting the empty test block. It's however able to
detect incorrect duration units passed.
6a68343ffb doc: Prepend 'build/' to binary paths under 'src/' in docs (Lőrinc)
91b3bc2b9c doc: Update documentation generation example in developer-notes.md (Lőrinc)
Pull request description:
In [the other readmes](6ce50fd9d0/src/test/README.md (L19)) we've provided a default build directory instead, unified the `developer-notes.md` to specify it explicitly.
In the next commit I've used this default to go over each reference to our binaries and changed their in-source references to the build directory.
Some of these changes were in example outputs - I haven't validated that the outputs are still the same.
I haven't modified the build folders in the devtools.
ACKs for top commit:
maflcko:
review ACK 6a68343ffb
pablomartin4btc:
ACK 6a68343ffb
fanquake:
ACK 6a68343ffb - we still need to followup with other scripts/devtools, and likely unify what we are doing in some way, but this is an improvement.
Tree-SHA512: 905d9c68cafe1e405e98d6aa089d7a36a34c9e03403df5c67ac2c9a98cfa54a0305b647cb92247dcb9f49e9b509a8ba88367392b95618c67059684c67b6c36fb
ddef914bbb doc: remove extraneous install statement (tdb3)
bc532c915e doc: add with_bdb to unix build docs (tdb3)
Pull request description:
Existing instructions for building legacy wallet support omit `-DWITH_BDB=ON`, which results in:
```
CMake Warning:
Manually-specified variables were not used by the project:
BerkeleyDB_INCLUDE_DIR
```
and a build without BDB support.
This PR updates the docs to include `-DWITH_BDB=ON`.
Also adds a minor correction to the OpenBSD build doc.
Checked by building on Linux (Debian 12.7), FreeBSD 14.1, and OpenBSD 7.5 and attempting to create a legacy wallet with the `createwallet` rpc (with `-deprecatedrpc=create_bdb`).
ACKs for top commit:
l0rinc:
utACK ddef914bbb
fanquake:
ACK ddef914bbb
Tree-SHA512: 261568700b95fc073e03db6ca64a5f0544d5aed337aee4275575c1d0d1373c2a96911947abd202da3ed7c3b7a662b700b0596c0dabefe4b50900a798eed7e118
fa09cb41f5 refactor: Remove unused LogPrint (MarcoFalke)
3333415890 scripted-diff: LogPrint -> LogDebug (MarcoFalke)
Pull request description:
`LogPrint` has many issues:
* It seems to indicate that something is being "printed", however config options such as `-printtoconsole` actually control what and where something is logged.
* It does not mention the log severity (debug).
* It is a deprecated alias for `LogDebug`, according to the dev notes.
* It wastes review cycles, because reviewers sometimes point out that it is deprecated.
* It makes the code inconsistent, when both are used, possibly even in lines right next to each other (like in `InitHTTPServer`)
Fix all issues by removing the deprecated alias.
I checked all conflicting pull requests and at the time of writing there are no conflicts, except in pull requests that are marked as draft, are yet unreviewed, or are blocked on feedback for other reasons. So I think it is fine to do now.
ACKs for top commit:
stickies-v:
ACK fa09cb41f5
danielabrozzoni:
utACK fa09cb41f5
TheCharlatan:
ACK fa09cb41f5
Tree-SHA512: 14270f4cfa3906025a0b994cbb5b2e3c8c2427c0beb19c717a505a2ccbfb1fd1ecf2fd03f6c52d22cde69a8d057e50d2207119fab2c2bc8228db3f10d4288d0f
faa382ae76 ci, doc: Drop reference to `src/.bear-tidy-config` (Hennadii Stepanov)
d71ac76842 build: Remove Autotools-based build system (Hennadii Stepanov)
e268b48419 doc: Adjust `doc/design/libraries.md` (Hennadii Stepanov)
d209e4f156 doc: Drop mentions of `share/genbuild.sh` (Hennadii Stepanov)
Pull request description:
This PR deletes the Autotools-based build system.
The MSVC build system is deleted in https://github.com/bitcoin/bitcoin/pull/30731.
ACKs for top commit:
maflcko:
re-ACK faa382ae76🍦
TheCharlatan:
ACK faa382ae76
fanquake:
ACK faa382ae76
Tree-SHA512: 53df977b5b199a1c38f7f61a042a62b24831c559ba65a461b4ac1c96a1a56e2dfd676df79f1358fd1cc1749ff27e7b548086157f337d4f596c1054cb3d2d5739
7de0c99804 doc: update dev note examples for CMake (fanquake)
Pull request description:
Update the examples in the developer notes to work with CMake.
Also added an explicit `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON` for clarity.
ACKs for top commit:
davidgumberg:
Tested ACK 7de0c99804
TheCharlatan:
ACK 7de0c99804
jonatack:
Tested ACK 7de0c99804 on arm64 macOS 14.6.1
Tree-SHA512: 561fe5e777c5b29a4f26309700c03a730c5bbb2f838630abfaa4174112ced66e733c2109cb429a1927f1f3692bf1945f6386bcaffe604a76ea24633932d39171
7ee5c3c5b2 Fix a few likely documentation typos (Lőrinc)
Pull request description:
Found them during CMake migration - and ran a quick spellcheck for the rest to cover any remaining ones
ACKs for top commit:
maflcko:
lgtm ACK 7ee5c3c5b2
Tree-SHA512: c6e7aa1e952e0d093745c4e6004c3907b7a215c6f998cc205307c0c68abcc067bf3f56e22af0deb1710186e8a871306f4bae8a35c74581e5299abcbbcddfaa75
a0abcbd382 doc: Mention multipath specifier (Ava Chow)
0019f61fc5 tests: Test importing of multipath descriptors (Ava Chow)
f97d5c137d wallet, rpc: Allow importdescriptors to import multipath descriptors (Ava Chow)
32dcbca3fb rpc: Allow importmulti to import multipath descriptors correctly (Ava Chow)
64dfe3ce4b wallet: Move internal to be per key when importing (Ava Chow)
1692245525 tests: Multipath descriptors for scantxoutset and deriveaddresses (Ava Chow)
cddc0ba9a9 rpc: Have deriveaddresses derive receiving and change (Ava Chow)
360456cd22 tests: Multipath descriptors for getdescriptorinfo (Ava Chow)
a90eee444c tests: Add unit tests for multipath descriptors (Ava Chow)
1bbf46e2da descriptors: Change Parse to return vector of descriptors (Ava Chow)
0d640c6f02 descriptors: Have ParseKeypath handle multipath specifiers (Ava Chow)
a5f39b1034 descriptors: Change ParseScript to return vector of descriptors (Ava Chow)
0d55deae15 descriptors: Add DescriptorImpl::Clone (Ava Chow)
7e86541f72 descriptors: Add PubkeyProvider::Clone (Ava Chow)
Pull request description:
It is convenient to have a descriptor which specifies both receiving and change addresses in a single string. However, as discussed in https://github.com/bitcoin/bitcoin/issues/17190#issuecomment-895515768, it is not feasible to use a generic multipath specification like BIP 88 due to combinatorial blow up and that it would result in unexpected descriptors.
To resolve that problem, this PR proposes a targeted solution which allows only a single pair of 2 derivation indexes to be inserted in the place of a single derivation index. So instead of two descriptor `wpkh(xpub.../0/0/*)` and `wpkh(xpub.../0/1/*)` to represent receive and change addresses, this could be written as `wpkh(xpub.../0/<0;1>/*)`. The multipath specifier is of the form `<NUM;NUM>`. Each `NUM` can have its own hardened specifier, e.g. `<0;1h>` is valid. The multipath specifier can also only appear in one path index in the derivation path.
This results in the parser returning two descriptors. The first descriptor uses the first `NUM` in all pairs present, and the second uses the second `NUM`. In our implementation, if a multipath descriptor is not provided, a pair is still returned, but the second element is just `nullptr`.
The wallet will not output the multipath descriptors (yet). Furthermore, when a multipath descriptor is imported, it is expanded to the two descriptors and each imported on its own, with the second descriptor being implicitly for internal (change) addresses. There is no change to how the wallet stores or outputs descriptors (yet).
Note that the path specifier is different from what was proposed. It uses angle brackets and the semicolon because these are unused characters available in the character set and I wanted to avoid conflicts with characters already in use in descriptors.
Closes#17190
ACKs for top commit:
darosior:
re-ACK a0abcbd382
mjdietzx:
reACK a0abcbd382
pythcoiner:
reACK a0abcbd
furszy:
Code review ACK a0abcbd
glozow:
light code review ACK a0abcbd382
Tree-SHA512: 84ea40b3fd1b762194acd021cae018c2f09b98e595f5e87de5c832c265cfe8a6d0bc4dae25785392fa90db0f6301ddf9aea787980a29c74f81d04b711ac446c2
41051290ab cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)
6ce50fd9d0 doc: Update for CMake-based build system (Hennadii Stepanov)
9730288a0c ci: Migrate CI scripts to CMake (Hennadii Stepanov)
c360837ca5 cmake, lint: Adjust `lint_includes_build_config` (Hennadii Stepanov)
3885441ee0 cmake: Add presets for native Windows builds (Hennadii Stepanov)
7681746b20 cmake: Add vcpkg manifest file (Hennadii Stepanov)
8b6f1c4353 cmake: Add `Coverage` and `CoverageFuzz` scripts (Hennadii Stepanov)
65bdbc1ff2 cmake: Add `docs` build target (Hennadii Stepanov)
fb75ebbc33 cmake: Add compiler diagnostic flags (Hennadii Stepanov)
e821f0a37a cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
747adb6ffe cmake: Add `Maintenance` module (Hennadii Stepanov)
1f60b30df0 cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables (Hennadii Stepanov)
2b43c45b13 cmake: Add `AddWindowsResources` module (Hennadii Stepanov)
973a3b0c5d cmake: Implement `install` build target (Hennadii Stepanov)
84ac35cfd4 cmake: Add cross-compiling support (Hennadii Stepanov)
0d01c228a7 build: Generate `toolchain.cmake` in depends (Hennadii Stepanov)
91a799247d depends: Add host-specific `cmake_system_version` variables (Hennadii Stepanov)
9b31209b4c depends: Rename `cmake_system` -> `cmake_system_name` (Hennadii Stepanov)
4a5208a81d Revert "build, qt: Do not install *.prl files" (Hennadii Stepanov)
6522af62af depends: Amend handling flags environment variables (Hennadii Stepanov)
90cec4d251 cmake: Add `MULTIPROCESS` option (Hennadii Stepanov)
bb1a450dcb cmake: Build `bitcoin-chainstate` executable (Hennadii Stepanov)
aed38ea58c cmake: Build `bitcoinkernel` library (Hennadii Stepanov)
975d67369b cmake: Build `test_bitcoin-qt` executable (Hennadii Stepanov)
10fcc668a3 cmake: Add `WITH_DBUS` option (Hennadii Stepanov)
5bb5a4bc75 cmake: Add `libqrencode` optional package support (Hennadii Stepanov)
57a6e2ef4a cmake: Build `bitcoin-qt` executable (Hennadii Stepanov)
30f642952c cmake: Add `WERROR` option (Hennadii Stepanov)
c98d4a4c34 cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov)
a01cb6e63f cmake: Add `HARDENING` option (Hennadii Stepanov)
a8a2e364ac cmake: Add Python-based tests (Hennadii Stepanov)
3d85379570 cmake: Add fuzzing options (Hennadii Stepanov)
908530e312 cmake: Add `SANITIZERS` option (Hennadii Stepanov)
8bb0e85631 cmake: Build `bench_bitcoin` executable (Hennadii Stepanov)
801735163a cmake: Add external signer support (Hennadii Stepanov)
353e0c9e96 cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov)
d2fda82b49 cmake: Add `libzmq` optional package support (Hennadii Stepanov)
ae7b39a0e1 cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov)
6480e1dcdb cmake: Add `libnatpmp` optional package support (Hennadii Stepanov)
e73e9304a1 cmake: Build `bitcoin-util` executable (Hennadii Stepanov)
027c6d7caa cmake: Build `bitcoin-tx` executable (Hennadii Stepanov)
d10c5c34c3 cmake: Add wallet functionality (Hennadii Stepanov)
ab2e99b0d9 cmake: Create test suite for `ctest` (Hennadii Stepanov)
959370bd76 cmake: Build `test_bitcoin` executable (Hennadii Stepanov)
b27bf9700d cmake: Build `bitcoin-cli` executable (Hennadii Stepanov)
a9813df826 cmake: Build `bitcoind` executable (Hennadii Stepanov)
97829ce2d5 cmake: Add `FindLibevent` module (Hennadii Stepanov)
3118e40c61 cmake: Build `bitcoin_consensus` library (Hennadii Stepanov)
809a2f1929 cmake: Build `bitcoin_util` static library (Hennadii Stepanov)
0a9a521a70 cmake: Build `bitcoin_crypto` library (Hennadii Stepanov)
958971f476 cmake: Build `univalue` static library (Hennadii Stepanov)
752747fda8 cmake: Generate `obj/build.h` header (Hennadii Stepanov)
1f0a78edf3 cmake: Build `minisketch` static library (Hennadii Stepanov)
12bfbc8154 cmake: Build `leveldb` static library (Hennadii Stepanov)
51985c5304 cmake: Build `crc32c` static library (Hennadii Stepanov)
db7a198f29 cmake: Build `secp256k1` subtree (Hennadii Stepanov)
dbb7ed14e8 cmake: Add `ccache` support (Hennadii Stepanov)
cedfdf6c72 cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
b6b5e732c8 cmake: Add global compiler and linker flags (Hennadii Stepanov)
f98327931b cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov)
4a0af29697 cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
35cffc497d cmake: Add POSIX threads support (Hennadii Stepanov)
fd72d00ffe cmake: Add position independent code support (Hennadii Stepanov)
07069e2bb0 cmake: Add introspection module (Hennadii Stepanov)
27d687fc1f cmake: Add `config/bitcoin-config.h` support (Hennadii Stepanov)
fe5cdace5f cmake: Print compiler and linker flags in summary (Hennadii Stepanov)
70683884c5 cmake: Introduce interface libraries to encapsulate common flags (Hennadii Stepanov)
a2317e27b7 cmake: Add root `CMakeLists.txt` file (Hennadii Stepanov)
Pull request description:
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of contributors, including (in alphabetical order):
- [**achow101**](https://github.com/achow101)
- [**fanquake**](https://github.com/fanquake)
- [**maflcko**](https://github.com/maflcko)
- [**m3dwards**](https://github.com/m3dwards)
- [**pablomartin4btc**](https://github.com/pablomartin4btc)
- [**real-or-random**](https://github.com/real-or-random)
- [**ryanofsky**](https://github.com/ryanofsky)
- [**sipsorcery**](https://github.com/sipsorcery)
- [**TheCharlatan**](https://github.com/TheCharlatan)
- [**theStack**](https://github.com/theStack)
- [**theuni**](https://github.com/theuni)
- [**vasild**](https://github.com/vasild)
Reviewing in a separate staging repo was suggested in https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431798320.
The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.
Please refer to the [build options parity table](https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e). The "auto" value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.
System requirements for using the CMake-based build system:
- CMake >= 3.22 (if not available in your system's repository, it can be downloaded from https://cmake.org/download/)
- a build tool of your choice:
- any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
- Ninja (https://ninja-build.org/)
- MSBuild
- Xcode
A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).
---
We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR's goal may be ignored.
Thank you all for your understanding.
ACKs for top commit:
maflcko:
review ACK 41051290ab🐥
sipsorcery:
ACK 41051290ab.
vasild:
ACK 41051290ab
TheCharlatan:
ACK 41051290ab
pablomartin4btc:
tACK 41051290ab
i-am-yuvi:
tACK [`4105129`](41051290ab)
theuni:
ACK 41051290ab.
fanquake:
ACK 41051290ab
Tree-SHA512: 6c1445054436c6c00ad63bfa0f19d64091a2b25c9bd694f85bf2218ac358ffb774d6c000685b3ca1e9b50401babed989fa2a0694b774c211d226bfd1944c9b39
16d8261181 doc: Drop no longer needed workaround for WSL (Hennadii Stepanov)
Pull request description:
This PR effectively reverts commit 4f890ba6bc from https://github.com/bitcoin/bitcoin/pull/11437, which fixed some build issues on WSL seven years ago.
Testing the current master branch @ 31a3ff5515 on Windows 11 + WSL using Ubuntu 24.04 or Debian images, I noticed that the workaround is no longer required. Moreover, it doesn't affect the build process at all, which means the hashes of the built packages in depends remain the same and the `configure` log in the main build system remains the same as well.
ACKs for top commit:
sipsorcery:
utACK 16d8261181.
Tree-SHA512: 703a2ac4647125c91aad47131e2723fd05af30b0cfae5677a26d3e89a77c2779e0197584208f3b378ed64dd7305512d9064fd073ec06517f86e9905af4ec8838
6bfa26048d testnet: Add timewarp attack prevention for Testnet4 (Fabian Jahr)
0100907ca1 testnet: Add Testnet4 difficulty adjustment rules fix (Fabian Jahr)
74a04f9e7a testnet: Introduce Testnet4 (Fabian Jahr)
Pull request description:
To supplement the [ongoing conceptual discussion about a testnet reset](https://groups.google.com/g/bitcoindev/c/9bL00vRj7OU/m/9yCPo3uUBwAJ) I have drafted a move to v4 including a fix to the difficulty adjustment mechanism, which was part of the motivation that started the discussion.
Conceptual considerations:
- The conceptual discussion about doing a testnet4 or softforking the fix into testnet3 is outside of the scope of this PR and I would ask reviewers to contribute their opinions on this on the ML instead. However, I am happy to adapt this PR to a softfork change on testnet3 if there is consensus for that instead.
- The difficulty adjustment fix suggested here touches the `CalculateNextWorkRequired` function and uses the same logic used in `GetNextWorkRequired` to find the last previous block that was not mined with difficulty 1 under the exceptionf. An alternative fix briefly mentioned on the mailing list by Jameson Lopp would be to "restrict the special testnet minimum difficulty rule so that it can't be triggered on the block right before a difficulty retarget". That would also fix the issue but I find my suggestion here a bit more elegant.
ACKs for top commit:
jsarenik:
tACK 6bfa26048d
achow101:
ACK 6bfa26048d
murchandamus:
tACK 6bfa26048d
Tree-SHA512: 0b8b69a621406a944da5be551b863d065358ba94d85dd3b80d83c412660e230ee93b27316081fbee9b4851cc4ff8585db64c7dfa26cb5148ac835663f2712c3d
1f93e3c360 add deprecation warning for mempoolfullrbf (glozow)
4400c979a3 [doc] update documentation for new mempoolfullrbf default (glozow)
Pull request description:
Followup to #30493. Update bips.md and policy/*.md to reflect new default rules around signaling requirements in RBF.
Also, log a warning when `-mempoolfullrbf=0` that this config option is deprecated and will be removed in a future release.
ACKs for top commit:
petertodd:
ACK 1f93e3c360
instagibbs:
ACK 1f93e3c360
tdb3:
ACK 1f93e3c360
Tree-SHA512: f60a9524f15cfaa4c10c40b6f62b787d3f9865aac48ca883def30efac4f8a118f1359532f1b209ea34e201f0b1c92398abc8bc1e439e6b60910cc7f75c51e9ae
e9de0a76b9 doc: release note for 30212 (willcl-ark)
87b1880525 rpc: clarify ALREADY_IN_CHAIN rpc errors (willcl-ark)
Pull request description:
Closes: #19363
Renaming this error improves clarity around the returned error both internally and externally when a transactions' outputs are already found in the utxo set (`TransactionError::ALREADY_IN_CHAIN -> TransactionError::ALREADY_IN_UTXO_SET`)
ACKs for top commit:
tdb3:
ACK e9de0a76b9
ismaelsadeeq:
ACK e9de0a76b9
ryanofsky:
Code review ACK e9de0a76b9.
Tree-SHA512: 7d2617200909790340951fe56a241448f9ce511900777cb2a712e8b9c0778a27d1f912b460f82335844224f1abb4322bc898ca076440959edade55c082a09237
fa895c7283 mingw: Document mode wbx workaround (MarcoFalke)
fa359255fe Add -blocksxor boolean option (MarcoFalke)
fa7f7ac040 Return XOR AutoFile from BlockManager::Open*File() (MarcoFalke)
Pull request description:
Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to block requests (via P2P, RPC, REST, ...).
Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat files when writing or reading them.
Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat files. Any program that intentionally wants to mess with the dat files can still trivially do so.
The XOR pattern is only applied when the blocksdir is freshly created, and there is an option to disable it (on creation), so that people can disable it, if needed.
ACKs for top commit:
achow101:
ACK fa895c7283
TheCharlatan:
Re-ACK fa895c7283
hodlinator:
ACK fa895c7283
Tree-SHA512: c92a6a717da83bc33a9b8671a779eeefde2c63b192362ba1d71e6535ee31d08e2802b74acc908345197de9daac6930e4771595ee25b09acd5a67f7ea34854720
6d33e13bd4 doc: tor.md: use -bind=127.0.0.1:8334=onion for the Tor bind (David Gumberg)
a7f5d188cc doc: add release notes for #22729 (Vasil Dimov)
Pull request description:
Add release notes for #22729.
ACKs for top commit:
davidgumberg:
reACK 6d33e13bd4
willcl-ark:
ACK 6d33e13bd4
Tree-SHA512: 9d7e66ee1d0bb1d75b8273707d30f20915d5040a768c2c5cd47c84997df2645c8bec35db6c09dc77ab917836622411b924373816cbc83c4be38e2e9156a139d8
75648cea5a test: add P2A ProduceSignature coverage (Greg Sanders)
7998ce6b20 Add release note for P2A output feature (Greg Sanders)
71c9b02a04 test: add P2A coverage for decodescript (Greg Sanders)
1349e9ec15 test: Add anchor mempool acceptance test (Greg Sanders)
9d89209937 policy: stop 3rd party wtxid malleability of anchor spend (Greg Sanders)
b60aaf8b23 policy: make anchor spend standard (Greg Sanders)
455fca86cf policy: Add OP_1 <0x4e73> as a standard output type (Greg Sanders)
Pull request description:
This is a sub-feature taken out of the original proposal for ephemeral anchors #30239
This PR makes *spending* of `OP_1 <0x4e73>` (i.e. `bc1pfeessrawgf`) standard. Creation of this output type is already standard.
Any future witness output types are considered relay-standard to create, but not to spend. This preserves upgrade hooks, such as a completely new output type for a softfork such as BIP341. It also gives us a bit of room to use a new output type for policy uses.
This particular sized witness program has no other known use-cases (https://bitcoin.stackexchange.com/a/110664/17078), s it affords insufficient cryptographic security for a secure commitment to data, such as a script or a public key. This makes this type of output "keyless", or unauthenticated.
As a witness program, the `scriptSig` of the input MUST be blank, by BIP141. This helps ensure txid-stability of the spending transaction, which may be required for smart contracting wallets. If we do not use segwit, a miner can simply insert an `OP_NOP` in the `scriptSig` without effecting the result of program execution.
An additional relay restriction is to disallow non-empty witness data, which an adversary may use to penalize the "honest" transactor when RBF'ing the transaction due to the incremental fee requirement of RBF rules.
The intended use-case for this output type is to "anchor" the transaction with a spending child to bring exogenous CPFP fees into the transaction package, encouraging the inclusion of the package in a block. The minimal size of creation and spending of this output makes it an attractive contrast to outputs like `p2sh(OP_TRUE)` and `p2wsh(OP_TRUE)` which
are significantly larger in vbyte terms.
Combined with TRUC transactions which limits the size of child transactions significantly, this is an attractive option for presigned transactions that need to be fee-bumped after the fact.
ACKs for top commit:
sdaftuar:
utACK 75648cea5a
theStack:
re-ACK 75648cea5a
ismaelsadeeq:
re-ACK 75648cea5a via [diff](e7ce6dc070..75648cea5a)
glozow:
ACK 75648cea5a
tdb3:
ACK 75648cea5a
Tree-SHA512: d529de23d20857e6cdb40fa611d0446b49989eaafed06c28280e8fd1897f1ed8d89a4eabbec1bbf8df3d319910066c3dbbba5a70a87ff0b2967d5205db32ad1e
3cd24aadb2 doc: remove obsolete mention and link to closed PR (Marnix)
Pull request description:
Remove the mention and link as the PR (https://github.com/bitcoin/bitcoin/pull/22341) is closed and the description is wrong/outdated anyway.
ACKs for top commit:
BrandonOdiwuor:
ACK 3cd24aadb2
tdb3:
ACK 3cd24aadb2
Tree-SHA512: 5cd97029337f0cdfe81b6be9401adc4fe51ae2868f8fcadcb03828531a38380a587c32840850a924b6428f62df7d20a1e16ef7414d4078e7bb2c4e359b1fae40
f70eb0eeef doc: Remove reference to resolved issue (Daniela Brozzoni)
b27ef8ec7f doc: Update issue reference for libbitcoinkernel (Daniela Brozzoni)
Pull request description:
- The discussion of libbitcoinkernel has moved from 24303 to 27587
- Issue 15732 has been resolved, removing it from the document
ACKs for top commit:
maflcko:
ACK f70eb0eeef
Tree-SHA512: 11b597d9710504010945aae66f7e488403895aa8e1e091f3a8f6737dc128a4fde185daff8d4709cbbb69f454d3a649c4217e82a6bfc8ee2b25c8a1c047b57f1b
d93b794709 tests: improve wallet multisig descriptor test and docs (Michael Dietz)
Pull request description:
It is best to store all key origin information
(master key fingerprint and all derivation steps)
in the multisig descriptor. Being explicit with
this information should be beneficial if this approach is used with other wallets/signers (whether hardware or software). There is no harm including all of this with xpubs (if anything it simplifies the test code) and makes this example/docs more complete and safer incase it is referenced by others.
ACKs for top commit:
S3RK:
Code Review ACK d93b794709
achow101:
ACK d93b794709
Tree-SHA512: 0e5c4d13f060489405e6cf50c8a09911f5a0cee71023649235afd80a5e3aae38d52c6e12ad4660205b9357b09f45596941391bdcf6fceccbe07c4e5a1592a482
fa8f53273c refactor: Remove no longer needed clang-15 workaround for std::span (MarcoFalke)
9999dbc1bd fuzz: Clarify Apple-Clang-16 workaround (MarcoFalke)
fa7462c67a build: Bump clang minimum supported version to 16 (MarcoFalke)
Pull request description:
Most supported operating systems ship with clang-16 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.
For reference:
* https://packages.debian.org/bookworm/clang-16
* https://packages.ubuntu.com/noble/clang (clang-18)
* CentOS-like 8/9 Stream: All Clang versions from 16 to 17
* FreeBSD 12/13: All Clang versions from 16 to 18
* OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang18`); No idea about OpenSuse Leap
On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example:
* https://packages.debian.org/bookworm/g++ (g++-12)
* https://packages.ubuntu.com/jammy/g++ (g++-11)
* https://apt.llvm.org/, or nix, or guix, or compile clang from source, ...
**Ubuntu 22.04 LTS does not ship with clang-16**, so one of the above workarounds is needed there.
macOS 13 is unaffected, and the previous minimum requirement of Xcode15.0 remains, see also b1ba1b178f/.github/workflows/ci.yml (L93). For macOS 11 (Big Sur) and 12 (Monterey) you need to install a more recent version of llvm, this remains unchanged as well, see b1ba1b178f/doc/build-osx.md (L54).
ACKs for top commit:
hebasto:
ACK fa8f53273c, I have reviewed the code and it looks OK.
TheCharlatan:
Re-ACK fa8f53273c
stickies-v:
ACK fa8f53273c
Tree-SHA512: 18b79f88301a63bb5e367d2f52fffccd5fb84409061800158e51051667f6581a4cd71d4859d4cfa6d23e47e92963ab637e5ad87e3170ed23b5bebfbe99e759e2
926b8e39dc [doc] add release note for TRUC (glozow)
19a9b90617 use version=3 instead of v3 in debug strings (glozow)
881fac8e60 scripted-diff: change names from V3 to TRUC (glozow)
a573dd2617 [doc] replace mentions of v3 with TRUC (glozow)
089b5757df rename mempool_accept_v3.py to mempool_truc.py (glozow)
f543852a89 rename policy/v3_policy.* to policy/truc_policy.* (glozow)
Pull request description:
Adds a release note for TRUC policy which will be live in v28.0.
For clarity, replaces mentions of "v3" with "TRUC" in most places. Suggested in
- https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1629749583
- https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1624500904
I changed error strings from "v3-violation" to "TRUC-violation" but left v3 in the debug strings because I think it might be clearer for somebody who is debugging. Similarly, I left some variables unchanged because I think they're more descriptive this way, e.g. `tx_v3_from_v2_and_v3`. I'm happy to debate places that should or shouldn't be documented differently in this PR, whatever is clearest to everyone.
ACKs for top commit:
instagibbs:
reACK 926b8e39dc
achow101:
ACK 926b8e39dc
ismaelsadeeq:
Code review ACK 926b8e39dc
Tree-SHA512: 16c88add0a29dc6d1236c4d45f34a17b850f6727b231953cbd52eb9f7268d1d802563eadfc8b7928c94ed3d7a615275dd103e57e81439ebf3ba2b12efa1e42af
73f0a6cbd0 doc: detail -rpccookieperms option (willcl-ark)
d2afa2690c test: add rpccookieperms test (willcl-ark)
f467aede78 init: add option for rpccookie permissions (willcl-ark)
7df03f1a92 util: add perm string helper functions (willcl-ark)
Pull request description:
This PR picks up #26088 by aureleoules which adds a bitcoind launch option `-rpccookieperms` to set the file permissions of the cookie generated by bitcoin core.
Example usage to make the generated cookie group-readable: `./src/bitcoind -rpccookieperms=group`.
Accepted values for `-rpccookieperms` are `[owner|group|all]`. We let `fs::perms` handle platform-specific permissions changes.
ACKs for top commit:
achow101:
ACK 73f0a6cbd0
ryanofsky:
Code review ACK 73f0a6cbd0. Main change since last review is no longer throwing a skip exception in the rpc test on windows, so other checks can run after it, and overall test result is passing, not skipped. Also were clarifying renames and documentation improvements.
tdb3:
cr ACK 73f0a6cbd0
Tree-SHA512: e800d59a44aca10e1c58ca69bf3fdde9f6ccf5eab4b7b962645af6d6bc0cfa3a357701e409c8c60d8d7744fcd33a91e77ada11790aa88cd7811ef60fab86ab11
b5fc6d46a3 guix: use glibc 2.31 (fanquake)
Pull request description:
Set minimum required glibc to 2.31.
The glibc 2.31 branch is still maintained: https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/release/2.31/master.
Remove the stack-protector check from test-security-check, as the test
no-longer fails, and given the control we have of the end, the actual
security-check test seems sufficient (this might also be applied to some
of the other checks).
Drops runtime support for Ubuntu Bionic 18.04 and RHEL-8 from the release binaries.
ACKs for top commit:
TheCharlatan:
ACK b5fc6d46a3
Tree-SHA512: ba7e727240fa0ebebfb8b749024c71cbfdec37c33b39627866d78f9318ccdc687fd5103a63ee0e98cf809d9954dde56b1b305691c33d1de275ed0519f716c921
94ed4fbf8e Add release note for size 2 package rbf (Greg Sanders)
afd52d8e63 doc: update package RBF comment (Greg Sanders)
6e3c4394cf mempool: Improve logging of replaced transactions (Greg Sanders)
d3466e4cc5 CheckPackageMempoolAcceptResult: Check package rbf invariants (Greg Sanders)
316d7b63c9 Fuzz: pass mempool to CheckPackageMempoolAcceptResult (Greg Sanders)
4d15bcf448 [test] package rbf (glozow)
dc21f61c72 [policy] package rbf (Suhas Daftuar)
5da3967815 PackageV3Checks: Relax assumptions (Greg Sanders)
Pull request description:
Allows any 2 transaction package with no in-mempool ancestors to do package RBF when directly conflicting with other mempool clusters of size two or less.
Proposed validation steps:
1) If the transaction package is of size 1, legacy rbf rules apply.
2) Otherwise the transaction package consists of a (parent, child) pair with no other in-mempool ancestors (or descendants, obviously), so it is also going to create a cluster of size 2. If larger, fail.
3) The package rbf may not evict more than 100 transactions from the mempool(bip125 rule 5)
4) The package is a single chunk
5) Every directly conflicted mempool transaction is connected to at most 1 other in-mempool transaction (ie the cluster size of the conflict is at most 2).
6) Diagram check: We ensure that the replacement is strictly superior, improving the mempool
7) The total fee of the package, minus the total fee of what is being evicted, is at least the minrelayfee * size of the package (equivalent to bip125 rule 3 and 4)
Post-cluster mempool this will likely be expanded to general package rbf, but this is what we can safely support today.
ACKs for top commit:
achow101:
ACK 94ed4fbf8e
glozow:
reACK 94ed4fbf8e via range-diff
ismaelsadeeq:
re-ACK 94ed4fbf8e
theStack:
Code-review ACK 94ed4fbf8e
murchandamus:
utACK 94ed4fbf8e
Tree-SHA512: 9bd383e695964f362f147482bbf73b1e77c4d792bda2e91d7f30d74b3540a09146a5528baf86854a113005581e8c75f04737302517b7d5124296bd7a151e3992
260f8da71a refactor: remove warnings globals (stickies-v)
9c4b0b7ce4 node: update uiInterface whenever warnings updated (stickies-v)
b071ad9770 introduce and use the generalized `node::Warnings` interface (stickies-v)
20e616f864 move-only: move warnings from common to node (stickies-v)
bed29c481a refactor: remove unnecessary AppendWarning helper function (stickies-v)
Pull request description:
This PR:
- moves warnings from common to the node library and into the node namespace (as suggested in https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570069541)
- generalizes the warnings interface to `Warnings::Set()` and `Warnings::Unset()` methods, instead of having a separate function and globals for each warning. As a result, this simplifies the `kernel::Notifications` interface.
- removes warnings.cpp from the kernel library
- removes warning globals
- adds testing for the warning logic
Behaviour change introduced:
- the `-alertnotify` command is executed for all `KernelNotifications::warningSet` calls, which now also covers the `LARGE_WORK_INVALID_CHAIN` warning
- the GUI is updated automatically whenever a warning is (un)set, covering some code paths where it previously wouldn't be, e.g. when `node::AbortNode()` is called, or for the `LARGE_WORK_INVALID_CHAIN` warning
Some discussion points:
- ~is `const std::string& id` the best way to refer to warnings? Enums are an obvious alternative, but since we need to define warnings across libraries, strings seem like a straightforward solution.~ _edit: updated approach to use `node::Warning` and `kernel::Warning` enums._
ACKs for top commit:
achow101:
ACK 260f8da71a
ryanofsky:
Code review ACK 260f8da71a. Only change since last review was rebasing
TheCharlatan:
Re-ACK 260f8da71a
Tree-SHA512: a3fcedaee0d3ad64e9c111aeb30665162f98e0e72acd6a70b76ff2ddf4f0a34da4f97ce353c322a1668ca6ee4d8a81cc6e6d170c5bbeb7a43cffdaf66646b588
Instead of having separate warning functions (and globals) for each
different warning that can be raised, encapsulate this logic into
a single class and allow to (un)set any number of warnings.
Introduces behaviour change:
- the `-alertnotify` command is executed for all
`KernelNotifications::warningSet` calls, which now also covers the
`LARGE_WORK_INVALID_CHAIN` warning.
- previously, warnings were returned based on a predetermined order,
e.g. with the "pre-release test build" warning always first. This
is no longer the case, and Warnings::GetMessages() will return
messages sorted by the id of the warning.
Removes warnings.cpp from kernel.
c7376babd1 doc: Clarify distinction between util and common libraries in libraries.md (Ryan Ofsky)
4f74c59334 util: Move util/string.h functions to util namespace (Ryan Ofsky)
4d05d3f3b4 util: add TransactionError includes and namespace declarations (Ryan Ofsky)
680eafdc74 util: move fees.h and error.h to common/messages.h (Ryan Ofsky)
02e62c6c9a common: Add PSBTError enum (Ryan Ofsky)
0d44c44ae3 util: move error.h TransactionError enum to node/types.h (Ryan Ofsky)
9bcce2608d util: move spanparsing.h to script/parsing.h (Ryan Ofsky)
6dd2ad4792 util: move spanparsing.h Split functions to string.h (Ryan Ofsky)
23cc8ddff4 util: move HexStr and HexDigit from util to crypto (TheCharlatan)
6861f954f8 util: move util/message to common/signmessage (Ryan Ofsky)
cc5f29fbea build: move memory_cleanse from util to crypto (Ryan Ofsky)
5b9309420c build: move chainparamsbase from util to common (Ryan Ofsky)
ffa27af24d test: Add check-deps.sh script to check for unexpected library dependencies (Ryan Ofsky)
Pull request description:
Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically:
- Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity.
- Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing.
- Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of the util library.
Motivation for this change is that the util library is a dependency of the kernel, and we should remove functionality from util that shouldn't be called by kernel code or kernel applications. These changes should also improve code organization and make functions easier to discover. Some of these same moves are (or were) part of #28690, but did not help with code organization, or made it worse, so it is better to move them and clean them up in the same PR so code only has to change one time.
ACKs for top commit:
achow101:
ACK c7376babd1
TheCharlatan:
Re-ACK c7376babd1
hebasto:
re-ACK c7376babd1.
Tree-SHA512: 5bcef16c1255463b1b69270548711e7ff78ca0dd34e300b95e3ca1ce52ceb34f83d9ddb2839e83800ba36b200de30396e504bbb04fa02c6d0c24a16d06ae523d
Set minimum required glibc to 2.31.
The glibc 2.31 branch is still maintained:
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/release/2.31/master.
Remove the stack-protector check from test-security-check, as the test
no-longer fails, and given the control we have of the end, the actual
security-check test seems sufficient (this might also be applied to some
of the other checks).
Drops runtime support for Ubuntu Bionic 18.04 and RHEL-8 from the release binaries.
429ec1aaaa refactor: Rename CTransaction::nVersion to version (Ava Chow)
27e70f1f5b consensus: Store transaction nVersion as uint32_t (Ava Chow)
Pull request description:
Given that the use of a transaction's nVersion is always as an unsigned int, it doesn't make sense to store it as signed and then cast it to unsigned everywhere it is used and displayed.
Since a few alternative implementations have recently been revealed to have made an error with this signedness that would have resulted in consensus failure, I think it makes sense for us to just make this always unsigned to make it clear that the version is treated as unsigned. This would also help us avoid future potential issues with signedness of this value.
I believe that this is safe and does not actually change what transactions would or would not be considered both standard and consensus valid. Within consensus, the only use of the version in consensus is in BIP68 validation which was already casting it to uint32_t. Within policy, although it is used as a signed int for the transaction version number check, I do not think that this change would change standardness. Standard transactions are limited to the range [1, 2]. Negative numbers would have fallen under the < 1 condition, but by making it unsigned, they are still non-standard under the > 2 condition.
Unsigned and signed ints are serialized and unserialized the same way so there is no change in serialization.
ACKs for top commit:
maflcko:
ACK 429ec1aaaa 🐿
glozow:
ACK 429ec1aaaa
shaavan:
ACK 429ec1aaaa💯
Tree-SHA512: 0bcd92a245d7d16c3665d2d4e815a4ef28207ad4a1fb46c6f0203cdafeab1b82c4e95e4bdce7805d80a4f4a46074f6542abad708e970550d38a00d759e3dcef1
fa780e1c25 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 (cfaac2a60f)
* 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 fa780e1c25
hebasto:
ACK fa780e1c25, I have reviewed the code and it looks OK.
willcl-ark:
crACK fa780e1c25
Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c
1f6ab1215b minor: remove unnecessary semicolons from RPC content type examples (Matthew Zipkin)
b225295298 test: use json-rpc 2.0 in all functional tests by default (Matthew Zipkin)
391843b029 bitcoin-cli: use json-rpc 2.0 (Matthew Zipkin)
d39bdf3397 test: remove unused variable in interface_rpc.py (Matthew Zipkin)
0ead71df8c 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 1f6ab1215b
cbergqvist:
ACK 1f6ab1215b
Tree-SHA512: 49bf14c70464081280216ece538a2f5ec810bac80a86a83ad3284f0f1b017edf755a1a74a45be279effe00218170cafde7c2de58aed07097a95c2c6b837a6b6c
In order to ensure that the change of nVersion to a uint32_t in the
previous commit has no effect, rename nVersion to version in this commit
so that reviewers can easily spot if a spot was missed or if there is a
check somewhere whose semantics have changed.
30a01134cd [doc] update bips.md for 431 (glozow)
9dbe6a03f0 [test] wallet uses CURRENT_VERSION which is 2 (glozow)
539404fe0f [policy] make v3 transactions standard (glozow)
052ede75af [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 30a01134cd
instagibbs:
utACK 30a01134cd
murchandamus:
ACK 30a01134cd
ismaelsadeeq:
ACK 30a01134cd🛰️
Tree-SHA512: 2a4aec0442c860e792a061d83e36483c1f1b426f946efbdf664c8db97a596e498b535707e1d3a900218429486ea69fd4552e3d476526a6883cbd5556c6534b48
9013e2b97e 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](b483084d86). 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](234bfbf6a5/doc/init.md (L39)). They should be updated to improve clarity.
ACKs for top commit:
edilmedeiros:
ACK 9013e2b97e
kevkevinpal:
reACK [9013e2b](9013e2b97e)
achow101:
ACK 9013e2b97e
stickies-v:
ACK 9013e2b97e
tdb3:
ACK for 9013e2b97e
Tree-SHA512: 5ac5ad672ad181d574e19e29c3727fb9e5373282444fae09b42d113d5c8915cb2829d496212638cdc4b70540b7e1794a751fcdc9539f956a594cddd70c8fd747
3c08e11c3e doc: JSON-RPC request Content-Type is application/json (Luke Dashjr)
Pull request description:
Specify json content type in RPC examples.
Picks up #29946. Which needed rebasing and the commit message fixing,
ACKs for top commit:
laanwj:
ACK 3c08e11c3e
tdb3:
ACK for 3c08e11c3e
Tree-SHA512: 770bbbc0fb324cb63628980b13583cabf02e75079851850170587fb6eca41a70b01dcedaf1926bb6488eb9816a3cc6616fe8cee8c4b7e09aa39b7df5834ca0ec
This supports lcov 2.x in the sense that we are no-longer hardcoding
version specific options, and instead will use the `LCOV_OPTS` variable
(which is the more correct/flexible thing to do in any case). It's also
quite likely that devs are already having to pass extra options to lcov
2.x, given it's more stringent in terms of coverage generation and error
checking. See this thread for an example:
https://github.com/linux-test-project/lcov/issues/238.
Added an example to the developer notes.
Tested on one machine (LCOV 2.0, gcc 13.2) with:
```bash
./autogen.sh
./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic" LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch"
make
make cov
<snip>
Processing file src/netaddress.cpp
lines=521 hit=480 functions=72 hit=72 branches=675 hit=499
Overall coverage rate:
lines......: 81.8% (79362 of 97002 lines)
functions......: 77.8% (10356 of 13310 functions)
branches......: 49.6% (130628 of 263196 branches)
```
and another machine (LCOV 2.1, Clang 18.1.3) with:
```bash
./autogen.sh
./configure --enable-lcov CC=clang CXX=clang++ LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent"
make
make cov
<snip>
Processing file src/util/strencodings.cpp
lines=315 hit=311 functions=38 hit=38 branches=425 hit=357
Overall coverage rate:
source files: 622
lines.......: 79.8% (70311 of 88132 lines)
functions...: 78.1% (13968 of 17881 functions)
branches....: 44.5% (157551 of 354317 branches)
Message summary:
101 warning messages:
count: 1
inconsistent: 100
3528 ignore messages:
inconsistent: 3528
```
5deb0b024e build, test, doc: Temporarily remove Android-related stuff (Hennadii Stepanov)
Pull request description:
Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++ (see https://github.com/bitcoin/bitcoin/issues/29360).
All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x.
This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not used at this moment.
ACKs for top commit:
vasild:
ACK 5deb0b024e
fanquake:
ACK 5deb0b024e - given none of this is currently tested/wont compile. Can be revisted in future.
Tree-SHA512: 3bc2ccfe881e11cc1d78c27acd6f1d86cfba86821ef3bb5eca2e80d978fdfa13659ec82284dcaadc507e2394524dea91d4b8f81d0030c1cef9708df8be76bf07
2fd34ba504 Add sanity checks for various ATMPArgs booleans (Greg Sanders)
20d8936d8b [refactor] make some members MemPoolAccept-wide (glozow)
cbbfe719b2 cpfp carveout is excluded in packages (glozow)
69f7ab05ba Add m_allow_sibling_eviction as separate ATMPArgs flag (Greg Sanders)
57ee3029dd Add description for m_test_accept (Greg Sanders)
Pull request description:
First few commits of https://github.com/bitcoin/bitcoin/pull/28984 to set the stage for the package RBF logic.
These refactors are preparation for evaluating an RBF in a multi-proposed-transaction context instead of only a single proposed transaction. Also, carveouts and sibling evictions only should work in single RBF cases so add logic to preclude multi-tx cases in the future.
No behavior changes aside from bailing earlier from failed carve-outs.
ACKs for top commit:
glozow:
reACK 2fd34ba504 via range-diff
sr-gi:
utACK [2fd34ba](2fd34ba504)
theStack:
re-ACK 2fd34ba504
Tree-SHA512: 5071c5b8d9b8d2c9faa278c8c4df31de288cb407a68e4d55544c588caff6c86160cce7825453549c6ed69e29d9ccb5ee2d4a518b18f563bfb12f2ced073fe42a
84900ac34f doc: add release-notes-27064.md (Matthew Zipkin)
855dd8d592 system: use %LOCALAPPDATA% as default datadir on windows (Matthew Zipkin)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/2391
This PR changes the default datadir location on Windows from `C:\Users\Username\AppData\Roaming\Bitcoin` to `C:\Users\Username\AppData\Local\Bitcoin`. This change only applies to fresh installs. To preserve backwards compatibility, on startup we check for the existence of `C:\Users\Username\AppData\Roaming\Bitcoin\chainstate` and if it is there, we continue using the "Roaming" directory as the default datadir location.
[Note that in Windows 11 this change may be moot:](https://learn.microsoft.com/en-us/uwp/api/windows.storage.applicationdata.roamingfolder?view=winrt-22621)
> Roaming data and settings is no longer supported as of Windows 11. The recommended replacement is [Azure App Service](https://learn.microsoft.com/en-us/azure/app-service/). Azure App Service is widely supported, well documented, reliable, and supports cross-platform/cross-ecosystem scenarios such as iOS, Android and web. Settings stored here no longer roam (as of Windows 11), but the settings store is still available.
ACKs for top commit:
achow101:
ACK 84900ac34f
BenWestgate:
crACK 84900ac34f
hebasto:
re-ACK 84900ac34f, only addressed feedback since my recent [review](https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2028718273).
Tree-SHA512: 807c6e89571287e2c8f4934229aec91ef28e7d0a675234acf1b7d085c24c7b73a08b6e345fbfc9038e6239187b6b69c08490ddaa1c057de5ea975c4a000bba42
The behavior is not new, but this rule exits earlier than before.
Previously, a carve out could have been granted in PreChecks() but then
nullified in PackageMempoolChecks() when CheckPackageLimits() is called
with the default limits.
It is best to store all key origin information
(master key fingerprint and all derivation steps)
in the multisig descriptor. Being explicit with
this information should be beneficial if this approach
is used with other wallets/signers (whether hardware
or software). There is no harm including all of this
with xpubs (if anything it simplifies the test code)
and makes this example/docs more complete and safer
incase it is referenced by others.
cbc6c440e3 doc: add comments and release-notes for JSON-RPC 2.0 (Matthew Zipkin)
e7ee80dcf2 rpc: JSON-RPC 2.0 should not respond to "notifications" (Matthew Zipkin)
bf1a1f1662 rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests (Matthew Zipkin)
466b90562f rpc: Add "jsonrpc" field and drop null "result"/"error" fields (Matthew Zipkin)
2ca1460ae3 rpc: identify JSON-RPC 2.0 requests (Matthew Zipkin)
a64a2b77e0 rpc: refactor single/batch requests (Matthew Zipkin)
df6e3756d6 rpc: Avoid copies in JSONRPCReplyObj() (Matthew Zipkin)
09416f9ec4 test: cover JSONRPC 2.0 requests, batches, and notifications (Matthew Zipkin)
4202c170da test: refactor interface_rpc.py (Matthew Zipkin)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/2960
Bitcoin Core's JSONRPC server behaves with a special blend of 1.0, 1.1 and 2.0 behaviors. This introduces compliance issues with more strict clients. There are the major misbehaviors that I found:
- returning non-200 HTTP codes for RPC errors like "Method not found" (this is not a server error or an HTTP error)
- returning both `"error"` and `"result"` fields together in a response object.
- different error-handling behavior for single and batched RPC requests (batches contain errors in the response but single requests will actually throw HTTP errors)
https://github.com/bitcoin/bitcoin/pull/15495 added regression tests after a discussion in https://github.com/bitcoin/bitcoin/pull/15381 to kinda lock in our RPC behavior to preserve backwards compatibility.
https://github.com/bitcoin/bitcoin/pull/12435 was an attempt to allow strict 2.0 compliance behind a flag, but was abandoned.
The approach in this PR is not strict and preserves backwards compatibility in a familiar bitcoin-y way: all old behavior is preserved, but new rules are applied to clients that opt in. One of the rules in the [JSON RPC 2.0 spec](https://www.jsonrpc.org/specification#request_object) is that the kv pair `"jsonrpc": "2.0"` must be present in the request. Well, let's just use that to trigger strict 2.0 behavior! When that kv pair is included in a request object, the [response will adhere to strict JSON-RPC 2.0 rules](https://www.jsonrpc.org/specification#response_object), essentially:
- always return HTTP 200 "OK" unless there really is a server error or malformed request
- either return `"error"` OR `"result"` but never both
- same behavior for single and batch requests
If this is merged next steps can be:
- Refactor bitcoin-cli to always use strict 2.0
- Refactor the python test framework to always use strict 2.0 for everything
- Begin deprecation process for 1.0/1.1 behavior (?)
If we can one day remove the old 1.0/1.1 behavior we can clean up the rpc code quite a bit.
ACKs for top commit:
cbergqvist:
re ACK cbc6c440e3
ryanofsky:
Code review ACK cbc6c440e3. Just suggested changes since the last review: changing uncaught exception error code from PARSE_ERROR to MISC_ERROR, renaming a few things, and adding comments.
tdb3:
re ACK for cbc6c440e3
Tree-SHA512: 0b702ed32368b34b29ad570d090951a7aeb56e3b0f2baf745bd32fdc58ef68fee6b0b8fad901f1ca42573ed714b150303829cddad4a34ca7ad847350feeedb36
42fb5311b1 rpc: return warnings as an array instead of just a single one (stickies-v)
Pull request description:
The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning (i.e. the latest one that is set) is returned, the other ones are ignored.
Fix that by returning all warnings as an array.
As a side benefit, clean up the GetWarnings() logic.
Since this PR changes the RPC result schema, I've added release notes. Users can temporarily revert to the old results by using `-deprecatedrpc=warnings`, until it's removed in a future version.
---
Some historical context from git log:
- when `GetWarnings` was introduced in 401926283a, it was used in the `getinfo` RPC, where only a [single error/warning was returned](401926283a (diff-7442c48d42cd5455a79915a0f00cce5e13359db46437a32b812876edb0a5ccddR250)) (similar to how it is now).
- later on, "warnings" RPC response fields were introduced, e.g. in ef2a3de25c, with the description [stating](ef2a3de25c (diff-1021bd3c74415ad9719bd764ad6ca35af5dfb33b1cd863c0be49bdf52518af54R411)) that it returned "any network warnings" but in practice still only a single warning was returned
ACKs for top commit:
achow101:
re-ACK 42fb5311b1
tdb3:
Re ACK for 42fb5311b1
TheCharlatan:
ACK 42fb5311b1
maflcko:
ACK 42fb5311b1🔺
Tree-SHA512: 4225ed8979cd5f030dec785a80e7452a041ad5703445da79d2906ada983ed0bbe7b15889d663d75aae4a77d92e302c93e93eca185c7bd47c9cce29e12f752bd3
Previously, our Android builds were geared towards generating APKs,
which relied on Qt. However, after migrating to C++20, compiling for
Android became unfeasible due to Qt 5.15's compatibility limitations
with NDK only up to r25, which includes an outdated embedded libc++.
All removed stuff will be reinstated after migrating the build system to
CMake and upgrading Qt to version 6.x."
5195baa600 depends: fix miniupnpc snprintf usage on Windows (fanquake)
3c2d440f14 depends: switch miniupnpc to CMake (Cory Fields)
f5618c79d9 depends: add upstream CMake patch to miniupnpc (fanquake)
6866b571ab depends: miniupnpc 2.2.7 (fanquake)
Pull request description:
This picks up one of the changes from #29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I've upstreamed https://github.com/miniupnp/miniupnp/pull/721, as well as some suggestions from the previous PR.
ACKs for top commit:
theuni:
ACK 5195baa600.
TheCharlatan:
utACK 5195baa600
Tree-SHA512: 5b27e132cd5eed285e9be34c8b96893417d92a1ae55c99345c9a89e1c1c5e40e4bc840bc061b879758b2b11fcb520cd98c3da985c1e153f2e5380cf63efe2d69
The RPC documentation for `getblockchaininfo`, `getmininginfo` and
`getnetworkinfo` states that "warnings" returns "any network and
blockchain warnings". In practice, only a single warning is returned.
Fix that by returning all warnings as an array.
As a side benefit, cleans up the GetWarnings() logic.
1ea8674316 [doc] update release-process.md and backports section of CONTRIBUTING (glozow)
Pull request description:
While doing various release process things for the first time, I noticed some of our docs are outdated and/or confusing.
ACKs for top commit:
achow101:
ACK 1ea8674316
Tree-SHA512: 4ad10d4ce2c33fe15cb02599353107bb72ecb867aefc6c120cfd5cdea42aa8fa3783f9e0218c2f3815f030e0694cc8fb24011ce88358a0206cb07416a256a962
2179e2c320 doc: i2p: improve `-i2pacceptincoming` mention (brunoerg)
Pull request description:
In i2p documentation, it says that "the first time Bitcoin Core connects to the I2P router,
it automatically generates a persistent I2P address and its corresponding private key by
default _**or if `-i2pacceptincoming=1` is set**_". This is weird, because `-i2pacceptincoming=1`
by itself does not have any effect. Moreover, `-i2pacceptincoming` is 1 by default anyway.
ACKs for top commit:
laanwj:
This documentation change is correct and makes the documentation slightly shorter, thus easier to read. ACK 2179e2c320
davidgumberg:
ACK 2179e2c320
achow101:
ACK 2179e2c320
byaye:
ACK 2179e2c320
Tree-SHA512: 18a6a627343fb0aa824029d99df8a232153ba288ce94ec8c5da25693885237381fba505ea1e71c756b2a611243a302d319ca7ae03b526020cd6588710fc2ac17
3c1ae3ee33 depends: switch libnatpmp to CMake (Cory Fields)
72ba7b5d26 depends: libnatpmp f2433bec24ca3d3f22a8a7840728a3ac177f94ba (fanquake)
Pull request description:
This picks up one of the changes from https://github.com/bitcoin/bitcoin/pull/29232, which is a switch to building libnatpmp with CMake. It includes an update to the most recent version of libnatpmp (f2433bec24), which includes (https://github.com/miniupnp/libnatpmp/pull/43).
From an initial look I couldn't find any significant difference between the Autotools and CMake produced libs.
ACKs for top commit:
m3dwards:
ACK 3c1ae3ee33
hebasto:
ACK 3c1ae3ee33.
TheCharlatan:
ACK 3c1ae3ee33
Tree-SHA512: 1dd9d9933a5fceb9f8c4e1d68cd5cb4456a10a6dd27a6f6316f14493f9d2efad981ef8be9570c09ca82d45163aebd7f4cb2b2449989ec6084268ddba9a564c83
4357158c47 wallet: return and display signer error (Sjors Provoost)
dc55531087 wallet: compare address returned by displayaddress (Sjors Provoost)
6c1a2cc09a test: use h marker for external signer mock (Sjors Provoost)
Pull request description:
* HWI returns the requested address: as a sanity check, we now compare that to what we expected
* external signer documentation now reflects that HWI alternatives must implement this check
* both RPC and GUI will now return an error text, rather than just fail (the GUI even failed silently in some cases)
ACKs for top commit:
brunoerg:
ACK 4357158c47
achow101:
ACK 4357158c47
Tree-SHA512: 4f56edf3846745c8e7d08ef55cf29e8bb468256457149377c5f02da097931f9ca0c06bdbd856dc2385cde4fd11e4dc3b634c5a48814ff27f5562c8a25d43da93
55b13ecd2e doc: explain what the wallet password does (Brandon Odiwuor)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/18085
Add a document explaining what the wallet `passphrase` does
ACKs for top commit:
fjahr:
ACK 55b13ecd2e
achow101:
ACK 55b13ecd2e
Tree-SHA512: 6cc621269fe51ba7bd5d859427906a520e989bebcb274b08017c9900a4d781a165fc62dea46ed45aa48239cacd3c1f6eb4dd0c916be940d680a03266109716b0
21d0e6c7b7 doc: release notes for PR 27679 (Matthew Zipkin)
791dea204e test: cover unix sockets in zmq interface (Matthew Zipkin)
c87b0a0ff4 zmq: accept unix domain socket address for notifier (Matthew Zipkin)
Pull request description:
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/27375, allowing ZMQ notifications to be published to a UNIX domain socket.
Fortunately, libzmq handles unix sockets already, all we really have to do to support it is allow the format in the actual option.
[libzmq](https://libzmq.readthedocs.io/en/latest/zmq_ipc.html) uses the prefix `ipc://` as opposed to `unix:` which is [used by Tor](https://gitlab.torproject.org/tpo/core/tor/-/blob/main/doc/man/tor.1.txt?ref_type=heads#L1475) and now also by [bitcoind](a85e5a7c9a/doc/release-notes-27375.md (L5)) so we need to switch that internally.
As far as I can tell, [LND](d20a764486/zmq.go (L38)) supports `ipc://` and `unix://` (notice the double slashes).
With this patch, LND can connect to bitcoind using unix sockets:
Example:
*bitcoin.conf*:
```
zmqpubrawblock=unix:/tmp/zmqsb
zmqpubrawtx=unix:/tmp/zmqst
```
*lnd.conf*:
```
bitcoind.zmqpubrawblock=ipc:///tmp/zmqsb
bitcoind.zmqpubrawtx=ipc:///tmp/zmqst
```
ACKs for top commit:
laanwj:
Code review ACK 21d0e6c7b7
tdb3:
crACK for 21d0e6c7b7. Changes lgtm. Will follow up with some testing within the next few days as time allows.
achow101:
ACK 21d0e6c7b7
guggero:
Tested and code review ACK 21d0e6c7b7
Tree-SHA512: ffd50222e80dd029d903e5ddde37b83f72dfec1856a3f7ce49da3b54a45de8daaf80eea1629a30f58559f4b8ded0b29809548c0638cd1c2811b2736ad8b73030
- Mention which directories contain the respective unsigned tarballs
- Clarify that bitcoin.conf might not need to be updated
- Specify where to put historical release notes if there is already
something in release-notes.md
- Clarify what exactly is the problem with running guix-codesign more
than once
- Correct number: 6 codesigned attestations are needed before uploading
binaries
- Remove scp command which is outdated
- Remove server path which is outdated
- Specify that translations update should happen before branch-off, not
before each release candidate
- Mention that you should notify lists when RCs are available
- Put "Archive the release notes" as a separate step, since creating the
github release has a dependency on it.
- Put bitcoincore.org website updates as a separate step, since
updating packaging repos may have a dependency on it.
- Update "bitcoin-dev mailing list" to "bitcoin-dev group"
- Document that maintainers should create PRs to collect backports
- Remove section about not uploading `*-debug` files, reader should
upload all build artifacts.
- Torrent is created automatically, so delete instructions.
- Mention that server also generates ots file automatically.
24b67fa9f6 doc: Add example of mixing private and public keys in descriptors (Anton A)
Pull request description:
closes: #27414
ACKs for top commit:
achow101:
ACK 24b67fa9f6
alfonsoromanz:
Re ACK 24b67fa9f6
Tree-SHA512: 8c063f23199ac0ff35909f786a5b0de1b4a9b15d1e93bdcdac10cb4bd2002c12e99b6fb1c2e56d16971e7622b67d910b79088429df92c48279be2d7797049911
