Skip to content

refactor(spv): isolate btcd script classification#4165

Open
mswilkison wants to merge 1 commit into
mainfrom
fix/spv-remove-btcd-dependency
Open

refactor(spv): isolate btcd script classification#4165
mswilkison wants to merge 1 commit into
mainfrom
fix/spv-remove-btcd-dependency

Conversation

@mswilkison

@mswilkison mswilkison commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace pkg/maintainer/spv's direct txscript classification with the existing bitcoin.GetScriptType abstraction
  • preserve P2PKH, P2WPKH, P2SH, and P2WSH handling
  • remove the last direct btcd import from pkg/maintainer/spv

Testing

  • go test ./pkg/bitcoin
  • go test ./pkg/maintainer/spv
  • go test -count=1 ./pkg/maintainer/...

Fixes #3677

Summary by CodeRabbit

  • Refactor
    • Updated transaction input classification to use the application’s Bitcoin script type handling.
    • Preserved existing detection and validation behavior for main and deposit inputs.
    • Removed an unnecessary underlying script-processing dependency.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a2d8091b-2af9-42d5-9b18-edf390bf8127

📥 Commits

Reviewing files that changed from the base of the PR and between 038b7ce and 74ffd53.

📒 Files selected for processing (1)
  • pkg/maintainer/spv/deposit_sweep.go

📝 Walkthrough

Walkthrough

The deposit sweep parser now classifies transaction inputs through bitcoin.GetScriptType and bitcoin script constants, removing its direct btcd/txscript dependency while preserving existing main-UTXO and deposit-input handling.

Changes

SPV script classification

Layer / File(s) Summary
Use bitcoin script types
pkg/maintainer/spv/deposit_sweep.go
Replaces txscript checks with bitcoin script-type checks for P2PKH/P2WPKH main UTXOs and P2SH/P2WSH deposit inputs, and removes the unused import.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving SPV script classification away from direct btcd usage.
Linked Issues check ✅ Passed The change removes direct btcd usage from pkg/maintainer/spv and routes script classification through pkg/bitcoin as required.
Out of Scope Changes check ✅ Passed The PR is narrowly scoped to the btcd dependency refactor in pkg/maintainer/spv with no evident unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/spv-remove-btcd-dependency

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mswilkison

Copy link
Copy Markdown
Contributor Author

CI note: I reran the non-required client-integration-test after its initial failure. The rerun failed for the same shared external-infrastructure reason as PR #4167: all 30 failures are fulcrum_tcp cases receiving dial tcp 193.30.123.70:50001: connect: connection refused; every other package passed. This endpoint is not touched by this patch, and the matching result on an unrelated PR confirms this is not a regression here. Rerun job: https://github.com/threshold-network/keep-core/actions/runs/29531102639/job/87752372643

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.

Package pkg/maintainer/spv should not depend on btcd directly

1 participant