fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd util: Treat Assume as Assert when evaluating at compile-time (MarcoFalke)
Pull request description:
There is no downside or cost of treating an `Assume` at compile-time as an `Assert` and it may even help to find bugs while compiling without `ABORT_ON_FAILED_ASSUME`.
This is also required for https://github.com/bitcoin/bitcoin/pull/31093
ACKs for top commit:
dergoegge:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
brunoerg:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
marcofleon:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
Tree-SHA512: 17604403f841343a6d5b6e5d777e1760d38e0c27dc1fd4479e3741894fba40cdb1fb659cf24519a51d051bd5884a75992d1227ec9fa40fbf53bc619fbfb304ad
def6dd0c597f2c7b0c55910792e646b8ff93e36c depends: sqlite 3.46.1 (fanquake)
Pull request description:
Update sqlite in depends from [3.38.5](https://sqlite.org/releaselog/3_38_5.html) to [3.46.1](https://sqlite.org/releaselog/3_46_1.html).
ACKs for top commit:
TheCharlatan:
ACK def6dd0c597f2c7b0c55910792e646b8ff93e36c
theuni:
Not opposed utACK def6dd0c597f2c7b0c55910792e646b8ff93e36c
Tree-SHA512: 1f12c8ed8d05600b8240bcdbad5cf7d073ea5ab0bbd4a0f49a39ccfe1a93c043ee855b6eb0c67028edec57d8c21588dc33246e64d0b94feafad1a6ec38839893
fa9747a896188f4dd70f275aec2469dba5cd435e ci: Temporary workaround for old CCACHE_DIR cirrus env (MarcoFalke)
Pull request description:
On a CI re-run, the historic env vars and CI config is used from Cirrus. However, the most recent CI config and CI scripts from this repo are used. This may lead to issues.
For example, `CCACHE_DIR` in the old location may be missing on new CI workers and lead to errors.
Fix it, by falling back to the old logic when the old `CCACHE_DIR` was detected.
ACKs for top commit:
fanquake:
ACK fa9747a896188f4dd70f275aec2469dba5cd435e - have seen this now.
Tree-SHA512: 04f0ca8d09ab0b8216a474fde1e05b79fbc6524884be173e8d728799739b026cda18d1797e0fe53d7e1b0ea69c0485acfe4f8a8f85408ea5bfdcffcf13a7ce55
c495731a316d9c97ee05a08cf5087c5535f84bd4 fuzz: wallet: add target for `CreateTransaction` (brunoerg)
3db68e29ec632b29f5417dbef095520e75adc26d wallet: move `ImportDescriptors`/`FuzzedWallet` to util (brunoerg)
Pull request description:
This PR adds a fuzz target for the `CreateTransaction` function. It is a regression target for https://github.com/bitcoin/bitcoin/pull/27271 and can be testing by applying:
```diff
@@ -1110,7 +1110,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN)
// and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways
if (selection_target == 0 && !coin_control.HasSelected()) {
- return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")};
+ // return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")};
}
```
Also, it moves `ImportDescriptors` function to `src/wallet/test/util.h` to avoid to duplicate same code.
ACKs for top commit:
marcofleon:
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4
maflcko:
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4 🏻
Tree-SHA512: a439f947b91b01e327e18cd18e63d5ce49f2cb9ca16ca9d56fe337b8cff239b3af4db18fe89478fe5faa5549d37ca935bd321913db7646fbf6818f825cb5d878
9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5 util: Remove RandAddSeedPerfmon (Hodlinator)
Pull request description:
`RegQueryValueExA(HKEY_PERFORMANCE_DATA, ...)` sometimes hangs *bitcoind.exe* on Windows during startup, at least on CI.
We have other sources of entropy to seed randomness with on Windows, so should be alright removing this. Might resurrect if less drastic fix is found.
Hopefully sufficient to fix#30390.
CI debugged with temporary Windows stack trace dumping + Symbols in #30956.
ACKs for top commit:
achow101:
ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5
fanquake:
ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5
hebasto:
ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5, I have reviewed the code and it looks OK.
laanwj:
Code review ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5
Tree-SHA512: d3f26b4dd0519ef957f23abaffc6be1fed339eae756aed18042422fc6f0bba4e8fa9a44bf903e54f72747e2d0108146c18fd80576d95fc20690a2daf9c83689d
33a28e252a7349c0aa284005aee97873b965fcfe Change default help arg to `-help` and mention `-h` and `-?` as alternatives (Lőrinc)
f0130ab1a1e65583637b6a362b879ea3253e7bb7 doc: replace `-?` with `-h` for bench_bitcoin help (Lőrinc)
Pull request description:
The question mark is interpreted as a wildcard for any single character in Zsh (see https://www.techrepublic.com/article/globbing-wildcard-characters-with-zsh), so `bench_bitcoin -?` will not show the help message on systems using Zsh, such as macOS.
Since `-h` provides equivalent help functionality (as defined in https://github.com/bitcoin/bitcoin/blob/master/src/common/args.cpp#L684-L693), the `benchmarking.md` documentation has been updated to ensure compatibility with macOS.
----
### -?
> % cmake -B build -DBUILD_BENCH=ON && cmake --build build && build/src/bench/bench_bitcoin -?
zsh: no matches found: -?
### -h
> % cmake -B build -DBUILD_BENCH=ON && cmake --build build && build/src/bench/bench_bitcoin -h
Usage: bench_bitcoin [options]
Options:
...
----
Based on the comments the args help default was also changed to `-help`, mentioning `-h` and `-?` (instead of `-?` being the default)
ACKs for top commit:
edilmedeiros:
tACK 33a28e252a7349c0aa284005aee97873b965fcfe
maflcko:
lgtm ACK 33a28e252a7349c0aa284005aee97873b965fcfe
achow101:
ACK 33a28e252a7349c0aa284005aee97873b965fcfe
rkrux:
tACK 33a28e252a
laanwj:
Code review ACK 33a28e252a7349c0aa284005aee97873b965fcfe
Tree-SHA512: 8c6e27488462be9ba9186b34abe6249c1d93026b3963acc0f42c75496f39407563766ae518cf1839156039cc0047e29d91f70d191cfb97e0fbde85665e88c71e
4feaa28728442b0fd29a677d2b170a05fdf967c0 refactor: Rely on returned value of GetCoin instead of parameter (Lőrinc)
46dfbf169b49466de06dd16f7c23c6668419ef01 refactor: Return optional of Coin in GetCoin (Lőrinc)
e31bfb26c21f8b72f8c317e016d375862e27502e refactor: Remove unrealistic simulation state (Lőrinc)
Pull request description:
While reviewing [the removal of the unreachable combinations from the Coin cache logic](https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1721727681), we've noticed that the related tests often [reflect impossible states](https://github.com/bitcoin/bitcoin/pull/30673/files#r1740154464).
Browsing the Coin cache refactoring history revealed that migrating `bool GetCoin` to `optional<Coin> GetCoin` was [already proposed a few times before](https://github.com/bitcoin/bitcoin/pull/18746#issuecomment-842393167).
This refactor makes certain invalid states impossible, reducing the possibility of errors and making the code easier to understand. This will let us remove test code that exercises the impossible states as well.
The PR is done in multiple small steps, first swapping the new `optional` return value, slowly strangling out the usages of the return parameter, followed by the removal of the parameter.
Most of the invalid test states were still kept, except for https://github.com/bitcoin/bitcoin/pull/30673/files#r1748087322, where the new design prohibits invalid usage and https://github.com/bitcoin/bitcoin/pull/30673/files#r1749350258 was just marked with a TODO, will be removed in a follow-up PR.
ACKs for top commit:
andrewtoth:
re-ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0
achow101:
ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0
laanwj:
Code review ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0
theStack:
Code-review ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0
Tree-SHA512: 818d60b2e97f58c489a61120fe761fb67a08dffbefe7a3fce712d362fc9eb8c2cced23074f1bec55fe71c616a3561b5a8737919ad6ffb2635467ec4711683df7
c98fc36d094a08d44f3c95431db2c5f34a96cc73 wallet: migration, consolidate external wallets db writes (furszy)
7c9076a2d2e5dd117c31ca871e81c9170aa0e371 wallet: migration, consolidate main wallet db writes (furszy)
9ef20e86d7fd5d775c463a7a752c5bba9ef1266b wallet: provide WalletBatch to 'SetupDescriptorScriptPubKeyMans' (furszy)
34bf0795fc0fe30bd04a5b3a89b779f502189262 wallet: refactor ApplyMigrationData to return util::Result<void> (furszy)
aacaaaa0d3aeb576e01e2b4e6b3b094ab10f602a wallet: provide WalletBatch to 'RemoveTxs' (furszy)
57249ff669748d60895c275255dfb9bffaecbb67 wallet: introduce active db txn listeners (furszy)
91e065ec175e3d40439230d0a70b2c2c73ff024b wallet: remove post-migration signals connection (furszy)
055c0532fc8bb3135102590b46e266b68e9d9edc wallet: provide WalletBatch to 'DeleteRecords' (furszy)
122d103ca22f347eef7b323910477b72736c64b9 wallet: introduce 'SetWalletFlagWithDB' (furszy)
6052c7891dc033e30b342ae5ea1690f8e7cbeb9e wallet: decouple default descriptors creation from external signer setup (furszy)
f2541d09e132ce17a5d166583be98f5cdbf06588 wallet: batch MigrateToDescriptor() db transactions (furszy)
66c9936455fc0b79629686fe6b5737a020662e5d bench: add coverage for wallet migration process (furszy)
Pull request description:
Last step in a chain of PRs (#26836, #28894, #28987, #29403).
#### Detailed Description:
The current wallet migration process performs only individual db writes. Accessing disk to
delete all legacy records, clone and clean each address book entry for every created wallet,
create each new descriptor (with their corresponding master key, caches and key pool), and
also clone and delete each transaction that requires to be transferred to a different wallet.
This work consolidates all individual disk writes into two batch operations. One for the descriptors
creation from the legacy data and a second one for the execution of the migration process itself.
Efficiently dumping all the information to disk at once atomically at the end of each process.
This represent a speed up and also a consistency improvement. During migration, we either
want to succeed or fail. No other outcomes should be accepted. We should never leave a
partially migrated wallet on disk and request the user to manually restore the previous wallet from
a backup (at least not if we can avoid it).
Since the speedup depends on the storage device, benchmark results can vary significantly.
Locally, I have seen a 15% speedup on a USB 3.2 pendrive.
#### Note for Testers:
The first commit introduces a benchmark for the migration process. This one can be
cherry-picked on top of master to compare results pre and post changes.
Please note that the benchmark setup may take some time (~70 seconds here) due to the absence
of a batching mechanism for the address generation process (`GetNewDestination()` calls).
ACKs for top commit:
achow101:
ACK c98fc36d094a08d44f3c95431db2c5f34a96cc73
theStack:
re-ACK c98fc36d094a08d44f3c95431db2c5f34a96cc73
pablomartin4btc:
re-ACK c98fc36d094a08d44f3c95431db2c5f34a96cc73
Tree-SHA512: a52d5f2eef27811045d613637c0a9d0b7e180256ddc1c893749d98ba2882b570c45f28cc7263cadd4710f2c10db1bea33d88051f29c6b789bc6180c85b5fd8f6
8523d8c0fc85c5511311628d47c78fa380b99f6e ci: display logs of failed tests automatically (furszy)
Pull request description:
Saw it here https://github.com/bitcoin/bitcoin/actions/runs/11488618084/job/31975712362?pr=31000.
The 'test-each-commit' and 'win64' CI jobs currently do not display test logs when an error occurs, making it almost impossible to debug issues that don't arise locally. Fix this by setting the CTest `--output-on-failure` flag (per [README](2f40e453cc/src/test/README.md (L130))).
ACKs for top commit:
hebasto:
ACK 8523d8c0fc85c5511311628d47c78fa380b99f6e, I have reviewed the code and it looks OK.
Tree-SHA512: 59c025099fb623e2ed430cfc1ba808e1d3ff72773d021e2280a44423ae53615c16e96a07014eb8581c95aae241b6d2777e388a8931ff0904feb84ca45cb22763
fb46d57d4e7263495c007f4a499a349bff2b21e0 cmake, qt, test: Remove problematic code (Hennadii Stepanov)
Pull request description:
Split from https://github.com/bitcoin/bitcoin/pull/30997.
The removed code aimed to make Qt's minimal integration plugin DLL available for `test_bitcoin-qt.exe` on Windows.
However, there are two issues:
1. The code is broken because the destination directory must end with a trailing slash (`/`).
2. It is unnecessary because Qt's minimal integration plugin is not used on Windows. For more details, please refer to the following code:fb46d57d4e/src/qt/test/CMakeLists.txt (L38-L44)
As a side effect, this PR fixes https://github.com/bitcoin-core/gui/issues/842.
ACKs for top commit:
fanquake:
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0
TheCharlatan:
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0
Tree-SHA512: b44d1c5e352e9bbfbba3c263ee03838cd490435da0490d9c8a152e60515520772c8a87aca08d4510f50c2e46b64ac92c666fa81accf43758af2e896693c44ffa
6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e build: Replace MAC_OSX macro with existing __APPLE__ (Lőrinc)
Pull request description:
This PR aims to standardize and simplify macOS-specific checks within our codebase by replacing the custom-defined `MAC_OSX` macro with the existing `__APPLE__`macro, defined in e.g. https://sourceforge.net/p/predef/wiki/OperatingSystems/#macos
We already use `__APPLE__` in our own codebase for e.g. https://github.com/bitcoin/bitcoin/blob/master/src/crypto/sha256.cpp#L22
Local Verification confirms that `MAC_OSX` isn't defined, but `__APPLE__` is:
```bash
% echo | cpp -dM | egrep 'MAC_OSX|__MACOS__|__APPLE__'
#define __APPLE__ 1
```
ACKs for top commit:
fanquake:
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.
Tree-SHA512: dbf87c96211d9d55426ee85d76ef1e05cda3efd1c9248b0974a82834dafc1c1aece3165bd46e4252f0460dc97079bdbcebe98bbd81e9de0d7399c0bc69d5c050
The removed code aimed to make Qt's minimal integration plugin DLL
available for `test_bitcoin-qt.exe` on Windows.
However, there are two issues:
1. The code is broken because the destination directory must end with a
trailing slash (`/`).
2. It is unnecessary because Qt's minimal integration plugin is not
used on Windows. For more details, please refer to the following
code.
fa1c5cc9df116411edca172f8b80fc225c776554 fees: Log non-fatal errors as [warning], instead of info-level (MarcoFalke)
ddddbac9c10ba80c94e1f3ceeffa7091d18ea48d fees: Pin required version to 149900 (MarcoFalke)
fa5126adcb1247efb7e480f37f9c23446f36ec53 fees: Pin "version that wrote" to 0 (MarcoFalke)
Pull request description:
Coupling the fees serialization with CLIENT_VERSION is problematic, because:
* `CLIENT_VERSION` may change, even though the serialization format does not change. This is harmless, but still confusing.
* If a serialization format change was backported (unlikely), it may lead to incorrect results.
* `CLIENT_VERSION` is changed at a different time during the release process than any serialization format change. This is harmless for releases of Bitcoin Core, but may be confusing when using the development branch.
* It is harder to reason about a global `CLIENT_VERSION` when changing the format, than to reason about a versioning local to the module.
Fix all issues by pinning the current version number in the module locally. In the future it can then be modified locally to the module, if needed.
ACKs for top commit:
hodlinator:
re-ACK fa1c5cc9df116411edca172f8b80fc225c776554
TheCharlatan:
Re-ACK fa1c5cc9df116411edca172f8b80fc225c776554
Tree-SHA512: 93870176ed50cc5a734576d66398a6036b31632228a9e05db1fa5452229e35ba4126f003e7db246aeb9891764ed47bde4470c674ec2bce7fd3ddd97e43944627
82e16e698321ea6fb69ce5a08b048347aab8c74e cmake: Refactor install kernel dependencies (Hennadii Stepanov)
42e62779873cf9d307c678c8867288ad4441855b build: Add static libraries to Kernel install component (TheCharlatan)
Pull request description:
Fixes the installation of the pkgconfig file and the static library when installing only the `Kernel` component.
This is a followup to fix#30835 and #30814, which were merged shortly after one another, but are interrelated. Can be tested with:
```
cmake -B build -DBUILD_SHARED_LIBS=OFF -DBUILD_KERNEL_LIB=ON
cmake --build build --target bitcoinkernel
cmake --install build --component Kernel
```
ACKs for top commit:
hebasto:
ACK 82e16e698321ea6fb69ce5a08b048347aab8c74e, tested on Ubuntu 23.10.
fanquake:
ACK 82e16e698321ea6fb69ce5a08b048347aab8c74e
Tree-SHA512: 07c18a341d4464e489c28fb262600338f1711248309ffb2af0ef3ab1abf06f10873c435895b63010e0be8e44af77046324896dfd872479792aa049831606dc45
a0c9595810c7d8bb17d8b5bea8d916db194b5239 doc: Make list of targets in depends README consistent (laanwj)
Pull request description:
The description of `i686-pc-linux-gnu` and `x86_64-pc-linux-gnu` is incomplete and inconsistent with the others. Fix this. Also use "64 bit" consistently instead of "64-bit".
ACKs for top commit:
maflcko:
lgtm ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
hebasto:
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239.
jarolrod:
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
rkrux:
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
Tree-SHA512: eedefb19639dd08f25627ceaab0d2c3745b256e561e55f8506d14721d0236978f1b1bef79f9fe126b7f42d869887ca988d04b3536d98a27e0eb182f0a7f64183
31cc5006c3de4dd6a1f7a238684163956604df45 init: Return fatal failure on snapshot validation failure (Martin Zumsande)
8f1246e833804789ee5d8b59026b49142df5c455 init: Improve chainstate init db error messages (TheCharlatan)
cd093049dda878e8424fdc1ef828b5f644bd91d4 init: Remove incorrect comment about shutdown condition (MarcoFalke)
635e9f85d76c28647120172d9524982ebe36cf3c init: Remove misleading log line when user chooses not to retry (TheCharlatan)
720ce880a355cf59a4f042a504750eb4e3ee68d3 init: Improve comment describing chainstate load retry behaviour (Martin Zumsande)
baea842ff184f98d2f07568f0a77e48a34d3cde3 init: Remove unneeded argument for mempool_opts checks (stickies-v)
Pull request description:
These are mostly followups from #30968, making the code, log lines, error messages, and comments more consistent.
The last commit is an attempt at improving the error reporting when loading the chainstate. It aims to more cleanly distinguish between errors arising from a specific database, and errors where the culprit may be less clear.
ACKs for top commit:
achow101:
ACK 31cc5006c3de4dd6a1f7a238684163956604df45
mzumsande:
Code Review / lightly tested ACK 31cc5006c3de4dd6a1f7a238684163956604df45
BrandonOdiwuor:
Code Review ACK 31cc5006c3de4dd6a1f7a238684163956604df45.
stickies-v:
ACK 31cc5006c3de4dd6a1f7a238684163956604df45
Tree-SHA512: 59fba4845ee45a3d91bf55807ae6b1c81458463b96bf664c8b1badfac503f6b01efd52a915fc399294e68a3f69985362a5a10a3844fa23f7707145ebe9ad349b
04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a test: add test for specifying custom pidfile via `-pid` (Sebastian Falbesoner)
b832ffe04468762f94c6b8a3efb2a978bd16e52e refactor: introduce default pid file name constant in tests (tdb3)
Pull request description:
This small PR adds test coverage for the `-pid` command line option, which allows to overrule the pid filename (`bitcoind.pid` by default). One can specify either a relative path (within the datadir) or an absolute one; the latter is tested using `self.options.tmpdir`. Note that the functional test file `feature_init.py` so far only contained a stress test; with this new sub-test added, both the description and the test name are adapted to be more generic.
ACKs for top commit:
achow101:
ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a
tdb3:
ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a
ryanofsky:
Code review ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a
naiyoma:
Tested ACK [04e4d52420)
Tree-SHA512: b2bc8a790e5d187e2c84345f344f65a176b62caecd9797c3b9edf10294c741c33a24e535be640b56444b91dcf9c65c7dd152cdffd8b1c1d9ca68e5e3c6ad1e99
The description of `i686-pc-linux-gnu` and `x86_64-pc-linux-gnu` is
incomplete and inconsistent with the rest. Fix this. Also use "64 bit"
consistently instead of "64-bit".
Also, remove not needed and possibly redundant function name and class
names from the log string. Also, minimally reword the log messages.
Also, remove redundant trailing newlines from log messages, while
touching.
fa71bedf8609f06618aa85342ea6f5c4d2c5fea0 ci: Approximate MAKEJOBS in image build phase (MarcoFalke)
Pull request description:
The `MAKEJOBS` env var is the default in image builds, which is fine, because it is only relevant when building msan (or iwyu) and only differs when setting MAKEJOBS to something other than `nproc` (currently used as an approximation).
So the normal workflow of `MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_msan.sh" ./ci/test_run_all.sh` already works today.
However, `MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_native_msan.sh" ./ci/test_run_all.sh` does not.
This is hard to fix, because making the env var a build arg means that changing it (and only it) requires a new (expensive and redundant) build.
So add an option `HAVE_CGROUP_CPUSET`, which can be set to approximate `MAKEJOBS` a bit. Can be tested via:
`HAVE_CGROUP_CPUSET=yo MAKEJOBS="-j_something" FILE_ENV="./ci/test/00_setup_env_native_msan.sh" ./ci/test_run_all.sh`
ACKs for top commit:
fanquake:
ACK fa71bedf8609f06618aa85342ea6f5c4d2c5fea0
Tree-SHA512: 43ef194c71d726f4cfa3fe08a5894c7872150f37da7e4fa0c2d89e4572bc63acadb5dae3286a5e5cc14a8ce3e1ebcc14571f1a3541e8db2d18d2f7503764a2f3
a16917fb5981d1465ffd4c036586f8729e683b73 rpc, net: improve `mapped_as` doc for getrawaddrman/getpeerinfo (brunoerg)
bdad0243be80c4c72f8cefd1233fd7c057a1945b rpc, net: getrawaddrman "mapped_as" follow-ups (brunoerg)
Pull request description:
- Change `addrman` to reference to const since it isn't modified (https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1612272793).
- Improve documentation of `mapped_as`/`source_mapped_as` in `getrawaddrman` RPC by mentioning that both fields will be only available if asmap flag is set. It is the same message for `mapped_as` field in `getpeerinfo`.
ACKs for top commit:
fjahr:
re-ACK a16917fb5981d1465ffd4c036586f8729e683b73
0xB10C:
re-ACK a16917fb5981d1465ffd4c036586f8729e683b73
laanwj:
re-ACK a16917fb5981d1465ffd4c036586f8729e683b73
Tree-SHA512: c66b2ee9d24da93d443be83f6ef3b2d39fd5bf3f73e2974574cad238ffb82035704cf4fbf1bac22a63734948e285e8e091c2884bb640202efdb473315e770233
RegQueryValueExA(HKEY_PERFORMANCE_DATA, ...) sometimes hangs bitcoind.exe on Windows during startup, at least on CI.
We have other sources of entropy to seed randomness with on Windows, so should be alright removing this. Might resurrect if less drastic fix is found.
4d3da08d1b9d07acb43420899e0d16fad2437fb0 guix: Enable CET for `glibc` package (Hennadii Stepanov)
Pull request description:
Pulled from #30685. This doesn't need to wait for anything.
ACKs for top commit:
laanwj:
ACK 4d3da08d1b9d07acb43420899e0d16fad2437fb0
TheCharlatan:
ACK 4d3da08d1b9d07acb43420899e0d16fad2437fb0
Tree-SHA512: 1f4645971381fd342adec52c826fc0023722519a3e28043c9fe8b64bbc1abad822fcc25a64f3f959e3f3a10f5c119029f4cae13c22bac6badcbec9ae8b501dfc
86e2a6b749c7fecbd086b361806ac9f6e9426d79 [test] A non-standard transaction which is also consensus-invalid should return the consensus error (Antoine Poinsot)
f859ff8a4e9c3aa23bf5be6eceb7099ca72b2290 [validation] Improve script check error reporting (dergoegge)
Pull request description:
An input script might be invalid for multiple reasons. For example, it might fail both a standardness check and a consensus check, which can lead to a `mandatory-script-verify-flag-failed` error being reported that includes the script error string from the standardness failure (e.g. `mandatory-script-verify-flag-failed (Using OP_CODESEPARATOR in non-witness script)`), which is confusing.
ACKs for top commit:
darosior:
re-ACK 86e2a6b749c7fecbd086b361806ac9f6e9426d79
ariard:
Re-Code Review ACK 86e2a6b7
instagibbs:
ACK 86e2a6b749c7fecbd086b361806ac9f6e9426d79
Tree-SHA512: 053939107c0bcd6643e9006b2518ddc3a6de47d2c6c66af71a04e8af5cf9ec207f19e54583b7a056efd77571edf5fd4f36c31ebe80d1f0777219c756c055eb42
Before, we did not explicity say that both fields
`{source_}mapped_as` (that are optional in getrawaddrman)
will be only available if the asmap config flag is set.
Co-authored-by: Jon Atack <jon@atack.com>
The wallet is isolated during migration and reloaded at the end
of the process. There is no benefit on connecting the signals
few lines before unloading the wallet.
fac6cfe5ac06547c90da6f976d7c8bed20da8bac lint: commit-script-check.sh: echo to stderr (MarcoFalke)
Pull request description:
This makes it easier to redirect the produced `git diff` on failure. On success, it shouldn't hurt, because the same output is still present, just on stderr.
Can be tested by introducing a fault in any scripted diff and then calling `commit-script-check.sh HEAD~..HEAD > any_file.txt`. Previously the file contained the full output, now it contains just the diff.
ACKs for top commit:
TheCharlatan:
ACK fac6cfe5ac06547c90da6f976d7c8bed20da8bac
Tree-SHA512: b4dfad10a4a902729a7ad7533ed0ef86b9e79761083f2ec623d448a551462b268fe04bdba387ca62160dae9ef7b1781e005dec60f18b111d9bfa6b97357108e6
cd0edf26c07c8c615f3ae3ac040c4774dcc8e650 tracing: cast block_connected duration to nanoseconds (0xb10c)
Pull request description:
When the `validation:block_connected` tracepoint was introduced in 8f37f5c2a562c38c83fc40234ade9c301fc4e685, the connect block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. This was 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, 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 a duration passed in an incorrect unit (1000x off).
A previous version of this PR casted the duration to microseconds `µs` - however, as the last three major releases have had the duration as nanoseconds (and this went unnoticed), we assume that this is the API now and changeing it back to microseconds would break the API again. See also https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2067867597
ACKs for top commit:
maflcko:
re-lgtm ACK cd0edf26c07c8c615f3ae3ac040c4774dcc8e650
laanwj:
re-ACK cd0edf26c07c8c615f3ae3ac040c4774dcc8e650
Tree-SHA512: 54a1eea0297e01c07c2d071ffafbf97dbd080f763e1dc0014ff086a913b739637c1634b1cf87c90b94a3c2f66006acfaada0414a15769cac761e03bc4aab2a77
