test: support mainnet base58 in address_to_scriptpubkey #34770

1. 
   tonykim525 commented at 11:58 am on March 7, 2026: none

   Summary

   • Extend test/functional/test_framework/address.py::address_to_scriptpubkey to support mainnet Base58 version bytes:
     • P2PKH: 0
     • P2SH: 5
   • Replace assert-based Base58 character validation with explicit ValueError.
   • Add explicit ValueError handling for:
     • empty input
     • unsupported Base58 version bytes
     • invalid payload lengths

   Motivation

   address_to_scriptpubkey supported Bech32 and testnet Base58, but not mainnet Base58 (0, 5), despite being a shared helper in functional tests.

   This change closes that gap and makes failure modes explicit and testable.

   Tests

   • cd test/functional && python3 -m unittest test_framework.address
   • test/functional/feature_framework_unit_tests.py
   • python3 -m ruff check --select=B006,B008,E101,E401,E402,E701,E702,E703,E711,E713,E714,E721,E722,E742,E743,F401,F402,F403,F404,F405,F406,F407,F541,F601,F602,F621,F631,F632,F811,F821,F822,F823,F841,PLE,W191,W291,W292,W293,W605 test/functional/test_framework/address.py
2. DrahtBot added the label Tests on Mar 7, 2026
3. 
   DrahtBot commented at 11:59 am on March 7, 2026: 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	Bortlesboat

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

4. test: support mainnet base58 in address_to_scriptpubkey and add error-path tests 013059dcc2
5. test: add explicit empty-address error in address helper 219daccf3b
6. tonykim525 force-pushed on Mar 7, 2026
7. 
   Bortlesboat commented at 10:16 pm on March 7, 2026: none

   Code Review ACK

   Reviewed the diff. Clean, well-tested improvement.

   • Correctly adds mainnet version bytes (0 for P2PKH, 5 for P2SH) alongside existing testnet bytes (111, 196)
   • Payload length validation (20 bytes) prevents silent corruption from malformed addresses
   • Replacing the bare assert c in b58chars with a descriptive ValueError and the assert False catch-all with raise ValueError improves debuggability
   • The wit_ver/wit_payload rename is a nice clarity improvement over shadowing version/payload
   • Unit tests cover both valid mainnet/testnet conversions and all error paths (empty address, unsupported version byte, wrong payload length, invalid base58 character)

   No issues found.

8. 
   Bortlesboat commented at 10:38 pm on March 7, 2026: none

   Tested ACK 219daccf3b

   Ran the address.py unit tests on Windows (Python 3.12):

   0$ python3 -m pytest test/functional/test_framework/address.py -v
   1test_address_to_scriptpubkey_base58 PASSED
   2test_address_to_scriptpubkey_invalid_base58 PASSED
   3test_base58encodedecode PASSED
   4test_bech32_decode PASSED
   

   All 4 tests pass, including the 2 new tests added by this PR. The mainnet version bytes (0 for P2PKH, 5 for P2SH) produce correct scriptPubKeys, and all error paths (empty address, unsupported version, wrong payload length, invalid base58 char) raise the expected ValueErrors.

9. 
   sedited commented at 11:52 am on March 9, 2026: contributor

   This change seems to be motivated by expanding unit tests for the functional tests without actual functional test code using it. I don’t think that is worthwhile doing with the motivation provided. Can you motivate this better? Otherwise I’ll close the PR.

Contributors
tonykim525 DrahtBot Bortlesboat sedited

Labels
Tests