wallet: Change default address type to bech32 #16884

pull instagibbs wants to merge 3 commits into bitcoin:master from instagibbs:bech32_default changing 11 files +16 −21
  1. instagibbs commented at 3:53 PM on September 16, 2019: member

    No description provided.

  2. emilengler commented at 4:00 PM on September 16, 2019: contributor

    Concept ACK, same is already in bitcoin-qt so this makes it just more coherent.

  3. instagibbs renamed this:
    Change default address type to bech32
    RPC: Change default address type to bech32
    on Sep 16, 2019
  4. MarcoFalke commented at 4:21 PM on September 16, 2019: member

    I think you can also revert fa5a4cd813c2f0225dcbc05cd16ae2d1c0d13234

  5. DrahtBot added the label GUI on Sep 16, 2019
  6. DrahtBot added the label RPC/REST/ZMQ on Sep 16, 2019
  7. DrahtBot added the label Tests on Sep 16, 2019
  8. DrahtBot added the label Wallet on Sep 16, 2019
  9. MarcoFalke removed the label GUI on Sep 16, 2019
  10. MarcoFalke removed the label RPC/REST/ZMQ on Sep 16, 2019
  11. MarcoFalke removed the label Tests on Sep 16, 2019
  12. MarcoFalke renamed this:
    RPC: Change default address type to bech32
    wallet: Change default address type to bech32
    on Sep 16, 2019
  13. fanquake added this to the milestone 0.20.0 on Sep 16, 2019
  14. nopara73 commented at 6:15 AM on September 17, 2019: none

    Concept ACK

  15. Sjors commented at 1:05 PM on September 17, 2019: member

    Concept ACK. Needs a release note.

  16. fanquake added the label Needs release note on Sep 17, 2019
  17. fanquake removed the label Needs release note on Sep 17, 2019
  18. DrahtBot commented at 1:42 AM on September 18, 2019: member

    <!--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:

    • #15606 ([experimental] UTXO snapshots by jamesob)

    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.

  19. in src/wallet/wallet.cpp:2905 in 30f3f5b636 outdated 
    2901 | @@ -2902,23 +2902,6 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec
2902 |          return change_type;
2903 |      }
2904 |  
2905 | -    // if m_default_address_type is legacy, use legacy address as change (even
    laanwj commented at 8:33 AM on September 18, 2019:

    I'd prefer factoring out this change from the "Change default address to bech32" commit, because it doesn't seem directly related, but maybe I'm wrong.

    instagibbs commented at 1:11 PM on September 18, 2019:

    IIRC mostly just I'm breaking tests twice and didn't want to bother, let me see if I swap around my commits if it makes it less stupid...

    instagibbs commented at 1:26 PM on September 18, 2019:

    ok, split it up with passing tests for each

  20. laanwj commented at 8:33 AM on September 18, 2019: member

    Concept ACK

  21. instagibbs force-pushed on Sep 18, 2019
  22. in test/functional/wallet_import_with_label.py:122 in 8e82acc8ad outdated 
     118 | @@ -119,7 +119,7 @@ def run_test(self):
 119 |          )
 120 |          priv_key4 = self.nodes[0].dumpprivkey(address4)
 121 |          self.nodes[1].importprivkey(priv_key4)
 122 | -        embedded_addr = self.nodes[1].getaddressinfo(address4)['embedded']['address']
 123 | +        embedded_addr = self.nodes[1].getaddressinfo(address4)["embedded"]['address']
    MarcoFalke commented at 7:42 PM on September 18, 2019:

    in commit 8e82acc8ade: Why?

    instagibbs commented at 7:50 PM on September 18, 2019:

    I had removed it, then manually reverted with my favorite quote: the double quote. fixed

    promag commented at 10:06 PM on September 18, 2019:

    my favorite quote: the double quote

    there are 10 types of people in this world..

  23. MarcoFalke approved
  24. MarcoFalke commented at 7:42 PM on September 18, 2019: member

    ACK

  25. instagibbs force-pushed on Sep 18, 2019
  26. in doc/release-notes-16884.md:1 in b4e8375f93 outdated 
       0 | @@ -0,0 +1,4 @@
   1 | +The wallet now by default uses bech32 addresses when using RPC, and creates native segwit
    promag commented at 10:09 PM on September 18, 2019:

    Drop "when using RPC"?

    instagibbs commented at 1:00 PM on September 19, 2019:

    QT already by default does bech32, so I believe this is more accurate?

  27. promag commented at 10:10 PM on September 18, 2019: member

    Concept ACK.

  28. fanquake commented at 12:40 AM on September 19, 2019: member

    Concept ACK

  29. darosior commented at 9:25 AM on September 19, 2019: member

    Concept ACK

  30. DrahtBot added the label Needs rebase on Sep 25, 2019
  31. instagibbs force-pushed on Sep 25, 2019
  32. MarcoFalke commented at 5:10 PM on September 25, 2019: member

    in commit 972142964f5549ee713a7d71e84fd7e2ce4f2bda:

    Does not compile because you forgot to remove the

    // Always fall back to bech32 in the gui

    case

  33. instagibbs force-pushed on Sep 25, 2019
  34. in src/wallet/init.cpp:39 in ce90999de8 outdated 
      35 | @@ -36,7 +36,7 @@ void WalletInit::AddWalletOptions() const
  36 |  {
  37 |      gArgs.AddArg("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(DEFAULT_ADDRESS_TYPE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  38 |      gArgs.AddArg("-avoidpartialspends", strprintf("Group outputs by address, selecting all or none, instead of selecting on a per-output basis. Privacy is improved as an address is only used once (unless someone sends to it after spending from it), but may result in slightly higher fees as suboptimal coin selection may result due to the added limitation (default: %u (always enabled for wallets with \"avoid_reuse\" enabled))", DEFAULT_AVOIDPARTIALSPENDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  39 | -    gArgs.AddArg("-changetype", "What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype, except when -addresstype=p2sh-segwit a native segwit output is used when sending to a native segwit address)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  40 | +    gArgs.AddArg("-changetype", "What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
    MarcoFalke commented at 5:55 PM on September 25, 2019:

    in commit ce90999de84f39c38bbda2e1c29fb131af2a4f4d:

    This should be part of the previous commit, no?

  35. in src/wallet/wallet.cpp:2923 in f069b9bf55 outdated 
    2918 | -            return OutputType::BECH32;
2919 | -        }
2920 | -    }
2921 | -
2922 |      // else use m_default_address_type for change
2923 |      return m_default_address_type;
    MarcoFalke commented at 5:57 PM on September 25, 2019:

    in commit f069b9bf55936d3793406f2ab1af107a6e1a40c4:

    (style-nit): Could remove CHANGE_AUTO completely. It is fine to keep, so feel free to not address this nit as it makes the diff larger.

  36. DrahtBot removed the label Needs rebase on Sep 25, 2019
  37. MarcoFalke approved
  38. MarcoFalke commented at 6:04 PM on September 25, 2019: member

    ACK b631464f54e4b826a52c04161492fec89a711866 looks good, did not compile

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK b631464f54e4b826a52c04161492fec89a711866 looks good, did not compile
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiFOQv6A3gQDyHxmmYJuYzWi4MYaCerjQ+6wUjnTgFvkDGsGo70m2yRf9zfb1Tx
vw24BOYmqq8C1eHHDl3JixPTxpYfemdu2tdyNdzM+QLJD4pzcZFGYsGlqpqu6GLO
TUV7MOIbzlBmJ1G4kIQFFeF3cJhcTeYPhQaNLqPbsGhvdpy4CDb8Mj2co7KSFVs3
kieSDVKK4VMbs52srHJel3WLNTW5HORfsTM7CrwAb0TMUn/HwjDmgPcZdZYhYZxU
0K7St/Dtf0NAYdDCxInzyXfMUkhbcE9w4hye2jbtnOIGbZH+DCfsjuaW/ITdmfxW
4HzP7dbYg5IRrY304ohKTgC6P3m+F7Qw57R7hrpo90sgdIy/m0k4E89Ifv08pP8O
bHWzPYOv560vX6zNRoVjztY6/yNwWceJ9/SX6L6otx4D28fsTJleDfEWTHIebqKk
gp71AUYwNDrSvcyvu/uayp93U1enrmS97pq1kz3eR6irRL1G6hLsCVDcgfW9Y9U2
3qR5na16
=QpM0
-----END PGP SIGNATURE-----

    Timestamp of file with hash 0979dba1d91ee3700a115a74e85034597fa657a8e983564f537c26f83c78585e -

    </details>

  39. instagibbs commented at 7:39 PM on September 26, 2019: member

    I'm going to revert my change-mimicking revert due to unpopularity on IRC.

  40. Change default address type to bech32 f50785ab56
  41. Revert "gui: Generate bech32 addresses by default (take 2, fixup)"
    This reverts commit fa5a4cd813c2f0225dcbc05cd16ae2d1c0d13234.
    b34f0180e3
  42. Add release note for bech32 by default in wallet 71d4eddf42
  43. instagibbs force-pushed on Sep 26, 2019
  44. instagibbs commented at 8:35 PM on September 26, 2019: member

    updated to reduce PR scope to just the default address option

  45. laanwj added the label Feature on Sep 30, 2019
  46. hebasto commented at 1:09 PM on September 30, 2019: member

    Concept ACK

  47. MarcoFalke removed the label Feature on Sep 30, 2019
  48. MarcoFalke added the label Feature on Sep 30, 2019
  49. MarcoFalke removed the label Feature on Sep 30, 2019
  50. MarcoFalke commented at 6:05 PM on September 30, 2019: member

    re-ACK 71d4eddf42eb5a15e434d2273f11d0a3942ef502 (only change is restore mimick behavior)

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-ACK 71d4eddf42eb5a15e434d2273f11d0a3942ef502 (only change is restore mimick behavior)
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUi42wv9E53RyaKcAWQFXqT0Tg+mZ0oi+mpCcxAUi5oQ7VYHS8tZCPyhTn0Xy3cl
yW4uf3N73WkJeRTpI5zvdqcBDm9yYKM7O33aS8kx7odWv1bOLOSu8aVqPT2P5xFq
CvZ+HpZP3HF8H7nAOVJmTX2gb6qRciPmyrbvqYsjwiG/Z5hVN4GXfGQTm9f8dE1K
rQHLOCym9MrIwRsDDRiNf08NxNwSgCqhO+eOtRo39gSmuGUj5nz9l+BsZmk8m8a5
CxqfHl4JOvwMzUjvbAmaq5r8hEQYYKiqJ09KoQn7ARsc6CquR+T086EGPArT+/Y5
RFbu/XcqGSUJsicinBaoywgrr1+xNfiRwOKW1iTviq5H54WrXTPYNhdo59Tg6HbY
yznrPkswly5eJE/dfrMEeEi2YZpM3JGhwZugN/svnBQszUa3wzMELl1rX8KWowmW
pAFXHDAD81J/XaKXj1eSzW2xF2WcnYCFcEE2/NxobUKkqzdIfqMAyP18TNuL7sUV
Uh3SQ4p5
=LUrX
-----END PGP SIGNATURE-----

    Timestamp of file with hash f7ab15c1dbf906029708de7346a7a88e7b8e07d3b53b29c8db5fbb4e4762c3f7 -

    </details>

  51. MarcoFalke commented at 7:01 PM on September 30, 2019: member

    Could add a commit to bump doc/bips.md (173)?

  52. laanwj added the label Feature on Oct 2, 2019
  53. MarcoFalke added the label Waiting for author on Oct 2, 2019
  54. MarcoFalke removed the label Waiting for author on Oct 2, 2019
  55. laanwj commented at 3:45 PM on October 2, 2019: member

    ACK 71d4eddf42eb5a15e434d2273f11d0a3942ef502 I think it's good to merge this early in the 0.20 cycle.

    Could add a commit to bump doc/bips.md (173)?

    Please do this in a follow-up PR.

  56. laanwj referenced this in commit fecc1be231 on Oct 2, 2019
  57. laanwj merged this on Oct 2, 2019
  58. laanwj closed this on Oct 2, 2019
  59. sidhujag referenced this in commit 2cd469768a on Oct 2, 2019
  60. MarkLTZ referenced this in commit bbae9dae49 on Nov 17, 2019
  61. DrahtBot locked this on Dec 16, 2021
Contributors
instagibbsemilenglerMarcoFalkenopara73SjorsDrahtBotlaanwjpromagfanquakedarosiorhebasto
Labels
FeatureWallet

Milestone
0.20.0

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-04-17 06:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me