SelectorParams mempool policy#47
Open
aagbotemi wants to merge 3 commits intobitcoindevkit:masterfrom
Open
Conversation
5f132dd to
21d52e8
Compare
21d52e8 to
c98521b
Compare
noahjoeris
reviewed
Apr 28, 2026
Contributor
noahjoeris
left a comment
There was a problem hiding this comment.
I think the policy checks should be integrated more directly into the normal transaction-building flow.
Right now the caller has to remember to do a separate post-build check:
let policy = MempoolPolicy { tip_height, tip_mtp };
policy.check_all(&selection, &tx)?;This is easy for callers to forget. It also asks them to provide both a Selection and a Transaction, even though the transaction should be derived from Selection + PsbtParams. A checked path should instead construct the transaction itself and validate exactly the transaction the crate is going to return.
Suggested shape:
-
Output checks in
SelectorParamsBuilderbuild()could validate against a default policy.build_with_policy(policy)could validate against a customMempoolPolicy.- For unchecked, e.g.
build_unchecked()
-
Final transaction checks should happen in
Selection.create_psbt(...)defaults to e.g. Core v30- Add
create_psbt_with_policy(policy)andcreate_psbt_unchecked() - This avoids a public
check_all(&Selection, &Transaction)API and makesSelectionTxMismatchunnecessary
-
So I think we should have some form of abstraction/config over policy.
- No hardcoded policy values but make them part of MempoolPolicy struct.
- Also chain-tip data is an unrelated runtime input that should be passed alongside, not embedded
- Use
MempoolPolicy::bitcoin_core_v30()as the default, for example. - Users should be able to pass a different policy if their node has different relay settings.
- Then this policy can be passed into both
SelectorParamsBuilder::build_with_policy(...)andSelection::create_psbt_with_policy(...).
Let me know what you think.
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
Add a validated builder interface for
SelectorParamsthat catches dust outputs, zero-value OP_RETURNs, oversized OP_RETURN aggregates, and non-standard script types at construction time. Pair it with a post-selectionMempoolPolicy::check_allpath for transaction-level standardness checks (weight, locktime, version, min-relay-fee, input spendability, witness stack limits, non-witness size).Closes #41
Notes to the reviewers
SelectorParams::check_standardnessruns output-side checks before coin selection.MempoolPolicy::check_allruns transaction-level checks after the Selection is built.change_dust_relay_feeratetodust_relay_feerate. The field applies to all outputs (target and change), not just change.Changelog notice
SelectorParamsBuilderwith chainable setters andcheck_standardnessvalidationMempoolPolicywithcheck_alland per-rule methodsSelectorParams::change_dust_relay_feeratetodust_relay_feerate.Checklists