From c962f597fdc9c696edbbdac655172ab9f5e217e7 Mon Sep 17 00:00:00 2001 From: julian Date: Mon, 7 Nov 2022 14:46:36 -0600 Subject: [PATCH] added extra checks to BCH as well as test cases --- .../coins/bitcoincash/bitcoincash_wallet.dart | 58 ++++--- .../bitcoincash/bitcoincash_wallet_test.dart | 164 +++++++++++++++++- 2 files changed, 199 insertions(+), 23 deletions(-) diff --git a/lib/services/coins/bitcoincash/bitcoincash_wallet.dart b/lib/services/coins/bitcoincash/bitcoincash_wallet.dart index 98a31ee0c..5b3b54663 100644 --- a/lib/services/coins/bitcoincash/bitcoincash_wallet.dart +++ b/lib/services/coins/bitcoincash/bitcoincash_wallet.dart @@ -208,9 +208,9 @@ class BitcoinCashWallet extends CoinServiceAPI { _getCurrentAddressForChain(0, DerivePathType.bip44); Future? _currentReceivingAddressP2PKH; - Future get currentReceivingAddressP2SH => - _currentReceivingAddressP2SH ??= - _getCurrentAddressForChain(0, DerivePathType.bip49); + // Future get currentReceivingAddressP2SH => + // _currentReceivingAddressP2SH ??= + // _getCurrentAddressForChain(0, DerivePathType.bip49); Future? _currentReceivingAddressP2SH; @override @@ -269,7 +269,11 @@ class BitcoinCashWallet extends CoinServiceAPI { try { if (bitbox.Address.detectFormat(address) == bitbox.Address.formatCashAddr) { - address = bitbox.Address.toLegacyAddress(address); + if (validateCashAddr(address)) { + address = bitbox.Address.toLegacyAddress(address); + } else { + throw ArgumentError('$address is not currently supported'); + } } } catch (e, s) {} try { @@ -294,11 +298,14 @@ class BitcoinCashWallet extends CoinServiceAPI { } catch (err) { // Bech32 decode fail } - if (_network.bech32 != decodeBech32!.hrp) { - throw ArgumentError('Invalid prefix or Network mismatch'); - } - if (decodeBech32.version != 0) { - throw ArgumentError('Invalid address version'); + + if (decodeBech32 != null) { + if (_network.bech32 != decodeBech32.hrp) { + throw ArgumentError('Invalid prefix or Network mismatch'); + } + if (decodeBech32.version != 0) { + throw ArgumentError('Invalid address version'); + } } } throw ArgumentError('$address has no matching Script'); @@ -1203,6 +1210,15 @@ class BitcoinCashWallet extends CoinServiceAPI { _transactionData = Future(() => cachedTxData!); } + bool validateCashAddr(String cashAddr) { + String addr = cashAddr; + if (cashAddr.contains(":")) { + addr = cashAddr.split(":").last; + } + + return addr.startsWith("q"); + } + @override bool validateAddress(String address) { try { @@ -1217,12 +1233,7 @@ class BitcoinCashWallet extends CoinServiceAPI { } if (format == bitbox.Address.formatCashAddr) { - String addr = address; - if (address.contains(":")) { - addr = address.split(":").last; - } - - return addr.startsWith("q"); + return validateCashAddr(address); } else { return address.startsWith("1"); } @@ -2085,7 +2096,8 @@ class BitcoinCashWallet extends CoinServiceAPI { String _convertToScriptHash(String bchAddress, NetworkType network) { try { if (bitbox.Address.detectFormat(bchAddress) == - bitbox.Address.formatCashAddr) { + bitbox.Address.formatCashAddr && + validateCashAddr(bchAddress)) { bchAddress = bitbox.Address.toLegacyAddress(bchAddress); } final output = Address.addressToOutputScript(bchAddress, network); @@ -2163,7 +2175,8 @@ class BitcoinCashWallet extends CoinServiceAPI { List allAddressesOld = await _fetchAllOwnAddresses(); List allAddresses = []; for (String address in allAddressesOld) { - if (bitbox.Address.detectFormat(address) == bitbox.Address.formatLegacy) { + if (bitbox.Address.detectFormat(address) == bitbox.Address.formatLegacy && + addressType(address: address) == DerivePathType.bip44) { allAddresses.add(bitbox.Address.toCashAddress(address)); } else { allAddresses.add(address); @@ -2882,7 +2895,12 @@ class BitcoinCashWallet extends CoinServiceAPI { String address = output["scriptPubKey"]["addresses"][0] as String; if (bitbox.Address.detectFormat(address) == bitbox.Address.formatCashAddr) { - address = bitbox.Address.toLegacyAddress(address); + if (validateCashAddr(address)) { + address = bitbox.Address.toLegacyAddress(address); + } else { + throw Exception( + "Unsupported address found during fetchBuildTxData(): $address"); + } } if (!addressTxid.containsKey(address)) { addressTxid[address] = []; @@ -2913,10 +2931,6 @@ class BitcoinCashWallet extends CoinServiceAPI { ); for (int i = 0; i < p2pkhLength; i++) { String address = addressesP2PKH[i]; - if (bitbox.Address.detectFormat(address) == - bitbox.Address.formatCashAddr) { - address = bitbox.Address.toLegacyAddress(address); - } // receives final receiveDerivation = receiveDerivations[address]; diff --git a/test/services/coins/bitcoincash/bitcoincash_wallet_test.dart b/test/services/coins/bitcoincash/bitcoincash_wallet_test.dart index 50ff8f741..077163809 100644 --- a/test/services/coins/bitcoincash/bitcoincash_wallet_test.dart +++ b/test/services/coins/bitcoincash/bitcoincash_wallet_test.dart @@ -60,7 +60,7 @@ void main() { }); }); - group("validate mainnet bitcoincash addresses", () { + group("mainnet bitcoincash addressType", () { MockElectrumX? client; MockCachedElectrumX? cachedClient; MockPriceAPI? priceAPI; @@ -136,6 +136,168 @@ void main() { verifyNoMoreInteractions(priceAPI); }); + test("P2PKH cashaddr with prefix", () { + expect( + mainnetWallet?.addressType( + address: + "bitcoincash:qrwjyc4pewj9utzrtnh0whkzkuvy5q8wg52n254x6k"), + DerivePathType.bip44); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("P2PKH cashaddr without prefix", () { + expect( + mainnetWallet?.addressType( + address: "qrwjyc4pewj9utzrtnh0whkzkuvy5q8wg52n254x6k"), + DerivePathType.bip44); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("Multisig cashaddr with prefix", () { + expect( + () => mainnetWallet?.addressType( + address: + "bitcoincash:pzpp3nchmzzf0gr69lj82ymurg5u3ds6kcwr5m07np"), + throwsArgumentError); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("Multisig cashaddr without prefix", () { + expect( + () => mainnetWallet?.addressType( + address: "pzpp3nchmzzf0gr69lj82ymurg5u3ds6kcwr5m07np"), + throwsArgumentError); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("Multisig/P2SH address", () { + expect( + mainnetWallet?.addressType( + address: "3DYuVEmuKWQFxJcF7jDPhwPiXLTiNnyMFb"), + DerivePathType.bip49); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + }); + + group("validate mainnet bitcoincash addresses", () { + MockElectrumX? client; + MockCachedElectrumX? cachedClient; + MockPriceAPI? priceAPI; + FakeSecureStorage? secureStore; + MockTransactionNotificationTracker? tracker; + + BitcoinCashWallet? mainnetWallet; + + setUp(() { + client = MockElectrumX(); + cachedClient = MockCachedElectrumX(); + priceAPI = MockPriceAPI(); + secureStore = FakeSecureStorage(); + tracker = MockTransactionNotificationTracker(); + + mainnetWallet = BitcoinCashWallet( + walletId: "validateAddressMainNet", + walletName: "validateAddressMainNet", + coin: Coin.bitcoincash, + client: client!, + cachedClient: cachedClient!, + tracker: tracker!, + priceAPI: priceAPI, + secureStore: secureStore, + ); + }); + + test("valid mainnet legacy/p2pkh address type", () { + expect( + mainnetWallet?.validateAddress("1DP3PUePwMa5CoZwzjznVKhzdLsZftjcAT"), + true); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("valid mainnet legacy/p2pkh cashaddr with prefix address type", () { + expect( + mainnetWallet?.validateAddress( + "bitcoincash:qrwjyc4pewj9utzrtnh0whkzkuvy5q8wg52n254x6k"), + true); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("valid mainnet legacy/p2pkh cashaddr without prefix address type", () { + expect( + mainnetWallet + ?.validateAddress("qrwjyc4pewj9utzrtnh0whkzkuvy5q8wg52n254x6k"), + true); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("invalid legacy/p2pkh address type", () { + expect( + mainnetWallet?.validateAddress("mhqpGtwhcR6gFuuRjLTpHo41919QfuGy8Y"), + false); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test( + "invalid cashaddr (is valid multisig but bitbox is broken for multisig)", + () { + expect( + mainnetWallet + ?.validateAddress("pzpp3nchmzzf0gr69lj82ymurg5u3ds6kcwr5m07np"), + false); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + + test("multisig address should fail for bitbox", () { + expect( + mainnetWallet?.validateAddress("3DYuVEmuKWQFxJcF7jDPhwPiXLTiNnyMFb"), + false); + expect(secureStore?.interactions, 0); + verifyNoMoreInteractions(client); + verifyNoMoreInteractions(cachedClient); + verifyNoMoreInteractions(tracker); + verifyNoMoreInteractions(priceAPI); + }); + test("invalid mainnet bitcoincash legacy/p2pkh address", () { expect( mainnetWallet?.validateAddress("mhqpGtwhcR6gFuuRjLTpHo41919QfuGy8Y"),