Merge #19253: Tests: tidy up address.py and segwit_addr.py

825fcae484f31182041dfacbf820e818d759b130 [tests] Replace bytes literals with hex literals (John Newbery)
64eca45100536579a3849631e59d4277bbc25be1 [tests] Fix pep8 style violations in address.py (John Newbery)
b230f8b3f3adcb1e2ae299094f9ae0a8bc7cc3d0 [tests] Correct docstring for address.py (John Newbery)
ea70e6a2ca0e183ef40cdb9b3b86f39e94366015 [tests] Tidy up imports in address.py (John Newbery)
7f639df0b8a15aaeccedab00b634925f568c2c9a [tests] Remove unused optional verify_checksum parameter (John Newbery)
011e784f74411bd5d5dbccfd3af39e0937fd8933 [tests] Rename segwit encode and decode functions (John Newbery)
e4557133f595f357df5e16ae4f2f19c579631396 [tests] Move bech32 unit tests to test framework (John Newbery)

Pull request description:

  Lots of small fixes:

  - moving unit tests to test_framework implementation files
  - renaming functions to be clearer
  - removing multiple imports
  - removing unreadable byte literals from the code
  - fixing pep8 violations
  - correcting out-of-date docstring

ACKs for top commit:
  jonatack:
    re-ACK 825fcae484f31182041dfacbf820e818d759b130 per `git range-diff a0a422c 7edcdcd 825fcae` and verified `wallet_address_types.py` and `wallet_basic.py --descriptors` (the failure on one travis job) are green locally.
  MarcoFalke:
    ACK 825fcae484f31182041dfacbf820e818d759b130
  fanquake:
    ACK 825fcae484f31182041dfacbf820e818d759b130 - looks ok to me.

Tree-SHA512: aea509c27c1bcb94bef11205b6a79836c39c62249672815efc9822f411bc2e2336ceb3d72b3b861c3f4054a08e16edb28c6edd3aa5eff72eec1d60ea6ca82dc4

4 files changed, 57 insertions, 51 deletions

diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py
index 9506b63f82..360962b8da 100644
--- a/test/functional/test_framework/address.py
+++ b/test/functional/test_framework/address.py
@@ -2,17 +2,17 @@
 # Copyright (c) 2016-2020 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Encode and decode BASE58, P2PKH and P2SH addresses."""
+"""Encode and decode Bitcoin addresses.
+
+- base58 P2PKH and P2SH addresses.
+- bech32 segwit v0 P2WPKH and P2WSH addresses."""
 
 import enum
 import unittest
 
 from .script import hash256, hash160, sha256, CScript, OP_0
-from .util import hex_str_to_bytes
-
-from . import segwit_addr
-
-from test_framework.util import assert_equal
+from .segwit_addr import encode_segwit_address
+from .util import assert_equal, hex_str_to_bytes
 
 ADDRESS_BCRT1_UNSPENDABLE = 'bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj'
 ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR = 'addr(bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj)#juyq9d97'
@@ -35,7 +35,7 @@ def byte_to_base58(b, version):
     str = chr(version).encode('latin-1').hex() + str
     checksum = hash256(hex_str_to_bytes(str)).hex()
     str += checksum[:8]
-    value = int('0x'+str,0)
+    value = int('0x' + str, 0)
     while value > 0:
         result = chars[value % 58] + result
         value //= 58
@@ -45,7 +45,10 @@ def byte_to_base58(b, version):
     return result
 
 
-def base58_to_byte(s, verify_checksum=True):
+def base58_to_byte(s):
+    """Converts a base58-encoded string to its data and version.
+
+    Throws if the base58 checksum is invalid."""
     if not s:
         return b''
     n = 0
@@ -65,66 +68,67 @@ def base58_to_byte(s, verify_checksum=True):
         else:
             break
     res = b'\x00' * pad + res
-    if verify_checksum:
-        assert_equal(hash256(res[:-4])[:4], res[-4:])
+
+    # Assert if the checksum is invalid
+    assert_equal(hash256(res[:-4])[:4], res[-4:])
 
     return res[1:-4], int(res[0])
 
 
-def keyhash_to_p2pkh(hash, main = False):
+def keyhash_to_p2pkh(hash, main=False):
     assert len(hash) == 20
     version = 0 if main else 111
     return byte_to_base58(hash, version)
 
-def scripthash_to_p2sh(hash, main = False):
+def scripthash_to_p2sh(hash, main=False):
     assert len(hash) == 20
     version = 5 if main else 196
     return byte_to_base58(hash, version)
 
-def key_to_p2pkh(key, main = False):
+def key_to_p2pkh(key, main=False):
     key = check_key(key)
     return keyhash_to_p2pkh(hash160(key), main)
 
-def script_to_p2sh(script, main = False):
+def script_to_p2sh(script, main=False):
     script = check_script(script)
     return scripthash_to_p2sh(hash160(script), main)
 
-def key_to_p2sh_p2wpkh(key, main = False):
+def key_to_p2sh_p2wpkh(key, main=False):
     key = check_key(key)
     p2shscript = CScript([OP_0, hash160(key)])
     return script_to_p2sh(p2shscript, main)
 
-def program_to_witness(version, program, main = False):
+def program_to_witness(version, program, main=False):
     if (type(program) is str):
         program = hex_str_to_bytes(program)
     assert 0 <= version <= 16
     assert 2 <= len(program) <= 40
     assert version > 0 or len(program) in [20, 32]
-    return segwit_addr.encode("bc" if main else "bcrt", version, program)
+    return encode_segwit_address("bc" if main else "bcrt", version, program)
 
-def script_to_p2wsh(script, main = False):
+def script_to_p2wsh(script, main=False):
     script = check_script(script)
     return program_to_witness(0, sha256(script), main)
 
-def key_to_p2wpkh(key, main = False):
+def key_to_p2wpkh(key, main=False):
     key = check_key(key)
     return program_to_witness(0, hash160(key), main)
 
-def script_to_p2sh_p2wsh(script, main = False):
+def script_to_p2sh_p2wsh(script, main=False):
     script = check_script(script)
     p2shscript = CScript([OP_0, sha256(script)])
     return script_to_p2sh(p2shscript, main)
 
 def check_key(key):
     if (type(key) is str):
-        key = hex_str_to_bytes(key) # Assuming this is hex string
+        key = hex_str_to_bytes(key)  # Assuming this is hex string
     if (type(key) is bytes and (len(key) == 33 or len(key) == 65)):
         return key
     assert False
 
 def check_script(script):
     if (type(script) is str):
-        script = hex_str_to_bytes(script) # Assuming this is hex string
+        script = hex_str_to_bytes(script)  # Assuming this is hex string
     if (type(script) is bytes or type(script) is CScript):
         return script
     assert False
@@ -135,15 +139,15 @@ class TestFrameworkScript(unittest.TestCase):
         def check_base58(data, version):
             self.assertEqual(base58_to_byte(byte_to_base58(data, version)), (data, version))
 
-        check_base58(b'\x1f\x8e\xa1p*{\xd4\x94\x1b\xca\tA\xb8R\xc4\xbb\xfe\xdb.\x05', 111)
-        check_base58(b':\x0b\x05\xf4\xd7\xf6l;\xa7\x00\x9fE50)l\x84\\\xc9\xcf', 111)
-        check_base58(b'A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111)
-        check_base58(b'\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111)
-        check_base58(b'\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111)
-        check_base58(b'\0\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111)
-        check_base58(b'\x1f\x8e\xa1p*{\xd4\x94\x1b\xca\tA\xb8R\xc4\xbb\xfe\xdb.\x05', 0)
-        check_base58(b':\x0b\x05\xf4\xd7\xf6l;\xa7\x00\x9fE50)l\x84\\\xc9\xcf', 0)
-        check_base58(b'A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0)
-        check_base58(b'\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0)
-        check_base58(b'\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0)
-        check_base58(b'\0\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0)
+        check_base58(bytes.fromhex('1f8ea1702a7bd4941bca0941b852c4bbfedb2e05'), 111)
+        check_base58(bytes.fromhex('3a0b05f4d7f66c3ba7009f453530296c845cc9cf'), 111)
+        check_base58(bytes.fromhex('41c1eaf111802559bad61b60d62b1f897c63928a'), 111)
+        check_base58(bytes.fromhex('0041c1eaf111802559bad61b60d62b1f897c63928a'), 111)
+        check_base58(bytes.fromhex('000041c1eaf111802559bad61b60d62b1f897c63928a'), 111)
+        check_base58(bytes.fromhex('00000041c1eaf111802559bad61b60d62b1f897c63928a'), 111)
+        check_base58(bytes.fromhex('1f8ea1702a7bd4941bca0941b852c4bbfedb2e05'), 0)
+        check_base58(bytes.fromhex('3a0b05f4d7f66c3ba7009f453530296c845cc9cf'), 0)
+        check_base58(bytes.fromhex('41c1eaf111802559bad61b60d62b1f897c63928a'), 0)
+        check_base58(bytes.fromhex('0041c1eaf111802559bad61b60d62b1f897c63928a'), 0)
+        check_base58(bytes.fromhex('000041c1eaf111802559bad61b60d62b1f897c63928a'), 0)
+        check_base58(bytes.fromhex('00000041c1eaf111802559bad61b60d62b1f897c63928a'), 0)
diff --git a/test/functional/test_framework/segwit_addr.py b/test/functional/test_framework/segwit_addr.py
index 02368e938f..00c0d8a919 100644
--- a/test/functional/test_framework/segwit_addr.py
+++ b/test/functional/test_framework/segwit_addr.py
@@ -3,7 +3,7 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Reference implementation for Bech32 and segwit addresses."""
-
+import unittest
 
 CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"
 
@@ -84,7 +84,7 @@ def convertbits(data, frombits, tobits, pad=True):
     return ret
 
 
-def decode(hrp, addr):
+def decode_segwit_address(hrp, addr):
     """Decode a segwit address."""
     hrpgot, data = bech32_decode(addr)
     if hrpgot != hrp:
@@ -99,9 +99,23 @@ def decode(hrp, addr):
     return (data[0], decoded)
 
 
-def encode(hrp, witver, witprog):
+def encode_segwit_address(hrp, witver, witprog):
     """Encode a segwit address."""
     ret = bech32_encode(hrp, [witver] + convertbits(witprog, 8, 5))
-    if decode(hrp, ret) == (None, None):
+    if decode_segwit_address(hrp, ret) == (None, None):
         return None
     return ret
+
+class TestFrameworkScript(unittest.TestCase):
+    def test_segwit_encode_decode(self):
+        def test_python_bech32(addr):
+            hrp = addr[:4]
+            self.assertEqual(hrp, "bcrt")
+            (witver, witprog) = decode_segwit_address(hrp, addr)
+            self.assertEqual(encode_segwit_address(hrp, witver, witprog), addr)
+
+        # P2WPKH
+        test_python_bech32('bcrt1qthmht0k2qnh3wy7336z05lu2km7emzfpm3wg46')
+        # P2WSH
+        test_python_bech32('bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj')
+        test_python_bech32('bcrt1qft5p2uhsdcdc3l2ua4ap5qqfg4pjaqlp250x7us7a8qqhrxrxfsqseac85')
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index c8cf173d5f..cb86305cc6 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -71,6 +71,7 @@ TEST_FRAMEWORK_MODULES = [
     "blocktools",
     "muhash",
     "script",
+    "segwit_addr",
     "util",
 ]
 
diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py
index 68e22b7e86..bba0b8974d 100755
--- a/test/functional/wallet_address_types.py
+++ b/test/functional/wallet_address_types.py
@@ -64,10 +64,6 @@ from test_framework.util import (
     assert_raises_rpc_error,
     connect_nodes,
 )
-from test_framework.segwit_addr import (
-    encode,
-    decode,
-)
 
 class AddressTypeTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -101,13 +97,6 @@ class AddressTypeTest(BitcoinTestFramework):
         """Return a list of balances."""
         return [self.nodes[i].getbalances()['mine'][key] for i in range(4)]
 
-    # Quick test of python bech32 implementation
-    def test_python_bech32(self, addr):
-        hrp = addr[:4]
-        assert_equal(hrp, "bcrt")
-        (witver, witprog) = decode(hrp, addr)
-        assert_equal(encode(hrp, witver, witprog), addr)
-
     def test_address(self, node, address, multisig, typ):
         """Run sanity checks on an address."""
         info = self.nodes[node].getaddressinfo(address)
@@ -132,7 +121,6 @@ class AddressTypeTest(BitcoinTestFramework):
             assert_equal(info['witness_version'], 0)
             assert_equal(len(info['witness_program']), 40)
             assert 'pubkey' in info
-            self.test_python_bech32(info["address"])
         elif typ == 'legacy':
             # P2SH-multisig
             assert info['isscript']
@@ -158,7 +146,6 @@ class AddressTypeTest(BitcoinTestFramework):
             assert_equal(info['witness_version'], 0)
             assert_equal(len(info['witness_program']), 64)
             assert 'pubkeys' in info
-            self.test_python_bech32(info["address"])
         else:
             # Unknown type
             assert False