388 - Add more motivation, and links to miniscript BIP

bigspider commented at 8:37 am on July 11, 2024: contributor

One important aspect of wallet policies is how they help preventing pubkey reuse. I think it’s worth explaining it in some detail in the motivation section.

Also adding links to BIP-379, and fixing a nit.

Strictly speaking, one might want to formally specify all the miniscript fragments in the grammar specification; however, that would require listing all the miniscript fragments, which doesn’t seem useful. As the grammar for miniscript rules are somewhat obvious, I think a simple link to BIP-379 is more useful in practice.

Add paragraph on key reuse 3fd971455a

Add references to the miniscript BIP-379 8c2f54d33b

Nit: it's not 'two' descriptors if one uses the multipath expressions per BIP-389 0adf7c36e1

in bip-0388.mediawiki:17 in 0adf7c36e1 outdated

13@@ -14,7 +14,7 @@
14 
15 == Abstract ==
16 
17-Wallet policies build on top of output script descriptors to represent the types of descriptors that are typically used to represent "accounts" in a software wallet, or a hardware signing device, in a compact, reviewable way. A wallet policy always represents exactly two descriptors, which produce the receive and change addresses that are logically part of the same account.
18+Wallet policies build on top of output script descriptors to represent the types of descriptors that are typically used to represent "accounts" in a software wallet, or a hardware signing device, in a compact, reviewable way. A wallet policy always represents descriptors which produce all the receive and change addresses that are logically part of the same account.

jonatack commented at 5:48 pm on July 11, 2024:

0Wallet policies build on top of output script descriptors to represent the types of descriptors that are typically used to represent "accounts" in a software wallet, or a hardware signing device, in a compact, reviewable way. A wallet policy always represents descriptors, which produce all the receive and change addresses that are logically part of the same account.

Grammar nit, s/ which/, which/, as this word is always preceded by a comma.

bigspider commented at 7:34 am on July 12, 2024:

I think this slightly changes the meaning, as here the part that the “which” is introducing is not optional for the meaning of the sentence, as it’s qualifying what descriptors. See for example here.

Perhaps the following is more readable?

A wallet policy always represents descriptors producing all the receive and change addresses that are logically part of the same account.

murchandamus commented at 4:12 pm on July 16, 2024:

The first sentence feels a bit dense. Perhaps if you separate out a definition of accounts, it becomes a bit more digestible:

0Software wallets and hardware signing devices sequester wallet uses into logically separate "accounts".
1Wallet policies build on top of output script descriptors to represent such accounts in a compact, reviewable way.
2An account encompasses a logical group of receive and change addresses, and each wallet policy represents all descriptors necessary to describe an account in its entirety.

jonatack commented at 6:56 pm on July 11, 2024: member

Approach ACK

in bip-0388.mediawiki:68 in 0adf7c36e1 outdated

63+
64+Reusing keys across different UTXOs harms user privacy by allowing external parties to link these UTXOs to the same entity once they are spent.
65+
66+By constraining the derivation path patterns to have a uniform structure, wallet policies prevent key reuse among the same or different UTXOs of the same account.
67+
68+Using distinct public keys obtained from hardened derivation paths guarantees that no key reuse can happen also across accounts, and is strongly recommended. However, wallet policies do not mandate hardened derivation paths for the public keys, in order to maintain compatibility with existing deployments that do not adhere to this recommendation.

murchandamus commented at 5:26 pm on July 16, 2024:

This paragraph could perhaps be simplified a bit:

0It is strongly recommended to avoid key reuse across accounts. Distinct public keys per account can be guaranteed per hardened derivation paths. This specification does not mandate hardened derivation to maintain compatibility with existing deployments that do not adhere to this recommendation.

murchandamus commented at 5:28 pm on July 16, 2024: contributor

Looks good to me, have a couple suggestions.

Apply suggestions from code review

Co-authored-by: Mark "Murch" Erhardt <murch@murch.one>

0b3c79c257

bigspider commented at 8:20 am on July 17, 2024: contributor

Thanks @murchandamus! I committed your suggestions, and I believe the changes also answer @jonatack’s concern above.

murchandamus approved

murchandamus commented at 12:52 pm on July 17, 2024: contributor

LGTM

in bip-0388.mediawiki:17 in 0b3c79c257 outdated

13@@ -14,7 +14,9 @@
14 
15 == Abstract ==
16 
17-Wallet policies build on top of output script descriptors to represent the types of descriptors that are typically used to represent "accounts" in a software wallet, or a hardware signing device, in a compact, reviewable way. A wallet policy always represents descriptors which produce all the receive and change addresses that are logically part of the same account.
18+Software wallets and hardware signing devices sequester wallet uses into logically separate "accounts".

jonatack commented at 7:54 pm on July 24, 2024:

“sequester” can have the (generally negative) meaning to remove or withdraw into solitude or retirement, to banish, to exile

suggestion:

Software wallets and hardware signing devices typically enable separating wallet uses into different "accounts".

bigspider commented at 5:45 pm on July 27, 2024:

Good point on ‘sequester’; I used ‘partition the funds’ in the rewrite, preferring to refer specifically to “funds” rather than the more generic “wallet uses”.

I spent quite some time trying to reformulate the sentence, never feeling fully happy with it. I think the reason is that the while the goal of the accounts (= keeping funds partitioned) was stated, the practical effect (= designing a UX that abstracts away UTXOs, matching the user’s expectation) was not stated at all in the entire abstract. So in c2655e0 I added a new sentence to this effect.

in bip-0388.mediawiki:70 in 0b3c79c257 outdated

66@@ -65,7 +67,7 @@ Reusing keys across different UTXOs harms user privacy by allowing external part
67 
68 By constraining the derivation path patterns to have a uniform structure, wallet policies prevent key reuse among the same or different UTXOs of the same account.
69 
70-Using distinct public keys obtained from hardened derivation paths guarantees that no key reuse can happen also across accounts, and is strongly recommended. However, wallet policies do not mandate hardened derivation paths for the public keys, in order to maintain compatibility with existing deployments that do not adhere to this recommendation.
71+It is strongly recommended to avoid key reuse across accounts. Distinct public keys per account can be guaranteed per hardened derivation paths. This specification does not mandate hardened derivation to maintain compatibility with existing deployments that do not adhere to this recommendation.

jonatack commented at 7:58 pm on July 24, 2024:

(In general, an active voice can be easier to read and more direct than a heavy passive voice.)

Perhaps s/per/by using/ and s/derivation to/derivation in order to

bigspider commented at 5:48 pm on July 27, 2024:

Accepted the suggestion in c2655e0, and I added the repeated word ‘distinct’ to the derivation paths, as ‘hardened’ is of course not enough if one uses the exact same derivation path. Please let me know if you think I should reformulate in order to avoid repeating the word, but actually my impression is that it helps clarity - especially for readers who are not fully familiar with the details of BIP-32.

murchandamus commented at 5:29 pm on July 30, 2024:

I would say that the compartmentalization of different uses of the wallet’s funds, i.e. the not mixing of different purposes is the main purpose and could be highlighted more. Abstracting away the UTXOs feels like an orthogonal goal that would apply to a wallet with a single account the same way.

bigspider commented at 3:32 pm on August 3, 2024:

You can only securely abstract away the UTXOs (getting the UX flows people are used to) if you properly segregate funds across accounts (that is, the signing device guarantees that you always know exactly how much you’re spending from each account).

That wasn’t much of a concern for single-sig accounts because malware tricking you into spending from another of your accounts is not an interesting attack.

jonatack added the label Proposed BIP modification on Jul 26, 2024

More adjustments from PR review c2655e0ab9

in bip-0388.mediawiki:17 in c2655e0ab9 outdated

13@@ -14,7 +14,10 @@
14 
15 == Abstract ==
16 
17-Wallet policies build on top of output script descriptors to represent the types of descriptors that are typically used to represent "accounts" in a software wallet, or a hardware signing device, in a compact, reviewable way. A wallet policy always represents exactly two descriptors, which produce the receive and change addresses that are logically part of the same account.
18+Software wallets and hardware signing devices typically partition funds into separate "accounts". When signing or visualizing transactions, this allows to show to the user aggregate in-flow or out-flow information for one or more involved accounts.

murchandamus commented at 5:34 pm on July 30, 2024:

0Software wallets and hardware signing devices typically feature "accounts" as a means to partition funds and transactions by purpose. This partitioning is also useful to show aggregate in-flow and out-flow information for the involved accounts when signing or visualizing transactions.

bigspider commented at 3:05 pm on August 3, 2024:

I don’t quite agree with the rewrite:

“accounts”/wallet policies logically partition funds based on the spending conditions; therefore, it’s mostly the ownership rules that is semantically represented in the “account”. While purpose has of course a lot of overlap with the ownership rules, I don’t think it’s the appropriate word to mention in this first paragraph.
wallet policies / accounts partition funds, but don’t quite partition transactions. That might seem the case today for most wallets, but just because they only allows creating transactions spending from (and receiving change to) a single account. More sophisticated wallets could (and hopefully will) allow creating transactions that involve multiple accounts, including with different spending conditions or with different owners.

jonatack commented at 3:05 pm on August 7, 2024:

s/this allows to show to the user/this allows displaying to the user/

I’m not sure this section is an improvement over the current BIP (this may account for why @murchandamus and I keep trying to rewrite it :). Here’s another attempt:

0Software wallets and hardware signing devices typically allow the user to partition funds into separate "accounts". When signing or visualizing a transaction, aggregate flows of all accounts affected by the transaction may (and should) be displayed to the user.

bigspider commented at 1:11 pm on August 14, 2024:

Accepted most of it in 1811613; only left out the allow the user to part as it’s generally not the user’s choice, but how (almost?) all software wallet structure funds.

in bip-0388.mediawiki:65 in c2655e0ab9 outdated

59@@ -57,6 +60,18 @@ The hardware signing device must guarantee that the user knows precisely what "p
60 
61 This makes it impossible for an attacker to surreptitiously modify the policy, therefore stealing or burning the user's funds.
62 
63+==== Avoiding key reuse ====
64+
65+Reusing public keys within a Script is a source of malleability when using miniscript policies, which has potential security implications.

murchandamus commented at 5:35 pm on July 30, 2024:

Capital-S Script usually refers to the Script language, perhaps go lowercase here:

0Reusing public keys within a script is a source of malleability when using miniscript policies, which has potential security implications.

murchandamus approved

murchandamus commented at 5:43 pm on July 30, 2024: contributor

Looks good to me, and please feel free to ignore my suggestions if you feel that it’s ready as is, but it seems to me that the actual isolation of funds and transactions across accounts is obfuscated by highlighting the partition’s use in showing account changes.

Happy to merge either way.

Nit from PR review

Co-authored-by: Mark "Murch" Erhardt <murch@murch.one>

00fbea5dcd

murchandamus approved

murchandamus commented at 5:29 pm on August 5, 2024: contributor

Seems ready to merge. I will merge this tomorrow, unless more review comments appear that indicate otherwise.

in bip-0388.mediawiki:73 in 00fbea5dcd outdated

68+
69+By constraining the derivation path patterns to have a uniform structure, wallet policies prevent key reuse among the same or different UTXOs of the same account.
70+
71+It is strongly recommended to avoid key reuse across accounts. Distinct public keys per account can be guaranteed by using distinct hardened derivation paths. This specification does not mandate hardened derivation in order to maintain compatibility with existing deployments that do not adhere to this recommendation.
72+
73+It is out of scope for this document to guarantee that users do not reuse extended public keys among different wallet accounts. This responsibility is left to the users and their software wallet.

jonatack commented at 3:10 pm on August 7, 2024:

Unless the idea was to remove this mention, perhaps keep it.

0It is out of scope for this document to guarantee that users do not reuse extended public keys among different wallet accounts. This is still very important, but responsibility is left to the users and their software wallet.

bigspider commented at 1:13 pm on August 14, 2024:

Added in 1811613, plus an additional the.

jonatack commented at 3:22 pm on August 7, 2024: member

Had another look.

Further improvements from PR review 1811613e07

murchandamus requested review from jonatack on Aug 19, 2024

murchandamus approved

murchandamus merged this on Aug 21, 2024

murchandamus closed this on Aug 21, 2024

murchandamus commented at 4:43 pm on August 21, 2024: contributor

Looks to me like @jonatack’s comments were addressed, so I went ahead and merged this.

bigspider deleted the branch on Aug 22, 2024

jonatack commented at 10:19 pm on August 22, 2024: member

Seems fine. Would these changes merit a changelog entry?

bigspider commented at 7:26 am on August 23, 2024: contributor

Seems fine. Would these changes merit a changelog entry?

Since it’s only cosmetic/explanatory changes (not affecting the specs), IMHO it’s not worth tracking.

388 - Add more motivation, and links to miniscript BIP #1644