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

Supporting multiple wallet policies in SIGN_PSBT #219

Open
kewde opened this issue Jan 23, 2024 · 3 comments
Open

Supporting multiple wallet policies in SIGN_PSBT #219

kewde opened this issue Jan 23, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@kewde
Copy link
Contributor

kewde commented Jan 23, 2024

When spending UTXOs with multiple wallet policies, you have to call SIGN_PSBT multiple times for each wallet policy. A common use case is that the user has both a Taproot (86) and Segwit (84) address and we're spending UTXOs from both.

@kewde kewde changed the title Supporting multiple wallet policies during SIGN_PSBT Supporting multiple wallet policies in SIGN_PSBT Jan 23, 2024
@bigspider
Copy link
Collaborator

That would indeed be a great idea and should happen at some point.

However, it's quite tricky to generalize the signing flow to work with multiple policies in the same transaction.

A wallet policy models an account, so spending from multiple policies makes the flow a lot more complicated, both in terms of signing logic and UX, so it's unlikely we'll have bandwidth to work on this any time soon.

For the time being, the suggestion is to suggest that users do not mix different "accounts" (and therefore, different address types) in the same transaction.

@bigspider bigspider added the enhancement New feature or request label Jan 23, 2024
@kewde
Copy link
Contributor Author

kewde commented Jan 24, 2024

@bigspider

However, it's quite tricky to generalize the signing flow to work with multiple policies in the same transaction.

Could you share why it would be tricky from a UX/engineering perspective? Or perhaps what an acceptable implementation of
this enhancement would require? This would be helpful, in case an external contributor decides to give this a go 😄

@bigspider
Copy link
Collaborator

bigspider commented Jan 26, 2024

@kewde

You can see the current signing flow (currently for a single wallet policy) in sign_psbt.c.

This is a rough summary of what would it take to generalize:

  • Figure out how to pass multiple wallet policies, instead of a single one. Currently, the wallet policy is sent as part of the SIGN_PSBT command, while you would probably want to incorporate multiple policies in the PSBT (using proprietary keys per BIP-0174, as there is no standard). Each input in the PSBT would need to specify which wallet policy they are using. We would want to add all the wallet policies in the global map, and then for each input/output a "pointer" specifying which policy is being used. As this is a change in the communication protocol, care needs to be taken to make sure it's backwards compatible, and client libraries need to be updated accordingly.
  • The signing flow currently categorizes each input/output as internal (matching the wallet policy) or 'external'. This needs to be generalized so that for each 'internal' input you also correctly match each wallet policy.
  • For each wallet policy, the flow has to keep track of how much is being spent from it (that is, compute total input amounts for that policy, and possibly subtract the matching change output amounts).
  • The concept of default wallet policies the doesn't need registration would also need to be either generalized, or excluded from this generalized signing flow.
  • UX: would need to show how much is spent for each wallet policy.

This generalized flow would be challenging to support on Nano S, as it would probably imply a somewhat increased RAM usage, which is a very scarce resource on Nano S.

I would also consider #124 a blocker for this issue, as the support for sighash flags is currently quite buggy, and figuring out how to do that correctly for the single-policy case seems wise before attempting a generalization to multiple policies.

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