After port collisions are no longer tolerated but lead to
a startup failure in v28.0, local setups of multiple nodes,
each with a different -port value would not be possible anymore
due to collision of the onion default port - even if the nodes
were using tor or not interested in receiving onion inbound connections.
Fix this by deriving the onion listening port to be -port + 1.
(idea by vasild / laanwj)
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
The mempool:replaced tracepoint now reports either a txid or a
package hash (previously it always was a txid). To let users know
if a txid or package hash is passed, a boolean argument is added
the the tracepoint.
In the functional test, a ctypes.Structure class for MempoolReplaced
is introduced as Python warns the following when not explcitly
casting it to a ctype:
Type: 'bool' not recognized. Please define the data with ctypes manually.
5c2e291060cca3be500f3af0f6f2d3fd2177a7c9 bench: Add basic CheckEphemeralSpends benchmark (Greg Sanders)
3f6559fa581b1f78cd9a9ef4dc0169e315ffa6b3 Add release note for ephemeral dust (Greg Sanders)
71a6ab4b33df383642cca49397a88b1606171225 test: unit test for CheckEphemeralSpends (Greg Sanders)
21d28b2f362708dd9206feb9ddc11a352063ef0c fuzz: add ephemeral_package_eval harness (Greg Sanders)
127719f516a6a8bbfb65f09827bbe22190df3a58 test: Add CheckMempoolEphemeralInvariants (Greg Sanders)
e2e30e89ba4b9bdbcabaf5b4346610922f0728bb functional test: Add ephemeral dust tests (Greg Sanders)
4e68f901390d512a9dfaf0de34daf822449e9bd2 rpc: disallow in-mempool prioritisation of dusty tx (Greg Sanders)
e1d3e81ab4d34485f1b82cb9c3b967e92a4e1f15 policy: Allow dust in transactions, spent in-mempool (Greg Sanders)
04b2714fbbc4a019d23743a488b9f9b42652617b functional test: Add new -dustrelayfee=0 test case (Greg Sanders)
Pull request description:
A replacement for https://github.com/bitcoin/bitcoin/pull/29001
Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type, and simple focusing on the feature of allowing temporary dust in the mempool.
Users of this can immediately use dust outputs as:
1. Single keyed anchor (can be shared by multiple parties)
2. Single unkeyed anchor, ala P2A
Which is useful when the parent transaction cannot have fees for technical or accounting reasons.
What I'm calling "keyed" anchors would be used anytime you don't want a third party to be able to run off with the utxo. As a motivating example, in Ark there is the concept of a "forfeit transaction" which spends a "connector output". The connector output would ideally be 0-value, but you would not want that utxo spend by anyone, because this would cause financial loss for the coordinator of the service: https://arkdev.info/docs/learn/concepts#forfeit-transaction
Note that this specific use-case likely doesn't work as it involves a tree of dust, but the connector idea in general demonstrates how it could be used.
Another related example is connector outputs in BitVM2: https://bitvm.org/bitvm2.html .
Note that non-TRUC usage will be impractical unless the minrelay requirement on individual transactions are dropped in general, which should happen post-cluster mempool.
Lightning Network intends to use this feature post-29.0 if available: https://github.com/lightning/bolts/issues/1171#issuecomment-2373748582
It's also useful for Ark, ln-symmetry, spacechains, Timeout Trees, and other constructs with large presigned trees or other large-N party smart contracts.
ACKs for top commit:
glozow:
reACK 5c2e291060c via range-diff. Nothing but a rebase and removing the conflict.
theStack:
re-ACK 5c2e291060cca3be500f3af0f6f2d3fd2177a7c9
Tree-SHA512: 88e6a6b3b91dc425de47ccd68b7668c8e98c5683712e892c588f79ad639ae95c665e2d5563dd5e5797983e7542cbd1d4353bc90a7298d45a1843b05a417f09f5
83fab3212c91d91fc5502f940c901a07772ff747 test: Add combinerawtransaction test to rpc_createmultisig (Ava Chow)
Pull request description:
The only coverage of combinerawtransaction is in a legacy wallet only test. So also use it in rpc_createmultisig so that this RPC remains tested after the legacy wallet is removed.
Split from #28710
ACKs for top commit:
maflcko:
re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
BrandonOdiwuor:
Re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
Abdulkbk:
ACK 83fab3212c91d91fc5502f940c901a07772ff747
brunoerg:
code review ACK 83fab3212c91d91fc5502f940c901a07772ff747
rkrux:
tACK 83fab3212c91d91fc5502f940c901a07772ff747
Tree-SHA512: 383d88ff6c9b54337ed81c714026e527b0fed41d976959fd5c6863b49d0defa4ea13fdc3d984885c86a2b6380825cd66c17842cc31f20fbec4bc42d86aecbbfa
- Some test methods in the functional test framework are independent
and do not require any previous context or setup defined in `run_test`.
- This commit adds a new option for running these specific methods within a test file,
allowing them to be executed individually without running the entire test suite.
- running test methods that require an argument or context will fail.
c189eec848e3c31f438151d4d3422718a29df3a3 doc: release note for mempoolrullrbf removal (Greg Sanders)
d47297c6aaba44672fdd19d817d9b11d2dc90bb7 rpc: Mark fullrbf and bip125-replaceable as deprecated (Greg Sanders)
04a5dcee8ab56f2089ab08192b97b67bc15bc3ba docs: remove requirement to signal bip125 (Greg Sanders)
111a23d9b3615094fbfdf6cc8c996adc3db2782c Remove -mempoolfullrbf option (Greg Sanders)
Pull request description:
Given https://github.com/bitcoin/bitcoin/pull/30493 and the related discussion on network uptake it's probably not helpful to have an option for a feature that will not be respected by the network in any meaningful way.
Wallet changes can be done in another PR on its own cadence to account for possible fingerprinting, waiting for fullrbf logic to permeate the network, etc.
ACKs for top commit:
stickies-v:
re-ACK c189eec848e3c31f438151d4d3422718a29df3a3
achow101:
ACK c189eec848e3c31f438151d4d3422718a29df3a3
murchandamus:
ACK c189eec848e3c31f438151d4d3422718a29df3a3
rkrux:
reACK c189eec848e3c31f438151d4d3422718a29df3a3
Tree-SHA512: 9447f88f8f291c56c5bde70af0a91b0a4f5163aaaf173370fbfdaa3c3fd0b44120b14d3a1977f7ee10e27ffe9453f8a70dd38aad0ffb8c39cf145049d2550730
The only coverage of combinerawtransaction is in a legacy wallet only
test. So also use it in rpc_createmultisig so that this RPC remains
tested after the legacy wallet is removed.
d7fd766feb2f579bdba0e778bacdeb13103e8282 test: added test to assert TX decode rpc error on submitpackage rpc (kevkevinpal)
Pull request description:
This PR adds coverage for this line https://github.com/bitcoin/bitcoin/blob/master/src/rpc/mempool.cpp#L996
If you run the following you will get no results for `submitpackage`
`grep -nri "TX decode failed" ./test/functional`
ACKs for top commit:
achow101:
ACK d7fd766feb2f579bdba0e778bacdeb13103e8282
instagibbs:
reACK d7fd766feb
tdb3:
ACK d7fd766feb2f579bdba0e778bacdeb13103e8282
rkrux:
reACK d7fd766feb2f579bdba0e778bacdeb13103e8282
Tree-SHA512: e92e0e2621a4efab35625d8da3ac61ccb7fa65c378aa977112bc132fd3b42431f8c3ceb081f7c9903ed2833c229042b65bdb11444e1d6367354ae65dc7504231
bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9 Fix unsigned integer overflows in interpreter (MarcoFalke)
Pull request description:
Unsigned integer overflow is well defined by the language and in some cases even useful or necessary. However, I think that it should be avoided in interpreter, as it makes the code harder to read and requires the whole file to be suppressed in the sanitizer. This puts more burden on reviewers to check that any changes to interpreter that involve unsigned integer overflow are sane.
This patch involves a few changes:
* Evaluate the addition in 64-bit "space". Previously, the first argument was `size_t` (unsigned, 32-bit or 64-bit, depending on platform) and the second was `int` (32-bit on all supported platforms). Thus the addition was done in 32-bit or 64-bit "unsigned space". Now the addition is done in 64-bit "signed space" on all platforms. This is safe because signed integer overflow (UB) isn't expected here with 64-bit integers.
* Clarify that the value passed to the "stack macros" always fits in an `int64_t`. This is done with the C++11 syntax `int64_t{i}`, which fails to compile if `i` needs to be narrowed to fit into `int64_t`.
* Explicitly convert the result of the addition to `size_t`. This isn't needed, because the called function already converts the value (see https://en.cppreference.com/w/cpp/container/vector/operator_at), however I have a slight preference for the explicit cast. (Happy to remove if reviewers prefer without)
The patch does not change the bitcoind binary on my 64-bit system with `clang++ -O2`. However, it does change with gcc.
ACKs for top commit:
achow101:
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9
ismaelsadeeq:
Code review ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9
hebasto:
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9, I have reviewed the code and it looks OK.
Tree-SHA512: 0e9cbc6a0afd3db0d1d9489fd5e32ff856217604abde370add1f01c2cae8c526f2afedeb372997217c3a70ab0f8f56442e8230f87456f8e21c9abcb7c6578f7c
e60cecc8115d3b28be076792baa5e4ea26d353a6 doc: add release note for 31156 (Martin Zumsande)
fc7dfb3df5b932cc015817c4461e7017601d607f test: Don't enforce BIP94 on regtest unless specified by arg (Martin Zumsande)
Pull request description:
The added arg `-test=bip94` is only used in a functional test for BIP94. This is done because the default regtest consensus rules should follow mainnet, not testnet.
Fixes#31137.
ACKs for top commit:
achow101:
ACK e60cecc8115d3b28be076792baa5e4ea26d353a6
tdb3:
cr and light test ACK e60cecc8115d3b28be076792baa5e4ea26d353a6
rkrux:
tACK e60cecc8115d3b28be076792baa5e4ea26d353a6
BrandonOdiwuor:
utACK e60cecc8115d3b28be076792baa5e4ea26d353a6
laanwj:
Code review ACK e60cecc8115d3b28be076792baa5e4ea26d353a6
Tree-SHA512: ca2f322f89d8808dfc3565fe020d2615cfcc110e188a02128ad7108fef51c735b33d55b5e6a70c505d78f7291f3c635dc7dfbcd78be1348d4d6e483883be4216
57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0 test: set P2PConnection.p2p_connected_to_node in peer_connect_helper() (Vasil Dimov)
22cd0e888c71b0f56171a524251c1557bcb6237b test: support WTX INVs from P2PDataStore and fix a comment (Vasil Dimov)
ebe42c00aa4a7a16900eff3aec45604c86b2dbf5 test: extend the SOCKS5 Python proxy to actually connect to a destination (Vasil Dimov)
ba621ffb9cb63a01053854bb270786c470c90392 test: improve debug log message from P2PConnection::connection_made() (Vasil Dimov)
Pull request description:
If requested, make the SOCKS5 Python proxy redirect connections to a set of given destinations. Actually act as a real proxy, connecting the client to a destination, except that the destination is not what the client asked for.
This would enable us to "connect" to Tor addresses from the functional tests.
Plus a few other minor improvements in the test framework as individual commits.
---
These changes are part of https://github.com/bitcoin/bitcoin/pull/29415 but they make sense on their own and would be good to have them, regardless of the fate of #29415. Also, if this is merged, that would reduce the size of #29415, thus the current standalone PR.
ACKs for top commit:
jonatack:
Approach ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
achow101:
ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
tdb3:
CR and test ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
mzumsande:
Code review / tested ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
Tree-SHA512: a2892c97bff2d337b37455c409c6136cb62423ce6cc32b197b36f220c1eec9ca046b599135b9a2603c0eb6c1ac4d9795e73831ef0f04378aeea8b245ea733399
0ea84bc362f395fd247623c22942eb5ca3d1b874 test: explicitly check boolean verbosity is disallowed (tdb3)
7a2e6b68cd928a32dd307273727a85890a74c7da doc: add rpc guidance for boolean verbosity avoidance (tdb3)
698f302df8b7cc6e4077c911d3c129960bdb5e07 rpc: disallow boolean verbosity in getorphantxs (tdb3)
63f5e6ec795f3d5ddfed03f3c51f79ad7a51db1e test: add entry and expiration time checks (tdb3)
808a708107e65e52f54373d2e26f807cf1e444e1 rpc: add entry time to getorphantxs (tdb3)
56bf3027144b4fa6ce9586d3d249b275acb7bcce refactor: rename rpc_getorphantxs to rpc_orphans (tdb3)
7824f6b07703463707bb4f10577ff6d34118e248 test: check that getorphantxs is hidden (tdb3)
ac68fcca701e0b3b90c6bb81d66bfa38b57f39bf rpc: disallow undefined verbosity in getorphantxs (tdb3)
Pull request description:
Implements follow-up suggestions from #30793.
- Now disallows undefined verbosity levels (below and above valid values) (https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786093549)
- Disallows boolean verbosity (https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1788273274) and adds guidance to developer-notes
- Checks that `getorphantxs` is a hidden rpc (https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786107786)
- Adds a test for `expiration` time
- Adds `entry` time to the returned orphan objects (verbosity >=1) to relieve the user from having to calculate it from `expiration`. Also adds associated test. (https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743687732)
- Minor cleanup (blank line removal and log message move) (https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786092641)
Included a commit to rename the test to a more generic `get_orphans` to better accommodate future orphanage-related RPCs (e.g. `getorphanangeinfo`). Can drop the refactor commit from this PR if people feel strongly about it.
ACKs for top commit:
achow101:
ACK 0ea84bc362f395fd247623c22942eb5ca3d1b874
glozow:
utACK 0ea84bc362f395fd247623c22942eb5ca3d1b874
rkrux:
tACK 0ea84bc362f395fd247623c22942eb5ca3d1b874
itornaza:
tACK 0ea84bc362f395fd247623c22942eb5ca3d1b874
Tree-SHA512: e48a088f333ebde132923072da58e970461e74362d0acebbc799c3043d5727cdf5f28e82b43cb38bbed27c603df6710695dba91ff0695e623ad168e985dce08e
c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6 test: Remove dead code from interface_zmq (Fabian Jahr)
Pull request description:
The loop removed here appears to be effectively dead code: In case `get_raw_seq` is behind `zmq_mem_seq` the loop runs and tries to get a more recent (higher) number for `get_raw_seq`. However, the exact number of `get_raw_seq` is asserted in the line above: `assert_equal(get_raw_seq, 6)`. If the loop would actually achieve its purpose this assert would need to be racy. This does not seem to be the case and 6 appears to be the final number. `zmq_mem_seq` however does take some time to catch up (if it were continue to be updated). But this is not handled by the loop and does not seem to be relevant at this point in the test. The backlog is consumed a bit later in another loop that handles this correctly already.
ACKs for top commit:
l0rinc:
ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
tdb3:
CR re ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
Tree-SHA512: 663a1711ba1ce04a3d2e2916e0df7a7bb51069e28bc2644b816a483628c95b5e6c29fc6eacc31a5f72b7d9af11096f3c437ea1dc57eaa1ee9ddce43cc20bacd3
6c9fe7b73ea1572b8b56c716ab13d9866f91c6e9 test: Prevent connection attempts to random IPs in p2p_seednodes.py (Martin Zumsande)
bb97b1ffa9f02bf9c05f653602cfb1cf48efb7fa test: fix intermittent timeout in p2p_seednodes.py (Martin Zumsande)
Pull request description:
Fixes#31103
On some CI runs, the seed node timer in `ThreadOpenConnection` was only started *after* the mocktime was set.
Fix this by waiting for the first connection attempt, which happens after the timer was started.
Also I noticed that the "unreachable" connections are not in fact unreachable, so that the functional test could attempt connections
to random IPs on the internet. This was already noted in https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1701616675 but the suggested fix never made it in, so I added it to this PR.
ACKs for top commit:
sr-gi:
tACK [6c9fe7b](6c9fe7b73e)
laanwj:
Code review ACK 6c9fe7b73ea1572b8b56c716ab13d9866f91c6e9
tdb3:
cr and light test ACK 6c9fe7b73ea1572b8b56c716ab13d9866f91c6e9
Tree-SHA512: 021b6d5325eab85d79708b4b137f61723a36f2b8a1faf681463bad2ea5283ea528b5ff1701467a86b035d3a6972750a61ace5020e58b7aa61ecaad97664488c8
The added regtest option -test=bip94 is only used in the functional
test for BIP94.
This is done because the default regtest consensus rules
should aim to follow to mainnet, not testnet.
The use of `PACKAGE_NAME` for the project's variable name is
problematic, as this name is commonly used in CMake's interface
variables. If third-party CMake code handles with scopes improperly,
our `PACKAGE_NAME` variable could end up with an unexpected value.
This change avoids such conflicts by renaming all `PACKAGE_*` variables
to `CLIENT_*`.
40e5f26a3ff77e50df808f6f850c617aec2df203 mapport: remove dead code in DispatchMapPort (Antoine Poinsot)
38fdf7c1fb1946820236c319ad44c7bcbf0c6a98 mapport: drop outdated comments (Antoine Poinsot)
b7b24352906f1dba64826e7a093069b5bfc504dc doc: add release note for #31130 (Antoine Poinsot)
1b6dec98da3025c19951daf209347cecf1f0c6ab depends: drop miniupnpc (Antoine Poinsot)
953533d0214819a05d36672d295821ef06ced8d6 doc: remove mentions of UPnP (Antoine Poinsot)
94ad614482f4f1f9d207509a209badbc2fb5700d ci: remove UPnP options (Antoine Poinsot)
a9598e5eaab861fd6e6ce279f1282a83eec407d6 build: drop miniupnpc dependency (Antoine Poinsot)
a5fcfb7385c10d83a294cb2bb2248d06b2ab931e interfaces: remove now unused 'use_upnp' arg from 'mapPort' (Antoine Poinsot)
038bbe7b20074cc2201585dcc631e81b9e1e306c daemon: remove UPnP support (Antoine Poinsot)
844770b05ebc34789dc46d70cd6398089539c915 qt: remove UPnP settings (Antoine Poinsot)
Pull request description:
This PR removes UPnP IGD support and drops our [miniupnp](https://github.com/miniupnp/miniupnp) dependency.
Miniupnpc is a C library (somewhat) maintained by a single person which had several vulnerabilities in the past (a couple dozens are listed [here](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=miniupnp)), some of which directly affected our software ([RCE in 2015](https://bitcoincore.org/en/2024/07/03/disclose_upnp_rce/), [OOM in 2020](https://bitcoincore.org/en/2024/07/31/disclose-upnp-oom/)).
The main purpose of this functionality is to have more (non-data-center) reachable nodes on the network. For a non-technical user running Bitcoin Core at home, the software would automatically open a port on their router to receive incoming connections. This way, users not able to manually open a port on their router would still provide the network with more resources and enhance its diversity.
However, due to past vulnerabilities (and a worry about unknown future ones) in miniupnpc this feature was disabled by default in https://github.com/bitcoin/bitcoin/pull/6795. Having it disabled by default kills (most of?) the purpose of having this functionality in the first place: someone technical enough to understand the `-upnp` startup option or the "enable UPnP" setting is most likely able to open a port on his box in the first place.
In addition, laanwj implemented PCP with a NAT-PMP fallback directly in Bitcoin Core in https://github.com/bitcoin/bitcoin/pull/30043. If we ever want to re-enable automatic NAT traversal by default in Bitcoin Core, this is the best option (and in my opinion the only sane one). The NAT-PMP fallback makes it so compatibility shouldn't be (much of) an issue.
On balance, i believe that keeping this functionality and this barely maintained C dependency has higher costs than benefits. Therefore i propose that we get rid of it.
ACKs for top commit:
jarolrod:
ACK 40e5f26a3f
1440000bytes:
Code Review ACK 40e5f26a3f
laanwj:
Code review ACK 40e5f26a3ff77e50df808f6f850c617aec2df203
i-am-yuvi:
Tested ACK 40e5f26a3ff77e50df808f6f850c617aec2df203
Tree-SHA512: 9ea48662775510f5ec6de7af65790f7c8d211603398e9d8c634a86387be81b28081419a95b4d6680d3d7fe6a9f16cec99f16516548201dc7e49781909899a657
f32c34d0c3d4041a301822b27e88d6db4cbf631e functional test: Additional package evaluation coverage (Greg Sanders)
Pull request description:
Current test coverage doesn't ensure that mempool trimming doesn't appear prior to the entire package, and not just the subpackage, is finished being submitted.
Add a scenario that covers this case, where package ancestors can make it in individually, but would be immadiately evicted if not for the package CPFP.
in response to https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1813272637 where if applied onto that PR's old commit, the test fails due to package failure.
ACKs for top commit:
sdaftuar:
re-ACK f32c34d0c3d4041a301822b27e88d6db4cbf631e
rkrux:
tACK f32c34d0c3d4041a301822b27e88d6db4cbf631e
glozow:
reACK f32c34d0c3d4041a301822b27e88d6db4cbf631e
Tree-SHA512: 739fcc5e66878b3def9b25dc588d8cb5349aaaa0901b11475879a413a03f6ea0e87d19de5bc4fb44ddd0436fdc052cdc3ed564f7e2ad510269aab9732d5c24eb
5c299ecafe6f336cffa145d28036b04b87e26712 test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped (kevkevinpal)
Pull request description:
After joining the bitcoin pr review club about https://github.com/bitcoin/bitcoin/pull/30793
I learned about [`CVE-2012-3789`](https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L4693)
So I was motivated to write a functional test that covers this part of the code,
This test should add the max number of orphans to a nodes orphanage and then attempt to add another, then asserts that the number of orphans is still at the max amount
ACKs for top commit:
achow101:
ACK 5c299ecafe6f336cffa145d28036b04b87e26712
rkrux:
ACK 5c299ecafe6f336cffa145d28036b04b87e26712
instagibbs:
ACK 5c299ecafe6f336cffa145d28036b04b87e26712
tdb3:
ACK 5c299ecafe6f336cffa145d28036b04b87e26712
Tree-SHA512: 687bba337978e0945e94af71632998221e5565a5d83cf5a59ecf2ee52c7262d8ff907b94dceea3b80bed441dd19b24790b2904e88e1da14d30827c5469fcb4d3
Current test coverage doesn't ensure that mempool trimming
doesn't appear prior to the entire package, and not just
the subpackage, is finished being submitted.
Add a scenario that covers this case, where package
ancestors can make it in individually, but would be
immadiately evicted if not for the package CPFP.
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
Keep the "-upnp" option as a hidden arg for one major version in order
to show a more user friendly error to people who had this option set in
their config file.
This aims to complete our test framework BDB parser to reflect
our read-only BDB parser in the wallet codebase. This could be
useful both for making review of #26606 easier and to also possibly
improve our functional tests for the BDB parser by comparing with
an alternative implementation.
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
These addrs aren't unreachable as the test claims.
Specify a (non-working) proxy to make sure the connections fails
even if the addr was reachable.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
