Change example address from legacy (P2PKH) to bech32m (P2TR) #808

1. 
   theStack commented at 4:55 pm on March 22, 2024: contributor

   Legacy addresses are less and less common these days and not recommended to use, so it seems senseful to also reflect that in the example addresses and update to the most recent address / output type (bech32m / P2TR). Also, as I couldn’t see any value in computing these at runtime, they are pre-generated. This was done with the following Python script, executed in ./test/functional (it’s also included in the commit body, though without the she-bang):

    0#!/usr/bin/env python3
    1from test_framework.segwit_addr import CHARSET, decode_segwit_address, encode_segwit_address
    2from test_framework.messages import sha256
    3
    4output_key = sha256(b'bitcoin dummy taproot output key')
    5for network, hrp in [('mainnet', 'bc'), ('signet', 'tb'), ('testnet', 'tb'), ('regtest', 'bcrt')]:
    6    dummy_address = encode_segwit_address(hrp, 1, output_key)
    7    while decode_segwit_address(hrp, dummy_address) != (None, None):
    8        last_char = CHARSET[(CHARSET.index(dummy_address[-1]) + 1) % 32]
    9        dummy_address = dummy_address[:-1] + last_char
   10    print(f'{network:7} example address: {dummy_address}')
   

   Note that the last bech32 character is modified in order to make the checksum fail.

   master (mainnet):

   

   PR (mainnet):

   

2. 
   DrahtBot commented at 4:55 pm on March 22, 2024: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, pablomartin4btc
   Concept ACK	MarnixCroes, kristapsk

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. theStack force-pushed on Mar 22, 2024
4. 
   MarnixCroes commented at 5:41 pm on March 22, 2024: contributor

   cACK

   Using the default address type (bech32) for this makes the most sense I think, instead of the latest, no?

5. in src/qt/guiutil.cpp:124 in 960b2d401e outdated

   133+    case ChainType::TESTNET:
   134+        return "tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg";
   135+    case ChainType::REGTEST:
   136+        return "bcrt1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqsr2427";
   137+    } // no default case, so the compiler can warn about missing cases
   138+    assert(false);
   

   ---

   ---

   

   maflcko commented at 7:20 am on March 28, 2024:

   Can the !IsValidDestinationString check be kept?

   ---

   

   theStack commented at 9:24 am on March 28, 2024:

   Sure, good idea. With an Assert? (I’m always a bit unsure when to use Assert and when Assume; the old assert should rather be avoided in general, from my understanding).

   ---

   

   maflcko commented at 10:12 am on March 28, 2024:

   As it is not critical, keeping the fallback to the empty string seems fine.

   0if (Assume(!IsValidDestinationString(s))) return s;
   1return {};
   

   ---

   

   theStack commented at 10:24 am on March 28, 2024:

   Thanks, done.
6. maflcko approved
7. 
   theStack commented at 9:27 am on March 28, 2024: contributor

   @MarnixCroes

   Using the default address type (bech32) for this makes the most sense I think, instead of the latest, no?

   P2TR addresses look similar enough to P2WPKH ones (with the only difference that they are longer and start with “bc1p…” rather than “bc1q…”), so I thought it’s fine if use the most recent one already. No hard feelings though, I’m happy to change this if requested, it doesn’t take too much effort to adapt the Python script used.

8. gui: change example address from legacy (P2PKH) to bech32m (P2TR)

   The dummy addresses have been computed with the following Python script
   (executed under ./test/functional):
   
   --------------------------------------------------------------------------------------------------------
   from test_framework.segwit_addr import CHARSET, decode_segwit_address, encode_segwit_address
   from test_framework.messages import sha256
   
   output_key = sha256(b'bitcoin dummy taproot output key')
   for network, hrp in [('mainnet', 'bc'), ('signet', 'tb'), ('testnet', 'tb'), ('regtest', 'bcrt')]:
       dummy_address = encode_segwit_address(hrp, 1, output_key)
       while decode_segwit_address(hrp, dummy_address) != (None, None):
           last_char = CHARSET[(CHARSET.index(dummy_address[-1]) + 1) % 32]
           dummy_address = dummy_address[:-1] + last_char
       print(f'{network:7} example address: {dummy_address}')
   --------------------------------------------------------------------------------------------------------
   
   Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>

   c6d1b8de89
9. theStack force-pushed on Mar 28, 2024
10. 
    maflcko commented at 10:33 am on March 28, 2024: contributor

    lgtm ACK c6d1b8de89d87fe4fd171dc85557299e429e6564
11. DrahtBot requested review from MarnixCroes on Mar 28, 2024
12. pablomartin4btc approved
13. 
    pablomartin4btc commented at 5:38 am on April 3, 2024: contributor

    tACK c6d1b8de89d87fe4fd171dc85557299e429e6564

    

    0/test/functional$ ./gen_add_type.py
    1mainnet example address: bc1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tq2jku9f
    2signet  example address: tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg
    3testnet example address: tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg
    4regtest example address: bcrt1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqsr2427
    

14. 
    kristapsk commented at 7:13 pm on April 3, 2024: contributor

    Concept ACK
15. 
    maflcko commented at 7:27 am on April 18, 2024: contributor

    rfm or is anything left to be done here?
16. hebasto merged this on Apr 18, 2024
17. hebasto closed this on Apr 18, 2024

    ---

18. theStack deleted the branch on Apr 18, 2024

Contributors
theStack DrahtBot MarnixCroes maflcko pablomartin4btc kristapsk

Review Requested
MarnixCroes