wallet: feebumper, fix crash when combined bump fee is unavailable#34870
Merged
achow101 merged 1 commit intobitcoin:masterfrom Mar 24, 2026
Merged
wallet: feebumper, fix crash when combined bump fee is unavailable#34870achow101 merged 1 commit intobitcoin:masterfrom
achow101 merged 1 commit intobitcoin:masterfrom
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste |
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.
furszy
force-pushed
the
2026_feebumper_crash_fix
branch
from
7bd27f9 to
6072a2a
Compare
luke-jr
approved these changes
Mar 20, 2026
rkrux
reviewed
Mar 23, 2026
Contributor
There was a problem hiding this comment.
crACK 6072a2a based on returning before accessing the null optional.
When a large cluster of unconfirmed transactions exceeds the limit,
calculateCombinedBumpFee() returns std::nullopt.
The error message caught my eye because it is dependent on the optional being empty, which is an underlying assumption made by the code. Nothing needs to changed for it in this PR but I found it noteworthy.
Some more investigation showed me there is no test coverage for this conditional, so I created a (relatively) good first issue for it here: #34902.
Member
|
ACK 6072a2a |
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Mar 24, 2026
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: bitcoin#34870 Rebased-From: 6072a2a
Merged
Member
|
Backported to |
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Mar 24, 2026
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: bitcoin#34870 Rebased-From: 6072a2a
Member
|
Backported to 30.x in #34856. |
Draft
achow101
added a commit
that referenced
this pull request
Mar 25, 2026
b241f3c doc: update example bitcoin conf for 31.0rc2 (fanquake) 718c31c doc: update manual pages for v31.0rc2 (fanquake) a30e505 build: bump version to v31.0rc2 (fanquake) ac13aca test: scale IPC mining wait timeouts by timeout_factor (Enoch Azariah) 39c8762 test: verify IPC error handling for invalid coinbase (Enoch Azariah) 6609473 test: move make_mining_ctx to ipc_util.py (Enoch Azariah) acd7e3d test: verify createNewBlock wakes promptly when tip advances (Enoch Azariah) e3d5716 test: Remove confusing assert_debug_log in wallet_reindex.py (MarcoFalke) 87d1691 wallet: feebumper, fix crash when combined bump fee is unavailable (furszy) 11b6992 wallet: fix amount computed as boolean in coin selection (furszy) d171afa ci: Temporarily use clang in valgrind tasks (MarcoFalke) 198bc4d ci: Clarify why valgrind task has gui disabled (MarcoFalke) 6993aa1 test: Scale feature_dbcrash.py timeout with factor (MarcoFalke) 051afe9 depends: Remove no longer necessary `dsymutil` (Hennadii Stepanov) 3b79852 depends: Fix cross-compiling on macOS for Windows (Hennadii Stepanov) e53c20d gui: Fix TransactionsView on setCurrentWallet (pablomartin4btc) 7118559 tests: applied PYTHON_GIL to the env for every test (kevkevinpal) d9a5791 ci: Avoid intermittent Windows generate download failures (MarcoFalke) 335a098 kernel: acquire coinstats cursor and block info atomically (w0xlt) e930c6d rpc: fix race condition in gettxoutsetinfo (w0xlt) ca781e4 cmake: Migrate away from deprecated SQLite3 target (Daniel Pfeifer) 0689512 test: [refactor] Use verbosity=0 named arg (MarcoFalke) 8379f00 test: Fix intermittent issue in feature_assumeutxo.py (MarcoFalke) 72d6c88 test: Move event loop creation to network thread (MarcoFalke) c7127f2 test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy() (MarcoFalke) a69f8c3 ci: Use arch-appropriate binaries in lint install (will) e3383ac ci: check macos bundle structure and codesigning (fanquake) ab37d3d macdeploy: use plugins dir to find plugins (fanquake) bb9fcff macdeploy: subprocess out to zip rather than shutil.make_archive (fanquake) d20ba02 build: Set AUTHOR_WARNING on warnings (MarcoFalke) 2724c39 guix: Make guix-clean less destructive (Hodlinator) a28d78c test: use static methods and clarify comment in addr_relay (stratospher) 5642a2b test: protect outbound connection from eviction in getaddr_test (stratospher) a3c1eda test: fix addr relay test silent pass and wrong peerinfo index (stratospher) 207087b ci: bump cirruslabs actions versions (will) a74dfe3 lint: Temporarily revert to vulture==2.14 (MarcoFalke) f7f7e68 ci: Bump GHA actions versions (MarcoFalke) a3ffff0 depends: delete Boost extra files (fanquake) 9852bbd depends: disable Qt sbom generation (fanquake) Pull request description: Backports: * #33144 * #34451 * #34589 * #34727 * #34750 * #34755 * #34776 * #34787 * #34802 * #34814 * #34815 * #34820 * #34852 * #34832 * #34848 * #34850 * #34857 * #34859 * #34869 * #34870 * #34878 * #34888 Gui: * bitcoin-core/gui#815 ACKs for top commit: Sjors: ACK b241f3c achow101: ACK b241f3c Tree-SHA512: bb68f5b6e569781805c741d63a6ad6f955c1964d9186defa892936160e8444900f1e4175a1ef4fff268b655d664ddf0b914795ef554ea60cb23a054b080b4805
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a large cluster of unconfirmed transactions exceeds the limit,
calculateCombinedBumpFee()returnsstd::nullopt.Previously, we continued executing and the optional value was
accessed unconditionally, leading to a
std::bad_optional_accessexception (https://en.cppreference.com/w/cpp/utility/optional/value.html).
Fix this by returning early when the bumped fee is null.
Note:
This is a crash for the GUI, and an uncaught exception for the RPC
bumpfeeandpsbtbumpfee.