Skip to content

rpc: Enable wallet import on pruned nodes #16037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
promag wants to merge 4 commits into from
Closed

rpc: Enable wallet import on pruned nodes #16037

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23cf377
rpc: Compute nLowestTimestamp on the first iteration in importmulti
promag May 17, 2019
db3256e
rpc: Compute nTimeBegin on the first iteration in importwallet
promag May 17, 2019
0791e64
rpc: Add EnsureBlockDataFromTime utility
promag May 17, 2019
cffd615
rpc: Enable wallet import on pruned nodes
promag May 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions src/wallet/rpcdump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver,
}
}

static void EnsureBlockDataFromTime(interfaces::Chain::Lock& locked_chain, int64_t timestamp)
{
const Optional<int> height = locked_chain.findFirstBlockWithTimeAndHeight(timestamp - TIMESTAMP_WINDOW, 0, nullptr);
Copy link
Member

@fanquake fanquake Mar 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use std::optional in new code.

if (height && !locked_chain.haveBlockOnDisk(*height)) {
Copy link
Member

@luke-jr luke-jr Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we fail if any blocks after this one are pruned too?

Copy link
Contributor Author

@promag promag Sep 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that possible? I mean, if block X is available then X+1 is also available?

Copy link
Member

@luke-jr luke-jr Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocks are pruned as entire files, so it's possible block X is in the same file as block X+2, and we don't want to prune block X+2, but we do want to prune block X+1. This can result in block X+1 being pruned before block X.

throw JSONRPCError(RPC_WALLET_ERROR, "Pruned blocks required to import keys");
}
}

UniValue importprivkey(const JSONRPCRequest& request)
{
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
Expand Down Expand Up @@ -551,13 +559,6 @@ UniValue importwallet(const JSONRPCRequest& request)
},
}.Check(request);

if (pwallet->chain().havePruned()) {
// Exit early and print an error.
// If a block is pruned after this check, we will import the key(s),
// but fail the rescan with a generic error.
throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled when blocks are pruned");
}

WalletRescanReserver reserver(pwallet);
if (!reserver.reserve()) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
Expand Down Expand Up @@ -615,15 +616,20 @@ UniValue importwallet(const JSONRPCRequest& request)
fLabel = true;
}
}
nTimeBegin = std::min(nTimeBegin, nTime);
keys.push_back(std::make_tuple(key, nTime, fLabel, strLabel));
} else if(IsHex(vstr[0])) {
std::vector<unsigned char> vData(ParseHex(vstr[0]));
CScript script = CScript(vData.begin(), vData.end());
int64_t birth_time = DecodeDumpTime(vstr[1]);
if (birth_time > 0) nTimeBegin = std::min(nTimeBegin, birth_time);
scripts.push_back(std::pair<CScript, int64_t>(script, birth_time));
}
}
file.close();
if (pwallet->chain().havePruned()) {
Copy link
Member

@luke-jr luke-jr Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should just check this inside EnsureBlockDataFromTime

EnsureBlockDataFromTime(*locked_chain, nTimeBegin);
}
// We now know whether we are importing private keys, so we can error if private keys are disabled
if (keys.size() > 0 && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI
Expand Down Expand Up @@ -652,8 +658,6 @@ UniValue importwallet(const JSONRPCRequest& request)

if (has_label)
pwallet->SetAddressBook(PKHash(keyid), label, "receive");

nTimeBegin = std::min(nTimeBegin, time);
progress++;
}
for (const auto& script_pair : scripts) {
Expand All @@ -666,9 +670,6 @@ UniValue importwallet(const JSONRPCRequest& request)
fGood = false;
continue;
}
if (time > 0) {
nTimeBegin = std::min(nTimeBegin, time);
}

progress++;
}
Expand Down Expand Up @@ -1370,10 +1371,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
// Verify all timestamps are present before importing any keys.
const Optional<int> tip_height = locked_chain->getHeight();
now = tip_height ? locked_chain->getBlockMedianTimePast(*tip_height) : 0;
for (const UniValue& data : requests.getValues()) {
GetImportTimestamp(data, now);
}

const int64_t minimumTimestamp = 1;

if (fRescan && tip_height) {
Expand All @@ -1382,6 +1379,16 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
fRescan = false;
}

for (const UniValue& data : requests.getValues()) {
const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
// Get the lowest timestamp.
if (timestamp < nLowestTimestamp) {
nLowestTimestamp = timestamp;
}
}
if (pwallet->chain().havePruned()) {
EnsureBlockDataFromTime(*locked_chain, nLowestTimestamp);
}
for (const UniValue& data : requests.getValues()) {
const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
const UniValue result = ProcessImport(pwallet, data, timestamp);
Expand All @@ -1395,11 +1402,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
if (result["success"].get_bool()) {
fRunScan = true;
}

// Get the lowest timestamp.
if (timestamp < nLowestTimestamp) {
nLowestTimestamp = timestamp;
}
}
}
if (fRescan && fRunScan && requests.size()) {
Expand Down