Skip to content
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

[Feature]: Improved SignPSBT method #286

Open
JakeFernandes98 opened this issue Dec 9, 2024 · 4 comments
Open

[Feature]: Improved SignPSBT method #286

JakeFernandes98 opened this issue Dec 9, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@JakeFernandes98
Copy link

Is your feature request related to a problem?
No

Describe the solution you'd like
Currently signPSBT takes in a psbt hex, with no control over which input(s) to sign. The sats connect library gives a good overview on how this could be designed.
https://docs.xverse.app/sats-connect/bitcoin-methods/signpsbt

Additional context
Working on a runes marketplace, the feature is critical to the implementation of rune buy/sell systems.

@JakeFernandes98 JakeFernandes98 added the enhancement New feature or request label Dec 9, 2024
@hathbanger hathbanger self-assigned this Dec 9, 2024
@hathbanger
Copy link
Contributor

yo @JakeFernandes98 dope! this looks like a good idea.
only thing that's tricky is that not every supported wallet offers this functionality. so perhaps, for the wallets that who don't allow for input index specification, we'll just toss out anything passed in, and hope the wallet can it out in the psbt?

lemme ask, how would that affect the buy sell systems if the wallet doesn't provide such specificity?

@JakeFernandes98
Copy link
Author

@hathbanger

  1. I think tossing out inputsToSign may end up confusing developers expecting the argument to apply across all wallets. Perhaps it should be split out to a different signPsbt method? (e.g. signPsbtWithInputs)

Side note: I know you're currently working on signMultipleTransactions/signPsbts which suffers from the same issue (some wallets support some don't). Perhaps implementing some 'Capability' object to each wallet and relevant methods (getCapabilities, hasCapability) may help developers navigate these disparities with wallets. We have something like this in our custom implementation.

  1. Based on my understanding, the issue arises when you have a PSBT with inputs from different wallets. By not being able to specify which input to sign, I imagine it tries to sign everything (I have only used wallets which support the sats-connect standard until now). On our marketplace you would have one or more inputs for runes (signed by the seller) and one or more inputs for BTC (signed by the buyer) and we specify the index of the inputs when presenting the PSBT to each user.

@hathbanger
Copy link
Contributor

hathbanger commented Dec 11, 2024

@JakeFernandes98 so as it stands, for the wallets that support it, we track down the inputs needed to sign during the signature. here's an example:

https://github.com/omnisat/lasereyes-mono/blob/main/packages/lasereyes-core/src/client/providers/xverse.ts#L266-L292

@JakeFernandes98
Copy link
Author

This should work in principle. Will test and come back to you

@hathbanger hathbanger removed their assignment Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants