doc: warn that ranged multi() descriptors are not BIP67 compatible #17023

pull Sjors wants to merge 1 commits into bitcoin:master from Sjors:2019/10/doc_descriptor_bip67_warning changing 2 files +9 −4
  1. Sjors commented at 3:06 pm on October 2, 2019: member

    This adds a quick warning to the deriveaddresses RPC help, and a more detailed explanation to descriptors.md.

    Personally I’m not a huge fan of BIP67 as it is. It results in a different ordering for each deriviation index, depending on the public keys at that index. I would prefer to order all derived addresses by BIP32 master fingerprint.

  2. [doc] warn that ranged multi() descriptors are not BIP67 compatible 3f6204c465
  3. laanwj added the label Docs on Oct 2, 2019
  4. instagibbs commented at 4:30 pm on October 2, 2019: member
    multi67() or something doesn’t sound like a bad idea to consider supporting in the future
  5. Sjors commented at 5:25 pm on October 2, 2019: member
    @sipa any thoughts on such a multi67() descriptor? It seems that at least Electrum and ColdCard use BIP67 in the wild. It would be the same as multi() but at every index the public keys are ordered canonically.
  6. fanquake requested review from sipa on Oct 4, 2019
  7. sipa commented at 6:06 pm on October 4, 2019: member

    I think adding a multi_bip67 descriptor fragment or so makes perfect sense; it’s easy to do, and composes fine with other things.

    One issue is that the script-to-descriptor inference code cannot guess multi vs multi_bip67.

  8. instagibbs commented at 6:12 pm on October 4, 2019: member
    @achow101 textually committed to trying to get an implementation out
  9. DrahtBot commented at 9:09 pm on October 4, 2019: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17056 (descriptors: Introduce sortedmulti descriptor by achow101)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  10. fanquake referenced this in commit 520d140e6e on Oct 8, 2019
  11. DrahtBot commented at 7:13 pm on October 8, 2019: member
  12. DrahtBot added the label Needs rebase on Oct 8, 2019
  13. instagibbs commented at 7:22 pm on October 8, 2019: member
    The feature got merged faster than the docs saying we don’t support it, I think? Close?
  14. Sjors commented at 8:03 pm on October 8, 2019: member

    Having a deriveaddresses RPC example could still be useful. Will rebase.

    Update: meh

  15. Sjors closed this on Oct 9, 2019

  16. laanwj removed the label Needs rebase on Oct 24, 2019
  17. jasonbcox referenced this in commit 4931365316 on Oct 8, 2020
  18. DrahtBot locked this on Dec 16, 2021


Sjors instagibbs sipa DrahtBot


sipa

Labels
Docs


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-11-21 12:12 UTC

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