SelectorParams mempool policy

[aagbotemi]

Description

Add a validated builder interface for SelectorParams that catches dust
outputs, zero-value OP_RETURNs, oversized OP_RETURN aggregates, and
non-standard script types at construction time. Pair it with a
post-selection MempoolPolicy that enforces transaction-level standardness
checks (weight, locktime, version, min-relay-fee, input spendability,
witness stack limits, non-witness size), and fold that validation into PSBT construction so callers can't forget to run it.

Closes #41
Closes #49

Notes to the reviewers

• OP_RETURN policy choice. The v30 alignment question is discussed on #41. I implemented option 1 (100,000-byte aggregate cap matching Core v30 defaults).
• Two-layer split: SelectorParams::check_standardness runs output-side
  checks before coin selection. MempoolPolicy runs transaction-level
  checks after the Selection is built and the PSBT is materialized,
  invoked from inside the validated create_psbt_* methods.
• Configurable MempoolPolicy. All limits previously hardcoded
  (dust feerate, min relay feerate, OP_RETURN cap, max weight, allowed
  versions, etc.) are now public fields, with MempoolPolicy::bitcoin_core_v30()
  as the canonical preset and Default. Chain state (tip_height,
  tip_mtp) is split out into a separate ChainTip struct passed
  per-call rather than baked into the policy.
• Renamed change_dust_relay_feerate to dust_relay_feerate. The field applies to all outputs (target and change), not just change.
• Validation folded into PSBT construction.

Changelog notice

• Added SelectorParamsBuilder with chainable setters and three terminal
  methods: build (validates against MempoolPolicy::default),
  build_with_policy (validates against a caller-supplied policy), and
  build_unchecked (skips validation).
• Added MempoolPolicy with configurable limits.
• Added ChainTip for passing chain state to policy checks that need it.
• Added Selection::create_psbt_with_policy and
  Selection::create_psbt_unchecked for std callers, plus
  create_psbt_with_rng, create_psbt_with_policy_and_rng, and
  create_psbt_unchecked_with_rng for no_std. Each entry point is
  explicit about which policy (if any) validates the constructed PSBT.
• Added CreatePsbtError::Policy(MempoolPolicyError) variant with a
  From<MempoolPolicyError> conversion.
• Renamed SelectorParams::change_dust_relay_feerate to
  dust_relay_feerate. The field applies to all outputs (target and
  change), not just change.
• Removed Selection::create_psbt in favor of the explicit
  create_psbt_with_policy / create_psbt_unchecked pair. Existing
  callers should migrate to create_psbt_with_policy(params, &policy, tip)
  for validated PSBTs or create_psbt_unchecked(params) to preserve the
  prior unchecked behavior.

Checklists

• I followed the contribution guidelines
• I've added tests for the new feature
• I've added docs for the new feature
• I'm linking the issue being fixed by this PR

[noahjoeris]

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 SelectorParamsBuilder

  • build() could validate against a default policy.
  • build_with_policy(policy) could validate against a custom MempoolPolicy.
  • 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) and create_psbt_unchecked()
  • This avoids a public check_all(&Selection, &Transaction) API and makes SelectionTxMismatch unnecessary

• 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(...) and Selection::create_psbt_with_policy(...).

Let me know what you think.

[aagbotemi]

Thank you for the review. I agree with all of this. The two-layer split made sense when I wrote it, but you're right that the manual policy.check_all(&selection, &tx) step is invisible at the type level. Nothing prompts the caller to run it, and accepting both a Selection and a Transaction lets the two drift. I will fold the validation into construction and have create_psbt_with_policy build the tx it validates.

[noahjoeris]

Nice, thanks for updating 🔥

I’d wait now for a Concept ACK on this direction by @ValuedMammal before spending more time on details.

• One API point: I’d prefer to keep create_psbt(...). It could either preserve the old unchecked behavior for compatibility, or become checked by default as create_psbt(params, tip) using MempoolPolicy::default(). I think we should discuss that explicitly. Right now most examples moved to create_psbt_unchecked(...), which makes the unchecked path look like the normal path.
• Also, MempoolPolicy only models part of Core v30 policy for now, so we should expect it to grow as more checks are added.

[ValuedMammal]

I did a first pass. Overall I like the idea and it appears to be well implemented. Biggest concern is the long term API stability as policies change and grow outdated.

Pros

• Conceptually I like the separation between standardness checks and post selection checks
• Certainly in favor of consensus-level validation to catch obvious mistakes in tx construction (e.g. negative fee)
• And maybe a subset of policy/standardness checks
• Important to differentiate tx building validation, bitcoin consensus, and local node policy
• From a maintenance point of view I would take a light touch approach to policy/standardness since it will likely be biased and incomplete

Cons

• MempoolPolicy::default() is going to be a maintenance liability since the defaults will inevitably change over time
• MempoolPolicy as a name is potentially misleading. We don't guarantee that if your tx passes MempoolPolicy, it will be accepted to mempools
• NegativeFee error is good for guarding against obviously invalid txs, but not exactly a policy preference

[ValuedMammal]

We could collapse the segwit/taproot conditions to script.is_witness_program(), which should also cover pay-to-anchor?

[ValuedMammal]

Confused why the dust-feerate of the params would override the MempoolPolicy when we're supposed to be validating the specified params against a chosen policy.