New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change example address from legacy (P2PKH) to bech32m (P2TR) #808
Change example address from legacy (P2PKH) to bech32m (P2TR) #808
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
c6fa0ab
to
960b2d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK
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. |
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>
960b2d4
to
c6d1b8d
Compare
lgtm ACK c6d1b8d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK c6d1b8d
Also tested the script provided in the description.
/test/functional$ ./gen_add_type.py
mainnet example address: bc1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tq2jku9f
signet example address: tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg
testnet example address: tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg
regtest example address: bcrt1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqsr2427
Concept ACK |
rfm or is anything left to be done here? |
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):Note that the last bech32 character is modified in order to make the checksum fail.
master (mainnet):
PR (mainnet):