This queue may be hundreds to thousands of items, each of
which are being revalidated at short interval. Allow the
node to be more responsive in this scenario.
Add a belt-and-suspenders feature, limit the amount of
memory and cpu possible when unlucky or simply misconfigured.
The worst case limit is roughly 400kB * 10,000 = 4GB, regardless
of usage pattern.
Before this change, sheer volume of broadcasts, mismatches in
standardness rules, or simply fee mismatches may result in unbounded
growth of memory usage. As the feature may be expanded in
the future, explicit bounds helps reasoning going forward.
3765b428d1 logging: More fully remove libevent log category (Ryan Ofsky)
Pull request description:
Libevent log category was partially removed in 39e9099da5, and this commit extends that with the following changes:
- Stops showing libevent in the list of supported log categories in `bitcoind -help` and `bitcoin-cli help logging` output.
- Stops returning `"libevent": false` in `logging` RPC output.
It's not good to treat libevent as a supported log category when it can't be enabled and trying to enable it results in warnings.
There's also no need to define an unused LIBEVENT constant value and keep more complicated logic for dealing with deprecated log categories, so this change also simplifies code internally.
ACKs for top commit:
l0rinc:
code review ACK 3765b428d1
pinheadmz:
ACK 3765b428d1
sedited:
ACK 3765b428d1
Tree-SHA512: 09e9514f905bb0a79d870689af491886baaa31fa19f2ad6aef4283fa20c2fa6ce8d384178139227aeeabffabff6e83d254114daaeefbbfe2c6172b9da8871298
a318f43254 bitcoin-util: Add netmagic command (ekzyis)
Pull request description:
This adds a `netmagic` command to `bitcoin-util`. It will return the network magic bytes of the selected chain:
```
$ bitcoin-util netmagic
f9beb4d9
$ bitcoin-util -regtest netmagic
fabfb5da
$ bitcoin-util -testnet4 netmagic
1c163f28
$ bitcoin-util -signet netmagic
0a03cf40
# default challenge
$ bitcoin-util -signet -signetchallenge=512103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae netmagic
0a03cf40
$ bitcoin-util -signet -signetchallenge=51 netmagic
54d26fbd
```
This will be particularly useful for https://github.com/bitcoin/bitcoin/pull/34566 to determine the datadir path of a custom signet, before starting bitcoind, since #34566 will add the network magic as a suffix. This was mentioned in https://github.com/bitcoin/bitcoin/pull/34566#issuecomment-4622356132. It uses the same code from init.cpp to print the signet derived magic:
ea9afb61a1/src/init.cpp (L967-L969)
Since it does not depend on #34566, and the changes are quite simple, I created a separate PR for this for easier review and discussion.
I have tested this by invoking the command with the options above.
ACKs for top commit:
stickies-v:
ACK a318f43254
sedited:
ACK a318f43254
Tree-SHA512: bcb5a1b14260e13a35f5682e4dac735f472ab6ff7781e7e77aef5895ddbecc03a2fc72a26419ba9a28354736a1eaaeddb047bafe276440e339f17ddfe7841862
2ee4fafa3f test: add fuzz test for private broadcast (kevkevinpal)
08b7c61fc7 private broadcast: enforce sending to unique node ids (Vasil Dimov)
Pull request description:
Add a fuzz test that exercises the public methods of the `PrivateBroadcast` class from `src/private_broadcast.h` and checks for correctness.
ACKs for top commit:
instagibbs:
ACK 2ee4fafa3f
nervana21:
re-ACK 2ee4fafa3f
frankomosh:
Code Review ACK 2ee4fafa3f.
Tree-SHA512: 35f9efcf9e7ea8bd071f6b607fd7c901b60ccf14610f501e6cf06f0d05c389424ab8a0c25a2aaffd5755c8f920af4834e0979cae72a474680170de2b94d06e1c
829255c8be cmake: Remove `SelectLibraryConfigurations` from `FindQRencode` module (Hennadii Stepanov)
5c55606da9 depends: Remove unused `lib/pkgconfig` in `qrencode` package (Hennadii Stepanov)
402ba10b20 cmake: Drop optional `PkgConfig` use in `FindQRencode` module (Hennadii Stepanov)
Pull request description:
This PR addresses:
- [this](https://github.com/bitcoin/bitcoin/pull/35602#issuecomment-4799507779) comment:
> If it works without pkg-config, then we should remove the usage of pkgconfig entirely, rather than say it will work without it, and leave the dependency in the code?
- and [this](https://github.com/bitcoin/bitcoin/pull/35602#issuecomment-4799947803) one:
> While cleaning up this module, you may also get rid of `SelectLibraryConfigurations`. We don't want that `QRencode_LIBRARY` or `QRencode_LIBRARIES` is used anywhere; there is no need to set a variable with that name.
ACKs for top commit:
purpleKarrot:
ACK 829255c8be
Tree-SHA512: fd87a5afd7c0b3271a008df8adcb2ff809053799a0658fbb5a1f15ed11f6abad25836c8e1b7507d0d4c35c027dcbba88b136ac827e17b283ee4ffbbe55d6f4ee
bbbbab86a8 ci: Bump tsan config to ubuntu:26.04 with -U_FORTIFY_SOURCE (MarcoFalke)
Pull request description:
The `-U_FORTIFY_SOURCE` should be harmless in the Tsan CI, and is required to work around https://github.com/bitcoin/bitcoin/issues/30586
ACKs for top commit:
fanquake:
ACK bbbbab86a8
Tree-SHA512: 7c4b434342861fabc52eda7b551ebe08fcde6079cd39d54a21938c88dcfd80cdb876dd7de859bac1201a2b928163c19704c24d5b5a5e76f4935e33cb3e6ae2bd
The `ERR` macro is defined on illumos-based systems in the `regset.h`
header included by the Boost.Test framework, which may cause a
compilation error.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./src/qt/psbtoperationsdialog.* ) ; }
ren StatusLevel::INFO StatusLevel::Info
ren INFO Info
ren StatusLevel::WARN StatusLevel::Warn
ren WARN Warn
ren StatusLevel::ERR StatusLevel::Error
ren ERR Error
-END VERIFY SCRIPT-
6fa4132298 fuzz: share a single mocked steady clock across FuzzedSock instances (Hao Xu)
Pull request description:
This is a follow-up of https://github.com/bitcoin/bitcoin/pull/35478#issuecomment-4667842057, inspired by maflcko .
Each FuzzedSock used to own its mocked steady clock and call MockableSteadyClock::SetMockTime() directly. Hold the clock by reference to an externally provided SteadyClockContext instead, so that several FuzzedSock instances sharing a test case (e.g. one per peer, or one created via Accept()) advance a single mocked clock, and the mocking goes through the SteadyClockContext RAII helper that resets mocktime on destruction.
SteadyClockContext is a LimitOne type, so each fuzz target constructs one instance per iteration and passes it to ConsumeSock / ConsumeNode / the FuzzedSock constructor.
ACKs for top commit:
maflcko:
review ACK 6fa4132298🌕
marcofleon:
crACK 6fa4132298
Tree-SHA512: 3c773b5c0c3ba42a8245c9ea6042b0bc767df4fad506305f3c200310616b48a59deb1542086eb4ce3e8a1407c4d6b42cef3b37cd84bfe80d4821972b8d3b4286
8791c4764c test: use ExtendedPrivateKey in wallet_taproot.py (rkrux)
89ceafafb9 test: use ExtendedPrivateKey in wallet_listdescriptors.py (rkrux)
bbfffcab58 test: use ExtendedPrivateKey in wallet_send.py (rkrux)
2ab6e590f7 test: use ExtendedPrivateKey in wallet_keypool.py (rkrux)
9e20118720 test: use ExtendedPrivateKey in wallet_fundrawtransaction.py (rkrux)
06af0cddbb test: use ExtendedPrivateKey in wallet_descriptor.py (rkrux)
4100fac20e test: use ExtendedPrivateKey in wallet_createwallet.py (rkrux)
ff3f6def9a test: use ExtendedPrivateKey in wallet_bumpfee.py (rkrux)
003f2a01f6 test: use ExtendedPrivateKey in feature_notifications.py (rkrux)
f988e6d6e6 test: use ExtendedPrivateKey in wallet_importdescriptors.py (rkrux)
d2a03d50ac test: add extendedkey.py unit tests by using BIP32 test vectors (rkrux)
afdb378082 test: introduce ExtendedPrivateKey and ExtendedPublicKey classes (rkrux)
4dbaa7cc65 test: generalise byte_to_base58 utility function to allow more version types (rkrux)
Pull request description:
Many a times there has been a need to come up with dynamic xprvs and xpubs
in the functional tests, but the lack of code that creates them dynamically has
led to the presence of several hardcoded keys in the testing framework. This
is not developer friendly and not self-documenting, clutters the testing code,
and makes it difficult to update the tests in the future.
This PR introduces two utility classes ExtendedPrivateKey and
ExtendedPublicKey that allows the developer to create them on the fly
to be used in the tests. I have intentionally not introduced any library for this
purpose and have reused the existing libraries and functions in the framework.
The implementation is supposed to provide basic functionality for creating
xprv randomly or from a fixed seed, creating corresponding xpub, and
deriving child xprvs and xpubs at custom derivation paths.
I've updated many tests to show how these can be used, there are more
tests as well that can be updated in the future to completely remove such
non-deterministic hardcoded keys.
ACKs for top commit:
achow101:
ACK 8791c4764c
w0xlt:
ACK 8791c4764c
Tree-SHA512: f8ec4e09eaa6cc44b0f1c9a91337e570b12fb882c258be89b470de1a8cecf9d2fd40d9f02ee739dcbf639462ea7710aa145a3726f0f537f5a1f1e7772e5b019d
The local `static constexpr auto ERR` shadowed the `Sock::ERR` static
data member. Rename it to `accept_error`, per the Developer Notes'
shadowing guidance.
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
1a3cfdf1b7 fuzz: connman: cover AddLocalServices/RemoveLocalServices (Bruno Garcia)
c507fb3063 fuzz: connman: add outbound-bytes invariants (Bruno Garcia)
4a6fce43ea fuzz: connman: add AddNode/RemoveAddedNode invariants (Bruno Garcia)
a5859edef4 fuzz: connman: set m_local_services/m_use_addrman_outgoing/m_max_automatic_connections (Bruno Garcia)
4b84c9125a fuzz: connman: add network activity invariants (Bruno Garcia)
Pull request description:
This PR improves the `connman` fuzz target by replacing some "`(void)`" calls with actual invariant checks, adding coverage for previously uncovered methods, and exercising more initialization states.
- Set `m_local_services`, `m_use_addrman_outgoing`, and
`m_max_automatic_connections` via fuzzed values before `Init()` to
explore more startup configurations.
- Add network activity and outbound-bytes invariants.
- Add `AddNode`/`RemoveAddedNode` invariants: e.g. a successful `AddNode`
increases `GetAddedNodeInfo()` by one; adding the same node again
must fail; a subsequent `RemoveAddedNode` must succeed and restore
the original count.
- Add coverage for `AddLocalServices`/`RemoveLocalServices`.
ACKs for top commit:
nervana21:
re-ACK 1a3cfdf1b7
frankomosh:
reACK 1a3cfdf1b7 . Change from the diff is the restoring `(void)connman.RemoveAddedNode(random_string)` arm
sedited:
ACK 1a3cfdf1b7
Tree-SHA512: c7b6799ca65d2e639d8ab9ab0cc77bae663f24fbda934446a8ee2e8ce9e8e36624d16b4f492b1714e2d67375edd35907cb9392d21f368d3d5298275ff1d05c72
8ebfff0f88 doc: add send RPC release note (Sjors Provoost)
5884f5a4fa wallet: remove experimental warning from send RPCs (Sjors Provoost)
Pull request description:
The `send` RPC was introduced in v21 an initially marked experimental. The `sendall` RPC was added in v24, based on `send` and also marked experimental.
I'm not aware of any proposed breaking changes, except #35433 which follows the regular deprecation flow.
Time to mark them as no longer experimental.
ACKs for top commit:
w0xlt:
ACK 8ebfff0f88
achow101:
ACK 8ebfff0f88
polespinasa:
ACK 8ebfff0f88
pablomartin4btc:
ACK 8ebfff0f88
Tree-SHA512: beb5321adaf871157bda396c8e5740daff95ffe342416914340ae4197accebe60236032d1329876b42405437b99f59079a56ec1e5ac592b753031ba2ebd36cfb
58560c281d ci: remove some packages from Chimera job (fanquake)
Pull request description:
This job doesn't build QT, so drop `ninja` and `xz`.
With libevent removed, and no Qt build, we can also drop `pkgconf`.
ACKs for top commit:
sedited:
ACK 58560c281d
hebasto:
ACK 58560c281d.
Tree-SHA512: 4537c8c77334637ea177321d95ad39ee103ab0080e8331f9fe9efb92a3bb4dc00fb59d2e250fd43c267c05f0a2b98e779949ff40723633c2f2bff345fa554a31
9e6546c517 test: raise reindex mining RPC timeout (Lőrinc)
Pull request description:
**Problem:** I often hit a timeout in `feature_reindex.py` when running functional tests locally in parallel in debug mode (especially on battery or in power-saving mode).
**Fix:** Increase the test-local RPC timeout for the reindex mining setup.
ACKs for top commit:
mercie-ux:
ACK 9e6546c517
sedited:
ACK 9e6546c517
Tree-SHA512: d3541dd6752f943921a030393b00e684b3b5d00b93aa0b2b1f85c017698bdfe2216d7f5441bb7dd5f153453385e4df6f9cc8055589e57bca83fa48f0b7de4252
0cdd817a82 add release note (Pol Espinasa)
517d37ce3e test: tests wallet migration with load_wallet disabled (Pol Espinasa)
b98dd63da7 rpc: Add load_wallet argument to migratewallet RPC (Pol Espinasa)
4acd063ba6 wallet: make loading the wallet after migrating optional (Pol Espinasa)
97d08d62ba refactor: store wallet names to MigrationResult (Pol Espinasa)
Pull request description:
This PR is motivated by this [Stack Exchange question](https://bitcoin.stackexchange.com/questions/130713/bitcoin-core-quickest-method-legacy-descriptor-wallet-migration).
Long story short, someone who has a node pruned before his legacy wallet birthday, is unable to migrate the wallet as it is not possible to load it.
Loading is not necessary for migration, and migrating without wanting to use the wallet in that node is a valid use-case.
This PR adds a new RPC argument to `migratewallet` that allow the user disabling the wallet loading.
Second commits adds tests for it.
Follow-up: Add an option to the GUI to not load the wallet after migrating.
ACKs for top commit:
achow101:
ACK 0cdd817a82
w0xlt:
ACK 0cdd817a82
pablomartin4btc:
ACK 0cdd817a82
Tree-SHA512: 8389599e63603b1a532e1bfba0b6c652653386c001f5a881bd49843302b74ff4dbaa4131b5b377c24f483d42e0e70a92b96f760244e3c2e2b44ce08cd04ca1e0
703a671fbc fuzz: compact coins view db during fuzzing (Lőrinc)
0868c85fd5 refactor: rename async coin compaction (Lőrinc)
Pull request description:
**Problem:** #35465 added async chainstate compaction, but the `coins_view_db` fuzz target did not exercise scheduling compaction alongside ordinary coins view operations.
The async wrapper also shared the `CompactFull()` name with the blocking `CDBWrapper` primitive.
**Fix:** Rename the coins DB wrapper to `CompactFullAsync()` and let `coins_view_db` randomly schedule it under `cs_main` (like in production).
The fuzz operation only starts compaction and any running job is joined by the `CCoinsViewDB` destructor at the end of the fuzz input.
ACKs for top commit:
sedited:
ACK 703a671fbc
andrewtoth:
ACK 703a671fbc
Tree-SHA512: 9854c3acbaace795155e7469cb10938fbd872726cbb8a4b4ef71d6d352d00824498747215aee1b237e64452eae2e690a3f7d7daa6b6b0030e75a6f2ccc0802fb
Libevent log category was partially removed in 39e9099da5, and this
commit extends that with the following changes:
- Stops showing libevent in the list of supported log categories in
`bitcoind -help` and `bitcoin-cli help logging` output.
- Stops returning `"libevent": false` in `logging` RPC output.
It's not good to treat libevent as a supported log category when it
can't be enabled and trying to enable it results in warnings.
There's also no need to define an unused LIBEVENT constant value and
keep more complicated logic for dealing with deprecated log categories,
so this change also simplifies code internally.
Co-authored-by: David Gumberg <davidzgumberg@gmail.com>
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
abc33ff043 test: announce field must be 0 or 1 in sendcmpct (brunoerg)
2d0dce0af5 net_processing: fix BIP152 first integer interpretation (brunoerg)
Pull request description:
Fixes#35542
According to the BIP152, the first integer in `sendcmpct` message shall be interpreted as a boolean (and MUST have a value of either 1 or 0). We currently correctly interpret it as boolean, however, we accept any value >=1 and treat it as `true`, deviating from the specification. This PR fixes it.
ACKs for top commit:
edilmedeiros:
utACK abc33ff043
davidgumberg:
crACK abc33ff043 Seems reasonable to comply with BIP152 strictly, test looks good as well.
Sjors:
ACK abc33ff043
jonatack:
re-ACK abc33ff043
achow101:
ACK abc33ff043
w0xlt:
ACK abc33ff043
Tree-SHA512: 77fed86d4de81f7c35ff002b6e1b2a90882ea55f159075da4d34a619d1075f625fca34f929cdd981f1b2eb06f76b64f42cda46502dcd1b4a634c690b0882ec7c
Exercise `CCoinsViewDB::CompactFullAsync()` from the `coins_view_db` fuzz target so the new chainstate compaction wrapper can run concurrently with ordinary coins view operations.
The fuzz operation only schedules compaction, matching production; outstanding work is waited for by the `CCoinsViewDB` destructor at the end of the fuzz input.
Rename the `CCoinsViewDB` async compaction wrapper to `CompactFullAsync()` so it is distinct from the blocking `CDBWrapper::CompactFull()` primitive it calls.
b847626562 test: refresh MiniWallet after node restart (Sjors Provoost)
f4e643cb15 test: merge mining options in package feerate check (Sjors Provoost)
280ce6a0ae miner: ensure block_max_weight is flattened before limit checks (Sjors Provoost)
65bd3164fb mining: clarify test_block_validity comment (Sjors Provoost)
978e7216e6 test: use shared default_ipc_timeout (Sjors Provoost)
Pull request description:
This implement the suggested followups from #33966. Each commit links to the original comment.
The most important change is the extra asserts added in `miner: ensure block_max_weight is flattened before limit checks`.
ACKs for top commit:
achow101:
ACK b847626562
enirox001:
tACK b847626562
sedited:
ACK b847626562
w0xlt:
ACK b847626562
Tree-SHA512: 47678eaed604228269bd892ccf8ff58804745bbc7675b4a93528da9a9292a2eb1e0562cdb8341edac77178563420885b48282bb9e5c2b997b28f2fc64ceeff3d
2fe34808fa wallet: reject sendtoaddress and sendmany for external signers (Sjors Provoost)
bd5a32f7db doc: add taproot descriptor to getdescriptors example (woltx)
7131c82937 doc: clarify which commands receive --chain, --fingerprint and --stdin (woltx)
4fdd4d8d29 doc: replace stale signtransaction wording with current signtx flow (woltx)
fab92257fe doc, rpc: document enumerate model field and fingerprint deduplication (woltx)
Pull request description:
This PR aligns the external signer documentation with current behavior, and makes one previously implicit behavior explicit.
Per review feedback, each commit fixes a limited set of issues:
* **doc, rpc: document enumerate model field and fingerprint deduplication** — the `enumerate` response uses the optional `model` field, which Bitcoin Core maps to the `name` field of the `enumeratesigners` RPC result. Duplicate fingerprints are skipped, and wallet operations require exactly one connected signer.
* **doc: replace stale signtransaction wording with current signtx flow** — spending from an external signer wallet uses `send`/`sendall` (and `bumpfee` for fee-bumping), which invoke `<cmd> --stdin` and pass the `signtx` subcommand and PSBT over stdin.
* **doc: clarify which commands receive --chain, --fingerprint and --stdin** — mark `--chain` and `--fingerprint` as required except for `enumerate`, keep `--stdin` required for protocol flexibility, and match the order and form of the actual invocations in the usage examples.
* **doc: add taproot descriptor to getdescriptors example** — show the BIP86 `tr()` descriptor alongside the other address types.
* **wallet: reject sendtoaddress and sendmany for external signers** — return a specific error instead of the misleading "Private keys are disabled for this wallet", with functional test coverage. Cherry-picked from #33112 (thanks Sjors).
How the documentation went stale:
* The `enumerate` example has shown a `name` field since external signer support landed in #16546, but the implementation has always read `model`.
* `sendtoaddress`/`sendmany` external signer support was effectively precluded by #21201, which was merged a few days before #16546, so the interaction was missed in review and the documented `signtransaction` flow never existed in this form.
* Fingerprint deduplication was added in #35251.
* The documentation was last updated in #33765.
ACKs for top commit:
Sjors:
ACK 2fe34808fa
optout21:
ACK 2fe34808fa
naiyoma:
ACK 2fe34808fa
Tree-SHA512: 86859d2f81ac337f3b4b6578c6ee0151ffb76b8374dfa58e28e00ce4eb69dc200cd6bd2d0a99f73d0475c3824d6ac1cb9e2542b119ca124dd835132dc95cd023
Each FuzzedSock used to own its mocked steady clock and call
MockableSteadyClock::SetMockTime() directly. Hold the clock by reference
to an externally provided FakeSteadyClock instead, so that several
FuzzedSock instances sharing a test case (e.g. one per peer, or one
created via Accept()) advance a single mocked clock, and the mocking goes
through the FakeSteadyClock RAII helper that resets mocktime on
destruction.
FakeSteadyClock is a LimitOne type, so each fuzz target constructs one
instance per iteration and passes it to ConsumeSock / ConsumeNode / the
FuzzedSock constructor.
