4ce53495e5e18370b7935551b3b8700faa720a33 doc: update tor docs to use bitcoind binary from path (ismaelsadeeq)
Pull request description:
I noticed this while trying to run a node over Tor.
Using `./bitcoind` as the executable path is incorrect.
This is a simple documentation update PR that fixes the path by removing the prefix and just
having `bitcoind` as the usage example targeting those who have Bitcoin Core in their PATH.
ACKs for top commit:
davidgumberg:
ACK 4ce53495e5
janb84:
ACK 4ce53495e5e18370b7935551b3b8700faa720a33
jonatack:
ACK 4ce53495e5e18370b7935551b3b8700faa720a33
Tree-SHA512: a23c94a175f77d66ee1a81599a15a809ad768090eebb619c8e4a67b8a020a2256da4f40cec3c00ec35775b265d3c53cdb70c09fbed48d399416fbc9156ebff31
One of the benefits of using a compilation database, which is available
after the CMake build system generation step, is that it is not
necessary to actually build the code in order to run `clang-tidy`.
fa4b8b16c37830bf7ee1419aae94e99f294f0e9f test: Add missing ipc subtree to lint (MarcoFalke)
Pull request description:
The subtree docs and lint checks list the subtrees in three places, making it hard to follow and update and easy to miss one.
Fix all issues by including the missing one and removing the list in one place.
ACKs for top commit:
l0rinc:
ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f
davidgumberg:
ACK fa4b8b16c3
achow101:
ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f
hebasto:
re-ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f, only a [comment](https://github.com/bitcoin/bitcoin/pull/32623#discussion_r2109243775) has been addressed since my recent [review](https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2871162934).
BrandonOdiwuor:
Re-ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f
Tree-SHA512: 97a39cf63be2707b2e03502546797ef639a611030e0a860618256c7c0683381be0f05c8bf850f9542a25266a52f58cef690abf9cc243498b6e026c7a6f9a52b2
a5ac43d98d1ad3ebed934f2c50208a85aae17e5e doc: Add release notes describing bitcoin wrapper executable (Ryan Ofsky)
258bda80c009a25d1f1bdeffccf9ed1ffde29cb2 doc: Mention bitcoin wrapper executable in documentation (Ryan Ofsky)
d2739d75c911c8bf73a4d3005c57add1ae4a67ae build: add bitcoin.exe to windows installer (Sjors Provoost)
ba649c00063a43b59a63db17b509179a658a8d9a ci: Run multiprocess tests through wrapper executable (Ryan Ofsky)
29bdd743bb843f8b8ed2e426b6df36e9d7e54215 test: Support BITCOIN_CMD environment variable (Ryan Ofsky)
9c8c68891b43053acfe7b8eb9d2e0d2bcfcb4e1e multiprocess: Add bitcoin wrapper executable (Ryan Ofsky)
5076d20fdb70a4bfafc4bdfe8293e347cb6bfa78 util: Add cross-platform ExecVp and GetExePath functions (Ryan Ofsky)
Pull request description:
Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don't cause confusion.
Idea and implementation of this were discussed in https://github.com/bitcoin/bitcoin/issues/30983.
---
Initial implementation of this feature is deliberately minimal so the UX can evolve in response to feedback and there are not too many details to debate and discuss in a single PR. But many improvements are possible or planned:
- Adding manpage and bash completions.
- Showing nicer error messages that detect if an executable isn't installed and suggest how to fix [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2073194474)
- Showing wrapper command lines in subcommand in help output [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2077800405). This could be done conditionally as suggested in the comment or be unconditional.
- Showing wrapper command lines in subcommand error output. There is a bitcoin-cli error pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243) that is needlessly confusing.
- Integrating help so `bitcoin help subcommand` invokes `bitcoin subcommand -h`. `bitcoin -h subcommand` should also be supported and be equivalent [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2093116725)
- Adding support for `bitcoin-util` subcommands. Ideal interface would probably be more like `bitcoin grind` not `bitcoin util grind` but this has been punted for now. Supporting subcommands directly would require some ArgsManager modifications
- Adding a dedicated python functional test for the wrapper. Right now there is some CI coverage by setting the `BITCOIN_CMD` variable, but this doesn't cover things like the help output and version output, and support for different directory layouts.
- Better `--multiprocess` (`-m`) / `--monolithic` (`-M`) default selection. Right now, default is monolithic but it probably makes sense to chose more intelligently depending on whether -ipc options are enabled and what binaries are available.
- Maybe parsing `bitcoin.conf` and supporting options to control wrapper behavior like custom locations or preferences or aliases.
- Better command command line usability. Allow combining short options like (`-ah`). Allow fuzzy matching of subcommands or suggestions if you misspell. (suggested by stickies in review club)
- Not directly related to this PR but `bitcoin-cli named` implementation used by the wrapper should do a better job disambiguating named arguments from base64 arguments ending in = as pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091886628)
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). A review club meeting for it took place in https://bitcoincore.reviews/31375
ACKs for top commit:
Sjors:
utACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
achow101:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
vasild:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
theStack:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
ismaelsadeeq:
fwiw my last review implied an ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
hodlinator:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
Tree-SHA512: 570e6a4ff8bd79ef6554da3d01f36c0a7c6d2dd7dace8f8732eca98f4a8bc2284474a9beadeba783114fe2f3dd08b2041b3da7753bae0b7f881ec50668cb821f
e8661aac752eb08fee318eb8f56e599578d78f9f wallet: drop watch-only things from interface (Sjors Provoost)
e99188e7daa19dcb14f387931949edf581ae5f37 qt: drop unused watch-only functionality (Sjors Provoost)
Pull request description:
The watch-only functionality in the GUI was only used for legacy wallets. Watch-only descriptor wallets do not use this.
The only visible changes of this PR should be:
- dropped "Spendable:" label from the overview tab
- column width cache is reset
This PR also removes some unused variables from the interface.
ACKs for top commit:
davidgumberg:
Review ACK e8661aac75.
hebasto:
ACK e8661aac752eb08fee318eb8f56e599578d78f9f, I have reviewed the code and it looks OK. The `src/qt/forms/overviewpage.ui` form was reviewed in Qt Designer.
Tree-SHA512: d7edb0f167e0b934075398a76eddca69890bb36848a918c932b1c2cea85ee87285e876cbfdf1f6dec7adf26b9f405fb558c70bec0c84585c0a9df33c2af78393
faf55fc80b11f3d9b0b12c1b26a9612ea9ce9b40 doc: Remove ParseInt mentions in documentation (MarcoFalke)
33332829333b589420f8038541d04ec6970f051d refactor: Remove unused Parse(U)Int* (MarcoFalke)
fa84e6c36cb0accf87123ca4eb98f6219d55fb5e bitcoin-tx: Reject + sign in MutateTxDel* (MarcoFalke)
face2519fac9e840d52f0334d9079e664be7eb28 bitcoin-tx: Reject + sign in vout parsing (MarcoFalke)
fa8acaf0b993c879ee8c516baa36339ff7b72406 bitcoin-tx: Reject + sign in replaceable parsing (MarcoFalke)
faff25a558ab15b5d8eeea5dd4c9c0d76350051b bitcoin-tx: Reject + sign in locktime (MarcoFalke)
dddd9e5fe38b81f1af6b343661b65e16b0de7c60 bitcoin-tx: Reject + sign in nversion parsing (MarcoFalke)
fab06ac03788243847b799a3feaac56bc918fba9 rest: Use SAFE_CHARS_URI in SanitizeString error msg (MarcoFalke)
8888bb499dec79258b1857b404d72f93650503f4 rest: Reject + sign in /blockhashbyheight/ (MarcoFalke)
fafd43c69192fcb48a9e04d52eeb07fff15655d0 test: Reject + sign when parsing regtest deployment params (MarcoFalke)
fa123afa0ef752e8645bf695d121da66d8f1382b Reject + sign when checking -ipcfd (MarcoFalke)
fa479857ed234d54df31d33b60de14c6ffab3d6f Reject + sign in SplitHostPort (MarcoFalke)
fab4c2967d554ddbc15f732cea6cd190c547d89f net: Reject + sign when parsing subnet mask (MarcoFalke)
fa89652e68fc07fb6c9f3d9e34dc11d35f0cc1e1 init: Reject + sign in -*port parsing (MarcoFalke)
fa9c45577dfbae67535e4965b5660288557d3631 cli: Reject + sign in -netinfo level parsing (MarcoFalke)
fa980413257e2004a8d48a8be66c6d67f90b76ad refactor: Use ToIntegral in CreateFromDump (MarcoFalke)
fa23ed7fc24212d85cdc7f52b317906b37a1a120 refactor: Use ToIntegral in ParseHDKeypath (MarcoFalke)
Pull request description:
The legacy int parsing is problematic, because it accepts the `+` sign for unsigned integers. In all cases this is either:
* Useless, because the `+` sign was already rejected.
* Erroneous and inconsistent, when third party parsers reject it. (C.f. https://github.com/bitcoin/bitcoin/pull/32365)
* Confusing, because the `+` sign is neither documented, nor can it be assumed to be present.
Fix all issues by removing the legacy int parsing.
ACKs for top commit:
stickies-v:
re-ACK faf55fc80b
brunoerg:
code review ACK faf55fc80b11f3d9b0b12c1b26a9612ea9ce9b40
Tree-SHA512: a311ab6a58fe02a37741c1800feb3dcfad92377b4bfb61b433b2393f52ba89ef45d00940972b2767b213a3dd7b59e5e35d5b659c586eacdfe4e565a77b12b19f
fa9198af55df74b0c19c9125d256ad4df83cf005 lint: Check for missing trailing newline (MarcoFalke)
fa2b2aa27c29fe810e296ef82126553b8f0d56e6 lint: Add archived notes to default excludes (MarcoFalke)
Pull request description:
A missing trailing newline is harmless, but a bit problematic:
* `git` shows a warning by default
* After another line is appended, the diff will be verbose and `git blame` will be wrong for the "untouched" line.
Fix the problems by just requiring what is already the default, see also 663a9cabf8/.editorconfig (L9) and 663a9cabf8/test/lint/test_runner/src/main.rs (L327)
ACKs for top commit:
l0rinc:
utACK fa9198af55df74b0c19c9125d256ad4df83cf005
fanquake:
ACK fa9198af55df74b0c19c9125d256ad4df83cf005
Tree-SHA512: d144eebdeee68fc3404aa4a66ecd5c130f907ed4b869bd300f6e9ed74d125561d1f4cdd6dd20d9e969471a7d007399f928f072d1c1f626275ca31f32bc23fdbc
In the dev notes, remove the whole section, because:
* ParseDouble was removed in commit
fa9d72a7947d2cff541794e21e0040c3c1d43b32
* The locale-dependent atoi is already checked by
test/lint/lint-locale-dependence.py
Co-authored-by: Fabian Jahr <fjahr@protonmail.com>
The watch-only functionality in the GUI was only used for legacy wallets.
Watch-only descriptor wallets do not use this.
The only visible changes of this commit are:
- dropped "Spendable:" label from the overview tab
- column width cache is reset
301993ebf7f8ec23050e91377e0fd05823bb372a init: drop -upnp (fanquake)
Pull request description:
This was slated for removal in `30.0`, so remove it.
ACKs for top commit:
i-am-yuvi:
ACK 301993ebf7f8ec23050e91377e0fd05823bb372a
maflcko:
review ACK 301993ebf7f8ec23050e91377e0fd05823bb372a
darosior:
tACK 301993ebf7f8ec23050e91377e0fd05823bb372a
Tree-SHA512: 635e374c013fa08c4cda7caadc465c89bb376d3ee2c66f67a27e3ed9031844674d3e996169aaffb9b65a67b0d44d92aaec000aaf69abe3dd10fce2f4138f3e27
fa2c6623626719b338880f7bb097b902019d5956 build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC" (MarcoFalke)
Pull request description:
Now that GitHub Actions has a fixed version and the Windows developers have updated their compiler, the workaround is no longer needed.
ACKs for top commit:
davidgumberg:
reACK fa2c662362
hodlinator:
ACK fa2c6623626719b338880f7bb097b902019d5956
Tree-SHA512: 952b36c917c91d78d82b5013e1df338b23f860fad7be43327150581f403050e61f748fc75557ec96fb2b115a2cc0246a506bc2ddc25e05f5a41339bd466c4b1a
This reverts commit b2d536100282bd901d3e0be7f7f4a6966e0ef817.
Also, adjust the doc to reflect the new minimum version. Versions 17.6
or 17.11 (or anything in between) may still work on a best-effor basis,
but it is not checked by CI or by developers.
e62423d6f1514b022155edb5bc930cecc4236731 doc: Improve dependencies.md documentation (Nicola Leonardo Susca)
a3520f9d561e499d0c05808cfc497d4fa113c798 doc: Add dependency self-compilation info (Nicola Leonardo Susca)
d1fdc84c54934931c3f49204a688e43458af94dd doc: Remove Linux Kernel from dep. table (Nicola Leonardo Susca)
Pull request description:
Small improvements to the `dependencies.md` documentation as a follow-up for #31634.
**Linux Kernel** does not need to be in the dependencies as it is not required for cross-compiling from other systems, and users building on Linux should not expect they can build using any EOL kernel, see: https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1957123270
**Runtime dependencies** can be in a separate table to improve readability. See: https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2589412550
**Version used** is redundant as the depends package definition is already linked in the table and can thus be removed, see: https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2063356972
ACKs for top commit:
maflcko:
lgtm ACK e62423d6f1514b022155edb5bc930cecc4236731 🛄
hebasto:
ACK e62423d6f1514b022155edb5bc930cecc4236731.
jonatack:
ACK e62423d6f1514b022155edb5bc930cecc4236731
Tree-SHA512: 586c450aec7ece5d543bcb12796a2bb7ff459e15c8813a7b5104a38d09fc51e7e902363ff023be48273ae2b1a1b0807a439c8523b4ea2e398b76b7c9a48d0dfb
- Remove the "Version used" column from the dependencies tables as the
depends package definition which defines the version used is already
linked. In case a developer is interested in which PR introduced this
file/version they can use `git blame` on the package definition as
usual. This removes doc. maintenance overhead and eliminates the risk
of stale information about the "Version used", see comment:
https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2063356972
- Separate dependency tables into build-time and run-time tables for
easier distinction of the two and to avoid repeating the same
information ("No"/"Yes") for better readability.
- Order dependencies alphabetically
The `dependencies.md` should mention that it is possible to self-compile
the dependencies and reference `depends/README.md` for instructions.
Also mention full path to `/doc/build-*.md` for clarity.
fa24fdcb7f474e6959ae4d8fe9759d365c6c021b lint: Remove string exclusion from locale check (MarcoFalke)
Pull request description:
The exclusion isn't needed. In fact, it prevents detection of `"bla" + wrong()`.
For example, the following is not detected:
```diff
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 1c2951deee..c1209013e5 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -336,7 +336,8 @@ RPCHelpMan addmultisigaddress()
RPCHelpMan keypoolrefill()
{
return RPCHelpMan{"keypoolrefill",
- "\nFills the keypool."+
+ "\nRefills each descriptor keypool in the wallet up to the specified number of new keys.\n"
+ "By default, descriptor wallets have 4 active ranged descriptors (\"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"), each with " + std::to_string(DEFAULT_KEYPOOL_SIZE) + " entries.\n" +
HELP_REQUIRING_PASSPHRASE,
{
{"newsize", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%u, or as set by -keypool", DEFAULT_KEYPOOL_SIZE)}, "The new keypool size"},
```
Fix the script by detecting it.
ACKs for top commit:
laanwj:
Code review ACK fa24fdcb7f474e6959ae4d8fe9759d365c6c021b.
rkrux:
ACK fa24fdcb7f474e6959ae4d8fe9759d365c6c021b
w0xlt:
ACK fa24fdcb7f
Tree-SHA512: cb7e6ed9fec5d2089e94031329ebf26b83a1814ffbbbca94f7527c127bc759d13c0f4ea79b71ff7f5f009d071dcf01958c8921163d6dc5e1ae6256cc40b57eea
The string exclusion would fail to detect `"bla" + wrong()`.
Also, remove /* */ comment exclusion, which would fail to detect stuff
like `/* bla */ wrong()`.
Instead, require the function to be called by adding \\( to the regex.
Finally, also remove the section in the dev notes, because:
* It was outdated and missing some functions such as std::to_string in
the list.
* The maintenance overhead of having to update two places is fragile and
questionable.
* Many other linters are also not mentioned in the dev notes, even
though they are important.
* A dev (and CI) is more likely to run the linters than to read the dev
notes.
* The dev notes are more than 1000 lines of dense information. It would
be easier to digest if they focused on the important stuff that is not
checked by automated tools.
abe43dfadd6325f80975a76aea57a549c3162191 doc: release note for #27826 (Sjors Provoost)
f9fa28788e63e2bd059a21ec0e76ae6903b2a6be Use LogBlockHeader for compact blocks (Sjors Provoost)
bad7c914793134abe2f64d96c367d5e9b07e60fd Log which peer sent us a header (Sjors Provoost)
9d3e39c29c31775fd82af319d1d4dfbbd3e21bfa Log block header in net_processing (Sjors Provoost)
Pull request description:
Fixes#27744
Since #27278 we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this PR provides.
Move the logging from validation to net_processing.
This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip.
The PR introduces a new helper method `LogBlockHeader`.
When receiving a _compact block_ we call `LogBlockHeader` from the exact same place as where we previously logged. So that log message doesn't change. What does change is that we no longer _also_ log from `AcceptBlockHeader`.
When receiving a regular header(s) message, _we only log the last one_. This is a change in behaviour because it was simpler to implement, but it's probably better anyway. It does mean that if a peer sends of a bunch of headers of which _any_ is invalid, we won't log it (here).
Lastly I expanded the code comment explaining why we log this. It initially only covered selfish mining, but we also care about peers sending us headers but not following up (see e.g. #27626).
Example log:
```
2023-06-05T13:12:21Z Saw new header hash=000000000000000000045910263ef84b575ae3af151865238f1e5c619e69c330 height=792964 peer=0
2023-06-05T13:12:23Z UpdateTip: new best=000000000000000000045910263ef84b575ae3af151865238f1e5c619e69c330 height=792964 version=0x20000000 log2_work=94.223098 tx=848176824 date='2023-06-05T13:11:49Z' progress=1.000000 cache=6.4MiB(54615txo)
2023-06-05T13:14:05Z Saw new cmpctblock header hash=00000000000000000003c6fd4ef2e1246a3f9e1fffab7247344f94cadb9de979 height=792965 peer=0
2023-06-05T13:14:05Z UpdateTip: new best=00000000000000000003c6fd4ef2e1246a3f9e1fffab7247344f94cadb9de979 height=792965 version=0x20000000 log2_work=94.223112 tx=848179461 date='2023-06-05T13:13:58Z' progress=1.000000 cache=7.2MiB(61275txo)
2023-06-05T13:14:41Z Saw new header hash=000000000000000000048e6d69c8399992782d08cb57f5d6cbc81a9f996c3f43 height=792966 peer=8
2023-06-05T13:14:42Z UpdateTip: new best=000000000000000000048e6d69c8399992782d08cb57f5d6cbc81a9f996c3f43 height=792966 version=0x2db3c000 log2_work=94.223126 tx=848182944 date='2023-06-05T13:14:35Z' progress=1.000000 cache=8.0MiB(69837txo)
```
ACKs for top commit:
danielabrozzoni:
tACK abe43dfadd6325f80975a76aea57a549c3162191
achow101:
ACK abe43dfadd6325f80975a76aea57a549c3162191
vasild:
ACK abe43dfadd6325f80975a76aea57a549c3162191
Tree-SHA512: 081e0de62cbd8a0b35cf54daaa09e3e6991d0cc9f706ef3eb50908752fe7815de69b367f7313381c90cd8d5de0ae5f532d1cd54948c5c1133b1832f266d9c232
This commit fixes the path listed in the documentation for the fuzz
testing test_runner.py. Previously the --help option worked but running
fuzz tests from the documented path did not.
3669ecd4ccd8e7a1e2b1a9dcbe708c51c78e4d6c doc: Document fuzz build options (Anthony Towns)
c1d01f59acc2067ecbf8a8b42ba0d8e596694439 fuzz: enable running fuzz test cases in Debug mode (Anthony Towns)
Pull request description:
When building with
BUILD_FOR_FUZZING=OFF
BUILD_FUZZ_BINARY=ON
CMAKE_BUILD_TYPE=Debug
allow the fuzz binary to execute given test cases (without actual fuzzing) to make it easier to reproduce fuzz test failures in a more normal debug build.
In Debug builds, deterministic fuzz behaviour is controlled via a runtime variable, which is normally false, but set to true automatically in the fuzz binary, unless the FUZZ_NONDETERMINISM environment variable is set.
ACKs for top commit:
maflcko:
re-ACK 3669ecd4ccd8e7a1e2b1a9dcbe708c51c78e4d6c 🏉
marcofleon:
re ACK 3669ecd4ccd8e7a1e2b1a9dcbe708c51c78e4d6c
ryanofsky:
Code review ACK 3669ecd4ccd8e7a1e2b1a9dcbe708c51c78e4d6c with just variable renamed and documentation added since last review
Tree-SHA512: 5da5736462f98437d0aa1bd01aeacb9d46a9cc446a748080291067f7a27854c89f560f3a6481b760b9a0ea15a8d3ad90cd329ee2a008e5e347a101ed2516449e
7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf doc: Add deps install notes for multiprocess (TheCharlatan)
Pull request description:
These just mirror the content in src/ipc/libmultiprocess/doc/install.md
ACKs for top commit:
Sjors:
re-ACK 7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf
ryanofsky:
Code review ACK 7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf just dropping dependencies.md update since last review
Tree-SHA512: f9bf4f54542323aa4a0600db874640e575e40355f08515331a27fb139e6e47ee58aa0c6635206f978696e3da7b5aa93efb45b181b02e99e308537fcb90bd6751
fa86190e6ed2aeda7bcceaf96f52403816bcd751 rpc: Allow fullrbf fee bump (MarcoFalke)
Pull request description:
The RPCs (psbt)bumpfee, and the GUI, reject fee bumps when BIP 125 signalling is absent in the transaction even when the mempool and other RPCs allow them. Fix the confusion by allowing the fee bump.
This is done after fullrbf is always on (https://github.com/bitcoin/bitcoin/pull/30592)
ACKs for top commit:
1440000bytes:
reACK fa86190e6e
achow101:
ACK fa86190e6ed2aeda7bcceaf96f52403816bcd751
w0xlt:
ACK fa86190e6e
rkrux:
reACK fa86190e6ed2aeda7bcceaf96f52403816bcd751
glozow:
ACK fa86190e6ed2aeda7bcceaf96f52403816bcd751
Tree-SHA512: b2ffe8dcadbe71e9be767a16cf8aa0bf383c2de7aa1aee9438d125f444e24f3f7e4f02ddb28981bd3b8b645b6a24a407b4ad6bb0b21946ae637e78f6386e05bf
Also, fix the incorrect documention of the 'replaceable' RPC argument
with respect to sequence number handling. The docs were incorrect
before, so the fix could be extracted, but it seems fine to include here
as well.
9ccee9cd022dcce72608831d4b1b93991345b306 doc: Document WITH_EXTERNAL_LIBMULTIPROCESS build option better (Ryan Ofsky)
Pull request description:
This includes a cmake documentation change suggested https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2039716492 and another change to mention the option in markdown documentation
ACKs for top commit:
hebasto:
ACK 9ccee9cd022dcce72608831d4b1b93991345b306, changes look good.
TheCharlatan:
ACK 9ccee9cd022dcce72608831d4b1b93991345b306
Tree-SHA512: c9103b001b970ac57afedc6dc384091f5661975d569573e93003cbd7df1891c54cefb06d7296eac5b9a5c57251803dcab2bd3b26c9d81aa476c62f211dcb3d6e
7a93544cdcc2874a18a8b6d528a75e84ac007880 doc: Fix and clarify description of ZMQ message format (Jiri Jakes)
Pull request description:
This change stresses that all ZMQ messages share the same structure and that they differ only in the format of the bodies. Previously this was not clear.
Further it removes the notion of endianness of 32-byte hashes, as it was misleading, and replaces it with the term 'reversed byte order' (as opposed to natural or normal byte order produced by hashing functions).
Additionally, it states that ZMQ 32-byte hashes are in the same format as in RPC. Previously it incorrectly stated that the two were in different formats.
[Rendered](https://github.com/jirijakes/bitcoin/blob/zmq-doc/doc/zmq.md).
Fixes#31856.
ACKs for top commit:
w0xlt:
Code review ACK 7a93544cdc
achow101:
ACK 7a93544cdcc2874a18a8b6d528a75e84ac007880
ryanofsky:
Code review ACK 7a93544cdcc2874a18a8b6d528a75e84ac007880. Nice changes. Documentation seems less repetitive and easier to understand now
Tree-SHA512: 8c5ab047c5fd9b5b6910d691b725886d7743dfd01510735b46e43d01c2d0d25ec52d79d71ec75dbeb142e96a88ad503d69ee14b971e3cdaeb8fd85e5292a8c21
8fe001d59722800417334a09142c559887b54b28 doc: Updates how to reproduce fuzz CI failure locally (Sergi Delgado Segura)
Pull request description:
The current version of the doc does not explain how to reproduce a recent fuzzing CI failure (not yet part of the corpora). Add instructions on how to manually create a crash file based on a report.
ACKs for top commit:
maflcko:
lgtm ACK 8fe001d59722800417334a09142c559887b54b28
glozow:
ACK 8fe001d59722800417334a09142c559887b54b28
Tree-SHA512: 7436d71a30bbbffc34770027f1deeacca2de528d8d1b333431d6070c2ba779ecfcdaf25dc791d2154ba4dd37824d06aed2695a8412d7ca1f29e5bd1796d42aeb
babb9f5db64145a4adec07dee1c8328195363777 depends: remove non-native libmultiprocess build (Cory Fields)
5d105fb8c3ffa39c3e716c3147ee74795b129113 depends: Switch libmultiprocess packages to use local git subtree (Ryan Ofsky)
9b35518d2f3ff67cbc87d30bd3b882fe72fd5d87 depends, moveonly: split up int_get_build_id function (Ryan Ofsky)
2d373e27071f54e04f8aaf6753e5481ccbc9ed3f lint: Add exclusions for libmultiprocess subtree (Ryan Ofsky)
e88ab394c163a8ebe59ee240b09430d788ef274e doc: Update documentation to explain libmultiprocess subtree (Ryan Ofsky)
d4bc5639829f0c98bd2faeddba9be4e8c6667875 cmake: Fix clang-tidy "no input files" errors (Ryan Ofsky)
abdf3cb6456f3f79b3130d2001bf780deadccf87 cmake: Fix warnings from boost headers (Ryan Ofsky)
8532fcb1c30d6556c273bd77a2a7daa6b0a2448d cmake: Fix ctest mptest "Unable to find executable" errors (Ryan Ofsky)
d597ab1dee6b3541c827e6be61fc8a9b61d1e23d cmake: Support building with libmultiprocess subtree (Ryan Ofsky)
69f0d4adb72c1dc5c261517899d87b0fe8b88304 scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake (Ryan Ofsky)
a2f28e4be96e92079a219567cf20214996aefc53 Squashed 'src/ipc/libmultiprocess/' content from commit 35944ffd23fa (Ryan Ofsky)
d6244f85c509d7644a57def892298a4dde988c3b depends: Update libmultiprocess library to simplify cmake subtree build (Ryan Ofsky)
Pull request description:
This adds the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library and code generator as a subtree in `src/ipc/libmultiprocess` and allows it to be built with the cmake `-DENABLE_IPC` option, which is disabled by default.
This PR does not entirely remove the depends system [libmultiprocess package](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/native_libmultiprocess.mk) because the package is useful when cross compiling. (A cross-compiling cmake build cannot easily build and run a native code generation tool.) However, it does update the depends package to build from the new git subtree, instead of being downloaded separately from github, so the same sources are used to build both the runtime library and the code generator.
This PR includes the following manual changes (not created automatically with `git subtree add`) which just update the build system and documentation:
- [`d6244f85c509` depends: Update libmultiprocess library to simplify cmake subtree build](d6244f85c5)
- [`69f0d4adb72c` scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake](69f0d4adb7)
- [`d597ab1dee6b` cmake: Support building with libmultiprocess subtree](d597ab1dee)
- [`8532fcb1c30d` cmake: Fix ctest mptest "Unable to find executable" errors](8532fcb1c3)
- [`abdf3cb6456f` cmake: Fix warnings from boost headers](abdf3cb645)
- [`d4bc5639829f` cmake: Fix clang-tidy "no input files" errors](d4bc563982)
- [`e88ab394c163` doc: Update documentation to explain libmultiprocess subtree](e88ab394c1)
- [`2d373e27071f` lint: Add exclusions for libmultiprocess subtree](2d373e2707)
- [`9b35518d2f3f` depends, moveonly: split up int_get_build_id function](9b35518d2f)
- [`5d105fb8c3ff` depends: Switch libmultiprocess packages to use local git subtree](5d105fb8c3)
- [`babb9f5db641` depends: remove non-native libmultiprocess build](babb9f5db6)
---
Previous minisketch subtree PR #23114 may be useful for comparison
Instructions for subtree verification can be found:
- https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees
- https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh
TL&DR:
```sh
git remote add --fetch libmultiprocess https://github.com/chaincodelabs/libmultiprocess.git
test/lint/git-subtree-check.sh -r src/ipc/libmultiprocess
```
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
Sjors:
re-ACK babb9f5db64145a4adec07dee1c8328195363777
TheCharlatan:
tACK babb9f5db64145a4adec07dee1c8328195363777
vasild:
ACK babb9f5db64145a4adec07dee1c8328195363777
Tree-SHA512: 43d4eecca5aab63e55c613de935965666eaced327f9fe859a0e9c9b85f7685dc16c5c8d6e03e09ca998628c5d468633f4f743529930b037049abe8e0101e0143
The current version of the doc does not explain how to reproduce a recent fuzzing CI failure
(not yet part of the corpora). Add instructions on how to manually create a crash file based
on a report.
On older systems, such as Ubuntu 22.04, `qt6-tools-dev-tools` and
`libgl-dev` are not treated as dependencies of `qt6-tools-dev` and
`qt6-base-dev`, respectively. This change explicitly lists them in the
installation documentation.
f00345727b8d2bf73409db5cd342e476671e6425 doc: Update `dependencies.md` for Qt 6 (Hennadii Stepanov)
80b917991ed7ff931f0a9211cebf859f674776c4 build, msvc: Update `vcpkg.json` for Qt 6 (Hennadii Stepanov)
30dd1f1644e0441b5310f1eceecfd6a5abc45f68 ci: Update for Qt 6 (Hennadii Stepanov)
629d292f4d846978c682c5f497240c62d62f4bd1 test: Update sanitizer suppressions for Qt 6 (Hennadii Stepanov)
551e13abf82522bad7fdde4ff4bd15d2c8f88b23 guix: Adjust for Qt 6 (Hennadii Stepanov)
c3e9bd086c498e9f1cbdc505949cb17ac1b39f7e qt: Fix compiling for Windows (Hennadii Stepanov)
ab399c4db2e98aee8e01323c4c6cca48b520dc7f depends: Add `native_qt` package (Hennadii Stepanov)
248613eb3ee034bf143821a51635e697dc114e6c depends: Factor out Qt modules' details (Hennadii Stepanov)
0268f52a4cd2b7aa63934526437bcf6912e47d3c depends: Introduce customizable `$(package)_patches_path` variables (Hennadii Stepanov)
5e794e62024eef612e1fbb71c76ea54d17435c14 depends: Bump `qt` package up to 6.7.3 (Hennadii Stepanov)
6d4214925fadc36d26aa58903db5788c742e68c6 cmake: Require Qt 6 to build GUI (Hennadii Stepanov)
Pull request description:
The currently used Qt 5.15 is approaching [EOL](https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders) and will reach it before the Bitcoin Core v30 release. The recent migration of the build system to CMake makes it possible to switch to Qt 6.
This PR updates the OS runtime compatibility requirements for the Bitcoin Core GUI as follows:
### 1. Linux
Starting with Qt 6.5.0, the `libxcb-cursor0` package is required to be installed at runtime.
### 2. Windows
Cross-compiling does not support LTO. We have to re-add it in a follow-up.
A new style plugin causes minor visual glitches, such as
which will be fixed in follow-ups.
### 3. macOS
`bitcoin-qt` now uses the [Metal](https://developer.apple.com/metal/) backend.
---
**IMPORTANT.** Don't forget to install [Ninja](https://ninja-build.org/).
---
For historical context, please refer to:
- https://github.com/bitcoin/bitcoin/issues/20627
- https://github.com/bitcoin/bitcoin/pull/24798
---
UPD 2024-10-09. Qt 6.8 has been [released](https://www.qt.io/blog/qt-6.8-released), but it has some [drawbacks](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346) for us. As a result, this PR will stick to Qt 6.7.
UPD 2025-03-18: [Standard support for Qt 5.15 will end after 26th of May 2025](https://www.qt.io/blog/extended-security-maintenance-for-qt-5.15-begins-may-2025)
ACKs for top commit:
laanwj:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
hodlinator:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
Tree-SHA512: 367f722e6c3ea4700b5395871c40b6df8c8062fdc822107090449ea4ae4ad2db75cc53a982a678f4c48ce8f9b2d43ed10e6d23b06165ab78713f161db712d895
