Skip to content

Refactor and test psbt::derive_sp #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

nymius
Copy link
Collaborator

@nymius nymius commented Jul 16, 2025

Description

Splits psbt::derive_sp in small logical units and adds unit tests.

Fixes #8 .

Checklists

  • I've signed all my commits
  • I followed the conventional commit guidelines
  • I ran just p (fmt, clippy and test) before committing
  • I've added tests for the new feature

@nymius nymius self-assigned this Jul 16, 2025
@nymius nymius added this to the bdk-sp 0.1.0 milestone Jul 16, 2025
nymius added 12 commits July 17, 2025 15:40
…e alias

Add supporting types for derive_sp refactoring:
- SpkWithSecret type alias for (ScriptBuf, SecretKey) tuple
- DataForPartialSecret struct to encapsulate input collection data
- Default implementation for DataForPartialSecret

No functional changes; preparation for function extraction.
Previously, PSBT `get_key` errors weren’t captured in `SpSendError`,
making failed key lookups hard to diagnose. This commit introduces a new
`KeyError` variant to represent any failure from the `K: GetKey`
provider and updates `SpSendError`’s `Display` impl with a generic error
message.

Note: the `GetKey` error type lacks a `Display` implementation, so the
underlying error can’t be shown; `KeyError` serves as a marker for
private‐key retrieval failures.
Introduce two new variants to the SpSendError enum:
- MissingDerivations: For cases where the PSBT lacks required key
  derivations.
- MissingOutputs: For scenarios where output counts do not match
  expectations.

These errors enable more precise handling of derivation and output
validation issues in the send process, without altering existing
behavior. This is a pure addition to improve error granularity for
future refactors and fixes.

No breaking changes; existing code remains unaffected.
The create_silentpayment_scriptpubkeys API no longer returns a Result
since it cannot fail on its own.

- create_silentpayment_scriptpubkeys never produces an error internally,
  so returning Result was misleading
- eliminates needless panic points and clarifies that failure cannot
  occur at this stage
- streamlines code paths and makes error propagation more consistent
  elsewhere
Extract prevout script retrieval logic into a standalone function. This
separates the logic for getting script_pubkey from either witness_utxo
or non_witness_utxo into its own function without changing behavior.

No changes in derive_sp logic.
Extract taproot private key derivation logic into a standalone function.
This separates the taproot key handling including parity adjustment and
tap tweaking into its own function.
…e_sp

Extract non-taproot private key derivation logic into a standalone
function.
This separates the BIP32 and public key-based key retrieval logic.
Extract transaction input building logic into a standalone function.

This isolates the logic for constructing a complete TxIn with witness
from PSBT input data.
Extract input data collection logic into a standalone function.

This separates the logic for gathering scripts with secrets and finding
the lexicographically minimum outpoint.
Extract PSBT output update logic into a standalone function.
This separates the logic to update PSBT outputs with silent payment
data.
Replace inline logic in derive_sp with calls to extracted functions.
This completes the refactoring by using collect_input_data and
update_outputs.
@nymius nymius force-pushed the refactor/psbt-derive-sp branch from 4a49ab0 to 0662119 Compare July 17, 2025 19:42
@nymius nymius force-pushed the refactor/psbt-derive-sp branch from 0662119 to 8e43b8d Compare July 17, 2025 22:07
@macgyver13
Copy link

Reviewed and tested 👍 - LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit tests for bdk_sp::send::psbt::derive_sp and dependency "stack"
2 participants