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
d6359937bf validation: check invariants when inserting into m_blocks_unlinked (stratospher)
0852925bd8 test/doc: remove misleading comment and improve tests (stratospher)
ca4a380281 test: add coverage for UB caused by FindMostWorkChain (stratospher)
c787b3b99b validation: avoid duplicates in m_blocks_unlinked (stratospher)
Pull request description:
This is joint work with @ mzumsande.
note: this requires a pruned node with deep reorgs to trigger. still it breaks assumptions in the codebase and is good to fix. A similar UB was fixed in https://github.com/bitcoin/bitcoin/pull/34521.
This PR prevents duplicate insertions into `m_blocks_unlinked` in `FindMostWorkChain`. There are 3 ways to insert into `m_blocks_unlinked`:
1. `LoadBlockIndex` - not problematic, as each block index is processed only once.
2. `ReceivedBlockTransactions` - not problematic, as this is usually only called once per block when it is first accepted in `AcceptBlock`. in the rare case it’s triggered again after pruning, the block would have been removed from `m_blocks_unlinked` when it was initially pruned, so duplicates still can’t arise.
3. `FindMostWorkChain` - problematic when multiple candidate tips share common chains of ancestors, traversals from each tip to the fork point may insert duplicate (`pprev`, `pindex`) entries for blocks whose parents have been pruned.
When the missing parent is later received and `ReceivedBlockTransactions` processes `m_blocks_unlinked`, the same entry may be processed multiple times. This can result in the block being re-added to `setBlockIndexCandidates` with a modified `nSequenceId`, violating its ordering invariants and leading to undefined behavior. So avoid duplicate insertions into `m_blocks_unlinked` in `FindMostWorkChain`.
### how to test:
use the updated `feature_pruning.py` which adds coverage for this scenario.
- on master: the test (with the below diff) fails since `nSequenceId` is being modified for an entry in `setBlockIndexCandidates`
- on this branch: the test (with the below diff) passes
```diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3814,6 +3814,12 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd
pindex->nHeight, pindex->m_chain_tx_count, prev_tx_sum(*pindex), CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT);
}
pindex->m_chain_tx_count = prev_tx_sum(*pindex);
+ for (const auto& c : m_chainstates) {
+ if (c->setBlockIndexCandidates.contains(pindex)) {
+ LogInfo("### pindex UB = %s", pindex);
+ assert(false);
+ }
+ }
pindex->nSequenceId = nBlockSequenceId++;
for (const auto& c : m_chainstates) {
c->TryAddBlockIndexCandidate(pindex);
```
ACKs for top commit:
sedited:
Re-ACK d6359937bf
marcofleon:
crACK d6359937bf
stringintech:
ACK d6359937bf
mzumsande:
Sure - Code Review ACK [d635993](d6359937bf)
Tree-SHA512: bb21adc2d92fe1865bbbcebf775a850ca3eccac6fe83d7bca10b78eee4c0abf782e44fa0ddfec9d9a70f42fa40bdc49b68a1d1b4905cdc371ba29117d3120619
b552f1713a ci: use warp docker buildkit cache (will)
Pull request description:
This was inadvertently broken in #35441 when we dropped the runners job. Unfortunately GHA seems to continue just fine when this field is empty, so it wasn't noticed at runtime.
ACKs for top commit:
m3dwards:
ACK b552f1713a
Tree-SHA512: 0bc61b42e94b0a73ef6026dea3dce27fdea611218936e585b5232fde358b4fab4728d59338b86c75affec80c11b6f1fada56ffb785999fabae0b01becb41268d
48df0939e7 fuzz: Remove unnecessary thread pool mutexes (marcofleon)
a4c3b003f8 fuzz: Speed up dbwrapper_concurrent_reads harness (marcofleon)
Pull request description:
Limiting how many read queries each worker executes significantly speeds up this test, especially when running with sanitizers. This still builds the full query list, and then takes the first 128/2000 after each worker shuffles it. I think that keeps some more operation diversity vs just lowering the query max directly. It also allowed me to reuse and test against a corpus I already had. Let me know if I'm wrong, but I don't think this test needs every worker to execute an identical query list to be effective.
This PR also reverts the `num_entries` max from 3000 back to 5000, as that didn't have much effect on input speed and restores a bit of lost coverage.
Lastly, as https://github.com/bitcoin/bitcoin/pull/35455#discussion_r3394357385 points out, remove the unnecessary `Mutex` from `StartReadPoolIfNeeded()`. Fuzz targets are entered sequentially within a process and parallel fuzzing uses separate processes/forks, so a mutex to prevent two in-process callers from racing to start the pool isn't needed.
ACKs for top commit:
sedited:
ACK 48df0939e7
brunoerg:
ACK 48df0939e7
Tree-SHA512: 24c35e13fa26790b36e5190283b78dd8511165d576841cb701f541a36c5b7c0a73f9bc265e71598a879fa1df9a9438e7841736f72b967214196fa6fe68569eff
61020b36c5 doc: add release note for #35182 replace libevent HTTP server (Matthew Zipkin)
39e9099da5 logging: deprecate libevent category (Matthew Zipkin)
8c1eea0777 http: remove libevent usage from this subsystem (Matthew Zipkin)
e427c227fa fuzz: switch http_libevent::HTTPRequest to http_bitcoin::HTTPRequest (Matthew Zipkin)
21c7542cf8 http: switch servers from libevent to bitcoin (Matthew Zipkin)
cbb8d1fb33 HTTPServer: disconnect after idle timeout (-rpcservertimeout) (Matthew Zipkin)
e5f242eef3 HTTPServer: implement control methods to match legacy API (Matthew Zipkin)
2ca645c2e4 refactor: split HTTPBindAddresses into config parse and libevent setup (Matthew Zipkin)
fec6b6bca8 refactor: split http_request_cb into libevent callback and dispatch (Matthew Zipkin)
f946ff5a0b Add helper methods to HTTPRequest to match original API (Matthew Zipkin)
dd11b5e01b define HTTP request methods at module level outside of class (Matthew Zipkin)
7ee7df988e HTTPServer: use a queue to pipeline requests from each connected client (Matthew Zipkin)
5ef1b80a09 Allow http workers to send data optimistically as an optimization (Matthew Zipkin)
a69bb9e1e6 HTTPServer: disconnect clients (Matthew Zipkin)
cdf71998e5 HTTPServer: compose and send replies to connected clients (Matthew Zipkin)
6734bcdeff HTTPserver: support "chunked" Transfer-Encoding (Matthew Zipkin)
80e1cfe5a2 HTTPServer: read requests from connected clients (Matthew Zipkin)
3c5226ab96 HTTPServer: start an I/O loop in a new thread and accept connections (Matthew Zipkin)
4ef4ebdc0c http: Introduce HTTPRemoteClient class (Matthew Zipkin)
a85286c5c7 HTTPServer: generate sequential Ids for each newly accepted connection (Matthew Zipkin)
5a3aa1af28 HTTPServer: implement and test AcceptConnection() (Matthew Zipkin)
f5bc018948 http: Introduce HTTPServer class and implement binding to listening socket (Matthew Zipkin)
9463e98781 http: Implement HTTPRequest class (Matthew Zipkin)
ad50aa4a0f http: Implement HTTPResponse class (Matthew Zipkin)
68b5d289d1 http: Implement HTTPHeaders class (Matthew Zipkin)
89c54ae4cb http: enclose libevent-dependent code in a namespace (Matthew Zipkin)
5aa3629b48 util/string: LineReader should only trim \r or \r\n (Matthew Zipkin)
0cdbb191b5 util/string: use string_view in LineReader (Matthew Zipkin)
881d4b6c75 test: cover common HTTP attacks and common malformed requests (Matthew Zipkin)
Pull request description:
Continued from #32061.
This is a major component of [removing libevent as a dependency of the project](https://github.com/bitcoin/bitcoin/issues/31194), by replacing the HTTP server used for RPC and REST with one implemented entirely within the Bitcoin Core codebase. The new `HTTPServer` class runs its own I/O thread, handling socket connections with code based on #30988, but tailored specifically for HTTP.
Commit strategy:
- Isolate the existing libevent-based HTTP server in a namespace `http_libevent`
- Implement HTTP in a new namespace `http_bitcoin` (classes like `HTTPRequest`, `HTTPClient`, etc...)
- Switch bitcoind from the libevent server to the new server
- Clean up (delete `http_libevent`)
## RFC compliance
I ran https://www.http-probe.com/ against v31.0 release and this PR branch at commit ddefe4263b42. While both HTTP servers failed several of the very picky compliance tests, the only differences in test results between the two branches is an *improvement* in the test results of the PR branch.
Raw test results:
- [v31.0](https://gist.github.com/pinheadmz/bb0debd49e8556417e9f0072a4463b31)
- [PR Branch](https://gist.github.com/pinheadmz/7267cdd90ff9deee8034788b5d1148de)
Test result comparison:
- https://gist.github.com/pinheadmz/08a5814345e02bf1ac1304e8e5b163a1
- Summary: `Changed results: 33 (Fail→Pass: 32, Pass→Fail: 0, other: 1)`
## Fuzz testing
### libfuzzer
- 4 days x8 cores at ddefe4263b
- generated corpus: https://github.com/pinheadmz/qa-assets/tree/httprequest-pr35182-ddefe4263b
- coverage report: https://thebitcoinblockclock.com/demo/libfuzzer_pr35182_ddefe4263b/index.html
### fuzzamoto
- 4 days x8 cores at commit ddefe4263b with a [patched fuzzer](https://github.com/pinheadmz/fuzzamoto/tree/http)
- coverage report: https://thebitcoinblockclock.com/demo/fuzzamoto_pr35182_ddefe4263b/coverage/bitcoin/index.html
## Integration testing:
I am testing the new HTTP server by forking projects that integrate with bitcoin via HTTP and running their integration tests with bitcoind built from this branch (on Github actions). I will continue adding integrations over time, and re-running these CI tests as this branch gets rebased:
- ✅ [lnd](https://github.com/pinheadmz/lnd/pull/7) master at `f21262dd6` tested with this branch at `ddefe4263b`
- ✅ [NodeJS `rpc-bitcoin`](https://github.com/pinheadmz/rpc-bitcoin/pull/4) master at `eb0c5da7d1094629cb4a381733cb01c9bb9a4608` tested with this branch at `ddefe4263b`
- ✅ [electrs](https://github.com/pinheadmz/electrs/pull/5) master at `d81dc35ae8f71c22b96cf23ee279b69683f29919` tested with this branch at `ddefe4263b`
- ✅ [eclair](https://github.com/pinheadmz/eclair/pull/4) master at `2dda79468a8b69a2acf7962cdac63245f7cc3ee8` tested with this branch at `ddefe4263b`
- ✅ [corepc](https://github.com/pinheadmz/corepc/pull/4) master at `818e5f2929ea5113176150b2d9f7c0248992d99c` tested with this branch at `ddefe4263b`
- ✅ [bitcoinjs-lib](https://github.com/pinheadmz/bitcoinjs-lib/pull/4) master at `ab9fad5978bc1f4fb6542d1cde903d4427c3344e` tested with this branch at `ddefe4263b`
- ✅ [NodeJS `bitcoin-core`](https://github.com/pinheadmz/bitcoin-core/pull/2) master at `24f3d0be7ab73089542de42cc8e31903727dbdf8` tested with this branch at `ddefe4263b`
## Benchmarks
To get a general idea of server speed vs. master I ran the functional test suite on all three platforms, three trials each, from the first and last commit of this branch. The first commit represents master, but adds a few extra functional tests, so the overall suite is the same in the last commit.
[Full results](https://docs.google.com/spreadsheets/d/1X8JI3k_mjKm9qUYNCV14TezTUBGOFTqHW-mucZOYRn4/edit?usp=sharing)
Summary:
Tested at 37ff331f60
There appears to be a 1-2% slow down on Ubuntu and MacOS but a 5% speed up on Windows
|| Ubuntu - master | Ubuntu - branch | MacOS - master | Macos - branch | Windows - master | Windows - branch |
|-|-|-|-|-|-|-|
|AVERAGE| 253| 260| 187| 190| 521| 493|
|% diff to master| | 2.766798419| | 1.604278075| | -5.37428023 |
ACKs for top commit:
vasild:
ACK 61020b36c5
theStack:
re-ACK 61020b36c5
Tree-SHA512: d39f782b9e6c7d67fdba5638fe9a1018b83062668c8b2ab0aa3fe363801f3c36a0ae97b8da0ea0395195e03a2c248a5d1511e5fa27c438c9bd7fca5c83d1ae4c
Remove the `Mutex` from the `threadpool` and `dbwrapper_concurrent_reads`
pool startup helpers. Fuzz targets are entered sequentially within a
process and parallel fuzzing uses separate processes/forks, which each
have their own copy of the global thread pool. Therefore, a mutex to
prevent two in-process callers from racing to start the pool isn't needed.
Limit how many read queries each worker executes. This significantly
speeds up the test, as each worker runs >90% fewer (2000 to 128)
expensive LevelDB operations (like `IteratorSeek`) but still
ends up hitting the intended target code.
Revert the `num_entries` max from 3000 back to 5000, as that didn't
have much effect on input speed and restores a bit of lost coverage.
Creates logic to deprecate logging categories but still
"support" them so the software doesn't quit with unknown category
on startup. Deprecated categories are always false and attempts
to switch them are logged as warnings.
The original function was already naturally split into two chunks:
First, we parse and validate the users' RPC configuration for IPs and
ports. Next we bind libevent's http server to the appropriate
endpoints.
This commit splits these chunks into two separate functions, leaving
the argument parsing in the common space of the module and moving the
libevent-specific binding into the http_libevent namespace.
A future commit will implement http_bitcoin::HTTPBindAddresses to
bind the validate list of endpoints by the new HTTP server.
The original function is passed to libevent as a callback when HTTP
requests are received and processed. It wrapped the libevent request
object in a http_libevent::HTTPRequest and then handed that off to
bitcoin for basic checks and finally dispatch to worker threads.
In this commit we split the function after the
http_libevent::HTTPRequest is created, and pass that object to a new
function that maintains the logic of checking and dispatching.
This will be the merge point for http_libevent and http_bitcoin,
where HTTPRequest objects from either namespace have the same
downstream lifecycle.
This is a refactor to prepare for matching the API of HTTPRequest
definitions in both namespaces http_bitcoin and http_libevent. In
particular, to provide a consistent return type for GetRequestMethod()
in both classes.
See https://www.rfc-editor.org/rfc/rfc7230#section-6.3.2
> A server MAY process a sequence of pipelined requests in
parallel if they all have safe methods (Section 4.2.1 of [RFC7231]),
but it MUST send the corresponding responses in the same order that
the requests were received.
We choose NOT to process requests in parallel. They are executed in
the order recevied as well as responded to in the order received.
This prevents race conditions where old state may get sent in response
to requests that are very quick to process but were requested later on
in the queue.
Sockets-touching bits copied and adapted from `CConnman::SocketSendData()`
Testing this requires adding a new feature to the SocketTestingSetup,
returning the DynSock I/O pipes from the mock socket so the received
data can be checked.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
`SocketHandlerConnected()` adapted from CConnman
Testing this requires adding a new feature to the SocketTestingSetup,
inserting a "request" payload into the mock client that connects
to us.
This commit also moves IOErrorIsPermanent() from sock.cpp to sock.h
so it can be called from the socket handler in httpserver.cpp
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Socket handling methods are copied from CConnMan:
`CConnman::GenerateWaitSockets()`
`CConnman::SocketHandlerListening()`
`CConnman::ThreadSocketHandler()` and `CConnman::SocketHandler()` are combined into ThreadSocketHandler()`.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Introduce a new low-level socket managing class `HTTPServer`.
BindAndStartListening() was copied from CConnMan's BindListenPort()
in net.cpp and modernized.
Unit-test it with a new class `SocketTestingSetup` which mocks
`CreateSock()` and will enable mock client I/O in future commits.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
This commit is a no-op to isolate HTTP methods and objects that
depend on libevent. Following commits will add replacement objects
and methods in a new namespace for testing and review before
switching over the server.
The utility can not be opinionated about CR or SP on either end of a
line it reads. That decision is up to the caller and in fact in the
case of HTTP should be allowed in some places and rejected in others.
Replace TrimStringView() with more surgical operation.
394e473d42 coins: compact chainstate in background (Lőrinc)
aa021b26f3 validation: randomly compact chainstate (Lőrinc)
b10889d107 coins: test chainstate flush baseline (Lőrinc)
Pull request description:
**Problem:** https://github.com/bitcoin-core/leveldb-subtree/pull/61 disabled read-triggered seek compactions to avoid large chainstate write amplification from random UTXO lookups.
That avoids repeated read-driven rewrites, but it also removes opportunistic cleanup that previously helped compact old chainstate data.
After IBD, normal chainstate churn can leave obsolete entries behind until ordinary LevelDB compaction naturally reaches the affected levels, keeping the chainstate database larger than necessary.
Also, chainstates created by pre-29 nodes can contain thousands of files from the old 2 MiB LevelDB table target.
After the mmap limit dropped back to 1000 and seek compaction was disabled, continuing from such a chainstate can leave many table reads on the non-mmap path until the database is compacted.
**Fix:** After each completed post-IBD full chainstate flush, give the chainstate a 1/320 chance to compact.
With roughly hourly full flushes, this averages about once every two weeks and makes a six-month stretch without compaction about a one-in-a-million event.
The randomized recurring trigger spreads compactions across nodes and keeps maintenance stateless, without storing last-compaction height or timestamp metadata in the chainstate database.
Compaction runs on a background thread (`utxocompact`) so validation only schedules the work.
Partially fixes#35298 and #35457
ACKs for top commit:
sipa:
ACK 394e473d42
optout21:
ACK 394e473d42
andrewtoth:
ACK 394e473d42
sedited:
ACK 394e473d42
Tree-SHA512: 536a298f86ca7d87a2dd5428354b05b693931c090c7a4afd4d09b95ce53e2bd8319e01c940489a1520d8cb1010a7ea9724d2c18d2e12887bd5ec70cb68c03517
855a3fee88 scripted-diff: Rename SteadyClockContext to FakeSteadyClock (Hao Xu)
Pull request description:
See: https://github.com/bitcoin/bitcoin/pull/35536#issuecomment-4715945494
`SteadyClockContext` and `FakeNodeClock` are both LimitOne RAII helpers that mock a clock in tests -- the steady clock and the node clock, respectively. Rename the former so the two follow a consistent FakeXClock naming scheme.
ACKs for top commit:
maflcko:
lgtm ACK 855a3fee88
w0xlt:
ACK 855a3fee88
sedited:
ACK 855a3fee88
Tree-SHA512: 72fd8dd3217d19f4b9f587e379a0aea2b7760ca6d61be6c363e7c06757cba40487df8b40cf3d62f9f9256fb4d0373610dd6f62c6e9ef3edde02265fc967a6bdf
fab2874269 lint: Require scripted-diff script to succeed (MarcoFalke)
Pull request description:
Currently, scripted diffs may silently pass with errors.
Fix this issue by calling the script from a Bash instance with error checking enabled: `bash -o errexit -o nounset -o pipefail -c "$SCRIPT"`.
Also, use Bash (not sh) when launching the script itself, because Bash is required anyway.
Can be tested by running something like this and observing the behavior before and after:
```
git commit --allow-empty -m $'scripted-diff: foo\n\n-BEGIN VERIFY SCRIPT-\n false;falseasfsafsaf;true;false|cat; echo "${NO_UN_SET}"|cat \n-END VERIFY SCRIPT-\n' && ./test/lint/commit-script-check.sh HEAD~..HEAD ; echo $?
```
Alternatively, an ancient brittle script can be tested:
```
./test/lint/commit-script-check.sh fb65dde147f63422c4148b089c2f5be0bf5ba80f~..fb65dde147f63422c4148b089c2f5be0bf5ba80f
ACKs for top commit:
hodlinator:
ACK fab2874269
sedited:
ACK fab2874269
Tree-SHA512: e3e8167e150be45a096d4883057640eb5624456f21b134cfa901fe490e5afb192855e55a752cb6121399314f65db155544a60645a101c39a084a62de4af23298
8f0354995b depends: latest config.guess & config.sub (fanquake)
Pull request description:
Pull the latest versions from upstream.
ACKs for top commit:
hebasto:
ACK 8f0354995b, verified using the upstream [repo](https://cgit.git.savannah.gnu.org/cgit/config.git).
Tree-SHA512: 6ea7ce70a7db614d32f0aea47006892766f6abb41e31fef25cadd4423a0f6e42f8500f03ea70a3990c2628ad1b534b7c52c19914289375f87289ca34349075b2
735b25519a support: clamp RLIMIT_MEMLOCK to size_t (Sjors Provoost)
8ab4b9fc85 init: clamp fd limits to int (Sjors Provoost)
4afbabdcef Fix startup failure with RLIM_INFINITY fd limits (Sjors Provoost)
Pull request description:
When setting the fd limit to unlimited, the node fails to start:
```sh
ulimit -n unlimited
build/bin/bitcoind
Error: Not enough file descriptors available. -1 available, 160 required.
```
This was caused by `RaiseFileDescriptorLimit()` (introduced in #2568) casting `limitFD.rlim_cur` to `int`, which for `RLIM_INFINITY` overflows to `-1`. Fix it by returning `std::numeric_limits<int>::max()` instead.
Some platforms implement `RLIM_INFINITY` as the maximum uint64, others as int64 (-1). So simply changing the return type to `uint64_t` wouldn't work.
Similarly, though unlikely to actually happen:
```sh
ulimit -n 214748364
build/bin/bitcoind
Error: Not enough file descriptors available. -2147483648 available, 160 required.
```
The second commit expands the fix by clamping all values above `std::numeric_limits<int>::max()` instead of letting them overflow.
This PR also expands `test/functional/feature_init.py` to cover these, using `resource.setrlimit`. The check is skipped on environments with a hard limit below infinity (or that don't have the Python [Resource module](https://docs.python.org/3/library/resource.html)).
macOS by default has a hard limit of infinity, but on e.g. Ubuntu the default hard limit is 524288.
The third commit applies a similar fix to `PosixLockedPageAllocator::GetLimit()` for 32-bit systems, but without a test.
ACKs for top commit:
winterrdog:
Re-ACK 735b25519a
achow101:
ACK 735b25519a
sedited:
Re-ACK 735b25519a
pinheadmz:
ACK 735b25519a
Tree-SHA512: 0ce0292ecd61456bdec6943b06cbb9ecfc5180ee6dce850f8496ef54af22c1fae6ea473085202f5ba6f72e4dc51a29247620c9a0eae31e96658adc77b293129f
2447385f47 rpc: remove unused RPCResult::Type::ELISION (satsfy (Renato Britto))
7a85118005 rpc: expand decodepsbt output script with explicit fields (satsfy (Renato Britto))
88e2a6ae89 rpc: expand getaddressinfo embedded with explicit fields (Renato Britto)
a9f9e7d17e rpc: extract fee estimate result helpers (Renato Britto)
8a615a8800 rpc: extract ListSinceBlockTxFields() helper (Renato Britto)
372ac283ac rpc: extend TxDoc() for getblock verbosity 2/3 (Renato Britto)
0380a1c46b rpc: extend TxDoc() for getrawtransaction verbosity 2 (Renato Britto)
44fc3a290d rpc: introduce HelpElision variant and ElideGroup helper (Renato Britto)
Pull request description:
Partially addresses #29912. Motivated by #34683, which exports OpenRPC from existing `RPCHelpMan` metadata. [Sample OpenRPC](https://gist.github.com/natobritto/8c4a1da04968d2325082ac4bca7d2408).
Some RPC help definitions rely on `RPCResult::Type::ELISION` entries whose structure is only described in prose. This keeps human-readable help concise, but leaves parts of the result layout implicit and prevents tools from deriving complete machine-readable schemas from `RPCHelpMan` metadata.
This PR replaces ELISION-based reuse with shared structured definitions, so result layouts are represented directly in metadata rather than only in text. At the same time, human-readable help remains compact via explicit help-rendering elision using `HelpElision`, so previously elided sections stay abbreviated without losing schema completeness.
Affected RPCs: `getrawtransaction`, `getblock`, `listsinceblock`, `estimaterawfee`, `getaddressinfo`.
RPC return values are unchanged. Human-readable help remains compact, while structured result metadata becomes explicit enough to derive complete machine-readable schemas.
A related `RPCResult::Type::ELISION` use in `importdescriptors` was split out into the follow-up PR #34867 because it changes the generated help output, per [this review comment](https://github.com/bitcoin/bitcoin/pull/34764#discussion_r2960236111).
Changes:
- Introduce `HelpElision` (`NONE`, `START`, `SKIP`) and `ElideGroup()`, replacing the tri-state `print_elision`
- Add an `RPCResult` copy-with-replacement-options constructor to support applying elision while keeping `m_opts` const
- Extend `TxDoc()` / `TxDocOptions` to support reusable transaction layouts with optional `prevout`, `fee`, `hex`, and elision behavior
- Replace ELISION-based reuse in `getrawtransaction` and `getblock` with explicit structured definitions
- Factor shared result layouts into `GetBlockFields()`, `ListSinceBlockTxFields()`, `FeeRateBucketDoc()`, `GetAddressInfoEmbeddedFields()` and `FeeEstimateHorizonDoc()`
- Expand `listsinceblock.removed`, `estimaterawfee` horizons/buckets and `getaddressinfo.embedded` into explicit metadata while preserving concise help output
ACKs for top commit:
achow101:
ACK 2447385f47
w0xlt:
reACK 2447385f47
janb84:
re ACK 2447385f47
willcl-ark:
ACK 2447385f47
Tree-SHA512: 8dc03c45c388ebdb4f8f1613af2576fc127a8d5425efe518cd0d0ed2439a38b2ed1236413471672c85f52ee22c4a17677c18fd4689bc6570496fc3af4cd4112f
