gui icon indicating copy to clipboard operation
gui copied to clipboard

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

Open theStack opened this issue 1 year ago • 3 comments

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

#!/usr/bin/env python3
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}')

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

master (mainnet):

image

PR (mainnet):

image

theStack avatar Mar 22 '24 16:03 theStack

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.

DrahtBot avatar Mar 22 '24 16:03 DrahtBot

@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.

theStack avatar Mar 28 '24 09:03 theStack

lgtm ACK c6d1b8de89d87fe4fd171dc85557299e429e6564

maflcko avatar Mar 28 '24 10:03 maflcko

Concept ACK

kristapsk avatar Apr 03 '24 19:04 kristapsk

rfm or is anything left to be done here?

maflcko avatar Apr 18 '24 07:04 maflcko