diff options
author | Andrew Chow <achow101-github@achow101.com> | 2021-06-04 17:35:47 -0400 |
---|---|---|
committer | Andrew Chow <achow101-github@achow101.com> | 2021-06-22 21:57:04 -0400 |
commit | 87a0e7a3b7c0ffd545e537bd497cdc3e67d045f6 (patch) | |
tree | 0b7a64337d330fcbeefe685b2808fa573f375e71 | |
parent | 6dbe4d10728f882986ed0d9ed77bc736f051c662 (diff) |
Disallow bech32m addresses for legacy wallet things
We don't want the legacy wallet to ever have bech32m addresses so don't
allow importing them. This includes addmultisigaddress as that is a
legacy wallet only RPC
Additionally, bech32m multisigs are not available yet, so disallow them
in createmultisig.
-rw-r--r-- | src/outputtype.cpp | 16 | ||||
-rw-r--r-- | src/outputtype.h | 3 | ||||
-rw-r--r-- | src/rpc/misc.cpp | 3 | ||||
-rw-r--r-- | src/script/descriptor.cpp | 16 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 9 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 9 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 5 | ||||
-rwxr-xr-x | test/functional/rpc_createmultisig.py | 7 | ||||
-rwxr-xr-x | test/functional/wallet_address_types.py | 10 | ||||
-rwxr-xr-x | test/functional/wallet_basic.py | 3 | ||||
-rwxr-xr-x | test/functional/wallet_importmulti.py | 21 | ||||
-rwxr-xr-x | test/functional/wallet_labels.py | 52 |
12 files changed, 113 insertions, 41 deletions
diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 9748fe24c7..8ede7b9974 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -108,3 +108,19 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, } // no default case, so the compiler can warn about missing cases assert(false); } + +std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest) { + if (std::holds_alternative<PKHash>(dest) || + std::holds_alternative<ScriptHash>(dest)) { + return OutputType::LEGACY; + } + if (std::holds_alternative<WitnessV0KeyHash>(dest) || + std::holds_alternative<WitnessV0ScriptHash>(dest)) { + return OutputType::BECH32; + } + if (std::holds_alternative<WitnessV1Taproot>(dest) || + std::holds_alternative<WitnessUnknown>(dest)) { + return OutputType::BECH32M; + } + return std::nullopt; +} diff --git a/src/outputtype.h b/src/outputtype.h index 8727d3f543..2b83235cd0 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -47,4 +47,7 @@ std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key); */ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, const CScript& script, OutputType); +/** Get the OutputType for a CTxDestination */ +std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest); + #endif // BITCOIN_OUTPUTTYPE_H diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index ab239fe79c..5178ce60e8 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -131,6 +131,9 @@ static RPCHelpMan createmultisig() if (!ParseOutputType(request.params[2].get_str(), output_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[2].get_str())); } + if (output_type == OutputType::BECH32M) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "createmultisig cannot create bech32m multisig addresses"); + } } // Construct using pay-to-script-hash: diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 5da249b7f9..d796ed26aa 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -640,22 +640,6 @@ public: std::optional<OutputType> GetOutputType() const override { return std::nullopt; } }; -static std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest) { - if (std::holds_alternative<PKHash>(dest) || - std::holds_alternative<ScriptHash>(dest)) { - return OutputType::LEGACY; - } - if (std::holds_alternative<WitnessV0KeyHash>(dest) || - std::holds_alternative<WitnessV0ScriptHash>(dest)) { - return OutputType::BECH32; - } - if (std::holds_alternative<WitnessV1Taproot>(dest) || - std::holds_alternative<WitnessUnknown>(dest)) { - return OutputType::BECH32M; - } - return std::nullopt; -} - /** A parsed addr(A) descriptor. */ class AddressDescriptor final : public DescriptorImpl { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 4e9ba83ead..35649ab02c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -286,6 +286,9 @@ RPCHelpMan importaddress() if (fP2SH) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); } + if (OutputTypeFromDestination(dest) == OutputType::BECH32M) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets"); + } pwallet->MarkDirty(); @@ -962,6 +965,9 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CP if (!IsValidDestination(dest)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address \"" + output + "\""); } + if (OutputTypeFromDestination(dest) == OutputType::BECH32M) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets"); + } script = GetScriptForDestination(dest); } else { if (!IsHex(output)) { @@ -1086,6 +1092,9 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID if (!parsed_desc) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } + if (parsed_desc->GetOutputType() == OutputType::BECH32M) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m descriptors cannot be imported into legacy wallets"); + } have_solving_data = parsed_desc->IsSolvable(); const bool watch_only = data.exists("watchonly") ? data["watchonly"].get_bool() : false; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8b1b6c6d95..951f575c2f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -269,6 +269,9 @@ static RPCHelpMan getnewaddress() if (!ParseOutputType(request.params[1].get_str(), output_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str())); } + if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses"); + } } CTxDestination dest; @@ -313,6 +316,9 @@ static RPCHelpMan getrawchangeaddress() if (!ParseOutputType(request.params[0].get_str(), output_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); } + if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses"); + } } CTxDestination dest; @@ -1004,6 +1010,9 @@ static RPCHelpMan addmultisigaddress() if (!ParseOutputType(request.params[3].get_str(), output_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str())); } + if (output_type == OutputType::BECH32M) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m multisig addresses cannot be created with legacy wallets"); + } } // Construct using pay-to-script-hash: diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index dedd40694e..2f7729a901 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -26,6 +26,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestinat error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated; return false; } + assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); error.clear(); @@ -299,6 +300,7 @@ bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool i if (LEGACY_OUTPUT_TYPES.count(type) == 0) { return false; } + assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); if (!CanGetAddresses(internal)) { @@ -1303,6 +1305,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type) { + assert(type != OutputType::BECH32M); // Remove from key pool WalletBatch batch(m_storage.GetDatabase()); batch.ErasePool(nIndex); @@ -1336,6 +1339,7 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, co bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, const OutputType type, bool internal) { + assert(type != OutputType::BECH32M); if (!CanGetAddresses(internal)) { return false; } @@ -1404,6 +1408,7 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType type) { + assert(type != OutputType::BECH32M); if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) { CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index af515f3a27..816ec67492 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -97,6 +97,9 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): sorted_key_desc = descsum_create('sh(multi(2,{}))'.format(sorted_key_str)) assert_equal(self.nodes[0].deriveaddresses(sorted_key_desc)[0], t['address']) + # Check that bech32m is currently not allowed + assert_raises_rpc_error(-5, "createmultisig cannot create bech32m multisig addresses", self.nodes[0].createmultisig, 2, self.pub, "bech32m") + def check_addmultisigaddress_errors(self): if self.options.descriptors: return @@ -108,6 +111,10 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): self.nodes[0].importaddress(a) assert_raises_rpc_error(-5, 'no full public key for address', lambda: self.nodes[0].addmultisigaddress(nrequired=1, keys=addresses)) + # Bech32m address type is disallowed for legacy wallets + pubs = [self.nodes[1].getaddressinfo(addr)["pubkey"] for addr in addresses] + assert_raises_rpc_error(-5, "Bech32m multisig addresses cannot be created with legacy wallets", self.nodes[0].addmultisigaddress, 2, pubs, "", "bech32m") + def checkbalances(self): node0, node1, node2 = self.nodes node0.generate(COINBASE_MATURITY) diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index 6d93cf412f..b05fedcfc7 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -373,5 +373,15 @@ class AddressTypeTest(BitcoinTestFramework): self.test_address(4, self.nodes[4].getrawchangeaddress(), multisig=False, typ='p2sh-segwit') self.test_address(4, self.nodes[4].getrawchangeaddress('bech32'), multisig=False, typ='bech32') + if self.options.descriptors: + self.log.info("Descriptor wallets do not have bech32m addreses by default yet") + # TODO: Remove this when they do + assert_raises_rpc_error(-12, "Error: No bech32m addresses available", self.nodes[0].getnewaddress, "", "bech32m") + assert_raises_rpc_error(-12, "Error: Keypool ran out, please call keypoolrefill first", self.nodes[0].getrawchangeaddress, "bech32m") + else: + self.log.info("Legacy wallets cannot make bech32m addresses") + assert_raises_rpc_error(-8, "Legacy wallets cannot provide bech32m addresses", self.nodes[0].getnewaddress, "", "bech32m") + assert_raises_rpc_error(-8, "Legacy wallets cannot provide bech32m addresses", self.nodes[0].getrawchangeaddress, "bech32m") + if __name__ == '__main__': AddressTypeTest().main() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index a052ec7477..b5afc3785e 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -420,6 +420,9 @@ class WalletTest(BitcoinTestFramework): # This will raise an exception for importing an invalid pubkey assert_raises_rpc_error(-5, "Pubkey is not a valid public key", self.nodes[0].importpubkey, "5361746f736869204e616b616d6f746f") + # Bech32m addresses cannot be imported into a legacy wallet + assert_raises_rpc_error(-5, "Bech32m addresses cannot be imported into legacy wallets", self.nodes[0].importaddress, "bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6") + # Import address and private key to check correct behavior of spendable unspents # 1. Send some coins to generate new UTXO address_to_import = self.nodes[2].getnewaddress() diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 0a00c5eed9..baeac655df 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -746,6 +746,27 @@ class ImportMultiTest(BitcoinTestFramework): assert 'hdmasterfingerprint' not in pub_import_info assert 'hdkeypath' not in pub_import_info + # Bech32m addresses and descriptors cannot be imported + self.log.info("Bech32m addresses and descriptors cannot be imported") + self.test_importmulti( + { + "scriptPubKey": {"address": "bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6"}, + "timestamp": "now", + }, + success=False, + error_code=-5, + error_message="Bech32m addresses cannot be imported into legacy wallets", + ) + self.test_importmulti( + { + "desc": descsum_create("tr({})".format(pub)), + "timestamp": "now", + }, + success=False, + error_code=-5, + error_message="Bech32m descriptors cannot be imported into legacy wallets", + ) + # Import some public keys to the keypool of a no privkey wallet self.log.info("Adding pubkey to keypool of disableprivkey wallet") self.nodes[1].createwallet(wallet_name="noprivkeys", disable_private_keys=True) diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 2d792bac52..a571454acf 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -135,31 +135,33 @@ class WalletLabelsTest(BitcoinTestFramework): # in the label. This is a no-op. change_label(node, labels[2].addresses[0], labels[2], labels[2]) - self.log.info('Check watchonly labels') - node.createwallet(wallet_name='watch_only', disable_private_keys=True) - wallet_watch_only = node.get_wallet_rpc('watch_only') - BECH32_VALID = { - '✔️_VER15_PROG40': 'bcrt10qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqxkg7fn', - '✔️_VER16_PROG03': 'bcrt1sqqqqq8uhdgr', - '✔️_VER16_PROB02': 'bcrt1sqqqq4wstyw', - } - BECH32_INVALID = { - '❌_VER15_PROG41': 'bcrt1sqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqajlxj8', - '❌_VER16_PROB01': 'bcrt1sqq5r4036', - } - for l in BECH32_VALID: - ad = BECH32_VALID[l] - wallet_watch_only.importaddress(label=l, rescan=False, address=ad) - node.generatetoaddress(1, ad) - assert_equal(wallet_watch_only.getaddressesbylabel(label=l), {ad: {'purpose': 'receive'}}) - assert_equal(wallet_watch_only.getreceivedbylabel(label=l), 0) - for l in BECH32_INVALID: - ad = BECH32_INVALID[l] - assert_raises_rpc_error( - -5, - "Address is not valid" if self.options.descriptors else "Invalid Bitcoin address or script", - lambda: wallet_watch_only.importaddress(label=l, rescan=False, address=ad), - ) + if self.options.descriptors: + # This is a descriptor wallet test because of segwit v1+ addresses + self.log.info('Check watchonly labels') + node.createwallet(wallet_name='watch_only', disable_private_keys=True) + wallet_watch_only = node.get_wallet_rpc('watch_only') + BECH32_VALID = { + '✔️_VER15_PROG40': 'bcrt10qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqxkg7fn', + '✔️_VER16_PROG03': 'bcrt1sqqqqq8uhdgr', + '✔️_VER16_PROB02': 'bcrt1sqqqq4wstyw', + } + BECH32_INVALID = { + '❌_VER15_PROG41': 'bcrt1sqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqajlxj8', + '❌_VER16_PROB01': 'bcrt1sqq5r4036', + } + for l in BECH32_VALID: + ad = BECH32_VALID[l] + wallet_watch_only.importaddress(label=l, rescan=False, address=ad) + node.generatetoaddress(1, ad) + assert_equal(wallet_watch_only.getaddressesbylabel(label=l), {ad: {'purpose': 'receive'}}) + assert_equal(wallet_watch_only.getreceivedbylabel(label=l), 0) + for l in BECH32_INVALID: + ad = BECH32_INVALID[l] + assert_raises_rpc_error( + -5, + "Address is not valid" if self.options.descriptors else "Invalid Bitcoin address or script", + lambda: wallet_watch_only.importaddress(label=l, rescan=False, address=ad), + ) class Label: |