Skip to content

Commit 0553d75

Browse files
laanwjlaanwj
committed
Merge bitcoin/bitcoin#22154: Add OutputType::BECH32M and related wallet support for fetching bech32m addresses
754f134 wallet: Add error message to GetReservedDestination (Andrew Chow) 87a0e7a Disallow bech32m addresses for legacy wallet things (Andrew Chow) 6dbe4d1 Use BECH32M for tr() desc, WitV1Taproot, and WitUnknown CTxDests (Andrew Chow) 699dfcd Opportunistically use bech32m change addresses if available (Andrew Chow) 0262536 Add OutputType::BECH32M (Andrew Chow) 177c15d Limit LegacyScriptPubKeyMan address types (Andrew Chow) Pull request description: Currently bech32m addresses are classfied as bech32. Because bech32m is incompatible with bech32, we need to define a new `OutputType` for it so that it can be handled correctly. This PR adds `OutputType::BECH32M`, updates all of the relevant `OutputType` classifications, and handle requests for bech32m addresses. There is now a `bech32m` address type string that can be used. * `tr()` descriptors now report their output type as `OutputType::BECH32M`. `WtinessV1Taproot` and `WitnessUnknown` are also classified as `OutputType::BECH32M`. * Bech32m addresses are completely disabled for legacy wallets. They cannot be imported (explicitly disallowed in `importaddress` and `importmulti`), will not be created when getting all destinations for a pubkey, and will not be added with `addmultisigaddress`. Additional protections have been added to `LegacyScriptPubKeyMan` to disallow attempting to retrieve bech32m addresses. * Since Taproot multisigs are not implemented yet, `createmultisig` will also disallow the bech32m address type. * As Taproot is not yet active, `DescriptorScriptPubKeyMan` cannot and will not create a `tr()` descriptor. Protections have been added to make sure this cannot occur. * The change address type detection algorithm has been updated to return `bech32m` when there is a segwit v1+ output script and the wallet has a bech32m `ScriptPubKeyMan`, falling back to bech32 if one is not available. ACKs for top commit: laanwj: re-review ACK 754f134 Sjors: re-utACK 754f134: only change is switching to `bech32m` in two `wallet_taproot.py` test cases. fjahr: re-ACK 754f134 jonatack: ACK 754f134 Tree-SHA512: 6ea90867d3631d0d438e2b08ce6ed930f37d01323224661e8e38f183ea5ee2ab65b5891394a3612c7382a1aff907b457616c6725665a10c320174017b998ca9f
2 parents b7565c7 + 754f134 commit 0553d75

19 files changed

+181
-61
lines changed

‎src/outputtype.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy";
1919
static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit";
2020
static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32";
21+
static const std::string OUTPUT_TYPE_STRING_BECH32M = "bech32m";
2122

2223
bool ParseOutputType(const std::string& type, OutputType& output_type)
2324
{
@@ -30,6 +31,9 @@ bool ParseOutputType(const std::string& type, OutputType& output_type)
3031
} else if (type == OUTPUT_TYPE_STRING_BECH32) {
3132
output_type = OutputType::BECH32;
3233
return true;
34+
} else if (type == OUTPUT_TYPE_STRING_BECH32M) {
35+
output_type = OutputType::BECH32M;
36+
return true;
3337
}
3438
return false;
3539
}
@@ -40,6 +44,7 @@ const std::string& FormatOutputType(OutputType type)
4044
case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY;
4145
case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT;
4246
case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32;
47+
case OutputType::BECH32M: return OUTPUT_TYPE_STRING_BECH32M;
4348
} // no default case, so the compiler can warn about missing cases
4449
assert(false);
4550
}
@@ -59,6 +64,7 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type)
5964
return witdest;
6065
}
6166
}
67+
case OutputType::BECH32M: {} // This function should never be used with BECH32M, so let it assert
6268
} // no default case, so the compiler can warn about missing cases
6369
assert(false);
6470
}
@@ -98,6 +104,23 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore,
98104
return ScriptHash(witprog);
99105
}
100106
}
107+
case OutputType::BECH32M: {} // This function should not be used for BECH32M, so let it assert
101108
} // no default case, so the compiler can warn about missing cases
102109
assert(false);
103110
}
111+
112+
std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest) {
113+
if (std::holds_alternative<PKHash>(dest) ||
114+
std::holds_alternative<ScriptHash>(dest)) {
115+
return OutputType::LEGACY;
116+
}
117+
if (std::holds_alternative<WitnessV0KeyHash>(dest) ||
118+
std::holds_alternative<WitnessV0ScriptHash>(dest)) {
119+
return OutputType::BECH32;
120+
}
121+
if (std::holds_alternative<WitnessV1Taproot>(dest) ||
122+
std::holds_alternative<WitnessUnknown>(dest)) {
123+
return OutputType::BECH32M;
124+
}
125+
return std::nullopt;
126+
}

‎src/outputtype.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ enum class OutputType {
1818
LEGACY,
1919
P2SH_SEGWIT,
2020
BECH32,
21+
BECH32M,
2122
};
2223

2324
static constexpr auto OUTPUT_TYPES = std::array{
2425
OutputType::LEGACY,
2526
OutputType::P2SH_SEGWIT,
2627
OutputType::BECH32,
28+
OutputType::BECH32M,
2729
};
2830

2931
[[nodiscard]] bool ParseOutputType(const std::string& str, OutputType& output_type);
@@ -45,4 +47,7 @@ std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key);
4547
*/
4648
CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, const CScript& script, OutputType);
4749

50+
/** Get the OutputType for a CTxDestination */
51+
std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest);
52+
4853
#endif // BITCOIN_OUTPUTTYPE_H

‎src/rpc/misc.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ static RPCHelpMan createmultisig()
131131
if (!ParseOutputType(request.params[2].get_str(), output_type)) {
132132
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[2].get_str()));
133133
}
134+
if (output_type == OutputType::BECH32M) {
135+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "createmultisig cannot create bech32m multisig addresses");
136+
}
134137
}
135138

136139
// Construct using pay-to-script-hash:

‎src/script/descriptor.cpp‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -640,20 +640,6 @@ class DescriptorImpl : public Descriptor
640640
std::optional<OutputType> GetOutputType() const override { return std::nullopt; }
641641
};
642642

643-
static std::optional<OutputType> OutputTypeFromDestination(const CTxDestination& dest) {
644-
if (std::holds_alternative<PKHash>(dest) ||
645-
std::holds_alternative<ScriptHash>(dest)) {
646-
return OutputType::LEGACY;
647-
}
648-
if (std::holds_alternative<WitnessV0KeyHash>(dest) ||
649-
std::holds_alternative<WitnessV0ScriptHash>(dest) ||
650-
std::holds_alternative<WitnessV1Taproot>(dest) ||
651-
std::holds_alternative<WitnessUnknown>(dest)) {
652-
return OutputType::BECH32;
653-
}
654-
return std::nullopt;
655-
}
656-
657643
/** A parsed addr(A) descriptor. */
658644
class AddressDescriptor final : public DescriptorImpl
659645
{
@@ -874,7 +860,7 @@ class TRDescriptor final : public DescriptorImpl
874860
{
875861
assert(m_subdescriptor_args.size() == m_depths.size());
876862
}
877-
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32; }
863+
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32M; }
878864
bool IsSingleType() const final { return true; }
879865
};
880866

‎src/wallet/rpcdump.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ RPCHelpMan importaddress()
286286
if (fP2SH) {
287287
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead");
288288
}
289+
if (OutputTypeFromDestination(dest) == OutputType::BECH32M) {
290+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
291+
}
289292

290293
pwallet->MarkDirty();
291294

@@ -962,6 +965,9 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CP
962965
if (!IsValidDestination(dest)) {
963966
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address \"" + output + "\"");
964967
}
968+
if (OutputTypeFromDestination(dest) == OutputType::BECH32M) {
969+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
970+
}
965971
script = GetScriptForDestination(dest);
966972
} else {
967973
if (!IsHex(output)) {
@@ -1086,6 +1092,9 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
10861092
if (!parsed_desc) {
10871093
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
10881094
}
1095+
if (parsed_desc->GetOutputType() == OutputType::BECH32M) {
1096+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m descriptors cannot be imported into legacy wallets");
1097+
}
10891098

10901099
have_solving_data = parsed_desc->IsSolvable();
10911100
const bool watch_only = data.exists("watchonly") ? data["watchonly"].get_bool() : false;

‎src/wallet/rpcwallet.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ static RPCHelpMan getnewaddress()
269269
if (!ParseOutputType(request.params[1].get_str(), output_type)) {
270270
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str()));
271271
}
272+
if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) {
273+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses");
274+
}
272275
}
273276

274277
CTxDestination dest;
@@ -313,6 +316,9 @@ static RPCHelpMan getrawchangeaddress()
313316
if (!ParseOutputType(request.params[0].get_str(), output_type)) {
314317
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str()));
315318
}
319+
if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) {
320+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses");
321+
}
316322
}
317323

318324
CTxDestination dest;
@@ -1004,6 +1010,9 @@ static RPCHelpMan addmultisigaddress()
10041010
if (!ParseOutputType(request.params[3].get_str(), output_type)) {
10051011
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str()));
10061012
}
1013+
if (output_type == OutputType::BECH32M) {
1014+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m multisig addresses cannot be created with legacy wallets");
1015+
}
10071016
}
10081017

10091018
// Construct using pay-to-script-hash:

‎src/wallet/scriptpubkeyman.cpp‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;
2222

2323
bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error)
2424
{
25+
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
26+
error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated;
27+
return false;
28+
}
29+
assert(type != OutputType::BECH32M);
30+
2531
LOCK(cs_KeyStore);
2632
error.clear();
2733

@@ -289,14 +295,22 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat
289295
return true;
290296
}
291297

292-
bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool)
298+
bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error)
293299
{
300+
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
301+
error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated;
302+
return false;
303+
}
304+
assert(type != OutputType::BECH32M);
305+
294306
LOCK(cs_KeyStore);
295307
if (!CanGetAddresses(internal)) {
308+
error = _("Error: Keypool ran out, please call keypoolrefill first").translated;
296309
return false;
297310
}
298311

299312
if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
313+
error = _("Error: Keypool ran out, please call keypoolrefill first").translated;
300314
return false;
301315
}
302316
address = GetDestinationForKey(keypool.vchPubKey, type);
@@ -1294,6 +1308,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const
12941308

12951309
void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex, const OutputType& type)
12961310
{
1311+
assert(type != OutputType::BECH32M);
12971312
// Remove from key pool
12981313
WalletBatch batch(m_storage.GetDatabase());
12991314
batch.ErasePool(nIndex);
@@ -1327,6 +1342,7 @@ void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, co
13271342

13281343
bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, const OutputType type, bool internal)
13291344
{
1345+
assert(type != OutputType::BECH32M);
13301346
if (!CanGetAddresses(internal)) {
13311347
return false;
13321348
}
@@ -1395,6 +1411,7 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key
13951411

13961412
void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType type)
13971413
{
1414+
assert(type != OutputType::BECH32M);
13981415
if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) {
13991416
CTxDestination witdest = WitnessV0KeyHash(key.GetID());
14001417
CScript witprog = GetScriptForDestination(witdest);
@@ -1706,10 +1723,9 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
17061723
return true;
17071724
}
17081725

1709-
bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool)
1726+
bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error)
17101727
{
17111728
LOCK(cs_desc_man);
1712-
std::string error;
17131729
bool result = GetNewDestination(type, address, error);
17141730
index = m_wallet_descriptor.next_index - 1;
17151731
return result;
@@ -1880,6 +1896,12 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
18801896

18811897
bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type)
18821898
{
1899+
if (addr_type == OutputType::BECH32M) {
1900+
// Don't allow setting up taproot descriptors yet
1901+
// TODO: Allow setting up taproot descriptors
1902+
return false;
1903+
}
1904+
18831905
LOCK(cs_desc_man);
18841906
assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
18851907

@@ -1909,6 +1931,7 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
19091931
desc_prefix = "wpkh(" + xpub + "/84'";
19101932
break;
19111933
}
1934+
case OutputType::BECH32M: assert(false); // TODO: Setup taproot descriptor
19121935
} // no default case, so the compiler can warn about missing cases
19131936
assert(!desc_prefix.empty());
19141937

‎src/wallet/scriptpubkeyman.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ class ScriptPubKeyMan
181181
virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; }
182182
virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; }
183183

184-
virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { return false; }
184+
virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) { return false; }
185185
virtual void KeepDestination(int64_t index, const OutputType& type) {}
186186
virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {}
187187

@@ -254,6 +254,13 @@ class ScriptPubKeyMan
254254
boost::signals2::signal<void ()> NotifyCanGetAddressesChanged;
255255
};
256256

257+
/** OutputTypes supported by the LegacyScriptPubKeyMan */
258+
static const std::unordered_set<OutputType> LEGACY_OUTPUT_TYPES {
259+
OutputType::LEGACY,
260+
OutputType::P2SH_SEGWIT,
261+
OutputType::BECH32,
262+
};
263+
257264
class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProvider
258265
{
259266
private:
@@ -357,7 +364,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
357364
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
358365
bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override;
359366

360-
bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override;
367+
bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override;
361368
void KeepDestination(int64_t index, const OutputType& type) override;
362369
void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override;
363370

@@ -566,7 +573,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
566573
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
567574
bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override;
568575

569-
bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override;
576+
bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override;
570577
void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override;
571578

572579
// Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file

‎src/wallet/spend.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,9 @@ bool CWallet::CreateTransactionInternal(
618618
// Reserve a new key pair from key pool. If it fails, provide a dummy
619619
// destination in case we don't need change.
620620
CTxDestination dest;
621-
if (!reservedest.GetReservedDestination(dest, true)) {
622-
error = _("Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.");
621+
std::string dest_err;
622+
if (!reservedest.GetReservedDestination(dest, true, dest_err)) {
623+
error = strprintf(_("Transaction needs a change address, but we can't generate it. %s"), dest_err);
623624
}
624625
scriptChange = GetScriptForDestination(dest);
625626
// A valid destination implies a change script (and

0 commit comments

Comments
 (0)