Skip to content

Commit 8679461

Browse files
newtonicknewtonick
authored
Merge pull request #667 from kdmukai/address_qr_all_caps
[Bugfix] Handle all-caps address QR data; refactor/cleanup; more tests
2 parents cbaaaf4 + defaed0 commit 8679461

File tree

2 files changed

+171
-109
lines changed

2 files changed

+171
-109
lines changed

‎src/seedsigner/models/decode_qr.py

+57-47
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,7 @@ def base43_decode(s):
507507
def is_bitcoin_address(s):
508508
if re.search(r'^bitcoin\:.*', s, re.IGNORECASE):
509509
return True
510-
elif re.search(r'^((bc1|tb1|bcr|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,62})$', s):
511-
# TODO: Handle regtest bcrt?
510+
elif re.search(r'^((bc1|tb1|bcr|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,62})$', s, re.IGNORECASE):
512511
return True
513512
else:
514513
return False
@@ -940,61 +939,72 @@ def __init__(self):
940939

941940

942941
def add(self, segment, qr_type=QRType.BITCOIN_ADDRESS):
943-
r = re.search(r'((bc1q|tb1q|bcrt1q|bc1p|tb1p|bcrt1p|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,64})', segment)
944-
if r != None:
945-
self.address = r.group(1)
946-
947-
if re.search(r'^((bc1q|tb1q|bcrt1q|bc1p|tb1p|bcrt1p|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,64})$', self.address) != None:
948-
self.complete = True
949-
self.collected_segments = 1
950-
951-
# get address type
952-
r = re.search(r'^((bc1q|tb1q|bcrt1q|bc1p|tb1p|bcrt1p|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,64})$', self.address)
953-
if r != None:
954-
r = r.group(2)
955-
956-
if r == "1":
957-
# Legacy P2PKH. mainnet
958-
self.address_type = (SettingsConstants.LEGACY_P2PKH, SettingsConstants.MAINNET)
942+
"""
943+
Input may be prefixed with "bitcoin:" but will be ignored.
944+
945+
RegEx searches for a recognizable bitcoin address.
946+
* The `^` ensures that the specified address prefixes can only match at
947+
the beginning of the address.
948+
949+
Result will yield the following match groups:
950+
* group 1: complete address
951+
* group 2: address prefix
952+
"""
953+
address_match = re.search(r'^((bc1q|tb1q|bcrt1q|bc1p|tb1p|bcrt1p|[123]|[mn])[a-zA-HJ-NP-Z0-9]{25,64})', segment.split(":")[-1], re.IGNORECASE)
954+
if address_match != None:
955+
self.address = address_match.group(1)
956+
self.complete = True
957+
self.collected_segments = 1
958+
959+
# Have to handle wallets that uppercase bech32 addresses.
960+
# Note that it's safe to lowercase the prefix for ALL addr formats.
961+
addr_prefix = address_match.group(2).lower()
962+
963+
if addr_prefix == "1":
964+
# Legacy P2PKH. mainnet
965+
self.address_type = (SettingsConstants.LEGACY_P2PKH, SettingsConstants.MAINNET)
959966

960-
elif r == "m" or r == "n":
961-
self.address_type = (SettingsConstants.LEGACY_P2PKH, SettingsConstants.TESTNET)
967+
elif addr_prefix in ["m", "n"]:
968+
self.address_type = (SettingsConstants.LEGACY_P2PKH, SettingsConstants.TESTNET)
962969

963-
elif r == "3":
964-
# Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); mainnet
965-
# TODO: Would be more correct to use a P2SH constant
966-
self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.MAINNET)
970+
elif addr_prefix == "3":
971+
# Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); mainnet
972+
# TODO: Would be more correct to use a P2SH constant
973+
self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.MAINNET)
967974

968-
elif r == "2":
969-
# Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); testnet / regtest
970-
self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.TESTNET)
975+
elif addr_prefix == "2":
976+
# Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); testnet / regtest
977+
self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.TESTNET)
971978

972-
elif r == "bc1q":
973-
# Native Segwit (single sig or multisig), mainnet
974-
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.MAINNET)
979+
elif addr_prefix == "bc1q":
980+
# Native Segwit (single sig or multisig), mainnet
981+
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.MAINNET)
975982

976-
elif r == "tb1q":
977-
# Native Segwit (single sig or multisig), testnet
978-
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.TESTNET)
983+
elif addr_prefix == "tb1q":
984+
# Native Segwit (single sig or multisig), testnet
985+
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.TESTNET)
979986

980-
elif r == "bcrt1q":
981-
# Native Segwit (single sig or multisig), regtest
982-
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.REGTEST)
987+
elif addr_prefix == "bcrt1q":
988+
# Native Segwit (single sig or multisig), regtest
989+
self.address_type = (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.REGTEST)
983990

984-
elif r == "bc1p":
985-
# Native Segwit (single sig or multisig), mainnet
986-
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.MAINNET)
991+
elif addr_prefix == "bc1p":
992+
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.MAINNET)
987993

988-
elif r == "tb1p":
989-
# Native Segwit (single sig or multisig), testnet
990-
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.TESTNET)
994+
elif addr_prefix == "tb1p":
995+
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.TESTNET)
991996

992-
elif r == "bcrt1p":
993-
# Native Segwit (single sig or multisig), regtest
994-
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.REGTEST)
995-
996-
return DecodeQRStatus.COMPLETE
997+
elif addr_prefix == "bcrt1p":
998+
self.address_type = (SettingsConstants.TAPROOT, SettingsConstants.REGTEST)
999+
# Note: there is no final "else" here because the regex won't return any other matches.
1000+
1001+
# If the addr type is case-insensitive, ensure we return it lowercase
1002+
if self.address_type[0] in [SettingsConstants.NATIVE_SEGWIT, SettingsConstants.TAPROOT]:
1003+
self.address = self.address.lower()
1004+
1005+
return DecodeQRStatus.COMPLETE
9971006

1007+
logger.debug(f"Invalid address: {segment}")
9981008
return DecodeQRStatus.INVALID
9991009

10001010

‎tests/test_decodepsbtqr.py

+114-62
Original file line numberDiff line numberDiff line change
@@ -281,69 +281,121 @@ def test_short_4_letter_mnemonic_qr():
281281
assert d.get_seed_phrase() == ["height", "demise", "useless", "trap", "grow", "lion", "found", "off", "key", "clown", "transfer", "enroll"]
282282

283283

284-
def test_bitcoin_address():
285-
bad1 = "loremipsum"
286-
bad2 = "0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae"
287-
bad3 = "121802020768124106400009195602431595117715840445"
288-
289-
legacy_address1 = "1KFHE7w8BhaENAswwryaoccDb6qcT6DbYY"
290-
legacy_address2 = "16ftSEQ4ctQFDtVZiUBusQUjRrGhM3JYwe"
291-
292-
main_bech32_address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq"
293-
test_bech32_address = "tb1qkurj377gtlmu0j5flcykcsh2xagexh9h3jk06a"
294-
295-
main_nested_segwit_address = "3Nu78Cqcf6hsD4sUBAN9nP13tYiHU9QPFX"
296-
test_nested_segwit_address = "2N6JbrvPMMwbBhu2KxqXyyHUQz3XKspvyfm"
297-
298-
main_bech32_address2 = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=12000"
299-
main_bech32_address3 = "BITCOIN:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?junk"
300-
301-
d = DecodeQR()
302-
d.add_data(bad1)
303-
304-
assert d.qr_type == QRType.INVALID
305-
306-
d = DecodeQR()
307-
d.add_data(legacy_address1)
308-
309-
assert d.get_address() == legacy_address1
310-
assert d.get_address_type() == (SettingsConstants.LEGACY_P2PKH, SettingsConstants.MAINNET)
311-
312-
d = DecodeQR()
313-
d.add_data(legacy_address2)
314-
315-
assert d.get_address() == legacy_address2
316-
assert d.get_address_type() == (SettingsConstants.LEGACY_P2PKH, SettingsConstants.MAINNET)
317-
318-
d = DecodeQR()
319-
d.add_data(main_bech32_address)
320-
321-
assert d.get_address() == main_bech32_address
322-
assert d.get_address_type() == (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.MAINNET)
323-
324-
d = DecodeQR()
325-
d.add_data(test_bech32_address)
326-
327-
assert d.get_address() == test_bech32_address
328-
assert d.get_address_type() == (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.TESTNET)
329-
330-
d = DecodeQR()
331-
d.add_data(main_nested_segwit_address)
332-
333-
assert d.get_address() == main_nested_segwit_address
334-
assert d.get_address_type() == (SettingsConstants.NESTED_SEGWIT, SettingsConstants.MAINNET)
335-
336-
d = DecodeQR()
337-
d.add_data(test_nested_segwit_address)
338-
339-
assert d.get_address() == test_nested_segwit_address
340-
assert d.get_address_type() == (SettingsConstants.NESTED_SEGWIT, SettingsConstants.TESTNET)
284+
# Test data for bitcoin address decoding. All generated from test key: ["abandon"] * 11 + ["about"]
285+
legacy_address_mainnet = "1LqBGSKuX5yYUonjxT5qGfpUsXKYYWeabA"
286+
legacy_address_testnet = "mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV"
287+
288+
nested_segwit_address_mainnet = "37VucYSaXLCAsxYyAPfbSi9eh4iEcbShgf"
289+
nested_segwit_address_testnet = "2Mww8dCYPUpKHofjgcXcBCEGmniw9CoaiD2"
290+
291+
native_segwit_address_mainnet = "bc1qcr8te4kr609gcawutmrza0j4xv80jy8z306fyu"
292+
native_segwit_address_testnet = "tb1q6rz28mcfaxtmd6v789l9rrlrusdprr9pqcpvkl"
293+
native_segwit_address_regtest = "bcrt1q6rz28mcfaxtmd6v789l9rrlrusdprr9pz3cppk"
294+
295+
taproot_address_mainnet = "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr"
296+
taproot_address_testnet = "tb1p8wpt9v4frpf3tkn0srd97pksgsxc5hs52lafxwru9kgeephvs7rqlqt9zj"
297+
taproot_address_regtest = "bcrt1p8wpt9v4frpf3tkn0srd97pksgsxc5hs52lafxwru9kgeephvs7rqjeprhg"
298+
299+
300+
301+
def test_bitcoin_address():
302+
"""
303+
Decoder should parse various types of valid bitcoin addresses with or without the
304+
"bitcoin:" prefix and optional query params.
305+
"""
306+
def decode(address, expected_script_type, expected_network=SettingsConstants.MAINNET):
307+
for data in [address, "bitcoin:" + address, "bitcoin:" + address + "?amount=12000"]:
308+
d = DecodeQR()
309+
d.add_data(data)
310+
assert d.get_address() == address
311+
assert d.get_address_type() == (expected_script_type, expected_network)
312+
313+
decode(legacy_address_mainnet, SettingsConstants.LEGACY_P2PKH)
314+
decode(legacy_address_testnet, SettingsConstants.LEGACY_P2PKH, SettingsConstants.TESTNET)
315+
decode(nested_segwit_address_mainnet, SettingsConstants.NESTED_SEGWIT)
316+
decode(nested_segwit_address_testnet, SettingsConstants.NESTED_SEGWIT, SettingsConstants.TESTNET)
317+
decode(native_segwit_address_mainnet, SettingsConstants.NATIVE_SEGWIT)
318+
decode(native_segwit_address_testnet, SettingsConstants.NATIVE_SEGWIT, SettingsConstants.TESTNET)
319+
decode(native_segwit_address_regtest, SettingsConstants.NATIVE_SEGWIT, SettingsConstants.REGTEST)
320+
decode(taproot_address_mainnet, SettingsConstants.TAPROOT)
321+
decode(taproot_address_testnet, SettingsConstants.TAPROOT, SettingsConstants.TESTNET)
322+
decode(taproot_address_regtest, SettingsConstants.TAPROOT, SettingsConstants.REGTEST)
323+
324+
325+
326+
def test_invalid_bitcoin_address():
327+
"""
328+
Decoder should fail to parse invalid address data.
329+
* Test incorrect "bitcoin:" prefix.
330+
* Test invalid addresses.
331+
* Test valid addresses w/additional prefixes (to ensure regexp is not finding
332+
mid-string matches) which make the data invalid.
333+
"""
334+
bad_inputs = [
335+
# wrong separator char
336+
"bitcoin=bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
337+
338+
# Unrecognized addr prefix
339+
"bitcoin:bcfakehrp1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
340+
341+
# valid addr w/garbage addr prefix
342+
"abcbc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
343+
344+
# valid "bitcoin:" prefix w/garbage addr prefix
345+
"bitcoin:abcbc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
346+
347+
# typo in "bitcoin:" prefix
348+
"bitcon:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
349+
]
341350

342-
d = DecodeQR()
343-
d.add_data(main_bech32_address2)
344-
345-
assert d.get_address() == "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq"
346-
assert d.get_address_type() == (SettingsConstants.NATIVE_SEGWIT, SettingsConstants.MAINNET)
351+
for bad_input in bad_inputs:
352+
d = DecodeQR()
353+
status = d.add_data(bad_input)
354+
assert status == DecodeQRStatus.INVALID
355+
356+
357+
358+
def test_bitcoin_address_ignores_case_where_allowed():
359+
"""
360+
Decoder should ignore case in QR data prefix and in the address itself (for the
361+
address types where case is ignored).
362+
"""
363+
def decode(address, expected_script_type, is_case_sensitive, expected_network=SettingsConstants.MAINNET):
364+
addr_variations = [address]
365+
if not is_case_sensitive:
366+
# Test as-is and all uppercase
367+
addr_variations.append(address.upper())
368+
369+
for addr_variation in addr_variations:
370+
# First add prefix capitalizations
371+
variations_1 = ["bitcoin:" + addr_variation, "BITCOIN:" + addr_variation]
372+
373+
# Now add query params
374+
variations_2 = [v + "?amount=12000" for v in variations_1]
375+
variations_3 = [v + "?AMOUNT=12000" for v in variations_1]
376+
for data in [addr_variation] + variations_1 + variations_2 + variations_3:
377+
d = DecodeQR()
378+
d.add_data(data)
379+
assert d.get_address_type() == (expected_script_type, expected_network)
380+
381+
if not is_case_sensitive:
382+
assert d.get_address() == addr_variation.lower()
383+
else:
384+
assert d.get_address() == addr_variation
385+
386+
# Case sensitive address types
387+
decode(legacy_address_mainnet, SettingsConstants.LEGACY_P2PKH, is_case_sensitive=True)
388+
decode(legacy_address_testnet, SettingsConstants.LEGACY_P2PKH, is_case_sensitive=True, expected_network=SettingsConstants.TESTNET)
389+
decode(nested_segwit_address_mainnet, SettingsConstants.NESTED_SEGWIT, is_case_sensitive=True)
390+
decode(nested_segwit_address_testnet, SettingsConstants.NESTED_SEGWIT, is_case_sensitive=True, expected_network=SettingsConstants.TESTNET)
391+
392+
# Case insensitive address types
393+
decode(native_segwit_address_mainnet, SettingsConstants.NATIVE_SEGWIT, is_case_sensitive=False)
394+
decode(native_segwit_address_testnet, SettingsConstants.NATIVE_SEGWIT, is_case_sensitive=False, expected_network=SettingsConstants.TESTNET)
395+
decode(native_segwit_address_regtest, SettingsConstants.NATIVE_SEGWIT, is_case_sensitive=False, expected_network=SettingsConstants.REGTEST)
396+
decode(taproot_address_mainnet, SettingsConstants.TAPROOT, is_case_sensitive=False)
397+
decode(taproot_address_testnet, SettingsConstants.TAPROOT, is_case_sensitive=False, expected_network=SettingsConstants.TESTNET)
398+
decode(taproot_address_regtest, SettingsConstants.TAPROOT, is_case_sensitive=False, expected_network=SettingsConstants.REGTEST)
347399

348400

349401

0 commit comments

Comments
 (0)