Disallow bech32m addresses for legacy wallet things

achow101 · achow101 · commit 87a0e7a3b7c0 · 2021-06-22T21:57:04.000-04:00
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.
diff --git 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
@@ -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
@@ -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
@@ -640,22 +640,6 @@ class DescriptorImpl : public Descriptor
     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
@@ -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
@@ -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
@@ -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
@@ -97,6 +97,9 @@ def run_test(self):
             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 @@ def check_addmultisigaddress_errors(self):
             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
@@ -373,5 +373,15 @@ def run_test(self):
         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
@@ -420,6 +420,9 @@ def run_test(self):
             # 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
@@ -746,6 +746,27 @@ def run_test(self):
         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
@@ -135,31 +135,33 @@ def run_test(self):
         # 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:

Original file line number	Diff line number	Diff line change
`@@ -131,6 +131,9 @@ static RPCHelpMan createmultisig()`
`131`	`131`	`if (!ParseOutputType(request.params[2].get_str(), output_type)) {`
`132`	`132`	`throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[2].get_str()));`
`133`	`133`	`}`
	`134`	`+ if (output_type == OutputType::BECH32M) {`
	`135`	`+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "createmultisig cannot create bech32m multisig addresses");`
	`136`	`+ }`
`134`	`137`	`}`
`135`	`138`
`136`	`139`	`// Construct using pay-to-script-hash:`
Original file line number	Diff line number	Diff line change
`@@ -269,6 +269,9 @@ static RPCHelpMan getnewaddress()`
`269`	`269`	`if (!ParseOutputType(request.params[1].get_str(), output_type)) {`
`270`	`270`	`throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str()));`
`271`	`271`	`}`
	`272`	`+ if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) {`
	`273`	`+ throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses");`
	`274`	`+ }`
`272`	`275`	`}`
`273`	`276`
`274`	`277`	`CTxDestination dest;`
`@@ -313,6 +316,9 @@ static RPCHelpMan getrawchangeaddress()`
`313`	`316`	`if (!ParseOutputType(request.params[0].get_str(), output_type)) {`
`314`	`317`	`throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str()));`
`315`	`318`	`}`
	`319`	`+ if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) {`
	`320`	`+ throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses");`
	`321`	`+ }`
`316`	`322`	`}`
`317`	`323`
`318`	`324`	`CTxDestination dest;`
`@@ -1004,6 +1010,9 @@ static RPCHelpMan addmultisigaddress()`
`1004`	`1010`	`if (!ParseOutputType(request.params[3].get_str(), output_type)) {`
`1005`	`1011`	`throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str()));`
`1006`	`1012`	`}`
	`1013`	`+ if (output_type == OutputType::BECH32M) {`
	`1014`	`+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m multisig addresses cannot be created with legacy wallets");`
	`1015`	`+ }`
`1007`	`1016`	`}`
`1008`	`1017`
`1009`	`1018`	`// Construct using pay-to-script-hash:`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestinat`
`26`	`26`	`error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated;`
`27`	`27`	`return false;`
`28`	`28`	`}`
	`29`	`+ assert(type != OutputType::BECH32M);`
`29`	`30`
`30`	`31`	`LOCK(cs_KeyStore);`
`31`	`32`	`error.clear();`
`@@ -299,6 +300,7 @@ bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool i`
`299`	`300`	`if (LEGACY_OUTPUT_TYPES.count(type) == 0) {`
`300`	`301`	`return false;`
`301`	`302`	`}`
	`303`	`+ assert(type != OutputType::BECH32M);`
`302`	`304`
`303`	`305`	`LOCK(cs_KeyStore);`
`304`	`306`	`if (!CanGetAddresses(internal)) {`
`@@ -1303,6 +1305,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const`
`1303`	`1305`
`1304`	`1306`	`void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type)`
`1305`	`1307`	`{`
	`1308`	`+ assert(type != OutputType::BECH32M);`
`1306`	`1309`	`// Remove from key pool`
`1307`	`1310`	`WalletBatch batch(m_storage.GetDatabase());`
`1308`	`1311`	`batch.ErasePool(nIndex);`
`@@ -1336,6 +1339,7 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, co`
`1336`	`1339`
`1337`	`1340`	`bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, const OutputType type, bool internal)`
`1338`	`1341`	`{`
	`1342`	`+ assert(type != OutputType::BECH32M);`
`1339`	`1343`	`if (!CanGetAddresses(internal)) {`
`1340`	`1344`	`return false;`
`1341`	`1345`	`}`
`@@ -1404,6 +1408,7 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key`
`1404`	`1408`
`1405`	`1409`	`void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType type)`
`1406`	`1410`	`{`
	`1411`	`+ assert(type != OutputType::BECH32M);`
`1407`	`1412`	`if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT \|\| type == OutputType::BECH32)) {`
`1408`	`1413`	`CTxDestination witdest = WitnessV0KeyHash(key.GetID());`
`1409`	`1414`	`CScript witprog = GetScriptForDestination(witdest);`