wallet: Avoid deserializing unused records when salvaging #19805
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a KeyFilterFn callback to ReadKeyValue which allows the caller to specify which types to actually deserialize. A KeyFilterFn takes the type as the parameter and returns a bool indicating whether deserialization should continue.
When salvaging a wallet, avoid deserializing any records that we don't care about, i.e. filter for keys only before the deserialization.
This was referenced Aug 25, 2020
What about scripts, can't they be critical as well to spend utxos? |
There are basically two types of scripts that we store: 1) watchonly imports, and 2) the segwit scripts for our keys. For watchonly imports, I don't really think that these are necessary to keep as presumably there is some place else that has those scripts. For segwit scripts for our keys, those are automatically added to the wallet's in-memory |
|
@achow101 Thanks! Clear to me now. |
laanwj
approved these changes
Sep 1, 2020
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 3, 2020
…salvaging 0bbe26a wallet: filter for keys only before record deser in salvage (Andrew Chow) 544e12a walletdb: Add KeyFilterFn to ReadKeyValue (Andrew Chow) Pull request description: When salvaging a wallet, the only things that matter are the private keys. It is not necessary to attempt to deserialize any other records, especially if those records are corrupted too. This PR adds a `KeyFilterFn` function callback to `ReadKeyValue` that salvage uses to filter for only the records that it wants. Of course doing it this way also lets us do other filters in the future from other places should we so desire. ACKs for top commit: ryanofsky: Code review ACK 0bbe26a. Looks great! This should make the recovery code more robust. Normally it'd be good to have a test case for the problem this fixes, but Marco already wrote one in bitcoin#19078, so I think we're covered laanwj: Code review ACK 0bbe26a Tree-SHA512: 8e3ee283a22a79273915711c4fb751f3c9b02ce94e6bf08dc468f1cfdf9fac35c693bbfd2435ce43c3a06c601b9b0a67e209621f6814bedfe3bc7a7ccc37bb01
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 22, 2021
Summary: PR description: > When salvaging a wallet, the only things that matter are the private keys. It is not necessary to attempt to deserialize any other records, especially if those records are corrupted too. > > This PR adds a KeyFilterFn function callback to ReadKeyValue that salvage uses to filter for only the records that it wants. Of course doing it this way also lets us do other filters in the future from other places should we so desire. > walletdb: Add KeyFilterFn to ReadKeyValue > > Add a KeyFilterFn callback to ReadKeyValue which allows the caller to > specify which types to actually deserialize. A KeyFilterFn takes the > type as the parameter and returns a bool indicating whether > deserialization should continue. > wallet: filter for keys only before record deser in salvage > > When salvaging a wallet, avoid deserializing any records that we don't > care about, i.e. filter for keys only before the deserialization. This is a backport of [[bitcoin/bitcoin#19805 | core#19805]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10176
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When salvaging a wallet, the only things that matter are the private keys. It is not necessary to attempt to deserialize any other records, especially if those records are corrupted too.
This PR adds a
KeyFilterFnfunction callback toReadKeyValuethat salvage uses to filter for only the records that it wants. Of course doing it this way also lets us do other filters in the future from other places should we so desire.