feat: introduce TxTemplate as an intermediate stage#73
Open
evanlinjin wants to merge 7 commits into
Open
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
27084ae to
e9c2962
Compare
4 tasks
Pure rename — same struct, same methods, same parameters. No behaviour change. The next commit adds the resolved tx-shape fields (version, lock_time, fallback_sequence), the corresponding setters, and the PSBT/AFS pipeline that consumes them. Selection -> TxTemplate Selection::new -> TxTemplate::from_parts (still pub(crate)) IntoSelectionError -> IntoTxTemplateError InputCandidates::into_selection -> into_tx_template Selector::try_finalize() -> Option<TxTemplate> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes bitcoindevkit#57. TxTemplate now owns the resolved tx-shape fields and the methods that mutate them. The selector hands you a TxTemplate already configured with sensible defaults; everything else is method calls on it. New fields on TxTemplate: - version (default V2) - lock_time (= max(input CLTV) or ZERO) - fallback_sequence (default ENABLE_RBF_NO_LOCKTIME) New setters with validation: - set_version -> SetVersionError::RelativeTimelockRequiresV2 - set_locktime -> SetLockTimeError::{BelowInputCltv, UnitMismatch} - set_fallback_sequence The PSBT/AFS pipeline is restructured around these fields: - PsbtParams -> PsbtBuildParams (PSBT-only knobs; version/locktime /AFS removed) - CreatePsbtError -> BuildPsbtError - create_psbt(params) -> (Psbt, Finalizer) (was just Psbt) - anti-fee-sniping moves off PsbtParams::anti_fee_sniping into TxTemplate::apply_anti_fee_sniping(tip, &mut rng), a separate chainable step that composes the public set_locktime / Input::set_sequence - to_unsigned_tx() materializes the tx for non-PSBT signing flows Chain ergonomics: sort_inputs_by / shuffle_inputs (etc.) now consume self and return Self. into_finalizer is dropped — Finalizer comes from create_psbt or from Finalizer::new for callers that want it standalone. What was previously silent is now an explicit error: - min_locktime of the wrong unit was silently ignored - min_locktime below an input's CLTV was silently clamped up Both now error via SetLockTimeError. Setting v < 2 with a relative- timelock input errors via SetVersionError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
create_psbt no longer needs an RNG (AFS — the only consumer — takes its own rng explicitly), so the create_psbt_with_rng wrapper and its thread_rng() call were dead weight. Collapses both into a single create_psbt(self, params) and moves rand to dev-dependencies. The library now depends only on rand_core (for the RngCore trait) + miniscript + bdk_coin_select. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e9c2962 to
de8bfc5
Compare
5 tasks
d07102f to
1521510
Compare
Settle on the "build" verb so the method, params, and error type agree: create_psbt -> build_psbt, PsbtBuildParams -> BuildPsbtParams (also fixing the word order). Move BuildPsbtParams/BuildPsbtError into a new build_psbt module; the build_psbt method stays inherent on TxTemplate since it touches private fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fee-sniping apply_anti_fee_sniping now consumes the template and returns a SealedTxTemplate exposing only reads + emission, so version/locktime/sequence/ordering can't be changed after AFS. TxTemplate wraps SealedTxTemplate and derefs to it for the shared read/emit surface; the free afs helper mutates &mut self in place and the method does the sealing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Provide selection_algorithm_single_random_draw, which shuffles candidates with a caller-supplied rng and selects until the target is met. Pass it to Selector::select_with_algorithm so randomness lives at the selection step and candidate construction stays deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Incremental builders to add an Input to the must-select group or as an optional can-select group, with outpoint de-duplication. Lets callers extend an InputCandidates after construction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
78bb570 to
0f1c8b2
Compare
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
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.
Description
Closes #57.
The transaction-building API had grown awkward, and a revisit traced the awkwardness back to how anti-fee-sniping (AFS) was bolted on. The concrete symptoms:
create_psbtmeant everyone had to reckon with two extra error variants and an extra input, whether or not they used the feature.PsbtParamswas doing two jobs. It carried both bitcoin-transaction-shape fields (version,min_locktime, sequence) and PSBT-specific emission options. Because AFS depended on those tx-shape fields, it was structurally pinned inside the PSBT step and couldn't move out.The right place for AFS is before PSBT creation — it decides tx fields, then those feed into emission. That mirrors Bitcoin Core, where
DiscourageFeeSnipingruns as a step before the transaction is built. But AFS couldn't be moved without first untangling the tx-shape fields fromPsbtParams— i.e. an architectural change, not a local one.This PR introduces that change: a
TxTemplatestage that sits betweenSelectorandPsbtand owns the fully-resolved tx shape (version, locktime, fallback sequence, per-input sequence, ordering).What this buys us:
TxTemplate;BuildPsbtParamsis now emission-only. AFS becomes one chainable step onTxTemplate(apply_anti_fee_sniping) that just drives the same public setters anyone else would — it is no longer privileged.apply_anti_fee_snipingconsumes theTxTemplateand returns aSealedTxTemplatethat exposes only reads and emission — no setters. The type system now guarantees nobody changes the version/locktime/sequence/ordering after AFS has pinned them.TxTemplateDerefs toSealedTxTemplate, so the shared read/emit surface isn't duplicated, and the two-type split maps cleanly onto an FFI boundary (where a type-state generic could not).TxTemplateandSealedTxTemplatecan emit abitcoin::Transactiondirectly, and PSBT v2 support (#48) now has an obvious home as a futurebuild_psbt_v2()— same construction logic, different emit step.A secondary win: with
build_psbtno longer needing an RNG, the library drops itsranddependency (onlyrand_core'sRngCoretrait remains).Notes to the reviewers
Commits (each is a self-contained, reviewable step):
refactor!: rename Selection to TxTemplate— pure rename, no behaviour change. Sets up commit 2.feat(tx-template)!: route tx-shape decisions through TxTemplate— the meat. Adds the resolved tx-shape fields and their validating setters, splitsPsbtParams→ emission-only params, makes PSBT emission return(Psbt, Finalizer), and turns AFS into a chainable pre-PSBT step. Closes IntroduceTxTemplateas a state betweenSelectionandPsbt#57.refactor!: drop rand dependency from the library— movesrandto dev-deps now that emission no longer needs it.refactor: rename create_psbt to build_psbt and extract its types— settles on the "build" verb so the method, params, and error agree (build_psbt/BuildPsbtParams/BuildPsbtError), and moves those types into their ownbuild_psbtmodule.feat(tx-template)!: seal TxTemplate into SealedTxTemplate after anti-fee-sniping— makes AFS terminal.apply_anti_fee_snipingnow consumes the template and returns a mutator-freeSealedTxTemplate;TxTemplatewraps it and derefs for the read/emit surface.feat: Add single_random_draw selection algorithm— aselection_algorithm_single_random_drawthat shuffles candidates with a caller-supplied RNG and selects until the target is met; randomness lives at the selection step so candidate construction stays deterministic.feat: Add InputCandidates::push_must_select / push_can_select— incremental builders to extend anInputCandidatesafter construction (with outpoint de-duplication).Two implementation points worth a look:
apply_anti_fee_sniping(&mut TxTemplate, …)), and the publicTxTemplate::apply_anti_fee_snipingmethod does the sealing. The helper drivesset_locktime_in_place/Input::set_sequenceand.expect()s the results. The expects hold by construction: the locktime path only fires whenafs_locktime >= current(both height-based, since AFS early-rejects time-based inputs), so the unit + ≥-CLTV checks always pass; the sequence path only touches taproot inputs without a relative timelock, so the CSV/CLTV-disable check can't fail. Worth a second pair of eyes on whether those invariants really are total.TxTemplate::from_partsispub(crate)— external users construct viaSelector/InputCandidates. If hand-rolling aTxTemplatebecomes a common ask, exposing it is a one-line change.Out of scope:
no_std. Blocked on abdk_coin_selectrelease: fix: add conditional FloatExt import for no_std builds coin-select#37 is merged but unreleased. Follow-up once0.4.2lands.Changelog notice
SelectiontoTxTemplate, now the single workspace for transaction shaping (version, locktime, fallback sequence, per-input sequence, ordering, anti-fee-sniping, emission).Selector::try_finalizenow returnsOption<TxTemplate>;InputCandidates::into_selectionis nowinto_tx_template, returningResult<TxTemplate, IntoTxTemplateError>(wasIntoSelectionError).PsbtParamsintoBuildPsbtParams(emission-only). Tx-shape options moved toTxTemplatesetters:set_version,set_locktime,set_fallback_sequence,apply_anti_fee_sniping.apply_anti_fee_snipingconsumes theTxTemplateand returns a newSealedTxTemplatethat exposes only reads and emission, so the tx shape cannot be mutated after AFS.TxTemplatederefs toSealedTxTemplate.build_psbt(renamed fromcreate_psbt) and returns(Psbt, Finalizer); its params/error areBuildPsbtParams/BuildPsbtError.shuffle_inputsis now consuming (returnsSelf).min_locktimewas silently ignored and a below-CLTV value silently clamped;set_locktimenow returnsSetLockTimeError::UnitMismatch/BelowInputCltv, andset_versionreturnsSetVersionError::RelativeTimelockRequiresV2. The hardcodedENABLE_RBF_NO_LOCKTIMEfallback sequence is now configurable viaset_fallback_sequence(default unchanged).selection_algorithm_single_random_draw(single-random-draw coin selection, RNG supplied at the selection step) andInputCandidates::push_must_select/push_can_selectfor extending candidates after construction.rand(onlyrand_core, for theRngCoretrait).Before submitting