wollet: Update::prune keeps rangeproofs of outputs paying scripts introduced by the update - #179
Open
ledegenbtc wants to merge 3 commits into
Open
ledegenbtc wants to merge 3 commits into
ledegenbtc wants to merge 3 commits into
Conversation
Update::prune() decides which outputs belong to the wallet by looking at wallet.cache.paths, but that map is only filled when an update is applied. With the documented usage (prune, then apply) the scripts introduced by the update being pruned are not there yet, so the outputs paying to them lose the whole witness, rangeproof included: at the first sync of a wallet restored from its descriptor, prune() removes as much as prune_witnesses(), and a wallet whose derivation index jumps ahead loses the rangeproofs of the outputs found on the new scripts. Also treat the scripts carried by the update (scripts_with_blinding_pubkey) as wallet scripts. DownloadTxResult::prune takes a predicate instead of a map so the paths map is not cloned at every sync. The new unit test prunes the update_many_txs vector against a wallet that never synced and fails on master.
Add to test_prune a wallet built from the descriptor after the transactions exist: its first update is pruned before being applied. Building a PSET must still find the input rangeproofs, which fails without the previous commit.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Update::prune()decides which outputs belong to the wallet by looking atwallet.cache.paths, but that map is only filled when an update is applied (apply_update_inner). With the documented usage, prune the update and then apply it (see the "Update Pruning" section of the book andtests/prune.rs), the scripts introduced by the update being pruned are not in the map yet, so their outputs are treated as foreign and lose the whole witness, rangeproof included.This is invisible when the wallet already synced before receiving funds, because the gap-limit scripts are already in the cache: that is what
test_prunedoes throughTestWollet::new. It shows up when a funded script is first seen by the update being pruned:full_scancarries the whole history andcache.pathsis empty, soprune()behaves exactly likeprune_witnesses(), contrary to its documentation;The affected outputs then have no
in_utxo_rangeproofwhen spent, andunblind_utxos_with()/reunblind()cannot see them.Fix:
Update::prune()also treats the scripts inscripts_with_blinding_pubkeyas wallet scripts.DownloadTxResult::prunetakes a predicate instead of a map, so the (possibly large)pathsmap is not cloned at every sync.Tests:
test_update_prune_keeps_rangeproofs_of_new_scriptsprunes theupdate_many_txsvector against a wallet that never synced and checks that the outputs paying the update's own scripts keep their rangeproofs, and that applying the pruned update yields the same utxos and balance. Fails on master.test_prunegains a wallet restored from the descriptor after the transactions exist: prune, apply, build a PSET and check thatin_utxo_rangeproofis present. Fails on master.Relates to #158 (found while pruning updates in production; numbers in that thread).