Skip to content
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.

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

Prevent wallet from rescanning beyond prune depth #751

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pinheadmz
Copy link
Member

This PR replaces #532 and is built on top of #749.

Once #749 is merged, the wallet will be able to determine whether the node it's connected to is pruned (whether its running as a plugin or standalone service). The prune depth is hard coded in networks.js for each network, so a wallet can prevent the user from rescanning more blocks than are available on disk.

Currently in bcoin, the rescanning process starts with a walletDB rollback (effectively "unconfirming" all the transactions in the DB from the rescan height to the tip), and THEN it scans the blockchain from the given height. If the given rescan height is lower than tip height - prune depth, those blocks will never be scanned and therefore the wallet will be out of sync with the state of the full blockchain.

This PR will throw if the user has requested a rescan beyond the available blocks on disk. The check is added to walletDB but is also added to wallet http -- the reason the http check is needed in addition is because the http call returns success: true before rescanning begins (since in practice its a background process).

Finally, to make testing more practical the regtest pruning settings have been lowered significantly. Added to the changelog is this message:

  • The pruning configuration has been changed for regtest network: The new
    settings will start pruning after height 500, and the node will keep 288
    blocks. The previous settings were 1000 and 10000, respectively.

@pinheadmz pinheadmz added api HTTP, RPC, authentication bug Unexpected or incorrect behavior ready for review Ready to be reviewed tests Has tests for code change is just an addtional test wallet Wallet related labels Apr 5, 2019
@codecov-io
Copy link

codecov-io commented Apr 5, 2019

Codecov Report

Merging #751 into master will increase coverage by 0.64%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #751      +/-   ##
==========================================
+ Coverage   56.64%   57.28%   +0.64%     
==========================================
  Files         112      115       +3     
  Lines       28140    28356     +216     
  Branches     4800     4811      +11     
==========================================
+ Hits        15939    16244     +305     
+ Misses      12201    12112      -89
Impacted Files Coverage Δ
lib/protocol/networks.js 100% <ø> (ø) ⬆️
lib/node/http.js 58.04% <ø> (+10.05%) ⬆️
lib/wallet/nodeclient.js 85.29% <100%> (+3.47%) ⬆️
lib/wallet/http.js 51.71% <100%> (+3.19%) ⬆️
lib/wallet/client.js 81.81% <100%> (ø)
lib/wallet/walletdb.js 77.72% <100%> (+0.42%) ⬆️
lib/wallet/nullclient.js 82.5% <100%> (+0.92%) ⬆️
lib/mempool/fees.js 52.5% <0%> (-8.84%) ⬇️
lib/node/spvnode.js 54.71% <0%> (ø)
lib/wallet/node.js 100% <0%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d601b6a...3f1543c. Read the comment docs.

@pinheadmz pinheadmz force-pushed the rerescan branch 2 times, most recently from e9e024e to a785c77 Compare April 7, 2019 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api HTTP, RPC, authentication bug Unexpected or incorrect behavior tests Has tests for code change is just an addtional test wallet Wallet related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants