Disallow duplicate leaves inside tr() descriptors #27104

issue darosior openend this issue on February 15, 2023
  1. darosior commented at 2:24 pm on February 15, 2023: member

    While working on adapting Miniscript to Tapscript, i noticed signatures in Tapscript only commit to the leaf (i previously assumed they committed to the whole branch). It’s been discussed lately on the mailing list as well (1, 2). This means if the same script is used in two different leaves, a signature for one can be rebound to the other.

    If one signed a transaction it shouldn’t matter what script path it used as long as the transaction itself didn’t change [0]. However, the possibility to change the script path used may cause two issues:

    1. It messes up with Miniscript’s malleability analysis (and is a malleability vector in itself).
    2. It allows to potentially hinder the confirmation of a transaction if the two leaves aren’t at the same height (h/t aj).

    I don’t think 1. is a valid point. Miniscript’s analysis is about malleability from a third party that only gets to see a single satisfaction. The composition of the tree would not be known to the third party here, therefore they wouldn’t be able to malleate.

    This leaves 2. which is more important in the context of presigned transactions, and in this case we’d expect protocol designers to take great care when designing the descriptors used so it would most likely not be an issue.

    On the other hand it really is inexpensive to check, and AFAICT it serves no purpose to duplicate a leaf. So we could avoid a potential future footgun we didn’t anticipate for cheap? @sipa @achow101 @apoelstra @sanket1729 thoughts?

    [0] Some may argue this doesn’t always hold, while it may be valid it’s imo out of scope here.

  2. darosior commented at 2:25 pm on February 15, 2023: member
    In the case of the Bitcoin Core wallet it would be a backward incompatible change though..
  3. sipa commented at 3:17 pm on February 15, 2023: member

    I think it’d be useful to provide some mechanism to detect duplicated leaves, and possibly a way to discourage/warn about their usage. I’m not sure that outlawing such descriptors in the first place is a good idea.

    E.g. could we add a (generic) mechanism to descriptors to mark silly/dangerous/… constructions, and e.g. prevent them from being imported in wallets (except maybe with a “force” argument), or prevent them from being used in deriveaddresses. This mechanism could perhaps also be used for non-sane miniscripts.

    Outlawing them in descriptors in the first place may be a step too far, as as you note it breaks backward compatibility, but it may also interfere with (vaguely) sensible use cases, like somehow you got funds in such a “bad” descriptor-derived address… ideally nothing prevents you from spending the coins. Whatever warning/discouragement we’d want, it’s too late at that point.

    Another weaker argument is that in a post-#24114 world, duplicated leaves would be trivial to bypass by replacing one of the leaves with a rawnode(HEX) expression.

  4. fanquake commented at 2:22 pm on March 3, 2023: member
    Also cc @instagibbs (don’t see any reaction)
  5. fanquake added the label Descriptors on Mar 3, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me