feat: introduce TxTemplate as an intermediate stage#73
Open
evanlinjin wants to merge 7 commits into
Open
Conversation
Member
Author
|
This is fully AI generated and not ready for review. |
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;PsbtBuildParamsis now emission-only. AFS becomes one ordinary, chainable method onTxTemplate(apply_anti_fee_sniping) that just calls the same public setters anyone else would — it is no longer privileged.TxTemplatecan emit abitcoin::Transactiondirectly, and PSBT v2 support (#48) now has an obvious home as a futureTxTemplate::create_psbt_v2()— same construction logic, different emit step.A secondary win: with
create_psbtno longer needing an RNG, the library drops itsranddependency (onlyrand_core'sRngCoretrait remains).Notes to the reviewers
Split into 5 commits:
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→PsbtBuildParams, makescreate_psbtreturn(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.feat: Add single_random_draw selection algorithm—selection_algorithm_single_random_draw, which shuffles candidates with a caller-supplied rng and selects until the target is met. Strictly speaking this is unrelated to theTxTemplaterefactor; it's here because the downstream wallet PoC (bitcoindevkit/bdk_wallet#502) needs a selection algorithm to drive, and the new "randomness lives at the selection step, candidate construction stays deterministic" split made this a natural home for it.feat: Add InputCandidates::push_must_select / push_can_select—Inputto a resolvedInputCandidates— to the must-select group, or as an optional can-select group — with outpoint de-duplication (InputCandidateswas previously construct-once vianew). Like commit 4, it's here only because the wallet PoC needs to push caller-supplied foreign inputs onto a candidate set after resolution.Both (4) and (5) are independent of the
TxTemplatechange and added purely for downstream convenience — happy to peel them into their own PR(s) if reviewers would rather keep this one focused.Two implementation points worth a look:
apply_anti_fee_snipinggoes through the publicset_locktime/Input::set_sequencepath and.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).PsbtParamsintoPsbtBuildParams(emission-only). Tx-shape options moved toTxTemplatesetters:set_version,set_locktime,set_fallback_sequence,apply_anti_fee_sniping.TxTemplate::create_psbtnow returns(Psbt, Finalizer).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).rand(onlyrand_core, for theRngCoretrait).selection_algorithm_single_random_draw, a single-random-draw coin selection algorithm for use withSelector::select_with_algorithm.InputCandidates::push_must_select/push_can_selectfor adding anInputto a resolvedInputCandidates(with outpoint de-duplication).Before submitting