Skip to content

Multisig/watchonly wallet transaction creation flow proof of concept#5743

Merged
NicolasDorier merged 28 commits into
btcpayserver:masterfrom
Kukks:multisig
Dec 10, 2024
Merged

Multisig/watchonly wallet transaction creation flow proof of concept#5743
NicolasDorier merged 28 commits into
btcpayserver:masterfrom
Kukks:multisig

Conversation

@Kukks
Copy link
Copy Markdown
Member

@Kukks Kukks commented Feb 8, 2024

This PR introduces a new concept, "pending transactions" (working title 💀). The idea is that if you have a watch only/multsig wallet, if you need to create a transaction, you currently have to

  • Wait until you have access to your (or all) private keys and hope you remember all the details of the transaction
  • Hand over a psbt to various parties manually to sign

With this PR, you can create a transaction as usual, but instead of signing you "create a pending transaction". A pending transaction is essentially a PSBT that fleshes out what coins to spend and where to spend them, and collects signatures as signers provide them. Once there are enough signatures, you can broadcast.

If the coins being spent by a pending transaction are spent elsewhere before the signatures are fully collected, the pending transaction is "invalidated".
If the transaction was signed out of band, using other means, BTCPay Server will detect it and mark it complete anyway.

With this system, a payout processor can create pending transactions out of payouts awaiting payments, and mark them as in progress. We can make the system send emails when a pending transaction is created to alert signing parties to come online and sign. If more payouts are added to the awaiting payment list, and the previous pending transaction did not collect any signatures, we can cancel that pending transaction, and create a fresh pending transaction with all the new payouts too, reducing the number of transactions needed.

Essentially, this system enables functionality that was deemed to be only possible for hot wallets.

There is also an optional expiry so that pending transactions can expire (as the fees are determined at the time of the transaction creation, you do not want the pending transaction to be there for too long and avoid the need of another fee bump transaction).

Currently, pending transactions show at the top of the transactions list.

@rockstardev
Copy link
Copy Markdown
Member

rebased branch on latest master, will take over this PR

@rockstardev rockstardev force-pushed the multisig branch 2 times, most recently from 3f04e86 to 92a10a0 Compare November 5, 2024 06:59
Comment thread BTCPayServer/Controllers/UIVaultController.cs Outdated
Comment thread BTCPayServer/Controllers/UIVaultController.cs
@rockstardev
Copy link
Copy Markdown
Member

So after a bit of time, we're finally at the point where this PR can be merged. All the added functionality is now hidden behind IsMultiSigOnServer switch so it will not have an impact on legacy wallets and settings. Here is new feature in action:

multisig.on.server.in.action.mp4

The video showcases full flow on mainnet with using different Jade hardware wallets to sign PSBT 2 times.

@rockstardev rockstardev marked this pull request as ready for review November 18, 2024 21:52
public string[] OutpointsUsed { get; set; }

[NotMapped][Obsolete("Use Blob2 instead")]
public byte[] Blob { get; set; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove this, there is no point in having this column... this column is present in some table just because I couldn't really migrate it easily.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It can't be removed since IHasBlob interface depends on it. That's why it's [NotMapped] so it doesn't go in the database. We need to first remove it from underlying interface.

Comment thread BTCPayServer.Data/Data/PendingTransaction.cs Outdated
Comment thread BTCPayServer/Controllers/UIVaultController.cs Outdated
Comment thread BTCPayServer/Controllers/UIVaultController.cs Outdated
Comment thread BTCPayServer/Controllers/UIVaultController.cs Outdated
Comment thread BTCPayServer/Controllers/UIVaultController.cs Outdated
Comment thread BTCPayServer/Controllers/UIWalletsController.cs Outdated
Comment thread BTCPayServer/DerivationSchemeSettings.cs Outdated
Comment thread BTCPayServer/DerivationSchemeSettings.cs Outdated
Comment thread BTCPayServer/Services/Wallets/BTCPayWalletProvider.cs Outdated
@rockstardev
Copy link
Copy Markdown
Member

@NicolasDorier addressed all the items you've brought up, take a look and let me know how it looks now.

@pavlenex pavlenex added this to the 2.1.0 milestone Nov 28, 2024
@NicolasDorier NicolasDorier merged commit b797cc9 into btcpayserver:master Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants