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

descriptor: Implement wallet-policies BIP #369

Merged
merged 25 commits into from
Sep 25, 2023
Merged

Conversation

jgriffiths
Copy link
Contributor

@jgriffiths jgriffiths commented Feb 13, 2023

See bitcoin/bips#1389 for details of wallet policies.

Note wally implements the /* extension to support non-BIP44 wallets such as Blockstream Green. Additionally, we do not limit allowable policies to the whitelist in the linked BIP. We allow all valid descriptors where the solved cardinality is either 1 or 2.

Includes some extra functionality that is useful for validating descriptors/policies, primarily the ability to iterate and query the keys in the parsed expression.

See bitcoin/bips#1389

Policies are a restriction on the general variable substitution
mechanism that wally already supports, combined with a shorthand
notation for path derivation.

1  - Key variables can only be named @n where n is an integer
2  - Key variable names must increase sequentially from 0 to n
3  - Key variables names must be followed by '/*', '/**', or '/<m;n>/*'
4  - Key variables can only be serialized BIP32 public keys without paths
5  - All key expressions to substitute must be unique
6  - Al least one key expression must be present
7  - Key variables must appear in the policy in order from 0 to n
     (back-references are allowed for repeated keys)
8  - All key expressions in a policy must be in the form of Key variables
9  - All key expression must share the same solved cardinality (keys
     using '/*', cannot be mixed with those using '/**' or '/<m;n>/*')
10 - The solved cardinality of a policy must be 1 or 2 (e.g. no combo())`.
11 - All repeated references to the same key must use distinct
     derivations.

This initial change implements and tests points 1-4.

This implementation will ignore the whitelisted expression lists given
in the BIP, and instead accept any valid descriptor that doesn't have a
solved cardinality greater than 2. See the above linked github PR discussion
for the rationale behind this decision.
Point 5 of the policy key requirements.
Point 6 of the policy key requirements.
Point 7 of the policy key requirements.
…ions

Point 8 of the policy key requirements.
Point 9 of the policy key requirements.
@jgriffiths jgriffiths force-pushed the wallet_policies branch 2 times, most recently from f0d8121 to 76defcd Compare September 7, 2023 20:01
Also align and re-use the context feature flags for node features (so we
can later expose the node features to callers).
This is required e.g. to differentiate x-only keys.
Also fix and test a few edge cases in ensuring substitution invariants.

The relevant BIP makes this mandatory, however given it is somewhat
expensive to verify, and the security concerns mentioned seem to only
relate to miniscript malleation via old tx signatures, make it opt-in.

It would be nice to support ensuring this for non-policy miniscript
descriptors. But, given the combination of possible descriptor key types
and path expressions I do not believe it is feasible for any implementation
to do this correctly without exactly the other limits that policies
enforce. A general solution would have to solve all possible paths and derive
all combinations of allowed keys which is not trivial to prove correct and
extremely compute intensive and thus not feasible on HWW.
@jgriffiths jgriffiths changed the title WIP: descriptor: Implement wallet-policies BIP descriptor: Implement wallet-policies BIP Sep 12, 2023
@jgriffiths jgriffiths merged commit 7446283 into master Sep 25, 2023
4 checks passed
@jgriffiths jgriffiths deleted the wallet_policies branch September 25, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants