#35442 test: remove usages of MAX_BIP125_RBF_SEQUENCE constant from functional tests
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35442 test: remove usages of MAX_BIP125_RBF_SEQUENCE constant from functional tests | Needs rebase | 0 | Blocked | 73 + 327 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Removes the `MAX_BIP125_RBF_SEQUENCE` constant from the functional test framework and eliminates its usage across wallet and non-wallet functional tests. Tests that previously used the constant to ensure non-final sequence numbers now use `MAX_SEQUENCE_NONFINAL` or default sequences. The constant is scoped strictly to `wallet_deprecated_rbf.py` to maintain coverage for deprecated BIP 125 RPC behavior until eventual removal.
Problem: Functional tests throughout the test suite unnecessarily rely on BIP 125 opt-in sequence numbers even when BIP 125 signaling is irrelevant to the scenario under test, conflicting with ongoing efforts to deprecate and remove BIP 125 signaling logic from the codebase.
Evidence of importance:
- Author description: 'Partially fixes https://github.com/bitcoin/bitcoin/issues/32661. This is in line with the removal and deprecation of BIP 125 RBF signalling from the codebase.'
- Commit 8e8765e46d: 'Remove the usage in all the tests where used except the ones where the sequence is set to unique values to avoid creating duplicate transactions simultaneously.'
Scope: The PR branch currently includes 11 commits imported directly from prerequisite PR #35433 (deprecating the `replaceable` RPC argument and altering wallet defaults). Only the final two commits represent the scope of this PR.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- author: author is waiting for #35433 to merge before progressing on this PR 2026-10-26
- author: address the sendall input SEQUENCE_FINAL locktime issue reported by rreview 2026-10-26
Agreement
Blocked
objection that needs a change, nobody has replied (rreview)
Thread notes: Reclassified maflcko's comment as suggestions and split into two parts; recorded suggestions from pablomartin4btc on PR description and release notes. Automated bot output from rreview is treated as neutral process output. / Re-classified maflcko's comment as suggestions and split it into two parts; added pablomartin4btc's release notes suggestion and rreview's correctness objection regarding sendall input sequences.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| maflcko (member) | suggestion, scope | Include the test-only commits in the prior pull request (#35433) instead of creating a separate PR | answered · author explained dependency on #35405 prevents folding commits cleanly into #35433 | answered | yes | 2026-06-08: 'Not sure if a small test-only fixup needs a separate pull request. Can those two commits not just be included in the prior pull?' Status from: 2026-06-10: rkrux: 'The first two commits here are from #35405 due to which these two commits can't be appended cleanly in #35433.' |
| maflcko (member) | suggestion, approach | Remove MAX_BIP125_RBF_SEQUENCE from the source code rather than tests to reduce fingerprint vectors | answered · author explained it cannot be cleanly removed from source code until feature removal post-deprecation | answered | yes | 2026-06-08: 'it seems more important to remove `MAX_BIP125_RBF_SEQUENCE` from the real source code, so that the risk of pre-existing and future fingerprint vectors in this source code is reduced.' Status from: 2026-06-10: rkrux: 'it can't be done cleanly until the feature is removed completely post deprecation.' |
| pablomartin4btc (member) | suggestion, scope | Add the dependency on #35405 and #35433 to the PR description | open | answered | no | 2026-06-12: 'Perhaps you can add that to the description too ("Depends on https://github.com/bitcoin/bitcoin/pull/35405 - first 2 commits - and #35433 - 9 commits").' |
| pablomartin4btc (member) | suggestion, maintenance | Update release notes according to discussion in PR 35405 | open | answered | no | 2026-06-12: 'I think that needs to be updated according to https://github.com/bitcoin/bitcoin/pull/35405/changes#r3340846939.' |
| rreview (none) | objection, correctness | sendall automatic input selection assigns SEQUENCE_FINAL, disabling caller-supplied locktimes and anti-fee-sniping | open | changed | no | 2026-10-26: 'Final sequences disable `nLockTime`, so a caller-supplied nonzero locktime is ineffective. The same sequence prevents `FinishTransaction` from applying anti-fee-sniping' |
Participants:
- maflcko (member, neutral, 1 statements 2026-06-08 to 2026-06-08): suggested folding into prior PR and prioritizing source code removal over tests
- pablomartin4btc (member, neutral, 2 statements 2026-06-12 to 2026-06-12): suggested documenting #35405 dependency in description and updating release notes
- rreview (none, objection, 1 statements 2026-10-26 to 2026-10-26): reported that default sendall inputs assign SEQUENCE_FINAL, disabling locktime activation and anti-fee-sniping
Dependencies
Depends on: #35433
Files
605 lines under test/bench/ci.
- test/functional/wallet_bumpfee.py +231/-235
- test/functional/wallet_deprecated_rbf.py +70/-5
- src/wallet/rpc/spend.cpp +41/-20
- test/functional/mempool_package_rbf.py +13/-13
- doc/release-notes-34917.md +20/-4
- src/rpc/rawtransaction.cpp +9/-5
- test/functional/rpc_psbt.py +5/-5
- test/functional/rpc_packages.py +2/-3
- test/functional/rpc_rawtransaction.py +0/-5
- test/functional/feature_rbf.py +2/-2
- test/functional/p2p_opportunistic_1p1c.py +2/-2
- test/functional/wallet_listsinceblock.py +1/-3
- test/functional/mempool_accept.py +1/-2
- src/rpc/rawtransaction_util.cpp +1/-1
- src/wallet/spend.cpp +1/-1
- src/wallet/wallet.h +1/-1
- test/functional/p2p_segwit.py +0/-2
- test/functional/test_framework/messages.py +0/-1
Card
The short text the ranking pass compares across PRs.
Removes references to the MAX_BIP125_RBF_SEQUENCE constant from functional tests and the shared test framework, switching to MAX_SEQUENCE_NONFINAL or default values. This refactor aligns test suite conventions with the broader deprecation and eventual removal of BIP 125 opt-in RBF signaling across Bitcoin Core (partially addressing #32661). It depends on #35433, which deprecates the replaceable RPC arguments, and bundles those commits while in draft.
Data
thread read through 2026-10-26 (8 statements), head 7e4a0c57fc
labels: Tests, Needs rebase · draft
Categories
Test infrastructure
P4 · cleanup (ranking pass: Placed after the assertion cleanups; it is blocked on #35433 and only partially advances BIP125 deprecation.)
Eliminates an unnecessary BIP 125 sequence constant from the functional test framework and test suites. Author notes this 'Partially fixes #32661' in line with removing BIP 125 signaling from the codebase, but the change itself is purely internal test cleanup.
Membership: Has the Tests label, removes `MAX_BIP125_RBF_SEQUENCE` from `test/functional/test_framework/messages.py`, and updates sequence handling across multiple functional tests.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1