fix(afs)!: preserve input timelock requirements by aagbotemi · Pull Request #65 · bitcoindevkit/bdk-tx

aagbotemi · 2026-05-12T21:28:55Z

Description

Fixes consensus-validity issues in BIP-326 anti-fee-sniping.

Closes #62
Closes #63
Closes #64

Bugs

The nLockTime strategy could overwrite tx.lock_time with a value lower than the accumulated input CLTV.
The nSequence strategy zeroed tx.lock_time, also breaking accumulated CLTV.
The nSequence strategy could select a Taproot input that already had a relative-timelock (CSV) requirement and overwrite its sequence.

Fixes

The locktime path only updates tx.lock_time when the AFS-proposed value still satisfies the existing requirement.
The sequence path leaves tx.lock_time alone and filters CSV-bearing Taproot inputs out of the candidate pool.

Breaking changes

PsbtParams::enable_anti_fee_sniping: bool + fallback_locktime → anti_fee_sniping: Option<absolute::Height> + min_locktime. The tip height is now required when AFS is enabled, and time-based tips are unrepresentable.
apply_anti_fee_sniping is now pub(crate); rbf_enabled param removed (derived from tx.is_explicitly_rbf()).
New AntiFeeSnipingError type, wrapped under CreatePsbtError::AntiFeeSniping.

evanlinjin

Thanks for working on this.

PR description is misleading - none of these bugs were flagged in the post-merge review. It also says this "fixes two consensus-validity issues", however this PR attempts to close 3 issues?

evanlinjin

There are two versions of the random_probability and random_range methods. The std versions add no meaningful value imo - I would just remove them.

aagbotemi · 2026-05-13T13:29:51Z

Thank you for the review

evanlinjin

Make apply_anti_fee_sniping explicitly private!

evanlinjin · 2026-05-15T18:39:53Z

We are lacking commit messages, PR description has factual errors, missing ! to signal breaking change.

I'll push it forward!

evanlinjin · 2026-05-15T20:08:52Z

@aagbotemi I pushed a Claude-generated test commit on top, let me know what you think.

Anti-fee-sniping could produce consensus-invalid transactions when inputs had timelock requirements: - The nLockTime strategy could overwrite `tx.lock_time` with a value lower than the accumulated input CLTV. - The nSequence strategy zeroed `tx.lock_time`, also breaking accumulated CLTV. - The nSequence strategy could select a Taproot input that already had a relative-timelock (CSV) requirement and overwrite its sequence. The locktime path now only updates `tx.lock_time` when the proposed AFS value still satisfies the existing requirement. The sequence path leaves `tx.lock_time` alone and filters CSV-bearing Taproot inputs out of the candidate pool. API changes: - `PsbtParams::enable_anti_fee_sniping: bool` and `fallback_locktime` are replaced with `anti_fee_sniping: Option<absolute::Height>` and `min_locktime`. The tip height is now required when AFS is enabled and time-based tips are unrepresentable. - `apply_anti_fee_sniping` is now `pub(crate)`; the `rbf_enabled` parameter is removed (derived from `tx.is_explicitly_rbf()`). - New `AntiFeeSnipingError` type, wrapped under `CreatePsbtError::AntiFeeSniping`. Closes bitcoindevkit#62 Closes bitcoindevkit#63 Closes bitcoindevkit#64

Three new tests, each verified to fail on the pre-fix algorithm: - `test_anti_fee_sniping_preserves_input_cltv` — input CLTV above tip must not be lowered. - `test_anti_fee_sniping_skips_taproot_csv_input` — a Taproot input carrying a CSV requirement must be excluded from the nSequence candidate pool; a regular Taproot input remains to ensure the sequence path is still reachable. - `test_anti_fee_sniping_rejects_time_based_locktime` — a time-based CLTV must surface `AntiFeeSnipingError::UnsupportedLockTime`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

evanlinjin

ACK 5df9bb1

aagbotemi mentioned this pull request May 12, 2026

refactor(afs)!: decouple anti-fee-sniping from create_psbt #56

Closed

3 tasks

aagbotemi self-assigned this May 12, 2026

aagbotemi requested a review from evanlinjin May 12, 2026 21:30

aagbotemi force-pushed the fix/afs-locktime branch from d6a0843 to dc4b147 Compare May 12, 2026 21:57

evanlinjin requested changes May 13, 2026

View reviewed changes

Comment thread src/utils.rs Outdated

Comment thread src/selection.rs

Comment thread src/utils.rs Outdated

evanlinjin reviewed May 13, 2026

View reviewed changes

Comment thread src/utils.rs

evanlinjin requested changes May 13, 2026

View reviewed changes

evanlinjin mentioned this pull request May 13, 2026

Rename utils.rs to afs.rs? #67

Closed

aagbotemi force-pushed the fix/afs-locktime branch from 67dcf29 to 8487ab4 Compare May 13, 2026 13:16

aagbotemi requested a review from evanlinjin May 13, 2026 13:29

This comment was marked as outdated.

Sign in to view

evanlinjin requested changes May 13, 2026

View reviewed changes

Comment thread src/utils.rs Outdated

aagbotemi force-pushed the fix/afs-locktime branch 2 times, most recently from f3d7e60 to 3b72e6d Compare May 14, 2026 09:59

aagbotemi requested a review from evanlinjin May 14, 2026 10:02

evanlinjin reviewed May 14, 2026

View reviewed changes

Comment thread src/selection.rs

aagbotemi force-pushed the fix/afs-locktime branch 2 times, most recently from 3b72e6d to b0b0eb1 Compare May 14, 2026 13:56

evanlinjin requested changes May 14, 2026

View reviewed changes

Comment thread src/utils.rs Outdated

Comment thread src/utils.rs Outdated

Comment thread src/utils.rs Outdated

Comment thread src/utils.rs Outdated

Comment thread src/utils.rs Outdated

Comment thread src/utils.rs Outdated

aagbotemi force-pushed the fix/afs-locktime branch 3 times, most recently from 15943a8 to adf472f Compare May 14, 2026 15:35

evanlinjin reviewed May 14, 2026

View reviewed changes

Comment thread examples/anti_fee_sniping.rs

aagbotemi force-pushed the fix/afs-locktime branch from adf472f to 8085f45 Compare May 14, 2026 20:19

evanlinjin mentioned this pull request May 15, 2026

AFS may complete the locktime-branch with all sequence values as Sequence::MAX, making AFS a no-op #70

Open

evanlinjin changed the title ~~fix(afs): preserve accumulated locktime~~ fix(afs)!: preserve input timelock requirements May 15, 2026

evanlinjin force-pushed the fix/afs-locktime branch 2 times, most recently from b8671c3 to c83079a Compare May 15, 2026 18:59

evanlinjin force-pushed the fix/afs-locktime branch from c83079a to 1211bc3 Compare May 15, 2026 19:42

evanlinjin requested a review from nymius May 15, 2026 20:01

aagbotemi commented May 15, 2026

View reviewed changes

Comment thread src/selection.rs Outdated

evanlinjin force-pushed the fix/afs-locktime branch 3 times, most recently from 059752f to 2fcd3c5 Compare May 16, 2026 12:31

aagbotemi and others added 2 commits May 16, 2026 12:41

evanlinjin force-pushed the fix/afs-locktime branch from 2fcd3c5 to 5df9bb1 Compare May 16, 2026 12:41

evanlinjin approved these changes May 16, 2026

View reviewed changes

evanlinjin merged commit 8b51d07 into bitcoindevkit:master May 16, 2026
7 checks passed

ValuedMammal mentioned this pull request Jun 2, 2026

Release 0.2.0 #30

Closed

Uh oh!

Conversation

aagbotemi commented May 12, 2026 • edited by evanlinjin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Bugs

Fixes

Breaking changes

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

aagbotemi commented May 13, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin commented May 15, 2026

Uh oh!

evanlinjin commented May 15, 2026

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aagbotemi commented May 12, 2026 •

edited by evanlinjin

Loading