Skip to content

Commit a9beff7

Browse files
Merge #7275: feat: new RPC migratewallet to migrate legacy wallets to backport wallets
d45f5a9 wallet: harden migratewallet backup filename and cover with a test (UdjinM6) e548fbe fix: return false from AdvanceNextIndexTo when descriptor write fails (UdjinM6) f8cc9c2 fix: issue with wallet names containings slashes (Konstantin Akimov) 3b28689 perf: speed up migratewallet and harden its error paths (UdjinM6) 355fcbd fix: loosen condition about P2PK / P2PKH scripts for non-hd imported keys (Konstantin Akimov) 3b30bde test: check mnemonic is set correctly for migrated wallet (Konstantin Akimov) 062dce5 fix: protection belt for more than 1 account for legacy wallets (Konstantin Akimov) 928c184 fix: advance active pkh() descriptor past the legacy counters (Konstantin Akimov) 0db21aa Merge bitcoin#28257: test: check backup from `migratewallet` can be successfully restored (fanquake) ecc9541 Merge bitcoin#26595: wallet: be able to specify a wallet name and passphrase to migratewallet (fanquake) 6be9b71 Merge bitcoin#26594: wallet: Avoid a segfault in migratewallet failure cleanup (fanquake) b38297c Merge bitcoin#26761: wallet: fully migrate address book entries for watchonly/solvable wallets (Andrew Chow) fd4f410 Merge bitcoin#26910: wallet: migrate wallet, exit early if no legacy data exist (Andrew Chow) a00fe15 Merge bitcoin#19602: wallet: Migrate legacy wallets to descriptor wallets (Andrew Chow) Pull request description: ## Issue being fixed or feature implemented Legacy wallets is subject to be removed in Bitcoin Core, here's bitcoin's timeline for reference: bitcoin#20160 Descriptor wallets is going to be created by default from Dash Core v24: #7204 ## What was done? Backports: - bitcoin#19602 - bitcoin#26910 - bitcoin#26761 - bitcoin#26594 - bitcoin#26595 - bitcoin#28257 And more to be done in further PR There are several key differences with Bitcoin Core: - bitcoin uses multiple path derivation and paths for legacy bip44 and descriptor bip44 is not matched; while Dash Core always uses the same derivation path for all HD wallets - combo descriptors can not be used as "active" descriptors so, they added as "inactive" descriptors - there's added 1 more coinjoin derivation path for compability with mobile. It covers a case if used used the same once on mobile wallet and run coinjoin - implementation of HD wallet in our legacy wallets and bitcoin's are not exactly same because this code hasn't been backported. Beside lookup of `mapKeys` and `mapCryptedKeys` there should be done extra lookup over `mapHdPubKeys` in migration code ## How Has This Been Tested? See updates for new functional test wallet_migration.py Also all my own legacy wallets (mainnet, testnet) has been successfully migrated to descriptor during testing of this PR, including coinjoin-status of mixed coins. One of my wallet on testnet highlighted existing bug of mainstream. This bug with watch-only addresses has been fixed by bitcoin#28868 and this fix will be included in the next batch of backports. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK d45f5a9 Tree-SHA512: 7e64bd0142aa0592ab77b69d27e6645adc0efba874f77200cd2999ce56b2ba483719ab5f146396e7bb1f12ba2e9e562b9c6386d53af8e519624861d1d3f79d46
2 parents 71453a8 + d45f5a9 commit a9beff7

17 files changed

Lines changed: 1775 additions & 38 deletions

doc/managing-wallets.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,24 @@ $ dash-cli -rpcwallet="restored-wallet" getwalletinfo
121121
```
122122

123123
The restored wallet can also be loaded in the GUI via `File` ->`Open wallet`.
124+
125+
## Migrating Legacy Wallets to Descriptor Wallets
126+
127+
Legacy wallets (traditional non-descriptor wallets) can be migrated to become Descriptor wallets
128+
through the use of the `migratewallet` RPC. Migrated wallets will have all of their addresses and private keys added to
129+
a newly created Descriptor wallet that has the same name as the original wallet. Because Descriptor
130+
wallets do not support having private keys and watch-only scripts, there may be up to two
131+
additional wallets created after migration. In addition to a descriptor wallet of the same name,
132+
there may also be a wallet named `<name>_watchonly` and `<name>_solvables`. `<name>_watchonly`
133+
contains all of the watchonly scripts. `<name>_solvables` contains any scripts which the wallet
134+
knows but is not watching the corresponding P2SH scripts.
135+
136+
Given that there is an extremely large number of possible configurations for the scripts that
137+
Legacy wallets can know about, be watching for, and be able to sign for, `migratewallet` only
138+
makes a best effort attempt to capture all of these things into Descriptor wallets. There may be
139+
unforeseen configurations which result in some scripts being excluded. If a migration fails
140+
unexpectedly or otherwise misses any scripts, please create an issue on GitHub. A backup of the
141+
original wallet can be found in the wallet directory with the name `<name>-<timestamp>.legacy.bak`.
142+
143+
The backup can be restored using the `restorewallet` command as discussed in the
144+
[Restoring the Wallet From a Backup](#16-restoring-the-wallet-from-a-backup) section

doc/release-notes-19602.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Wallet
2+
======
3+
4+
Migrating Legacy Wallets to Descriptor Wallets
5+
---------------------------------------------
6+
7+
An experimental RPC `migratewallet` has been added to migrate Legacy (non-descriptor) wallets to
8+
Descriptor wallets. More information about the migration process is available in the
9+
[documentation](https://github.com/dashpay/dash/blob/master/doc/managing-wallets.md#migrating-legacy-wallets-to-descriptor-wallets).

src/script/descriptor.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ class DescriptorImpl : public Descriptor
596596
return AddChecksum(ret);
597597
}
598598

599-
bool ToPrivateString(const SigningProvider& arg, std::string& out) const override final
599+
bool ToPrivateString(const SigningProvider& arg, std::string& out) const override
600600
{
601601
bool ret = ToStringHelper(&arg, out, StringType::PRIVATE);
602602
out = AddChecksum(out);
@@ -686,6 +686,7 @@ class AddressDescriptor final : public DescriptorImpl
686686
}
687687
}
688688
bool IsSingleType() const final { return true; }
689+
bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; }
689690
};
690691

691692
/** A parsed raw(H) descriptor. */
@@ -711,6 +712,7 @@ class RawDescriptor final : public DescriptorImpl
711712
}
712713
}
713714
bool IsSingleType() const final { return true; }
715+
bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; }
714716
};
715717

716718
/** A parsed pk(P) descriptor. */

src/wallet/rpc/wallet.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,74 @@ RPCHelpMan simulaterawtransaction()
10581058
};
10591059
}
10601060

1061+
static RPCHelpMan migratewallet()
1062+
{
1063+
return RPCHelpMan{"migratewallet",
1064+
"EXPERIMENTAL warning: This call may not work as expected and may be changed in future releases\n"
1065+
"\nMigrate the wallet to a descriptor wallet.\n"
1066+
"A new wallet backup will need to be made.\n"
1067+
"\nThe migration process will create a backup of the wallet before migrating. This backup\n"
1068+
"file will be named <wallet name>-<timestamp>.legacy.bak and can be found in the directory\n"
1069+
"for this wallet. In the event of an incorrect migration, the backup can be restored using restorewallet."
1070+
"\nEncrypted wallets must have the passphrase provided as an argument to this call.",
1071+
{
1072+
{"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to migrate. If provided both here and in the RPC endpoint, the two must be identical."},
1073+
{"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The wallet passphrase"},
1074+
},
1075+
RPCResult{
1076+
RPCResult::Type::OBJ, "", "",
1077+
{
1078+
{RPCResult::Type::STR, "wallet_name", "The name of the primary migrated wallet"},
1079+
{RPCResult::Type::STR, "watchonly_name", /*optional=*/true, "The name of the migrated wallet containing the watchonly scripts"},
1080+
{RPCResult::Type::STR, "solvables_name", /*optional=*/true, "The name of the migrated wallet containing solvable but not watched scripts"},
1081+
{RPCResult::Type::STR, "backup_path", "The location of the backup of the original wallet"},
1082+
}
1083+
},
1084+
RPCExamples{
1085+
HelpExampleCli("migratewallet", "")
1086+
+ HelpExampleRpc("migratewallet", "")
1087+
},
1088+
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
1089+
{
1090+
std::string wallet_name;
1091+
if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) {
1092+
if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) {
1093+
throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets");
1094+
}
1095+
} else {
1096+
if (request.params[0].isNull()) {
1097+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Either RPC endpoint wallet or wallet_name parameter must be provided");
1098+
}
1099+
wallet_name = request.params[0].get_str();
1100+
}
1101+
1102+
SecureString wallet_pass;
1103+
wallet_pass.reserve(100);
1104+
if (!request.params[1].isNull()) {
1105+
wallet_pass = std::string_view{request.params[1].get_str()};
1106+
}
1107+
1108+
WalletContext& context = EnsureWalletContext(request.context);
1109+
util::Result<MigrationResult> res = MigrateLegacyToDescriptor(wallet_name, wallet_pass, context);
1110+
if (!res) {
1111+
throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(res).original);
1112+
}
1113+
1114+
UniValue r{UniValue::VOBJ};
1115+
r.pushKV("wallet_name", res->wallet_name);
1116+
if (res->watchonly_wallet) {
1117+
r.pushKV("watchonly_name", res->watchonly_wallet->GetName());
1118+
}
1119+
if (res->solvables_wallet) {
1120+
r.pushKV("solvables_name", res->solvables_wallet->GetName());
1121+
}
1122+
r.pushKV("backup_path", fs::PathToString(res->backup_path));
1123+
1124+
return r;
1125+
},
1126+
};
1127+
}
1128+
10611129
// addresses
10621130
RPCHelpMan getaddressinfo();
10631131
RPCHelpMan getnewaddress();
@@ -1179,6 +1247,7 @@ Span<const CRPCCommand> GetWalletRPCCommands()
11791247
{"wallet", &listwallets},
11801248
{"wallet", &loadwallet},
11811249
{"wallet", &lockunspent},
1250+
{"wallet", &migratewallet},
11821251
{"wallet", &newkeypool},
11831252
{"wallet", &removeprunedfunds},
11841253
{"wallet", &rescanblockchain},

0 commit comments

Comments
 (0)