SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
661 stars 152 forks source link

Feature Request: Miniscript Support #306

Open Rob1Ham opened 1 year ago

Rob1Ham commented 1 year ago

Checking through the Seedsigner library, it is built on the embit library, which already has miniscript support. So from a backend perspective the feature shouldn't require much additional code!

Miniscript enables more robust Bitcoin Scripting to users, including timelocks, hashlocks, as well as AND/OR logic. A very direct feature this enables is a decaying multisig, where a wallet can start as a 3 of 3, then if the Bitcoin hasn't moved in 6 months (or ~26,000 blocks), it become a 2 of 3 multisig, and after an additional 2 years, become a 1 of 3 multisig. The full documentation is available at https://www.miniscript.com for more details, but these details details have already been implemented in embits.

Here is a link to the relevant miniscript code: https://github.com/diybitcoinhardware/embit/blob/master/src/embit/descriptor/miniscript.py

I will concede, the UI/UX on being able to communicate involved scripts will take some thought. I'm happy to brainstorm with the team on how they would tackle this.

A user would have to compile from Policy to a Miniscript Output Descriptor outside of seed signer, but once in output descriptor form, the seed signer should have the necessary tooling with embits to enable deposit/spends from a miniscript output descriptor.

For reference there is a proposed BIP that the team at Ledger has put together, for registering a wallet. This would be a one time operation that has the user very every element of the output descriptor and other signer XPUBs, enablinig expedited future use of a given output descriptor using miniscript. I'm not sure how the team feels about this BIP applying to the Seed Signer security model, but thought I'd share it since it is relevant to more advanced output descriptors that miniscript enables.

https://github.com/bitcoin/bips/blob/fd3b84da83f5b22953ab8c8cc7c2f1239026692a/bip-wallet-policies.mediawiki

This code is in production on the Python Ledger Client as well: https://github.com/LedgerHQ/app-bitcoin-new/tree/develop/bitcoin_client/ledger_bitcoin

pythcoiner commented 1 year ago

Hi guys, i'm also interested in this topic (mainly for using seedsigner together wizardsardine liana wallet). I'm new to the project (just start my first seedsigner yesterday), so i not yet have a clear view of how to integrate this in the seedsigner process.

So if i understand well:

is that the right way to thinking how to handle that feature?

Rob1Ham commented 1 year ago

I would defer to the seed signer team around their preferred flow of execution, but here are my thoughts:

  • implement the process for import & register wallet policy => output proof of registration to be keep by wallet

The seed signer does not "keep" anything inherently. All data is required to be exported via a QR.

This introduces a UX hurdle, as today the seed signer to maintain a full airgap has a user manually generate a QR code to store your seed phrase. You'd have to replicate this flow when it came time to "register" the wallet.

Here is a reference HMAC i've used for testing with ledgers, that would need to be converted to a QR:

bbeea6345c05b74caa012451f590da6e39ac86acabd41ce30353c718e3788686

This would get encoded as a QR, and kept with a QR of the output descriptor as well, which the seedsigner would encode as a policy ID on device and check to make sure the output descriptor supplied matches the policy id, and the HMAC corresponds to the policy ID.

As a one time process, assume you are able to keep the output descriptor and corresponding HMAC secured, this would allow users to know that if they are using a seedsigner for a miniscript supported output descriptor, that the user has previously verified these contents, and don't have signings occur for unitnended policies (malicious change paths come to mind).

  • implement the process for import (miniscript psbt + proof of registration) & sign it => output signed psbt

When it came time to spend, a user would either:

pythcoiner commented 1 year ago

I would defer to the seed signer team around their preferred flow of execution, but here are my thoughts:

  • implement the process for import & register wallet policy => output proof of registration to be keep by wallet

The seed signer does not "keep" anything inherently. All data is required to be exported via a QR.

yeah, what i mean is the following:

Registration step(only once or if software wallet lost the proof):

Signature step:

kdmukai commented 1 year ago

I'm starting at zero on miniscript knowledge so y'all will have to talk me through the finer details.

@Rob1Ham seemed to imply above that the registration proof should be stored separate from the coordinator software (thus the need for a manual QR code version of it).

But @pythcoiner's bullet points just has the software store the registration proof and provide it back to SeedSigner when needed. That sounds more correct to me. The descriptor (provided by the software) plus the PK (loaded only into SeedSigner) can produce the same registration proof, yes?

And then in the later bullet points, showing the policy details before signing is just for user experience purposes / sanity checking, right? They already approved the descriptor in the registration step. So this maybe sounds like an optional UI step ("Review Policy" or "Next").

kdmukai commented 1 year ago

The user flows are solvable and it sounds like @pythcoiner is pretty much there or close to it.

The real challenge is how to display the policy to the user in a way that is human-readable AND fits on our tiny screen.

For example, simply displaying the script is NOT an acceptable SeedSigner UI:

and(pk(A),or(pk(B),or(9@pk(C),older(1000))))

Even if we tell people to externally verify the script (e.g. paste it from their software into some other miniscript decoding tool), we'd still want the user to meticulously confirm that the script they see on the SeedSigner screen is the same as the one they entered into the decoding tool. Definitely not a user-friendly task.

Nor is the compiled version acceptable:

<B> OP_CHECKSIG OP_NOTIF
  <C> OP_CHECKSIG OP_NOTIF
    <e803> OP_CHECKSEQUENCEVERIFY OP_VERIFY
  OP_ENDIF
OP_ENDIF
<A> OP_CHECKSIG

And neither format would fit well on our screen anyway.

I've mostly been picturing a visual block diagram instead. But even that can get really hard to read.

image6

Or probably more practical and straightforward: converting the policy to a human-readable sentence, probably with some formatting blocks to help readability. Something like:

Any of the following:
    * 1234abcd
    * fedc0987 after 1000 blocks
    * 84c38fa2

This would require us to have a pretty robust miniscript-to-human-UI conversion layer. Maybe that would be more or less straightforward (follow the miniscript spec, only worry about the supported options). But still sounds kind of scary. Also hard to know how robust our UI would be against edge cases (e.g. how many levels of nesting can the logic go?).

Or am I overthinking/overcomplicating this?

pythcoiner commented 1 year ago

I'm starting at zero on miniscript knowledge so y'all will have to talk me through the finer details.

in fact we don't need to care about tech stuff of miniscript, embit care for us

pythcoiner commented 1 year ago

But @pythcoiner's bullet points just has the software store the registration proof and provide it back to SeedSigner when needed. That sounds more correct to me. The descriptor (provided by the software) plus the PK (loaded only into SeedSigner) can produce the same registration proof, yes?

yes, that's already this workflow used by Liana wallet with ledger/specter DIY

pythcoiner commented 1 year ago

And then in the later bullet points, showing the policy details before signing is just for user experience purposes / sanity checking, right? They already approved the descriptor in the registration step. So this maybe sounds like an optional UI step ("Review Policy" or "Next").

it's absolutely optional for avoid friction (you need ton check your wallet policy only once), but you can check the wallet policy at each use also

pythcoiner commented 1 year ago

Or probably more practical and straightforward: converting the policy to a human-readable sentence, probably with some formatting blocks to help readability. Something like:

Any of the following:
    * 1234abcd
    * fedc0987 after 1000 blocks
    * 84c38fa2

i also think something like that, but even more simpler for user:

PMK commented 1 year ago

I've mostly been picturing a visual block diagram instead. But even that can get really hard to read.

image6

This screenshot is not taken from https://bitcoindevkit.org/bdk-cli/playground/, is it? Where did you get this from?

kdmukai commented 1 year ago

@PMK no, hadn't seen the playground before, but looks like the same concept. I just grabbed the screenshot above as an example of visual drag-and-drop coding.

pythcoiner commented 1 year ago

i made a draft of how we can merge descriptor workflow into the actual workflow:

as @kdmukai says previously the user can skip the PoR (Proof of Registration) saving process to avoid friction, but still need to review it

looking for your comments guys

graph TB
classDef add fill:#00aa00, stroke:#000000,stroke-width:4px;
classDef modify fill:#aaaa7f;

subgraph  Legend 
    direction BT

    L1[Original]
    L2[Added]
    L2:::add
    L3[modified]
    L3:::modify
end

A[ScanView] --> T1{decoder.is_seed}
T1 -- False --> T2{decoder.is_psbt}
T2 -- False --> T3{decoder.is_settings}
T3 -- False --> T4{decoder.is_descriptor}
T4 -- False --> T5{decoder.is_address}
T5 -- True --> V50[AddressVerificationStartView]
T5 -- False --> V99[NotYetImplementedView]

T1 -- True --> T10{passphrase_required}
T10 -- True --> V10[SeedAddPassphraseView]
T10 -- False --> V11[SeedFinalizeView]

T3 -- True --> V30[SettingsUpdatedView]
V30 -- any_other_button ---> V32[MainMenuView]

T4 -- True --> T40{descriptor.is_basic_multisig}
T40 -- False --> T41{descriptor.is_miniscript} -- False --> V99[NotYetImplementedView]
T41:::add -- True --> T42{"descriptor.is_registered"} 
T42:::add -- False --> V43[DescriptorRegisterView]
T42:::add -- True --> T410{"descriptor.owns(psbt)"} 

V43:::add
V43 -- save to software wallet --> V44[DescriptorDisplayView] --> AAA
V43 -- sign --> T410
T410:::add -- False --> AAA[ScanView]
T410 -- True --> V202
V44:::add

T40 -- True --> V42[MultisigWalletDescriptorView]

T2 -- True --> V20[PSBTSelectSeedView]

V20 -- "choose seed" --> V21[PSBTOverviewView]
V20 -- "12 words" --> V22[SeedMnemonicEntryView]
V20 -- "24 words" --> V22[SeedMnemonicEntryView]
V20 -- "scan seed" --> AA[ScanView]
V21 --> T20
T20 -- else --> T21{psbt_parser.change_amount} -- == 0 ---> V212[PSBTNoChangeWarningView]
T20 -- == miniscript --> T200{"controller.miniscript_descriptor"} 
T200:::add -- True --> T201{"descriptor.owns(psbt)"} 
T200 -- False --> V203[PSBTScanMiniscriptDescriptor]
V203:::add --> V2030[ScanView]

T201:::add
V202:::add
T201 -- True --> V202[DescriptorShowPolicy] --> T21
T201 -- False --> V2160[PSBTAddressVerificationFailedView]

T20{"psbt_parser.policy"} -- None ----> V211[PSBTUnsupportedScriptTypeWarningView]

T21 -- > 0  --> V213

V211  --> V214[PSBTAddressDetailsView]
V214 -- next --> V214
V214 -- display change --> V215
V212   --> V213[PSBTMathView]

V213 --> T210{psbt_parser.destination_addresses} 
T210 -- > 0 --> V214
T210 -- = 0 --> V215[PSBTChangeDetailsView]

V215 --> T22{is_change_addr_verified}
T22 -- False --> V216[PSBTAddressVerificationFailedView] ---> V0[MainMenuView]
T22 --> T220["Else"]
T220 -- Verify multisig --> V218[LoadMultisigWalletDescriptorView]
T220 -- Next --> V217[PSBTFinalizeView]
V217 -- sign success --> V220[PSBTSignedQRDisplayView] --> V0
V217 -- error --> V219[PSBTSigningErrorView] --> V200[PSBTSelectSeedView] 
pythcoiner commented 10 months ago

@kdmukai any opinion?

kdmukai commented 10 months ago

@Rob1Ham I'd love to discuss the possibilities at TABConf next week!

We'll have a Builder's Day table set up on Day 1 and will be around the rest of the week.