Skip to content

Commit

Permalink
[electrum] drop support for "bitcoincash:..." addresses
Browse files Browse the repository at this point in the history
Summary:
After this diff, bch cash addresses are no longer supported in the "Pay to" field, in BIP21 URIs or any method of entering an address that uses `Address.from_string` without setting `support_arbitrary_prefix=True`.

The address converter in the GUI still support any prefix. This diff also makes the command line / RPC `addressconvert` command support arbitrary prefixes.
But now we no longer support bch addresses without an explicit prefix.

This diff also incidentally fixes support for "ectest:..." URIs when running in testnet mode. Previously `web.parseable_schemes` did not correctly account for the net parameter, and always returned `("ecash", "bitcoincash")`

This is a follow-up to #250 (dropping of display of BCH addresses in the UI).

Test Plan:
```
python test_runner.py
pytest electrumabc.tests.regtest
```

Try the address converter tool in the prefix with various address formats. It should still support everything it did before, except for prefixless "bitcoincash:..." cash addresses.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D15245
  • Loading branch information
PiRK authored and abc-bot committed Jan 24, 2024
1 parent 41f118c commit e9da99f
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 121 deletions.
43 changes: 9 additions & 34 deletions electrumabc/address.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@
minikey_to_private_key,
push_script_bytes,
)
from .constants import (
WHITELISTED_PREFIXES,
WHITELISTED_REGTEST_PREFIXES,
WHITELISTED_TESTNET_PREFIXES,
)
from .util import cachedproperty

_sha256 = hashlib.sha256
Expand Down Expand Up @@ -366,7 +361,8 @@ class Address(namedtuple("AddressTuple", "hash160 kind"), DestinationType):
FMT_LEGACY = "Legacy"

# We keep this for now for the address converter tool and hw wallets, but it
# can no longer be shown in the rest of the UI.
# can no longer be shown in the rest of the UI and is no longer supported in
# the "Pay to" field.
FMT_CASHADDR_BCH = "CashAddr BCH"

# Default to CashAddr
Expand Down Expand Up @@ -411,50 +407,29 @@ def from_cashaddr_string(
support_arbitrary_prefix: bool = False,
):
"""Construct from a cashaddress string.
If the prefix is not specified, "ecash:" and "bitcoincash:" are tried.
:return: Instance of :class:`Address`
"""
if net is None:
net = networks.net
string = string.lower()

whitelisted_prefixes = (
WHITELISTED_REGTEST_PREFIXES
if net.REGTEST
else WHITELISTED_TESTNET_PREFIXES
if net.TESTNET
else WHITELISTED_PREFIXES
)

if ":" in string:
# Case of prefix being specified
try:
prefix, kind, addr_hash = cashaddr.decode(string)
except ValueError as e:
raise AddressError(str(e))

if not support_arbitrary_prefix and prefix not in whitelisted_prefixes:
if not support_arbitrary_prefix and prefix != net.CASHADDR_PREFIX:
raise AddressError(f"address has unexpected prefix {prefix}")
else:
# The input string can omit the prefix, in which case
# we try supported prefixes
prefix, kind, addr_hash = None, None, None
errors = []
for p in whitelisted_prefixes:
full_string = ":".join([p, string])
try:
prefix, kind, addr_hash = cashaddr.decode(full_string)
except ValueError as e:
errors.append(str(e))
else:
# accept the first valid address
break
if len(errors) >= len(whitelisted_prefixes):
full_string = ":".join([net.CASHADDR_PREFIX, string])
try:
prefix, kind, addr_hash = cashaddr.decode(full_string)
except ValueError as e:
raise AddressError(
"Unable to decode CashAddr with supported prefixes.\n\n".join(
[f"{err}" for err in errors]
)
+ "\n"
f"Unable to decode CashAddr with supported prefix '{net.CASHADDR_PREFIX}'."
f"\n\n{str(e)}\n"
)

if kind == cashaddr.PUBKEY_TYPE:
Expand Down
2 changes: 1 addition & 1 deletion electrumabc/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def addressconvert(self, address):
a legacy or a Cash Address and both forms will be returned as a JSON
dict."""
try:
addr = Address.from_string(address)
addr = Address.from_string(address, support_arbitrary_prefix=True)
except Exception as e:
raise AddressError(f"Invalid address: {address}") from e
return {
Expand Down
14 changes: 1 addition & 13 deletions electrumabc/constants.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from decimal import Decimal
from typing import List, Mapping, Sequence
from typing import Mapping, Sequence

PROJECT_NAME: str = "Electrum ABC"
PROJECT_NAME_NO_SPACES = "ElectrumABC"
Expand Down Expand Up @@ -70,18 +70,6 @@ def satoshis_to_unit(self, satoshis: int) -> Decimal:
CASHADDR_REGTEST_PREFIX = "ecregtest"
CASHADDR_REGTEST_PREFIX_BCH = "bchreg"

WHITELISTED_PREFIXES: List[str] = [CASHADDR_PREFIX, CASHADDR_PREFIX_BCH]

WHITELISTED_TESTNET_PREFIXES: List[str] = [
CASHADDR_TESTNET_PREFIX,
CASHADDR_TESTNET_PREFIX_BCH,
]

WHITELISTED_REGTEST_PREFIXES: List[str] = [
CASHADDR_REGTEST_PREFIX,
CASHADDR_REGTEST_PREFIX_BCH,
]

PROOF_DUST_THRESHOLD: int = XEC.unit_to_satoshis(Decimal("100_000_000.00"))
"""Lowest amount in satoshis that can be used as stake in a proof."""

Expand Down
16 changes: 16 additions & 0 deletions electrumabc/tests/regtest/test_rpc_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,19 @@ def test_signtransaction(fulcrum_service): # noqa: F811
),
)
assert success


def test_addressconvert(fulcrum_service): # noqa: F811
cashaddr_no_prefix = "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqcrl5mqkt"
cashaddr = "ecregtest:" + cashaddr_no_prefix
cashaddr_bch = "bchreg:qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqha9s37tt"
legacy_addr = "mfWxJ45yp2SFn7UciZyNpvDKrzbhyfKrY8"
for addr_str in (cashaddr, cashaddr_no_prefix, cashaddr_bch, legacy_addr):
res = poll_for_answer(
EC_DAEMON_RPC_URL, request("addressconvert", params={"address": addr_str})
)
assert res == {
"cashaddr": cashaddr,
"bitcoincashaddr": cashaddr_bch,
"legacy": legacy_addr,
}
22 changes: 13 additions & 9 deletions electrumabc/tests/test_address.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,14 @@ def test_from_legacy(self):
self._test_addr(Address.from_string(LEGACY_ADDRESS))

def test_from_bch_cashaddr(self):
self._test_addr(Address.from_string(BCH_CASHADDR_WITH_PREFIX))
self._test_addr(Address.from_string(BCH_CASHADDR_NO_PREFIX))
self._test_addr(Address.from_string(BCH_CASHADDR_WITH_PREFIX.upper()))
self._test_addr(Address.from_string(BCH_CASHADDR_NO_PREFIX.upper()))
self._test_addr(
Address.from_string(BCH_CASHADDR_WITH_PREFIX, support_arbitrary_prefix=True)
)
self._test_addr(
Address.from_string(
BCH_CASHADDR_WITH_PREFIX.upper(), support_arbitrary_prefix=True
)
)

def test_from_ecashaddr(self):
self._test_addr(Address.from_string(ECASHADDR_WITH_PREFIX))
Expand All @@ -73,11 +77,11 @@ def test_from_legacy_tesnet(self):

def test_from_bch_cashaddr_tesnet(self):
self._test_addr(
Address.from_string(BCH_CASHADDR_WITH_PREFIX_TESTNET, net=networks.TestNet),
networks.TestNet,
)
self._test_addr(
Address.from_string(BCH_CASHADDR_NO_PREFIX_TESTNET, net=networks.TestNet),
Address.from_string(
BCH_CASHADDR_WITH_PREFIX_TESTNET,
net=networks.TestNet,
support_arbitrary_prefix=True,
),
networks.TestNet,
)

Expand Down
91 changes: 38 additions & 53 deletions electrumabc/tests/test_bip21.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@
import unittest

from ..address import Address
from ..web import DuplicateKeyInURIError, create_URI, parse_URI
from ..networks import MainNet, RegtestNet, TestNet
from ..web import (
BadSchemeError,
BadURIParameter,
DuplicateKeyInURIError,
create_URI,
parse_URI,
parseable_schemes,
)


class TestParseURI(unittest.TestCase):
Expand All @@ -12,39 +20,42 @@ def _do_test(self, uri, expected):
self.assertEqual(expected, result)

def test_address(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma"},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma"},
)

def test_testnet(self):
with self.assertRaises(BadSchemeError):
parse_URI("ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme", net=TestNet)

with self.assertRaises(BadURIParameter):
# correct prefix with bad checksum
parse_URI("ectest:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme", net=TestNet)

self.assertEqual(
parse_URI("ectest:qrh3ethkfms79tlcw7m736t38hp9kg5f7gzncerkcg", net=TestNet),
{"address": "qrh3ethkfms79tlcw7m736t38hp9kg5f7gzncerkcg"},
)

self.assertEqual(
parse_URI("qrh3ethkfms79tlcw7m736t38hp9kg5f7gzncerkcg", net=TestNet),
{"address": "qrh3ethkfms79tlcw7m736t38hp9kg5f7gzncerkcg"},
)

def test_only_address(self):
self._do_test(
"15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma"},
)

def test_address_label(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?label=electrum%20test",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "label": "electrum test"},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?label=electrum%20test",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "label": "electrum test"},
)

def test_address_message(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?message=electrum%20test",
{
"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
"message": "electrum test",
},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?message=electrum%20test",
{
Expand All @@ -54,23 +65,12 @@ def test_address_message(self):
)

def test_address_amount(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=1.03",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "amount": 103},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=1.03",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "amount": 103},
)

def test_address_request_url(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?r=http://domain.tld/page?h%3D2a8628fc2fbe",
{
"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
"r": "http://domain.tld/page?h=2a8628fc2fbe",
},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?r=http://domain.tld/page?h%3D2a8628fc2fbe",
{
Expand All @@ -80,28 +80,12 @@ def test_address_request_url(self):
)

def test_ignore_args(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?test=test",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "test": "test"},
)
self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?test=test",
{"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma", "test": "test"},
)

def test_multiple_args(self):
self._do_test(
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=10.04&label=electrum-test&message=electrum%20test&test=none&r=http://domain.tld/page",
{
"address": "15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma",
"amount": 1004,
"label": "electrum-test",
"message": "electrum test",
"r": "http://domain.tld/page",
"test": "none",
},
)

self._do_test(
"ecash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?"
"amount=10.04&"
Expand All @@ -120,17 +104,12 @@ def test_multiple_args(self):
)

def test_no_address_request_url(self):
self._do_test(
"bitcoincash:?r=http://domain.tld/page?h%3D2a8628fc2fbe",
{"r": "http://domain.tld/page?h=2a8628fc2fbe"},
)
self._do_test(
"ecash:?r=http://domain.tld/page?h%3D2a8628fc2fbe",
{"r": "http://domain.tld/page?h=2a8628fc2fbe"},
)

def test_invalid_address(self):
self.assertRaises(Exception, parse_URI, "bitcoincash:invalidaddress")
self.assertRaises(Exception, parse_URI, "ecash:invalidaddress")

def test_invalid(self):
Expand All @@ -140,11 +119,6 @@ def test_invalid(self):

def test_parameter_polution(self):
# amount specified twice
self.assertRaises(
Exception,
parse_URI,
"bitcoincash:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=0.0003&label=test&amount=30.0",
)
self.assertRaises(
Exception,
parse_URI,
Expand Down Expand Up @@ -255,5 +229,16 @@ def test_op_return(self):
)


class TestParseableSchemes(unittest.TestCase):
def test_mainnet(self):
self.assertEqual(parseable_schemes(MainNet), ("ecash",))

def test_testnet(self):
self.assertEqual(parseable_schemes(TestNet), ("ectest",))

def test_regtest(self):
self.assertEqual(parseable_schemes(RegtestNet), ("ecregtest",))


if __name__ == "__main__":
unittest.main()
12 changes: 6 additions & 6 deletions electrumabc/tests/test_wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def test_restore_wallet_from_text_mnemonic(self):
self.assertEqual(text, wallet.keystore.get_seed(password))
self.assertEqual(encrypt_file, wallet.storage.is_encrypted())
self.assertEqual(
Address.from_string("qrrqa5sv8xrg7lrq3l4c3ememxfwwsa09gcgf0r5jf"),
Address.from_string("qrrqa5sv8xrg7lrq3l4c3ememxfwwsa09gp9aycw57"),
wallet.get_receiving_addresses()[0],
)

Expand All @@ -124,7 +124,7 @@ def test_restore_wallet_from_text_xpub(self):
wallet: StandardWallet = d["wallet"]
self.assertEqual(text, wallet.keystore.get_master_public_key())
self.assertEqual(
Address.from_string("qzrseeup3rhehuaf9e6nr3sgm6t5eegufu96l404mu"),
Address.from_string("qzrseeup3rhehuaf9e6nr3sgm6t5eegufuuht750at"),
wallet.get_receiving_addresses()[0],
)

Expand All @@ -134,16 +134,16 @@ def test_restore_wallet_from_text_xprv(self):
wallet: StandardWallet = d["wallet"]
self.assertEqual(text, wallet.keystore.get_master_private_key(password=None))
self.assertEqual(
Address.from_string("qr2q6aadv6nxmqwjt8qmax76yqp09mlqzq5jsz5fe9"),
Address.from_string("qr2q6aadv6nxmqwjt8qmax76yqp09mlqzqdlyf0nlj"),
wallet.get_receiving_addresses()[0],
)

def test_restore_wallet_from_text_addresses(self):
text = "qr2q6aadv6nxmqwjt8qmax76yqp09mlqzq5jsz5fe9"
text = "qr2q6aadv6nxmqwjt8qmax76yqp09mlqzqdlyf0nlj"
d = restore_wallet_from_text(text, path=self.wallet_path, config=self.config)
wallet: AbstractWallet = d["wallet"]
self.assertEqual(
Address.from_string("qr2q6aadv6nxmqwjt8qmax76yqp09mlqzq5jsz5fe9"),
Address.from_string("qr2q6aadv6nxmqwjt8qmax76yqp09mlqzqdlyf0nlj"),
wallet.get_receiving_addresses()[0],
)
self.assertEqual(1, len(wallet.get_receiving_addresses()))
Expand All @@ -154,7 +154,7 @@ def test_restore_wallet_from_text_privkeys(self):
wallet: AbstractWallet = d["wallet"]
addr0 = wallet.get_receiving_addresses()[0]
self.assertEqual(
Address.from_string("qzrseeup3rhehuaf9e6nr3sgm6t5eegufu96l404mu"), addr0
Address.from_string("qzrseeup3rhehuaf9e6nr3sgm6t5eegufuuht750at"), addr0
)
self.assertEqual(
"Kz7FS9Adyj6RgSVGx5YLjZPanUhuze4yvcziZ1qLA24a3GJJZvBr",
Expand Down
Loading

0 comments on commit e9da99f

Please sign in to comment.