Make bech32m the default for RPC, opt-in for GUI #22260

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2021/06/bech32_gui changing 8 files +22 −6
  1. Sjors commented at 3:09 PM on June 16, 2021: member

    RPC This makes bech32m the default whenever possible. For legacy wallets the default remains bech32. Same for descriptor wallets without an active Taproot descriptor.

    GUI A new wallet setting allows users to opt-in to Taproot. If they opt in, the GUI bech32 checkbox now produces either bech32 or bech32m depending on the wallet. This avoids having to explain the difference.

    <img width="591" alt="Schermafbeelding 2021-10-20 om 20 13 17" src="https://user-images.githubusercontent.com/10217/138150773-353bd519-56b2-4b24-a2a0-998778cc56ab.png">

    Considerations This lets users and developers experiment with taproot wallets. When merged before #22364, the user will have to manually import a taproot descriptor. After that PR, taproot descriptors will be present for all new wallets.

    It is better to stick to bech32 addresses for the time being, because:

    1. It is expected that it will take a while for enough wallets and exchanges to correctly handle sending to bech32m addresses
    2. Taproot savings for a single signature wallet are not huge
    3. Explaining the difference between bech32 and bech32m is not great UX

    But we don't want to prevent users and developers who do understand the difference to use Taproot. Without this PR they can only do this via the RPC (getnewaddress some_label bech32m). With this PR it's a simple checkbox.

    Changing the RPC default to bech32m seems acceptable to me, since those users will have a better understanding of the wallet. Though I can see the case against it because it might be a breaking change, e.g. if someone has an automated setup that creates new wallets (existing wallets currently don't auto upgrade to taproot, so their behavior is not changed).

    Suggested testing

    • call getnewaddress without type and getnewaddress Test bech32 / bech32m
    • create a receive address in the GUI with and without the bech32 checkbox
    1. Legacy wallet
    2. Default descriptor wallet (current does not have taproot descriptor)
    3. Wallet with taproot descriptor:
      • build #22364
      • create new descriptor wallet
      • build this PR again
  2. DrahtBot added the label Descriptors on Jun 16, 2021
  3. DrahtBot added the label GUI on Jun 16, 2021
  4. DrahtBot added the label RPC/REST/ZMQ on Jun 16, 2021
  5. DrahtBot added the label Wallet on Jun 16, 2021
  6. Sjors force-pushed on Jun 16, 2021
  7. Sjors force-pushed on Jun 16, 2021
  8. Sjors force-pushed on Jun 16, 2021
  9. Sjors cross-referenced this on Jun 16, 2021 from issue Basic Taproot signing support for descriptor wallets by sipa
  10. DrahtBot commented at 11:44 PM on June 16, 2021: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #23308 (Update basic multisig test/docs to use multipath descriptor by mjdietzx)
    • #22928 (refactor: Remove gArgs from wallet.h and wallet.cpp (2) by kiminuo)
    • #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
    • #22805 (refactor: use CWallet const shared pointers in dump{privkey,wallet} by theStack)
    • #22787 (refactor: actual immutable pointing by kallewoof)

    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.

  11. DrahtBot cross-referenced this on Jun 17, 2021 from issue Remove `gArgs` from `wallet.h` and `wallet.cpp` by kiminuo
  12. DrahtBot cross-referenced this on Jun 17, 2021 from issue wallet test: Add test for subtract fee from recipient behavior by ryanofsky
  13. DrahtBot cross-referenced this on Jun 17, 2021 from issue Add OutputType::BECH32M and related wallet support for fetching bech32m addresses by achow101
  14. DrahtBot cross-referenced this on Jun 17, 2021 from issue wallet, rpc: add an option to list private descriptors by S3RK
  15. DrahtBot cross-referenced this on Jun 17, 2021 from issue wallet, refactor: make DescriptorScriptPubKeyMan agnostic of internal flag by S3RK
  16. DrahtBot cross-referenced this on Jun 17, 2021 from issue wallet: Migrate legacy wallets to descriptor wallets by achow101
  17. Sjors force-pushed on Jun 17, 2021
  18. DrahtBot added the label Needs rebase on Jun 24, 2021
  19. Sjors force-pushed on Jun 24, 2021
  20. DrahtBot removed the label Needs rebase on Jun 24, 2021
  21. Sjors cross-referenced this on Jun 29, 2021 from issue wallet: Make a tr() descriptor by default by achow101
  22. DrahtBot added the label Needs rebase on Jul 27, 2021
  23. Sjors cross-referenced this on Jul 27, 2021 from issue v22.0 patches by Sjors
  24. michaelfolkson commented at 10:16 AM on August 2, 2021: contributor

    My understanding from discussion at they July 30th Core wallet meeting is that this will only be merged once the (super)majority of the ecosystem is able to make payments to bech32m addresses. So definitely post Taproot activation but uncertain at this stage when exactly.

    Assuming I've understood above, Concept ACK. Thanks for the instructions for testing in the PR description, will do this testing at a later date.

  25. Sjors commented at 11:37 AM on August 2, 2021: member

    @michaelfolkson note that this doesn't do anything unless there's actually a taproot descriptor in the wallet, which this PR doesn't add.

  26. Sjors force-pushed on Aug 4, 2021
  27. DrahtBot removed the label Needs rebase on Aug 4, 2021
  28. unknown approved
  30. ghost commented at 7:58 AM on August 11, 2021: none

    Not sure why I don't see QR in bitcoin-qt

    image

  31. DrahtBot cross-referenced this on Aug 24, 2021 from issue refactor: actual immutable pointing by kallewoof
  32. DrahtBot cross-referenced this on Aug 26, 2021 from issue refactor: use CWallet const shared pointers in dump{privkey,wallet} by theStack
  33. DrahtBot cross-referenced this on Aug 30, 2021 from issue descriptors: Be able to specify change and receiving in a single descriptor string by achow101
  34. in src/qt/receivecoinsdialog.cpp:154 in 2c2a8c0e29 outdated 
     153 | +            address_type = OutputType::BECH32;
 154 | +        }
 155 |      } else {
 156 | -        address_type = model->wallet().getDefaultAddressType();
 157 | -        if (address_type == OutputType::BECH32) {
 158 | +        if (address_type == OutputType::BECH32 ||address_type == OutputType::BECH32M) {
    benthecarman commented at 2:17 AM on August 31, 2021:

    spacing/formatting is weird here

  35. Sjors force-pushed on Aug 31, 2021
  36. Sjors commented at 10:31 AM on August 31, 2021: member

    Rebased. @prayank23 do you have libqrencode on your system? See https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#dependencies

  37. ghost commented at 9:53 PM on September 1, 2021: none

    libqrencode was missing. I installed it with sudo apt-get install qrencode, still QR doesn't work. Its not related to this PR and maybe something wrong with this VM. I will try to fix this.

    ACK https://github.com/bitcoin/bitcoin/commit/1ba76c2ce3446d0fd95f89bb91089c6664b29afc

  38. benthecarman commented at 10:10 PM on September 1, 2021: contributor

    libqrencode was missing. I installed it with sudo apt-get install qrencode, still QR doesn't work. Its not related to this PR and maybe something wrong with this VM. I will try to fix this.

    you probably need to reconfigure and rebuild

  39. DrahtBot cross-referenced this on Sep 9, 2021 from issue refactor: Remove `gArgs` from `wallet.h` and `wallet.cpp` (2) by kiminuo
  40. bitcoin deleted a comment on Sep 11, 2021
  41. DrahtBot cross-referenced this on Sep 29, 2021 from issue Remove `-rescan` startup parameter by meshcollider
  42. DrahtBot cross-referenced this on Oct 15, 2021 from issue tests: remove usage of LegacyScriptPubKeyMan from some wallet tests by achow101
  43. DrahtBot cross-referenced this on Oct 20, 2021 from issue Update basic multisig test/docs to use multipath descriptor by mjdietzx
  44. Sjors force-pushed on Oct 20, 2021
  45. Sjors commented at 12:17 PM on October 20, 2021: member

    Rebased.

    My understanding from discussion at they July 30th Core wallet meeting is that this will only be merged once the (super)majority of the ecosystem is able to make payments to bech32m addresses.

    I added a note in the PR description. I tend to agree it's preferable to wait a little bit until more wallets and services support sending to bech32m, otherwise we have to add another checkbox for bech32 fallback and explain all that.

  46. schildbach commented at 1:06 PM on October 20, 2021: contributor

    I wonder why this isn't already merged for Testnet and Signet (not Mainnet), both of which have Taproot activated already. It would make Taproot testing much easier.

  48. sipa commented at 5:21 PM on October 20, 2021: member

    @schildbach I don't think we can merge this until there is wide support for sending to bech32m in commonly deployed clients/services, which will probably take a while after activation even.

  49. Sjors commented at 5:40 PM on October 20, 2021: member

    I'm open to the idea of enabling this behavior on signet (and testnet) and turning it off on mainnet until a later release.

  50. Sjors renamed this:
    Make bech32m the default, except where needed. Update GUI checkbox.
    Make bech32m the default for RPC, opt-in for GUI
    on Oct 20, 2021
  51. Sjors commented at 6:38 PM on October 20, 2021: member

    I added a GUI settings checkbox to enable "taproot" (maybe needs rewording). See updated PR description.

  52. DrahtBot added the label Needs rebase on Oct 22, 2021
  53. wallet: make bech32m default address type
    Except for legacy wallets and descriptor wallets without taproot. Update the default after importing a taproot descriptor.
    7bfa223753
  54. gui: interpret bech32 checkbox as bech32m where needed d6b3f6e982
  55. Sjors force-pushed on Oct 22, 2021
  56. DrahtBot removed the label Needs rebase on Oct 22, 2021
  57. Rspigler commented at 9:54 PM on October 22, 2021: contributor

    Use Taproot -> "Generate taproot addresses for wallets that support it..." Instead, could the new GUI option only be shown to users who have updated to taproot capable wallets? Then once clicking 'Use Taproot', the GUI bech32 checkbox should auto-select and produce bech32m addresses.

  58. Sjors commented at 10:39 AM on October 23, 2021: member

    Do you mean moving it from the wallet Settings view to the Receive view? (and then only showing it for taproot capable wallets)

  59. Rspigler commented at 2:51 PM on October 23, 2021: contributor

    I didn't, but I believe that's a better idea

  60. DrahtBot added the label Needs rebase on Oct 29, 2021
  61. DrahtBot commented at 10:01 AM on October 29, 2021: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

    <sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>

  62. Sjors cross-referenced this on Oct 29, 2021 from issue Address type dropdown, add Taproot (Receive tab) by Sjors
  63. Sjors commented at 12:11 PM on October 29, 2021: member

    Closing this in favor of a GUI-only PR: https://github.com/bitcoin-core/gui/pull/459

  64. Sjors closed this on Oct 29, 2021
  65. MarcoFalke referenced this in commit 63b5dfac21 on Dec 22, 2021
  66. sidhujag referenced this in commit b960097cf4 on Dec 22, 2021
  67. RandyMcMillan referenced this in commit aaa89c92e4 on Jan 27, 2022
  68. bitcoin locked this on Oct 30, 2022
Contributors
SjorsDrahtBotmichaelfolksonghostbenthecarmanschildbachsipaRspigler
Labels
GUIWalletRPC/REST/ZMQNeeds rebaseDescriptors
Linked (view graph)
#21365 Basic Taproot signing support for descriptor wallets#22154 Add OutputType::BECH32M and related wallet support for fetching bech32m addresses#22364 wallet: Make a tr() descriptor by default
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: 2026-05-19 06:53 UTC