phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 02:33:24 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#27279: Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet

Browse source 
7ccdd741fe test: fix importmulti/importdescriptors assertion (Jon Atack)
19d888ce40 rpc: move WALLET_FLAG_CAVEATS to the compilation unit of its caller (Jon Atack)
01df011ca2 doc: release note for wallet RPCs "warning" field deprecation (Jon Atack)
9ea8b3739a test: createwallet "warning" field deprecation test (Jon Atack)
645d7f75ac rpc: deprecate "warning" field in {create,load,unload,restore}wallet (Jon Atack)
2f4a926e95 test: add test coverage for "warnings" field in createwallet (Jon Atack)
4a1e479ca6 rpc: add "warnings" field to RPCs {create,load,unload,restore}wallet (Jon Atack)
079d8cdda8 rpc: extract wallet "warnings" fields to a util helper (Jon Atack)
f73782a903 doc: fix/improve warning helps in {create,load,unload,restore}wallet (Jon Atack)

Pull request description:

  Based on discussion and concept ACKed in #27138, add a `warnings` field to RPCs createwallet, loadwallet, unloadwallet, and restorewallet as a JSON array of strings to replace the `warning` string field in these 4 RPCs. The idea is to more gracefully handle multiple warning messages and for consistency with other wallet RPCs.  Then, deprecate the latter fields, which represent all the remaining RPC `warning` fields.

  The first commit f73782a903 implements https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1474789198 as an alternative to #27138. One of those two could potentially be backported to our currently supported releases.

ACKs for top commit:
  achow101:
    ACK 7ccdd741fe
  1440000bytes:
    utACK 7ccdd741fe
  vasild:
    ACK 7ccdd741fe
  pinheadmz:
    re-ACK 7ccdd741fe

Tree-SHA512: 314e0a4c41fa383d95e2817bfacf359d449e460529d235c3eb902851e2f4eacbabe646d9a5a4beabc4964cdfabf6397ed8301366a58d344a2f787f83b75e9d64
This commit is contained in:
Andrew Chow 2023-04-12 13:02:11 -04:00
commit 6a167325f0
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
11 changed files with 125 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
doc/release-notes-27279.md Normal file
View file

@ -0,0 +1,10 @@
Wallet
------
- In the createwallet, loadwallet, unloadwallet, and restorewallet RPCs, the
"warning" string field is deprecated in favor of a "warnings" field that
returns a JSON array of strings to better handle multiple warning messages and
for consistency with other wallet RPCs. The "warning" field will be fully
removed from these RPCs in v26. It can be temporarily re-enabled during the
deprecation period by launching bitcoind with the configuration option
`-deprecatedrpc=walletwarningfield`. (#27279)

2
src/rpc/output_script.cpp
View file

@ -162,7 +162,7 @@ static RPCHelpMan createmultisig()
// Only warns if the user has explicitly chosen an address type we cannot generate
warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present.");
}
if (!warnings.empty()) result.pushKV("warnings", warnings);
PushWarnings(warnings, result);
return result;
},

23
src/rpc/util.cpp
View file

@ -1174,3 +1174,26 @@ UniValue GetServicesNames(ServiceFlags services)
return servicesNames;
}
/** Convert a vector of bilingual strings to a UniValue::VARR containing their original untranslated values. */
[[nodiscard]] static UniValue BilingualStringsToUniValue(const std::vector<bilingual_str>& bilingual_strings)
{
CHECK_NONFATAL(!bilingual_strings.empty());
UniValue result{UniValue::VARR};
for (const auto& s : bilingual_strings) {
result.push_back(s.original);
}
return result;
}
void PushWarnings(const UniValue& warnings, UniValue& obj)
{
if (warnings.empty()) return;
obj.pushKV("warnings", warnings);
}
void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj)
{
if (warnings.empty()) return;
obj.pushKV("warnings", BilingualStringsToUniValue(warnings));
}

9
src/rpc/util.h
View file

@ -381,4 +381,13 @@ private:
const RPCExamples m_examples;
};
/**
* Push warning messages to an RPC "warnings" field as a JSON array of strings.
*
* @param[in] warnings Warning messages to push.
* @param[out] obj UniValue object to push the warnings array object to.
*/
void PushWarnings(const UniValue& warnings, UniValue& obj);
void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj);
#endif // BITCOIN_RPC_UTIL_H

2
src/wallet/rpc/addresses.cpp
View file

@ -300,7 +300,7 @@ RPCHelpMan addmultisigaddress()
// Only warns if the user has explicitly chosen an address type we cannot generate
warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present.");
}
if (!warnings.empty()) result.pushKV("warnings", warnings);
PushWarnings(warnings, result);
return result;
},

15
src/wallet/rpc/backup.cpp
View file

@ -1225,7 +1225,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64
result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields"));
}
if (warnings.size()) result.pushKV("warnings", warnings);
PushWarnings(warnings, result);
return result;
}
@ -1579,7 +1579,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
result.pushKV("success", UniValue(false));
result.pushKV("error", e);
}
if (warnings.size()) result.pushKV("warnings", warnings);
PushWarnings(warnings, result);
return result;
}
@ -1903,7 +1903,11 @@ RPCHelpMan restorewallet()
RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR, "name", "The wallet name if restored successfully."},
{RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
{RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
{RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring the wallet.",
{
{RPCResult::Type::STR, "", ""},
}},
}
},
RPCExamples{
@ -1933,7 +1937,10 @@ RPCHelpMan restorewallet()
UniValue obj(UniValue::VOBJ);
obj.pushKV("name", wallet->GetName());
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
}
PushWarnings(warnings, obj);
return obj;

45
src/wallet/rpc/wallet.cpp
View file

@ -13,6 +13,7 @@
#include <wallet/rpc/wallet.h>
#include <wallet/rpc/util.h>
#include <wallet/wallet.h>
#include <wallet/walletutil.h>
#include <optional>
@ -20,6 +21,14 @@
namespace wallet {
static const std::map<uint64_t, std::string> WALLET_FLAG_CAVEATS{
{WALLET_FLAG_AVOID_REUSE,
"You need to rescan the blockchain in order to correctly mark used "
"destinations in the past. Until this is done, some destinations may "
"be considered unused, even if the opposite is the case."},
};
/** Checks if a CKey is in the given CWallet compressed or otherwise*/
bool HaveKey(const SigningProvider& wallet, const CKey& key)
{
@ -207,7 +216,11 @@ static RPCHelpMan loadwallet()
RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR, "name", "The wallet name if loaded successfully."},
{RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
{RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
{RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to loading the wallet.",
{
{RPCResult::Type::STR, "", ""},
}},
}
},
RPCExamples{
@ -240,7 +253,10 @@ static RPCHelpMan loadwallet()
UniValue obj(UniValue::VOBJ);
obj.pushKV("name", wallet->GetName());
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
}
PushWarnings(warnings, obj);
return obj;
},
@ -335,7 +351,11 @@ static RPCHelpMan createwallet()
RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR, "name", "The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path."},
{RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
{RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
{RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating the wallet.",
{
{RPCResult::Type::STR, "", ""},
}},
}
},
RPCExamples{
@ -405,7 +425,10 @@ static RPCHelpMan createwallet()
UniValue obj(UniValue::VOBJ);
obj.pushKV("name", wallet->GetName());
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
}
PushWarnings(warnings, obj);
return obj;
},
@ -422,7 +445,11 @@ static RPCHelpMan unloadwallet()
{"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
},
RPCResult{RPCResult::Type::OBJ, "", "", {
{RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."},
{RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
{RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to unloading the wallet.",
{
{RPCResult::Type::STR, "", ""},
}},
}},
RPCExamples{
HelpExampleCli("unloadwallet", "wallet_name")
@ -460,11 +487,13 @@ static RPCHelpMan unloadwallet()
throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
}
}
UniValue result(UniValue::VOBJ);
if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
result.pushKV("warning", Join(warnings, Untranslated("\n")).original);
}
PushWarnings(warnings, result);
UnloadWallet(std::move(wallet));
UniValue result(UniValue::VOBJ);
result.pushKV("warning", Join(warnings, Untranslated("\n")).original);
return result;
},
};

7
src/wallet/wallet.cpp
View file

@ -52,13 +52,6 @@
using interfaces::FoundBlock;
namespace wallet {
const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS{
{WALLET_FLAG_AVOID_REUSE,
"You need to rescan the blockchain in order to correctly mark used "
"destinations in the past. Until this is done, some destinations may "
"be considered unused, even if the opposite is the case."
},
};
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
{

2
src/wallet/wallet.h
View file

@ -146,8 +146,6 @@ static const std::map<std::string,WalletFlags> WALLET_FLAG_MAP{
{"external_signer", WALLET_FLAG_EXTERNAL_SIGNER}
};
extern const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS;
/** A wrapper to reserve an address from a wallet
*
* ReserveDestination is used to reserve an address.

7
test/functional/wallet_backwards_compatibility.py
View file

@ -265,7 +265,12 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
)
load_res = node_master.loadwallet("u1_v16")
# Make sure this wallet opens without warnings. See https://github.com/bitcoin/bitcoin/pull/19054
assert_equal(load_res['warning'], '')
if int(node_master.getnetworkinfo()["version"]) >= 249900:
# loadwallet#warnings (added in v25) -- only present if there is a warning
assert "warnings" not in load_res
else:
# loadwallet#warning (deprecated in v25) -- always present, but empty string if no warning
assert_equal(load_res["warning"], '')
wallet = node_master.get_wallet_rpc("u1_v16")
info = wallet.getaddressinfo(v16_addr)
descriptor = f"wpkh([{info['hdmasterfingerprint']}{hdkeypath[1:]}]{v16_pubkey})"

31
test/functional/wallet_createwallet.py
View file

@ -15,12 +15,17 @@ from test_framework.util import (
)
from test_framework.wallet_util import bytes_to_wif, generate_wif_key
EMPTY_PASSPHRASE_MSG = "Empty string given as passphrase, wallet will not be encrypted."
LEGACY_WALLET_MSG = "Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future."
class CreateWalletTest(BitcoinTestFramework):
def add_options(self, parser):
self.add_wallet_options(parser)
def set_test_params(self):
self.num_nodes = 1
self.extra_args = [["-deprecatedrpc=walletwarningfield"]]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
@ -55,7 +60,7 @@ class CreateWalletTest(BitcoinTestFramework):
else:
result = w1.importmulti([{'scriptPubKey': {'address': key_to_p2wpkh(eckey.get_pubkey().get_bytes())}, 'timestamp': 'now', 'keys': [privkey]}])
assert not result[0]['success']
assert 'warning' not in result[0]
assert 'warnings' not in result[0]
assert_equal(result[0]['error']['code'], -4)
assert_equal(result[0]['error']['message'], 'Cannot import private keys to a wallet with private keys disabled')
@ -159,7 +164,9 @@ class CreateWalletTest(BitcoinTestFramework):
assert_equal(walletinfo['keypoolsize_hd_internal'], keys)
# Allow empty passphrase, but there should be a warning
resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='')
assert 'Empty string given as passphrase, wallet will not be encrypted.' in resp['warning']
assert_equal(resp["warning"], EMPTY_PASSPHRASE_MSG if self.options.descriptors else f"{EMPTY_PASSPHRASE_MSG}\n{LEGACY_WALLET_MSG}")
assert_equal(resp["warnings"], [EMPTY_PASSPHRASE_MSG] if self.options.descriptors else [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG])
w7 = node.get_wallet_rpc('w7')
assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 60)
@ -174,8 +181,24 @@ class CreateWalletTest(BitcoinTestFramework):
if self.is_bdb_compiled():
self.log.info("Test legacy wallet deprecation")
res = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, passphrase=None)
assert_equal(res["warning"], "Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future.")
result = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, passphrase=None)
assert_equal(result, {
"name": "legacy_w0",
"warning": LEGACY_WALLET_MSG,
"warnings": [LEGACY_WALLET_MSG],
})
result = self.nodes[0].createwallet(wallet_name="legacy_w1", descriptors=False, passphrase="")
assert_equal(result, {
"name": "legacy_w1",
"warning": f"{EMPTY_PASSPHRASE_MSG}\n{LEGACY_WALLET_MSG}",
"warnings": [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG],
})
self.log.info('Test "warning" field deprecation, i.e. not returned without -deprecatedrpc=walletwarningfield')
self.restart_node(0, extra_args=[])
result = self.nodes[0].createwallet(wallet_name="w7_again", disable_private_keys=False, blank=False, passphrase="")
assert "warning" not in result
if __name__ == '__main__':
CreateWalletTest().main()