Skip to content

fix(bitcoin-adapter): initialize ECC lib for Taproot PSBT finalization - #1149

Merged
pedromcunha merged 1 commit into
mainfrom
pedro/int2-1684-unisat-taproot-swaps-fail-before-broadcast
Oct 5, 2026
Merged

pedromcunha merged 1 commit into
mainfrom
pedro/int2-1684-unisat-taproot-swaps-fail-before-broadcast

Conversation

@pedromcunha

Copy link
Copy Markdown
Member

Taproot key-path finalization requires an ECC library; without it bitcoinjs-lib throws 'No ECC Library provided'. Also skip inputs the wallet already finalized, which otherwise throw 'Can not finalize input #0' (OKX autoFinalized).

Fixes INT2-1684, FE-8536

Taproot key-path finalization requires an ECC library; without it bitcoinjs-lib throws 'No ECC Library provided'. Also skip inputs the wallet already finalized, which otherwise throw 'Can not finalize input #0' (OKX autoFinalized).

Fixes INT2-1684, FE-8536
@pedromcunha
pedromcunha requested a review from ted-palmer October 5, 2026 19:48
@vercel

vercel Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
relay-sdk-demo Ready Ready Preview Oct 5, 2026 7:48pm UTC

Request Review

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @pedromcunha's task in 25s —— View job


Code Review

  • Gather context (diff, commits, full file)
  • Review for correctness, security, error handling, repo guidelines

Code Review — No issues found.

Reviewed 4 files, 27 additions / 1 deletion. Checked for correctness, security, error handling, and repo guidelines compliance.

The fix is correct and well-scoped:

  • bitcoin.initEccLib(ecc) before finalization resolves the No ECC Library provided throw for Taproot key-path inputs (@bitcoinerlab/secp256k1 is the standard ECC provider for bitcoinjs-lib v7, and its @noble/curves dependency aligns with v7's expectations).
  • Guarding each input on !input.finalScriptSig && !input.finalScriptWitness before calling finalizeInput(index) correctly skips wallet-auto-finalized inputs (OKX), avoiding the Can not finalize input #0 error, while still finalizing the ones the SDK must finalize.
  • Dependency pinned to an exact version (1.2.0), consistent with how bitcoinjs-lib is pinned in this package.
  • Changeset is reader-facing and names the exact errors and surface — matches the AGENTS.md changeset guidance.

A couple of minor, non-blocking observations (not issues):

  • bitcoin.initEccLib(ecc) is idempotent, so calling it per handleSendTransactionStep invocation is harmless; it could equally live at module scope. Current placement is fine.
  • The comment on line 65 accurately describes the behavior and is evergreen.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High risk] Adds a cryptographic dependency to Bitcoin transaction signing.

The PR appears safe to merge, with non-blocking regression coverage needed for both wallet finalization paths.

Findings

  1. P2 Finalization paths lack tests ▶
Summary

The PR adds an ECC implementation to the Bitcoin adapter and initializes it before finalization, while leaving wallet-finalized PSBT inputs untouched.

  • These changes target UniSat Taproot and OKX pre-broadcast failures.
  • Regression tests for the two changed paths are still needed.
  • Greptile automatically discovered related tickets that helped explain the purpose of this PR: restoring UniSat Taproot finalization and handling OKX wallet-finalized inputs.

Reviews (1) · Last reviewed commit: "fix(bitcoin-adapter): initialize ECC lib..."

Comment on lines +64 to +70
bitcoin.initEccLib(ecc)
// Some wallets return already-finalized inputs; re-finalizing a Taproot input throws.
signedPsbt.data.inputs.forEach((input, index) => {
if (!input.finalScriptSig && !input.finalScriptWitness) {
signedPsbt.finalizeInput(index)
}
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Finalization paths lack tests

There are no automated tests for the two paths changed here: finalizing a signed Taproot input after ECC initialization, and extracting a transaction when the wallet has already finalized an input. The adapter has no test script, and PR CI runs only the SDK tests, so a recurrence of either reported failure would go undetected. Greptile automatically discovered related tickets stating that both the UniSat Taproot and OKX finalized-input fixes need regression coverage, which informed this comment.

Source Used: Linear — UniSat Taproot swaps fail before broadcast
Source Used: Linear — OKX Taproot PSBTs fail during finalization

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @pedromcunha's task in 34s —— View job


📝 Docs Drift Check — No docs impact

This PR does not touch surfaces that require a relayprotocol/relay-docs update.

@pedromcunha
pedromcunha merged commit 06036f8 into main Oct 5, 2026
10 of 11 checks passed
@pedromcunha
pedromcunha deleted the pedro/int2-1684-unisat-taproot-swaps-fail-before-broadcast branch October 5, 2026 20:26

This branch was successfully deployed

1 active deployment
Preview — 79e53476 Deployed Oct 5, 2026 by vercel[bot]
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.

2 participants