Skip to content

feat: signing of a message working for ledger and trezor#238

Open
krrish-sehgal wants to merge 7 commits intocaravan-bitcoin:mainfrom
krrish-sehgal:message-signing
Open

feat: signing of a message working for ledger and trezor#238
krrish-sehgal wants to merge 7 commits intocaravan-bitcoin:mainfrom
krrish-sehgal:message-signing

Conversation

@krrish-sehgal
Copy link
Contributor

@krrish-sehgal krrish-sehgal commented Apr 10, 2025

This fixes #113

  1. Added the sign message option in UI
  2. Once the message is signed , the signature is returned by caravan-wallet api and is verified with the address using bip322-js
  3. The library bip322-js has been shifted to package level.
  4. Changeset switched to minor.

Demo:
Screenshot 2025-04-10 at 2 40 32 PM

Screen.Recording.2025-04-10.at.2.55.15.PM.mov

@changeset-bot
Copy link

changeset-bot bot commented Apr 10, 2025

🦋 Changeset detected

Latest commit: c7e8928

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

This PR includes changesets to release 1 package
Name Type
caravan-coordinator Minor

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

vercel bot commented Apr 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
caravan-coordinator ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2025 9:27am

@krrish-sehgal krrish-sehgal changed the title fix: signing of a message working for ledger and tenzor feat: signing of a message working for ledger and tenzor Apr 10, 2025
@krrish-sehgal
Copy link
Contributor Author

krrish-sehgal commented Apr 10, 2025

Note :

  1. I wasnt able to test it with actual wallets, let me know if there's a way to do that ( I just have jade and seedsigner)
    However I made sure the functions running are working with each other just fine
    (used : https://github.com/LedgerHQ/ledger-live/tree/develop/libs/ledgerjs/packages/hw-app-btc#signmessage)

@krrish-sehgal krrish-sehgal changed the title feat: signing of a message working for ledger and tenzor feat: signing of a message working for ledger and trezor Apr 10, 2025
@Legend101Zz
Copy link
Contributor

@krrish-sehgal sorry for my lack of knowledge here but just wanted to confirm if this was this done for the QR-project or it's seperate ?

@krrish-sehgal
Copy link
Contributor Author

@Legend101Zz , Its separate.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2025

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.

@krrish-sehgal
Copy link
Contributor Author

@bucko13 , making a comment here to mark it for review.

@bucko13
Copy link
Contributor

bucko13 commented Jun 10, 2025

Thanks for the ping!

@tomunchained
Copy link

tomunchained commented Jun 12, 2025

Hello @krrish-sehgal, thank you for working on this. I'd like to help test this and provide feedback. Fair warning: I'm not a software engineer and have limited experience in GitHub in general.

First: I've loaded up the most recent vercel deployment, and tried to test with a redeem script, but I'm getting an error This keystore is not supported when exporting public keys. I get this if I try Trezor or Ledger, either option. Do you know what might be causing this?

Screenshot 2025-06-12 at 5 52 23 PM

Second: Am I correct that this message signing feature is currently limited to the script explorer section of Caravan? Ideally I'd like to see it subtly built into the addresses within a wallet. For example, shown as an option "sign message" alongside "confirm on device" as shown in screenshot below. (Functionally, "sign message" could be built into the wallet itself, or maybe the button just takes the user to the script explorer and automatically loads in the redeem script. Either way, it will be more friendly to users who don't understand they'd need to manually copy the redeem script and take it to the script explorer.)

Screenshot 2025-06-12 at 5 57 56 PM

Third: I'd like to make sure this project includes all of the necessary features to be practically useful. The ultimate purpose is to demonstrate control over the funds in an address to a third party. Therefore, in a k-of-n multisig quorum, the message will often be signed with k number of keys, not just one key. Does BIP322 allow for multiple signatures to be grouped together like this? (I think so, but my understanding is fuzzy.)

Once k number of keys have signed, there also needs to be a verification tool for the third party. The third party should be able to input the address, the message, and the signature (I don't think they need the redeem script, as it is built into the signature, if BIP322 works like how I understand BIP137 to work). Where this verification tool should exist, I'm not sure. The script explorer doesn't feel like the right place, if the third party is not required to enter a redeem script. Therefore I'm tempted to consider an entirely new menu item, called "sign/verify message" where all of this behavior could live, rather than within the script explorer.

Screenshot 2025-06-12 at 6 28 46 PM

This is how Trezor builds it for BIP137:

Screenshot 2025-06-12 at 6 33 09 PM Screenshot 2025-06-12 at 6 33 17 PM

Happy to hear your thoughts and discuss this more going forward!

@github-actions
Copy link
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.

@krrish-sehgal
Copy link
Contributor Author

needs some attention by me.
So doesn't need to be closed just yet.

@Legend101Zz
Copy link
Contributor

needs some attention by me. So doesn't need to be closed just yet.

great I'll add the long lived tag

@Legend101Zz Legend101Zz added long-lived qr-code-interactions related to adding support for qr-based interactions and removed qr-code-interactions related to adding support for qr-based interactions labels Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add "sign message" feature

4 participants