Skip to content

Fix ColdCard Q PSBT signature validation error#488

Open
harrshita123 wants to merge 3 commits intocaravan-bitcoin:mainfrom
harrshita123:fix-coldcard-psbt-signatures
Open

Fix ColdCard Q PSBT signature validation error#488
harrshita123 wants to merge 3 commits intocaravan-bitcoin:mainfrom
harrshita123:fix-coldcard-psbt-signatures

Conversation

@harrshita123
Copy link
Copy Markdown

@harrshita123 harrshita123 commented Feb 24, 2026

What kind of change does this PR introduce?

Bugfix

Issue Number:

Fixes #377

Snapshots/Videos:

(Verified with E2E suite in Docker; 11/11 tests passing)

If relevant, did you update the documentation?

N/A

Summary

Fixed the signature validation error reported when uploading ColdCard Q PSBTs.

The issue stems from validateMultisigPsbtSignature relying strictly on bip32Derivation metadata. ColdCard Q sometimes strips these derivations from the signed PSBT, causing the validator to return an error even if the signature itself is valid.

Added a fallback to check the public keys directly from the redeemScript/witnessScript when derivation info is missing. I also updated SignatureImporter to provide more specific error messages to help distinguish between metadata issues and actual cryptographic failures.

Does this PR introduce a breaking change?

No

Checklist

  • I have tested my changes thoroughly.
  • I have added or updated tests to cover my changes (if applicable).
  • I have verified that test coverage meets or exceeds 95% (if applicable).
  • I have run the test suite locally, and all tests pass.
  • I have written tests for all new changes/features
  • I have followed the project's coding style and conventions.
  • I have created a changeset to document my changes (npm run changeset)

Other information

Added a regression test in psbt.test.ts that verifies validation passes even after stripping derivation data from a PSBT.

Have you read the contributing guide?

Yes

image

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Feb 24, 2026

🦋 Changeset detected

Latest commit: afcbe3c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@caravan/psbt Patch
caravan-coordinator Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 24, 2026

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

Project Deployment Actions Updated (UTC)
caravan-coordinator Ready Ready Preview, Comment Apr 28, 2026 7:06pm

Request Review

}
}
}
} catch (e) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's not silently catch and swallow errors

@harrshita123
Copy link
Copy Markdown
Author

@bucko13
Fixed. I have added a console.warn to log the parsing error if the fallback fails.
Thanks for the feedback!

Copy link
Copy Markdown
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

thanks for adding this! interesting that the ColdCard Q "sometimes" behaves differently.

We'll need to add a changest for the package to bump the version as well as reverting the swallowing of the thrown error to make sure that gets surfaced. otherwise this should be good to go.

@harrshita123
Copy link
Copy Markdown
Author

Thanks for the feedback! I've removed the try/catch block to let the errors surface properly and added a changeset for the package bump. The CI should now be green.

bucko13
bucko13 previously approved these changes Mar 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been inactive for 30 days and has been marked as stale. It will be closed in 7 days if no further activity occurs. To keep this PR open, add the "long-lived" label or comment on it.

@github-actions github-actions Bot added the stale label Apr 24, 2026
@harrshita123
Copy link
Copy Markdown
Author

Fixed the CI issues. I've updated the PR ,please take another look when you get a chance.

@harrshita123
Copy link
Copy Markdown
Author

I fixed the CI issue . The failure was due to Prettier formatting in SignatureImporter.jsx after adding the new validation error handling. I formatted the file, verified the changed files with Prettier, checked coordinator lint, and confirmed the changeset status.

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.

Error uploading signature to caravan

2 participants