No other *_destroy function in the Kernel API carries this annotation.
Following the convention set by free(), destroy functions should accept
null pointers.
Github-Pull: #34982
Rebased-From: 75608547b4
Waste is a CAmount, which is an int64_t. This will overflow an int, so
`best_waste` should also be a `CAmount`.
Github-Pull: #34939
Rebased-From: 890a09b1e4
These are unused and removing them avoids clang warnings like:
src/test/fuzz/deserialize.cpp:42:26: error: variable g_setup set but not used [-Werror,-Wunused-but-set-variable]
Github-Pull: #34918
Rebased-From: fabbfec3b0
70f632bda8f Merge bitcoin-core/libmultiprocess#265: ci: set LC_ALL in shell scripts
8e8e564259a Merge bitcoin-core/libmultiprocess#249: fixes for race conditions on disconnects
05d34cc2ec3 ci: set LC_ALL in shell scripts
e606fd84a8c Merge bitcoin-core/libmultiprocess#264: ci: reduce nproc multipliers
ff0eed1bf18 refactor: Use loop variable in type-context.h
ff1d8ba172a refactor: Move type-context.h getParams() call closer to use
1dbc59a4aa3 race fix: m_on_cancel called after request finishes
1643d05ba07 test: m_on_cancel called after request finishes
f5509a31fcc race fix: getParams() called after request cancel
4a60c39f24a test: getParams() called after request cancel
f11ec29ed20 race fix: worker thread destroyed before it is initialized
a1d643348f4 test: worker thread destroyed before it is initialized
336023382c4 ci: reduce nproc multipliers
b090beb9651 Merge bitcoin-core/libmultiprocess#256: ci: cache gnu32 nix store
be8622816da ci: cache gnu32 nix store
975270b619c Merge bitcoin-core/libmultiprocess#263: ci: bump timeout factor to 40
09f10e5a598 ci: bump timeout factor to 40
db8f76ad290 Merge bitcoin-core/libmultiprocess#253: ci: run some Bitcoin Core CI jobs
55a9b557b19 ci: set Bitcoin Core CI test repetition
fb0fc84d556 ci: add TSan job with instrumented libc++
0f29c38725b ci: add Bitcoin Core IPC tests (ASan + macOS)
3f64320315d Merge bitcoin-core/libmultiprocess#262: ci: enable clang-tidy in macOS job, use nullptr
cd9f8bdc9f0 Merge bitcoin-core/libmultiprocess#258: log: add socket connected info message and demote destroy logs to debug
b5d6258a42f Merge bitcoin-core/libmultiprocess#255: fix: use unsigned char cast and sizeof in LogEscape escape sequence
d94688e2c32 Merge bitcoin-core/libmultiprocess#251: Improved CustomBuildField for std::optional in IPC/libmultiprocess
a9499fad755 mp: use nullptr with pthread_threadid_np
f499e37850f ci: enable clang-tidy in macOS job
98f1352159d log: add socket connected info message and demote destroy logs to debug
554a481ea73 fix: use unsigned char cast and sizeof in LogEscape escape sequence
1977b9f3f65 Use std::forward in CustomBuildField for std::optional to allow move semantics, resolves FIXME
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 70f632bda8f80449b6240f98da768206a535a04e
When a large cluster of unconfirmed transactions exceeds the limit,
calculateCombinedBumpFee() returns std::nullopt.
Previously, we continued executing and the optional value was
accessed unconditionally, leading to a std::bad_optional_access
exception.
Fix this by returning early when the returned bumped fee is null.
Note:
This is a crash for the GUI, and an uncaught exception for the RPC
bumpfee and psbtbumpfee.
Github-Pull: #34870
Rebased-From: 6072a2a6a1
The comparison is evaluated before the assignment, so total_amount
ends up holding a boolean instead of the actual amount:
total_amount = (a - b < c)
which is not what we want here. This has been working by accident.
Github-Pull: #34888
Rebased-From: 0026b330c4
Making sure that if the privacy mode is activaded during
the wallet selection, the transaction view is not shown.
Github-Pull: #gui815
Rebased-From: 0dc337f73d
Acquire the cursor and block index under the same cs_main lock to
eliminate a potential race where a new block could be connected
between capturing the block info and acquiring the cursor, causing
the reported stats to reference a different block than the one
being iterated.
Github-Pull: #34451
Rebased-From: f3bf63ec4f
Fix an assertion failure in gettxoutsetinfo (issue #34263) caused by
capturing the best block before releasing cs_main, then checking it
against a potentially newer best block in GetUTXOStats().
Remove the early pindex capture since ComputeUTXOStats() independently
fetches the current best block under lock. Use stats.hashBlock and
stats.nHeight (the actual computed values) instead of the potentially
stale pindex when building the response.
Github-Pull: #34451
Rebased-From: 5e77072fa6
CMake version 4.3 deprecated the imported target `Sqlite::Sqlite3`.
Use the preferred name `Sqlite3::Sqlite3` instead and provide an
alias for older versions of CMake. Also define the same alias when
using vcpkg.
Github-Pull: #34848
Rebased-From: 498b6eb6b5
d67c8ed788 clusterlin: update SFL comments for deterministic order (Pieter Wuille)
Pull request description:
This was missed in #34257.
ACKs for top commit:
marcofleon:
ACK d67c8ed788
achow101:
ACK d67c8ed788
Tree-SHA512: e381da09eb686e69c0fb32cc16dff7ae108f13ecb07bc1466f504a7b4c773d4557599c659f6d2e9ba0037ed89179c2e187f383a917e0242c4c795cf6e1c9cec6
501a3dd4ad walletdb: hash pubkey/privkey in one shot to avoid leaking secret data (Sebastian Falbesoner)
Pull request description:
In several places in the wallet DB module, byte strings containing serialized public keys and secret keys are created in order to be hashed. To avoid sensitive data lingering in memory (and potentially leaking), don't store the preimage, but hash both public key and secret key in one shot, using the overloaded `Hash` function:
d198635fa2/src/hash.h (L82-L88)
See e.g. #31166 and #31774 for similarly themed PRs (Note that in #31166 we used the explicit `memory_cleanse` approach though, as changing the allocator was not possible.)
ACKs for top commit:
davidgumberg:
crACK 501a3dd4ad
furszy:
ACK 501a3dd4ad
rkrux:
ACK 501a3dd
theuni:
ACK 501a3dd4ad
Tree-SHA512: 8a71685b26bf89fca181aed6512a8db843b6d1dc740a468bb33fb2a629a23167a9676c228d1077ad8db2df9db80f47e32ec013737e93df8ee6f4ba505d3d50c9
fec58229fa contrib: Update fixed feeds (Ava Chow)
27fbdb009f makeseeds: Choose node info with most recent success when deduplicating (Ava Chow)
982883a1bc makeseeds: Update known user agents (Ava Chow)
Pull request description:
ACKs for top commit:
fjahr:
ACK fec58229fa
Tree-SHA512: 2852a9a6a7c299ce04ee4dc438af9547d56a860858201ad2ccdea14640b17876e7e9841ce3a30030e2482cd04e9b386f7ede5c4e51582ebd09b9ce0a2a0bc43b
89386e700e kernel: Use fs:: namespace and unicode path in kernel tests (sedited)
Pull request description:
Add support for unicode characters in paths to the kernel tests by using our fs:: wrappers for std::filesystem calls and adding the windows application manifest to the binary. This exercises their handling through the kernel API.
ACKs for top commit:
hebasto:
ACK 89386e700e.
w0xlt:
ACK 89386e700e
Tree-SHA512: 7b541f482d84a66c89eec63aea0e7f7626bbbd62082ad7a7fb2c7a517296c291a6ff301c628e5e9e1d7b850ead89005141481a2bfd06d8a9081622e32f7340cc
faa016af54 refactor: Use aliasing shared_ptr in Sock::Wait (MarcoFalke)
Pull request description:
Currently, a no-op lambda is used as the deleter for the temporary shared pointer helper in `Sock::Wait`. This is perfectly fine, but has a few style issues:
* The lambda needs to be allocated on the heap
* It triggers a false-positive upstream GCC-16-trunk bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123912
Fix all issues by just using an aliasing shared pointer, which points to `this`, but is otherwise empty (sits on the stack without any heap allocations).
ACKs for top commit:
hodlinator:
ACK faa016af54
sedited:
ACK faa016af54
vasild:
ACK faa016af54
Tree-SHA512: b7330862204e79fb61f30694accb16f9a24e5722bd0ceb098ca27c877cff921afa00c0cfd953d4cbb355e6433706961a25b628efefdbe0b48bdec2941eaaee7a
a067ca3410 [doc] coin selection filters by max cluster count, not descendant (glozow)
f7be5fb8fc [refactor] rename variable to clarify it is unused and cluster count (glozow)
Pull request description:
Followup to #33629.
Fix misleading docs and variable names. Namely, `getTransactionAncestry` returns the cluster count, not max descendant count of ancestor set (not worth reimplementing as it is merely a heuristic). No behavior changes - I don’t think much needs to be changed for the first release containing cluster mempool.
Current `CoinEligibilityFilter`s enforce a maximum ancestor count (summed across all outputs, potentially overestimating) and max descendant count across ancestors of the output.
Since #33629, these filters have started using cluster count instead of max desc count across ancestors. The change isn’t dangerous, as the cluster count bounds descendant count as well. Currently, the wallet is essentially enforcing a mixture of both limits - this is good while we are transitioning. Note that the cluster count enforced is 25, not 64, since it's grabbing the node's descendant count limit. While it is not an apples-to-apples comparison, a cluster count limit of 25 helps us avoid busting legacy descendant limits (which will be common on the network for a while).
Potential things for the future, out of scope for this PR:
- When we get rid of the ancestor/descendant config options, `getPackageLimits` can probably be replaced with hard-coded values.
- Change the `OutputGroup`s to track the actual cluster count that results from spending these outputs and merging their clusters.
- Loosen from 25 after that policy is no longer common.
- Clean up `getPackageLimits`.
ACKs for top commit:
achow101:
ACK a067ca3410
ismaelsadeeq:
reACK a067ca3410
rkrux:
crACK a067ca3410
Tree-SHA512: d7cacd5bf668d42e26e8b83e42a42c280929c3bfd554c3db1de605e5939f8b36c14ecfd2839abeb4eec352363df1891b3420a693c250916391ab10a5ce26396b
Avoid storing the privkey in a vector, which could linger in memory
and potentially leak sensitive data. An alternative approach is to
use `secure_allocator` for the `std::vector` instances, but this
commit has the advantage of also deduplicating code at the same shot.
Thanks to @theuni for suggesting this.
44538f8ada kernel: Add recent assumeutxo snapshot info (Ava Chow)
58c2e23fca kernel: Update headerssync params (Ava Chow)
cf261b071f kernel: update chainTxData (Ava Chow)
8eaf1d26d4 kernel: update defaultAssumeValid and minimumChainWork (Ava Chow)
5ca0c55517 kernel: update assumed blockchain and chainstate sizes (Ava Chow)
Pull request description:
Update chainparams and headerssync params per the release process.
Also added new assumeutxo snapshots for each network. I've uploaded snapshots to https://achow101.com/files/utxo-snapshots/
ACKs for top commit:
Sjors:
ACK 44538f8ada
fjahr:
ACK 44538f8ada
janb84:
ACK 44538f8ada
sipa:
ACK 44538f8ada. I re-did all the mainnet parameters, but did not look closely at the other networks.
jaonoctus:
ACK 44538f8ada
Tree-SHA512: f9b6ccc967c5ef58f734245df459c3136491e9b6a0f6e36f4272bc0787e7b59eabe47a8c8b19a90267eca4a0b5851dfbf45153f96eac599c417f148b3cf264cf
f580cc7e9f doc: Fix `fee` field in `getblock` RPC result (nervana21)
Pull request description:
The `fee` field in the `getblock` RPC result (verbosity 2 and 3) may be omitted when block undo data is not available. Marking it optional in the `RPCResult` aligns the documented schema with the runtime behavior.
ACKs for top commit:
mercie-ux:
ACK f580cc7e9f
satsfy:
ACK f580cc7e9f
instagibbs:
ACK f580cc7e9f
w0xlt:
ACK f580cc7e9f
luke-jr:
ACK f580cc7e9f
Tree-SHA512: e3b44a48e17e21b906967aef124688a34aea2c6af3b6df3c47693fd3002d33e824f764c0060a7ab07751b98567c29eb16f3b3c07bf2999db080ff7adfd087dfd
44feab23a7 script: Fix undefined behavior in Clone() -- std::transform writes past end of empty vector (Weixie Cui)
Pull request description:
# Motivation
This patch fixes undefined behavior in Clone() in src/script/descriptor.cpp.
When std::transform is used with providers.begin() or subdescs.begin() as the output iterator, the vectors have been reserve()d but have size 0. Writing through begin() in that case writes past the logical end of the vector, which is undefined behavior.
ACKs for top commit:
maflcko:
lgtm ACK 44feab23a7
rkrux:
ACK 44feab23a7 because it gets rid of the possible undefined behaviour.
frankomosh:
Code Review ACK 44feab23a7. Fix seems minimal and correct.
Tree-SHA512: 8af3b6d97c139b32bd47d4c452b6b16befdaa7028a7bc1b6de0ab1f0a8cb35eb068710316a2c07fa60856e17e25307931aa3125b4f41d0fe7726b435483a52db
4ae9a10ada doc: add release notes for dbcache bump (Andrew Toth)
c510d126ef doc: update dbcache default in reduce-memory.md (Andrew Toth)
027cac8527 qt: show GetDefaultDBCache() in settings (Andrew Toth)
5b34f25184 dbcache: bump default from 450MB -> 1024MB if enough memory (Andrew Toth)
Pull request description:
Alternative to #34641
This increases the default `dbcache` value from `450MiB` to `1024MiB` if:
- `dbcache` is unset
- The system is 64 bit
- At least 4GiB of RAM is detected
Otherwise fallback to previous `450MiB` default.
This should be simple enough to get into v31. The bump to 1GiB shows significant performance increases in #34641. It also alleviates concerns of too high default for steady state, and of lowering the current dbcache for systems with less RAM.
This change only changes bitcoind behavior, while kernel still defaults to 450 MiB.
ACKs for top commit:
ajtowns:
ACK 4ae9a10ada
kevkevinpal:
reACK [4ae9a10](4ae9a10ada)
svanstaa:
ACK [4ae9a10](4ae9a10ada)
achow101:
ACK 4ae9a10ada
sipa:
ACK 4ae9a10ada
Tree-SHA512: ee3acf1fb08523ac80e37ec8f0caca226ffde6667f3a75ae6f4f4f54bc905a883ebcf1bf0e8a8a15c7cfabff96c23225825b3fff4506b9ab9936bf2c0a2c2513
20ae9b98ea Extend functional test for setBlockIndexCandidates UB (marcofleon)
854a6d5a9a validation: fix UB in LoadChainTip (marcofleon)
9249e6089e validation: remove LoadChainTip call from ActivateSnapshot (marcofleon)
Pull request description:
Addresses https://github.com/bitcoin/bitcoin/issues/34503. See this issue for more details as well.
Fixes a bug where, under certain conditions, `setBlockIndexCandidates` had blocks in it that were worse than the tip. The block index candidate set uses `nSequenceId` as a sort key, so modifying this field while blocks are in the set results in undefined behavior. This PR populates `setBlockIndexCandidates` after the `nSequenceId` modifications, avoiding the UB.
ACKs for top commit:
achow101:
ACK 20ae9b98ea
sedited:
Re-ACK 20ae9b98ea
sipa:
Code review ACK 20ae9b98ea
Tree-SHA512: 121c170bb70fb6365089d578db63c811e7926e129d7206e569947f7a1f6c5ddc8d5f4937b80f1ba1b7d7daa42789b143ca5b56f154b7ab968a1cd55f925f378d
97e7e79435 test: Enable `system_tests/run_command` "stdin" test on Windows (Hennadii Stepanov)
a4324ce095 test: Remove `system_tests/run_command` runtime dependencies (Hennadii Stepanov)
Pull request description:
`system_tests` currently rely on `cat`, `echo`, `false` and `sh` being available in `PATH` at runtime.
This PR:
1. Removes these dependencies.
2. Reduces the number of platform-specific code paths.
The change is primarily motivated by my work on maintaining the [`bitcoin-core`](https://packages.guix.gnu.org/packages/bitcoin-core) package in Guix. It enables the removal of the existing `bash` and `coreutils` native inputs, which in turn makes it possible to drop the implicit dependency on `qtbase@5` (see https://codeberg.org/guix/guix/pulls/4386#issuecomment-8613333).
ACKs for top commit:
maflcko:
re-ACK 97e7e79435👓
janb84:
ACK 97e7e79435
sedited:
ACK 97e7e79435
Tree-SHA512: 1375c676f85c75d571df1ddfc3a4405767dbf0ed7bfea2927c93ec01b29f9f7ae3383e546d2658f595e8ffafa9ab20bba6fcc628a9f5ebdb288bbef03b645fb6
15c4889497 index: document TxoSpenderIndex::FindSpender (furszy)
f8b9595aaa test: Add missing txospenderindex coverage in feature_init (Fabian Jahr)
a1074d852a index, rpc, test: Misc formatting fixes (Fabian Jahr)
Pull request description:
This addresses my own comments in the last review of #24539: https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-3829110465
The first commit fixes three small formatting errors.
The second commit adds some missing coverage in `feature_init` and refactors the code a bit as well so these misses don't happen so easily in the future.
The third commit is by furzy:
> TxoSpenderIndex::FindSpender returns an Expected<optional<TxoSpender>> but
the two levels of the return type were undocumented, making it unclear what a returned
nullopt means. So added doc clarifying each return case.
ACKs for top commit:
furszy:
ACK 15c4889497
sedited:
ACK 15c4889497
rkrux:
crACK 15c4889497
Tree-SHA512: 2e0f060a54b558d2967ebae0835cf81bd86c2d8d983d670a48d1bee7d347f186623e75db7ae311ca1566807f715c1b3fa67cf734c9467d35e13b84d082f28253
bff8a7a80d subprocess: replace __USING_WINDOWS__ with WIN32 (kevkevinpal)
Pull request description:
## Summary
Motivated by https://github.com/bitcoin/bitcoin/pull/34385#pullrequestreview-3826616188
In `subprocess.h` we are now renaming `__USING_WINDOWS__` with `WIN32`
In the rest of the codebase, we are using `WIN32`, so it makes sense to update `subprocess.h` to match that.
---
Use the following `grep` to assert there is no `__USING_WINDOWS__` in the codebase
```
grep -nri --exclude-dir=build "WIN32" ./ -I
rep -nri --exclude-dir=build "__USING_WINDOWS__" ./ -I
```
ACKs for top commit:
sedited:
ACK bff8a7a80d
hebasto:
ACK bff8a7a80d, I have reviewed the code and it looks OK.
Tree-SHA512: 18c3c8b87cf880053bbf69f837a0a135c5da51cfb15ab1d9fd554d8f931b2ea8202cf0f4d5e6f317d6234480128c2f41a7a1a9d9bd0504697a3c4c6a21797762
The removal of the chain tip from setBlockIndexCandidates was
happening after nSequenceId was modified. Since the set uses
nSequenceId as a sort key, modifying it while the element is in the
set is undefined behavior, which can cause the erase to fail.
With assumeutxo, a second form of UB exists: two chainstates each
have their own candidate set, but share the same CBlockIndex
objects. Calling LoadChainTip on one chainstate mutates nSequenceIds
that are also in the other chainstate's set.
Fix by populating setBlockIndexCandidates after all changes to
nSequenceId.
This call is a no-op. PopulateAndValidateSnapshot already sets both
the chain tip and the coins cache best block to the snapshot block,
so LoadChainTip always hits the early return when it finds that the
two match (tip->GetBlockHash() == coins_cache.GetBestBlock()).
f51665bee7 psbt: validate pubkeys in MuSig2 pubnonce/partial sig deserialization (tboy1337)
Pull request description:
The previous fix for invalid MuSig2 pubkeys (bitcoin/bitcoin#34010) only
addressed the PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS field. However, the
PSBT_IN_MUSIG2_PUB_NONCE and PSBT_IN_MUSIG2_PARTIAL_SIG fields also
deserialize pubkeys without validation, which could lead to crashes when
invalid pubkeys are processed.
This commit adds validation to the DeserializeMuSig2ParticipantDataIdentifier
function to ensure all pubkeys in MuSig2 pubnonce and partial signature
fields are fully valid elliptic curve points.
The fix:
- Validates both aggregate and participant pubkeys in MuSig2 pubnonce and
partial signature deserialization
- Throws std::ios_base::failure with descriptive error messages for invalid
pubkeys
- Prevents potential crashes from invalid elliptic curve points
- Maintains backward compatibility for valid PSBTs
This completes the fix for issues [#33999](https://github.com/bitcoin/bitcoin/issues/33999) and [#34201](https://github.com/bitcoin/bitcoin/issues/34201).
ACKs for top commit:
rkrux:
lgtm ACK f51665bee7
w0xlt:
ACK f51665bee7
darosior:
utACK f51665bee7
Tree-SHA512: 8454d77a05aa003a3121b0a5975e8a000125ee0d62343bfa625a75db113358ba7a210ae0376ca1666957b7de7005e06e5a54c95170430ee5e9e1416719b8af53
Add support for unicode characters in paths to the kernel tests by using
our fs:: wrappers for std::filesystem calls and adding the windows
application manifest to the binary. This exercises their handling
through the kernel API.
98e8af4bb9 wallet: Drain validation interface queue after notifications disconnect (Ava Chow)
52992ebe1c interfaces: Add waitForNotifications() to call SyncWithValidationInterfaceQueue() (Ava Chow)
Pull request description:
When the wallet disconnects chain notifications, it is expecting no further notifications to execute, but this is not the case. This results in test failures such as in #34354. Instead of disconnecting the notifications and continuing shutdown, we should wait for the validation interface queue to be drained before the rest of wallet shutdown. This is achieved by adding an `interfaces::Chain::waitForNotifications()` function which calls `SyncWithValidationInterfaceQueue()`.
Fixes#34354
ACKs for top commit:
stickies-v:
utACK 98e8af4bb9
furszy:
ACK 98e8af4bb9
rkrux:
crACK 98e8af4bb9
sedited:
ACK 98e8af4bb9
Tree-SHA512: 263628556f740cb633d3970c22a0dfdb52a644bd1d0cd5a69c2970524edbb0e25d592cb39fc9bf1d0c281eebce09578526e2958dffee9026fc7473db35bd0dec
da7f70a532 test: use port 0 for I2P addresses in p2p_private_broadcast.py (Vasil Dimov)
a8ebcfd34c test: let connections happen in any order in p2p_private_broadcast.py (Vasil Dimov)
67696b207f net: extend log message to include attempted connection type (Vasil Dimov)
Pull request description:
If the following two events happen:
* (likely) the automatic 10 initial connections are not made to all
networks
* (unlikely) the network-specific logic kicks in almost immediately.
It is using exponential distribution with a mean of 5 minutes
(`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).
So if both happen, then the 11th connection may not be the expected
private broadcast, but a network-specific connection.
Fix this by retrieving the connection type from
`destinations_factory()`. This is more flexible because it allows
connections to happen in any order and does not break if e.g. the 11th
connection is not the expected first private broadcast.
This also makes the test run faster:
before: 19-44 sec
now: 10-25 sec
because for example there is no need to wait for the initial 10
automatic outbound connections to be made in order to proceed.
Fixes: https://github.com/bitcoin/bitcoin/issues/34387
ACKs for top commit:
achow101:
ACK da7f70a532
andrewtoth:
ACK da7f70a532
mzumsande:
Code Review ACK da7f70a532
Tree-SHA512: 7c293e59c15c148a438e0119343b05eb278205640658c99336d4caf4848c5bae92b48e15f325fa616cbc9d5f394649abfa02406a76e802cffbd3d312a22a6885
The `fee` field in the `getblock` RPC result (verbosity 2 and 3) may be
omitted when block undo data is not available. Marking it optional in
the `RPCResult` aligns the documented schema with the runtime behavior.