This commit adds support for mainnet addresses (P2PKH and P2SH) in the address_to_scriptpubkey function in the test framework. Previously, the function only supported testnet addresses and segwit addresses.
The changes include:
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32060.
See 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.
Reviewers, this pull request conflicts with the following ones:
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.
192@@ -193,7 +193,10 @@ def address_to_scriptpubkey(address):
193 return keyhash_to_p2pkh_script(payload)
194 elif version == 196: # testnet script hash
195 return scripthash_to_p2sh_script(payload)
196- # TODO: also support other address formats
192@@ -193,7 +193,10 @@ def address_to_scriptpubkey(address):
193 return keyhash_to_p2pkh_script(payload)
194 elif version == 196: # testnet script hash
195 return scripthash_to_p2sh_script(payload)
196- # TODO: also support other address formats
197+ elif version == 0: # mainnet pubkey hash
198+ return keyhash_to_p2pkh_script(payload)
199+ elif version == 5: # mainnet script hash
200+ return scripthash_to_p2sh_script(payload)
0 if version == 111 or version == 0: # testnet or mainnet pubkey hash
1 return keyhash_to_p2pkh_script(payload)
2 elif version == 196 or version == 5: # testnet or mainnet script hash
3 return scripthash_to_p2sh_script(payload)
4 # TODO: also support other address formats
Suggested change return keyhash_to_p2pkh_script(payload) elif version == 196: # testnet script hash return scripthash_to_p2sh_script(payload) # TODO: also support other address formats elif version == 0: # mainnet pubkey hash return keyhash_to_p2pkh_script(payload) elif version == 5: # mainnet script hash return scripthash_to_p2sh_script(payload) if version == 111 or version == 0: # testnet or mainnet pubkey hash return keyhash_to_p2pkh_script(payload) elif version == 196 or version == 5: # testnet or mainnet script hash return scripthash_to_p2sh_script(payload) # TODO: also support other address formats Applying suggestions on deleted lines is currently not supported.
I can’t commit your suggestion @davidgumberg
This commit adds support for mainnet addresses (P2PKH and P2SH) in the
address_to_scriptpubkey function in the test framework. Previously, the
function only supported testnet addresses and segwit addresses.
The changes include:
- Adding support for mainnet P2PKH addresses (version 0)
- Adding support for mainnet P2SH addresses (version 5)
- Adding tests to verify the new functionality
- Removing the TODO comment as the requested functionality has been implemented
This enhancement improves the test framework's ability to handle different
address formats, making it more versatile for testing scenarios that involve
mainnet addresses.
@VolodymyrBg, unlike in many other open source projects that are eager to receive any contribution whatsoever, the PoW (i.e., the amount of preparation and effort required for your contribution to minimize the workload for reviewers) needed in Bitcoin Core is a lot higher. Most people won’t have the patience to explain how to do git interactive rebases, how to tell a story in logical, reviewable commits, or how to update your PR after receiving feedback. Contributors are expected to research these processes themselves and demonstrate technical competence before their changes will be seriously considered. It’s painful, but often the lack of feedback is feedback. We all have to learn this the hard way.
I completely agree with you, sorry for taking your time.
Concept NACK
Why? This functionality is not used or needed in any tests. If it is needed in a test in a (future) PR, then it can be added at that time.