feat!: extract anti-fee-sniping from create_psbt#6
Open
evanlinjin wants to merge 27 commits into
Open
Conversation
AFS now lives behind a dedicated `apply_anti_fee_sniping` free function and
a `Selection::apply_anti_fee_sniping` wrapper, both taking an explicit
`tip_height: absolute::Height`. `create_psbt` is rng-free and deterministic.
Breaking changes:
- `PsbtParams::enable_anti_fee_sniping` removed.
- `Selection::create_psbt_with_rng` removed; `create_psbt` no longer takes
an rng and is no longer `#[cfg(feature = "std")]`-gated.
- `CreatePsbtError::InvalidLockTime` and `CreatePsbtError::UnsupportedVersion`
removed; replaced by `AntiFeeSnipingError`.
New surface:
- `AntiFeeSnipingError { UnsupportedVersion, TimeBasedLocktime,
TipBelowExistingLocktime }` — the latter two enforce that AFS cannot
silently violate a time-based or future-block CLTV requirement.
- `apply_anti_fee_sniping(tx, inputs, tip_height, rng)` re-exported from
the crate root; lives in `src/anti_fee_sniping.rs`.
- `Selection::apply_anti_fee_sniping(self, psbt, tip_height, rng) -> Psbt`.
Behavior fix:
- Confirmation math now uses the actual chain tip rather than the
post-accumulate `tx.lock_time`, fixing inflated confirmation counts when
any input requires a CLTV above tip.
- The locktime branch clamps the written value to `>= existing tx.lock_time`,
preserving any input-required CLTV through the random offset.
- The sequence branch — which zeroes `tx.lock_time` — is forced off when
`tx.lock_time` is non-zero, so AFS never erases an input-required CLTV.
Other:
- `rand` dropped from runtime `[dependencies]`; the lib uses `rand_core`
only. `rand` is now a dev-dependency for tests/examples.
- `src/utils.rs` removed (its only contents were the AFS function and its
RNG helpers, all moved into `src/anti_fee_sniping.rs`).
- See bitcoin/bitcoin#28944 for the analogous Bitcoin Core split between
construction and finishing.
Migration:
// before
let psbt = selection.create_psbt(PsbtParams {
fallback_locktime: LockTime::from_consensus(tip),
enable_anti_fee_sniping: true,
..Default::default()
})?;
// after
let psbt = selection.create_psbt(PsbtParams {
fallback_locktime: LockTime::from_consensus(tip),
..Default::default()
})?;
let psbt = selection.apply_anti_fee_sniping(
psbt,
Height::from_consensus(tip)?,
&mut rng,
)?;
Refs #4, #5.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Mirrors Bitcoin Core's `FinishTransaction`, which mutates a `CMutableTransaction` *before* wrapping it as a PSBT: AFS is now applied before `create_psbt` rather than after. `PsbtParams` stays AFS-free. API change: // before (post-processing on a built PSBT) let psbt = selection.create_psbt(params)?; let psbt = selection.apply_anti_fee_sniping(psbt, tip, &mut rng)?; // after (pre-construction mutation of Selection + PsbtParams) selection.apply_anti_fee_sniping(&mut params, tip, &mut rng)?; let psbt = selection.create_psbt(params)?; Surface changes: - `Selection::apply_anti_fee_sniping` is now `(&mut self, &mut PsbtParams, tip, rng) -> Result<(), AntiFeeSnipingError>`. Locktime branch rewrites `params.fallback_locktime`; sequence branch calls `self.inputs[i].set_sequence(...)`. - `Input::set_sequence(&mut self, Sequence)` and a `sequence_override` field. `Input::sequence()` now prefers the override. - The free function `apply_anti_fee_sniping(tx, inputs, tip, rng)` is removed — `Selection::apply_anti_fee_sniping` is the only entry point. Selection-less callers can compose `Selection` directly. - `AntiFeeSnipingError::LockTypeMismatch` added (returned when the selection's inputs already have inconsistent locktime units — `create_psbt` would have failed the same way). - `Selection::accumulate_max_locktime` is `pub(crate)` so the AFS module can simulate the locktime `create_psbt` would produce. Why this is better than the post-processing shape: - `PsbtParams` has no AFS-specific fields and `create_psbt` stays deterministic / rng-free. - AFS now operates on Selection + Params (a *plan*) rather than a built PSBT, so the "before signing" precondition is structurally enforced — there's no PSBT-input signature data to silently invalidate. - Matches Bitcoin Core's split between pre-PSBT mutation and PSBT-fill. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
A lock_time above tip means the tx is already future-locked beyond what AFS could add — no fee-sniping window exists for any block <= lock_time, so AFS doesn't need to protect it. Mechanically, accumulate_max_locktime takes max(input_CLTVs, params.fallback_locktime) inside create_psbt, so even if AFS writes a tip-ish value below an input-required CLTV, the input wins and tx.lock_time ends up at the higher value. The error variant was protecting an invariant AFS cannot violate in the pre-construction shape. Replaces the dedicated error test with `test_anti_fee_sniping_accepts_ existing_above_tip` that documents the new pass-through behavior. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…ence
`Input::sequence_override` was duplicate state for the `PsbtInput`
variant, which already has a mutable `sequence` field. Move the override
into the `PlanOrPsbtInput::Plan` variant — the only place it adds new
information — and have `set_sequence` mutate the appropriate slot
depending on the variant.
`Input::set_sequence` is now fallible: it rejects a value that would not
satisfy the input's existing relative-timelock requirement (BIP68 / CSV).
This prevents AFS — or any future caller — from silently producing an
invalid transaction by overriding a sequence below its required value.
New surface:
- `pub enum SetSequenceError { IncompatibleRelativeTimelock { required, new } }`
- `Input::set_sequence(&mut self, Sequence) -> Result<(), SetSequenceError>`
- `AntiFeeSnipingError::InputSequence(SetSequenceError)` — wraps the
inner error when AFS's sequence branch chooses an input whose plan-
required relative timelock the freshness signal can't satisfy.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
… erroring `AntiFeeSnipingError::InputSequence` removed. AFS now pre-filters the taproot input pool to those without a relative-timelock requirement; if filtering empties the pool, the existing `taproot_inputs.is_empty()` clause forces the locktime branch. The `set_sequence` call inside AFS is now infallible-by-construction (`.expect`), since every input that reaches it was filtered to have no relative-timelock requirement. Sequence-branch AFS is meant for "fresh" inputs without prior consensus commitments — falling through to the locktime branch is the right thing when no such inputs exist, not raising an error. New test `test_anti_fee_sniping_skips_inputs_with_relative_timelock` verifies that a taproot input with a stored `older(50)` sequence forces the locktime branch across 50 RNG iterations, and that its sequence is preserved (not overridden by AFS). https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
`AntiFeeSnipingError::UnsupportedVersion` removed. When `params.version` is below `Version::TWO`, AFS now silently bumps it to `Version::TWO` — BIP326 requires v2 semantics (BIP68/CSV) for the sequence branch, and v1 transactions have no modern wallet use case worth preserving here. The behavior is documented loudly in the `apply_anti_fee_sniping` docstring so callers who want a specific lower version know to not call this function. `AntiFeeSnipingError` is now down to two variants: `TimeBasedLocktime` and `LockTypeMismatch`. The previous `test_anti_fee_sniping_unsupported_version_error` is replaced by `test_anti_fee_sniping_bumps_version_to_two`, which sets `params.version = Version::ONE`, applies AFS, and asserts the version ends at `Version::TWO`. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…CKTIME
Match Bitcoin Core's wallet default (`MAX_BIP125_RBF_SEQUENCE = 0xfffffffd`).
Core uses this unconditionally in `CreateTransactionInternal` when the
caller doesn't opt out, and virtually every modern wallet signals BIP125
on every tx.
The previous default (`ENABLE_LOCKTIME_NO_RBF = 0xfffffffe`) was a
historical conservative choice: it didn't signal RBF, which used to mean
the tx couldn't be replaced in the mempool. Since Bitcoin Core 28
(October 2024) defaults `mempoolfullrbf=1`, that's no longer load-bearing
for replaceability — any tx can be replaced regardless of signaling. The
remaining reasons to signal still apply, though:
- Downstream tooling (block explorers, hardware wallet UIs, fee-bumping
software) reads BIP125 status and displays it to users.
- Explicit intent: "this tx is meant to be replaceable."
- AFS interaction: the sequence branch of `Selection::apply_anti_fee_sniping`
is gated on `rbf_enabled` (any sequence < 0xfffffffe). With the old
default, AFS could not take the sequence branch on a typical-cased tx;
with the new default, it can.
Behavior change for callers that took the default: their txs now signal
BIP125 RBF. To opt out, set `PsbtParams { fallback_sequence:
Sequence::ENABLE_LOCKTIME_NO_RBF, .. }` explicitly.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…nstead Previously `apply_anti_fee_sniping` silently mutated `params.version` to v2 when it was below. The locktime branch (nLockTime) works at any tx version, though — only the sequence branch needs v2 (BIP68/CSV relative- locktime semantics). So a more honest behavior is to respect the caller's version choice and just narrow AFS to the locktime branch at v1. Implemented by adding `params.version < Version::TWO` to the `must_use_locktime` chain. No version mutation. The branch decision gracefully degrades. Test updated: `test_anti_fee_sniping_bumps_version_to_two` → `test_anti_fee_sniping_v1_forces_locktime_branch`, which asserts `params.version` is preserved at `Version::ONE` and `fallback_locktime` was rewritten by AFS, across 50 RNG iterations. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Previously `apply_anti_fee_sniping` would return `TimeBasedLocktime` even when the sequence branch was perfectly applicable — taproot input, confirmed, RBF, v2 — just because some input required a time-based CLTV or `params.fallback_locktime` was time-based. The sequence branch is structurally orthogonal to `tx.lock_time`: it overrides one input's `sequence` and never touches `params.fallback_locktime`. So a tx ending up with a time-based `tx.lock_time` (from the input CLTV via `accumulate_max_locktime`) AND a freshness `sequence` signal is functionally valid. BIP326 framing assumes `lock_time = 0` for the sequence-branch case, but it doesn't address the time-based-CLTV edge case explicitly; degrading gracefully here is more useful than rejecting. Implementation: - `effective_height` becomes `Option<absolute::Height>` (None for time-based). - New `must_use_sequence = effective_height.is_none()` flag. - Early return removed. Time-based effective_locktime forces the sequence branch; the locktime branch's `expect` documents the invariant. - If BOTH `must_use_locktime` and `must_use_sequence` are true (e.g. time-based CLTV with a non-taproot input), AFS returns `TimeBasedLocktime` — the unrecoverable case is preserved. Tests: - `test_anti_fee_sniping_time_based_locktime_error` renamed to `test_anti_fee_sniping_time_based_locktime_forces_sequence_branch` and rewritten: across 50 iterations with a time-based fallback, asserts AFS succeeds, sequence override is set, and `params.fallback_locktime` is preserved verbatim. - New `test_anti_fee_sniping_time_based_locktime_and_non_taproot_errors` covers the still-unrecoverable case (non-taproot input + time-based fallback → `TimeBasedLocktime`). https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
`create_psbt(&self, ...)` borrows; selection stays accessible after the call. The clone was a leftover from an earlier shape of the example. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
`fallback_locktime: LockTime::ZERO` and `fallback_sequence: Sequence::ENABLE_RBF_NO_LOCKTIME` now both match `PsbtParams::default()` after the recent flip of the sequence default to BIP125-signaling. The explicit fields no longer carry information. Dropped the now-unused `Sequence` import. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Builder-style ownership for the Selection side enables chaining straight
into create_psbt:
let psbt = selection
.apply_anti_fee_sniping(&mut params, tip, &mut rng)?
.create_psbt(params)?;
`params` continues to be `&mut PsbtParams` because AFS reads other fields
on it (`version`, `fallback_sequence`, `fallback_locktime`) to decide
which branch to take — the caller's choices on those fields must be in
place before AFS runs so its decisions stay consistent with them. So the
two arguments have intentionally different ownership models: Selection
flows through the chain by value, PsbtParams is mutated in place. The
docstring explains the asymmetry.
All call sites updated:
- Tests rebind via `let selection = selection.apply_anti_fee_sniping(...)?`
before subsequent `.inputs` access or `.create_psbt(params)?` calls.
- Example uses the same rebind shape (continues to read `selection.inputs`
after for the offset-reporting loop).
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Pre-compute the per-input confirmations vector before AFS so the subsequent `create_psbt` call can be chained directly off the `apply_anti_fee_sniping?` return value. Confirmations are static (based on `input.status.height` vs `tip_height`) so lifting the lookup out is safe. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…sbt" This reverts commit f87f88c.
Clone `selection.inputs` upfront so the subsequent offset-reporting loop can still look up `confirmations(tip_height)` per input after `selection` has been consumed by the chain. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…eBranch The previous name was misleading after the recent change to force the sequence branch when the effective lock_time is time-based: a time-based lock_time on its own is no longer an error. The error now only fires when *both* branches are unavailable — time-based effective lock_time blocks the locktime branch AND the sequence branch is also ineligible (non-taproot input, unconfirmed input, v1, etc.). `NoApplicableBranch(LockTime)` describes the situation; the docstring spells out the eligibility conditions and the remedies (remove the time-based locktime, switch to taproot+v2+RBF inputs, or skip AFS). Test `test_anti_fee_sniping_time_based_locktime_and_non_taproot_errors` renamed to `test_anti_fee_sniping_no_applicable_branch`. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Moves the structural "all inputs agree on absolute-locktime unit" check
to Selector::new, which already exists as a fallible constructor that
takes &InputCandidates. Catching it here — after the caller has had a
chance to use InputCandidates::filter to narrow the pool — means the
"filter out the time-based-CLTV subset" workflow remains expressible:
a wallet with mixed locktime UTXOs can put them all into
InputCandidates, filter to a single-unit subset, and proceed.
API changes:
- New `SelectorError::LockTypeMismatch` variant. Selector::new returns
this when the supplied InputCandidates have absolute timelocks of
mixed units.
- `Selection` is now `#[non_exhaustive]`. External crates can't bypass
Selector::new by struct-literal-constructing `Selection { inputs,
outputs }`. All public paths to a Selection route through Selector,
so the invariant is preserved downstream.
- `CreatePsbtError::LockTypeMismatch` removed.
- `AntiFeeSnipingError::LockTypeMismatch` removed; only
`NoApplicableBranch(LockTime)` remains.
- `Selection::accumulate_max_locktime` returns `absolute::LockTime`
directly (no Result); a `debug_assert!` documents the invariant.
- `InputCandidates::new` stays infallible. Per the design discussion:
validating only once, at the boundary that actually matters for
Selection construction, avoids two-place divergence and keeps the
filter workflow expressible.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
evanlinjin
force-pushed
the
claude/review-plan-MzzcO
branch
from
207e7ac to
d788278
Compare
The previous name implied "use this when nothing else applies," but the field is actually a *floor* on `tx.lock_time`: when input CLTVs and the fallback share a unit, `accumulate_max_locktime` returns `max(input_CLTVs, fallback)`. So the fallback wins whenever it's higher than all input requirements. `min_locktime` captures the semantic honestly, removes the asymmetry with `fallback_sequence` (which IS a per-input default), and aligns with how `Selection::apply_anti_fee_sniping` uses it (as the AFS locktime write target — a floor, not a fallback). The docstring spells out the contract: - Used directly when no input requires an absolute locktime. - Acts as a floor when all input CLTVs of the same unit are below it. - Ignored when its unit differs from input-required CLTVs (so a height-based default doesn't conflict with a time-based CLTV). Tests `test_fallback_locktime_height` and `test_fallback_locktime_respects_lock_type` renamed accordingly. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…mping `min_version` is a floor on `tx.version`. Two stages can bump it above the caller's value: 1. `Selection::create_psbt`: if any input has `relative_timelock` (`Some(...)`), the spending script uses CSV (BIP112), which fails the consensus check unless `tx.version >= 2`. `tx.version` is computed as `max(params.min_version, V2-if-CSV-required)`. 2. `Selection::apply_anti_fee_sniping`: if AFS picks the sequence branch (BIP68 semantics, requires v2), `params.min_version` is bumped to `Version::TWO`. The locktime branch never bumps version. The rename makes this honest. Previously we'd argued AFS shouldn't silently mutate the user's explicit `version` choice — that argument no longer applies once the field's name says "minimum": bumping above the floor is the documented contract, not a silent override. v1 has no modern wallet use case worth protecting against. The `must_use_locktime` chain in AFS drops its `version < V2` clause — AFS now narrows to locktime only via `preserve_existing_locktime` or the per-input eligibility checks, not via version. Closes the v1-with-CSV-input gap I flagged: under the old design, a user could build a Selection of CSV-locked inputs at v1 and get an unspendable PSBT. Now `create_psbt` bumps automatically. Test `test_anti_fee_sniping_v1_forces_locktime_branch` → `test_anti_fee_sniping_v1_min_version_is_bumped_opportunistically`, which asserts both branches eventually fire across iterations: sequence branch bumps `min_version` to V2, locktime branch leaves it at V1. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…hash_type The tx-level `params.sighash_type` field was mostly dead weight: its default of `None` covered the SIGHASH_ALL case (the 99% common one), and the only callers who'd want to override usually want **per-input** control (atomic swaps, coordinated txs, ANYONECANPAY workflows). It also had an awkward asymmetry: it only applied to Plan-variant inputs, while PsbtInput-variant inputs carried their own sighash in `psbt::Input::sighash_type`. Replaces the tx-level knob with per-input methods on `Input`: - `Input::sighash_type() -> Option<psbt::PsbtSighashType>` — reads from the Plan variant's stored override or the PsbtInput variant's embedded PSBT sighash, whichever applies. - `Input::set_sighash_type(Option<PsbtSighashType>)` — writes to whichever variant. Symmetric with `Input::set_sequence`. `Selection::create_psbt` now reads `plan_input.sighash_type()` instead of `params.sighash_type` when filling the PSBT input. The default behavior is unchanged for callers that didn't set the field — `None` propagates and the signer uses SIGHASH_ALL. `PlanOrPsbtInput::Plan` gained a `sighash: Option<psbt::PsbtSighashType>` field (None default), mirroring `sequence_override`. New test `test_set_sighash_type` covers the round-trip. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
A new `TxTemplate` type sits between `Selection` and `Psbt`: a
fully-resolved tx shape (concrete `version`, `lock_time`, per-input
sequences) without the PSBT envelope overhead. This separates
tx-construction concerns from PSBT-specific concerns.
Benefits:
- AFS lives on `TxTemplate` as a single-self method, no `&mut params`.
- `TxTemplate::into_tx()` lets non-PSBT consumers grab the raw
transaction directly.
- Future validated mutators (`set_version`, `set_locktime`, etc.) have a
natural home with explicit invariant checks.
- `PsbtParams` splits into:
- `TemplateParams { min_version, min_locktime, fallback_sequence }`
— floors and defaults resolved at template construction.
- `PsbtBuildParams { mandate_full_tx_for_segwit_v0 }` — PSBT-specific
only.
- `Selection` is no longer responsible for PSBT construction. Its only
remaining job is to be a validated input/output pool produced by
`Selector`, which `into_template` consumes.
API:
- `Selection::into_template(TemplateParams) -> TxTemplate` resolves
floors, applies `fallback_sequence` to inputs that lack one, and
bumps `tx.version` to V2 if any input requires CSV.
- `TxTemplate::create_psbt(&self, PsbtBuildParams) -> Result<Psbt, _>`
(non-consuming so `into_finalizer` can be called next).
- `TxTemplate::into_tx() -> bitcoin::Transaction`.
- `TxTemplate::into_finalizer() -> Finalizer`.
- `TxTemplate::apply_anti_fee_sniping(self, tip, rng) -> Result<Self, _>`
— single-`self` method, no `&mut params`. Bumps `template.version`
in the sequence branch, writes `template.lock_time` in the locktime
branch, sets one input's sequence via `Input::set_sequence`.
Removed:
- `PsbtParams` (replaced by the two split types).
- `Selection::create_psbt` (moved to `TxTemplate::create_psbt`).
- `Selection::apply_anti_fee_sniping` (moved to `TxTemplate`).
- `CreatePsbtError` on `Selection` — re-defined on `TxTemplate`, no
longer carries `LockTypeMismatch` (still enforced by `Selector::new`).
`Selection::into_finalizer` stays as a convenience for users who don't
need the template (it just takes the inputs and produces a Finalizer).
`Selection::accumulate_max_locktime` stays as `pub(crate)` and is used
by `into_template`.
The chain reads:
let psbt = selection
.into_template(TemplateParams::default())
.apply_anti_fee_sniping(tip, &mut rng)?
.create_psbt(PsbtBuildParams::default())?;
Or for both psbt + finalizer:
let template = selection.into_template(TemplateParams::default());
let psbt = template.create_psbt(PsbtBuildParams::default())?;
let finalizer = template.into_finalizer();
Or for the no-PSBT path:
let tx = selection.into_template(TemplateParams::default()).into_tx();
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Add opt-in shuffle methods to `TxTemplate` for input and output reordering.
The order in which a coin selector picks inputs can leak information about
the wallet (selection algorithm, UTXO freshness), and the position of the
change output is a strong heuristic for tx analysis. Bitcoin Core shuffles
both by default; this library leaves the choice to the caller.
Implementation: in-place Fisher–Yates using only `rand_core::RngCore`
(no runtime `rand` dep). Rejection sampling for unbiased index selection.
Allocation-free.
Three unit tests cover: multiset preservation, eventual permutation across
iterations, and trivial-length cases (empty / single-element).
Usage:
let psbt = selection
.into_template(TemplateParams::default())
.shuffle_inputs(&mut rng)
.shuffle_outputs(&mut rng)
.apply_anti_fee_sniping(tip, &mut rng)?
.create_psbt(PsbtBuildParams::default())?;
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
`Finalizer` is now `Default`-constructible empty, and `Selection` /
`TxTemplate` each expose a `populate_finalizer(self, &mut Finalizer) ->
Self` method that adds their `(outpoint, plan)` pairs to the finalizer
without consuming the source. The source is returned by value so it
chains cleanly into further construction stages.
This solves the awkward "need both psbt AND finalizer from the same
template" problem that previously required breaking the chain into
three named bindings. Now:
let mut finalizer = Finalizer::default();
let mut psbt = selection
.into_template(TemplateParams::default())
.populate_finalizer(&mut finalizer)
.create_psbt(PsbtBuildParams::default())?;
API changes:
- `Finalizer::default()` (derive `Default`).
- `Finalizer::insert(&mut self, OutPoint, Plan)` public method for
population.
- `Selection::populate_finalizer(self, &mut Finalizer) -> Self`.
- `TxTemplate::populate_finalizer(self, &mut Finalizer) -> Self`.
- `Selection::into_finalizer` and `TxTemplate::into_finalizer` removed.
One way to do it; callers that want the old shape can do
`let mut f = Finalizer::default(); selection.populate_finalizer(&mut f);`.
PsbtInput-variant inputs (which already carry their own satisfaction
data in `psbt::Input`) contribute nothing to the finalizer — the
filter on `input.plan()` ensures only plan-backed inputs are added.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
External callers previously had direct mutable access to fields on
`Selection { pub inputs, pub outputs }` and
`TxTemplate { pub version, pub lock_time, pub inputs, pub outputs }`,
which let them bypass invariants enforced elsewhere:
- `Selection`: `selection.inputs.push(some_input)` after `Selector::new`
would skip the locktime-unit consistency check.
- `TxTemplate`: `template.version = V1` after AFS picked the sequence
branch would silently break BIP68 semantics. Direct
`template.inputs[i].sequence_override = ...` could bypass
`Input::set_sequence`'s relative-timelock validation. Setting
`template.lock_time` to a different unit than input CLTVs would
produce an unbuildable tx.
Fields are now `pub(crate)`. External callers read via:
- `Selection::inputs() -> &[Input]`
- `Selection::outputs() -> &[Output]`
- `TxTemplate::version() -> Version`
- `TxTemplate::lock_time() -> LockTime`
- `TxTemplate::inputs() -> &[Input]`
- `TxTemplate::outputs() -> &[Output]`
Mutation is only possible via the validated methods we already expose
(`apply_anti_fee_sniping`, `shuffle_inputs`, `shuffle_outputs`,
`populate_finalizer`, `create_psbt`, `into_tx`, `into_template`).
Internal code (tests, AFS, shuffle implementations) keeps direct
field access via `pub(crate)`.
Examples updated to use `selection.inputs().to_vec()` and
`selection.inputs().iter()` instead of `selection.inputs.clone()` and
`selection.inputs.iter()`.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
Per-input validated mutators for the two `Input` fields that can legitimately be changed after template construction: - `set_input_sequence(self, index, Sequence) -> Result<Self, SetSequenceError>` delegates to `Input::set_sequence`, which validates the new value against the input's existing relative-timelock requirement. - `set_input_sighash(self, index, Option<PsbtSighashType>) -> Self` delegates to `Input::set_sighash_type`. No validation needed. Both consume `self` and return it for chain composition. Both panic on out-of-bounds index (same as direct slice indexing). Use cases: - Atomic swaps / coordinated txs setting per-input ANYONECANPAY or SIGHASH_NONE / SIGHASH_SINGLE. - Manual relative-timelock signaling beyond what AFS does. These were previously unreachable from outside the crate because `TxTemplate.inputs` is `pub(crate)`. https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
…sbtFinalizer)
Two related changes:
1. `Finalizer` → `PsbtFinalizer`. With `TxTemplate::into_tx()` now a
first-class path, the library has two consumption branches. Naming
the finalizer for its PSBT-specificity signals which branch it
belongs to.
2. `TxTemplate::create_psbt` now consumes `self` and returns
`Result<(Psbt, PsbtFinalizer), CreatePsbtError>`. The finalizer is
built during PSBT construction (we already walk every input and
know which have plans), so this is essentially free.
Callers who only want the PSBT: `let (psbt, _) = ...?;`. The chain
guarantees the finalizer is paired with the PSBT it goes with — no
risk of finalizing the wrong PSBT.
Removed surface (now unreachable / unneeded):
- `PsbtFinalizer::default` (no longer `Default`).
- `PsbtFinalizer::new`.
- `PsbtFinalizer::insert`.
- `Selection::populate_finalizer`.
- `TxTemplate::populate_finalizer`.
`PsbtFinalizer` is now obtained only via `TxTemplate::create_psbt` —
single creation path enforces that PSBT and finalizer come from the
same template.
The chain reads:
let (mut psbt, finalizer) = selection
.into_template(TemplateParams::default())
.apply_anti_fee_sniping(tip, &mut rng)?
.create_psbt(PsbtBuildParams::default())?;
// psbt and finalizer are now structurally tied.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
evanlinjin
force-pushed
the
claude/review-plan-MzzcO
branch
from
6a3a788 to
4528a32
Compare
… → TxTemplateParams
File name + type name now match `TxTemplate` itself, consistent with the
`PsbtFinalizer` / `psbt_finalizer.rs` pattern in the sibling module.
Removes the residual asymmetry where the module/type was named by topic
("template") rather than by type ("tx_template" / `TxTemplate`).
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ
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
2 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.
AFS now lives behind a dedicated
apply_anti_fee_snipingfree function anda
Selection::apply_anti_fee_snipingwrapper, both taking an explicittip_height: absolute::Height.create_psbtis rng-free and deterministic.Breaking changes:
PsbtParams::enable_anti_fee_snipingremoved.Selection::create_psbt_with_rngremoved;create_psbtno longer takesan rng and is no longer
#[cfg(feature = "std")]-gated.CreatePsbtError::InvalidLockTimeandCreatePsbtError::UnsupportedVersionremoved; replaced by
AntiFeeSnipingError.New surface:
AntiFeeSnipingError { UnsupportedVersion, TimeBasedLocktime, TipBelowExistingLocktime }— the latter two enforce that AFS cannotsilently violate a time-based or future-block CLTV requirement.
apply_anti_fee_sniping(tx, inputs, tip_height, rng)re-exported fromthe crate root; lives in
src/anti_fee_sniping.rs.Selection::apply_anti_fee_sniping(self, psbt, tip_height, rng) -> Psbt.Behavior fix:
post-accumulate
tx.lock_time, fixing inflated confirmation counts whenany input requires a CLTV above tip.
>= existing tx.lock_time,preserving any input-required CLTV through the random offset.
tx.lock_time— is forced off whentx.lock_timeis non-zero, so AFS never erases an input-required CLTV.Other:
randdropped from runtime[dependencies]; the lib usesrand_coreonly.
randis now a dev-dependency for tests/examples.src/utils.rsremoved (its only contents were the AFS function and itsRNG helpers, all moved into
src/anti_fee_sniping.rs).sendandsendallbitcoin/bitcoin#28944 for the analogous Bitcoin Core split betweenconstruction and finishing.
Migration:
// before
let psbt = selection.create_psbt(PsbtParams {
fallback_locktime: LockTime::from_consensus(tip),
enable_anti_fee_sniping: true,
..Default::default()
})?;
// after
let psbt = selection.create_psbt(PsbtParams {
fallback_locktime: LockTime::from_consensus(tip),
..Default::default()
})?;
let psbt = selection.apply_anti_fee_sniping(
psbt,
Height::from_consensus(tip)?,
&mut rng,
)?;
Refs #4, #5.
https://claude.ai/code/session_01RzD32JgjJCTtsHs4sRELXJ