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

re-enable wallet in autoprune #6057

Merged
merged 3 commits into from Jun 10, 2015
Merged

Conversation

@jonasschnelli
Copy link
Member

jonasschnelli commented Apr 24, 2015

No description provided.

@morcos
Copy link
Member

morcos commented Apr 24, 2015

Nice! I'm glad you looked at this.

The way you checked pindexRescan isn't sufficient however. You can't assume a pruned node has all the blocks past a certain point. Blocks are deleted from disk in the order they were written, which doesn't necessarily correspond with order in the chain. But perhaps you could scan backwards from the Tip until you get to either pindexRescan or one which isn't BLOCK_HAVE_DATA.

@jonasschnelli
Copy link
Member Author

jonasschnelli commented Apr 24, 2015

Right. I did ignore the fact that blocks are not fly in in order. Will change it to your proposed way of scanning backwards.

Same needs to be done for #6058 (edit: already correct there)

else
return InitError(_("Can't run with a wallet in prune mode."));
if (GetBoolArg("-rescan", false)) {
return InitError(_("Rescans are not possible in pruned mode. You might use -reindex which then download the whole blockchain again."));

This comment has been minimized.

Copy link
@PRabahy

PRabahy Apr 24, 2015

Contributor

This message is unclear to me. Do you mean:
"Rescans are not possible in pruned mode. You will need to use -reindex which will download the whole blockchain again."?

This comment has been minimized.

Copy link
@jonasschnelli

jonasschnelli Apr 25, 2015

Author Member

Thanks!
English is not my mother language and I always struggle with it.
Feel free to also correct/rewrite the other warnings and errors (as well as the code comments).

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2015/04/autoprune_wallet branch from 37cf0f4 to 9c28a03 Apr 26, 2015
@jonasschnelli
Copy link
Member Author

jonasschnelli commented Apr 26, 2015

Fixed pindexRescan check mentioned by @morcos.

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2015/04/autoprune_wallet branch 2 times, most recently from 81cfaf6 to 35d22de Apr 26, 2015
@sipa
Copy link
Member

sipa commented Apr 27, 2015

Concept & code review ACK, but this definitely needs testing.

@jonasschnelli
Copy link
Member Author

jonasschnelli commented Apr 29, 2015

Also applied an additional test during wallet catchup-rescan if the previous block has TXs but no data to be sure we have pruned this block.

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2015/04/autoprune_wallet branch 2 times, most recently from 3942591 to 3774c90 Apr 30, 2015
@laanwj laanwj added the Wallet label May 6, 2015
@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2015/04/autoprune_wallet branch from 3774c90 to d36bee5 May 8, 2015
@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2015/04/autoprune_wallet branch from d36bee5 to 7e6569e May 28, 2015
@jonasschnelli
Copy link
Member Author

jonasschnelli commented May 28, 2015

Rebased.

else
return InitError(_("Can't run with a wallet in prune mode."));
if (GetBoolArg("-rescan", false)) {
return InitError(_("Rescans are not possible in pruned mode. You will need to use -reindex which will download the whole blockchain again."));

This comment has been minimized.

Copy link
@luke-jr

luke-jr Jun 2, 2015

Member

What prevents this error if the user has passed -reindex? In particular, a pruned node may not have the space for the whole blockchain, and may need to redownload it with pruning enabled.

(... btw, does -reindex actually work correctly with pruned data? I think last time I tried, it ignored blocks after the first missing one, and redownloaded/reappended them...)

@laanwj
Copy link
Member

laanwj commented Jun 10, 2015

utACK.

@laanwj laanwj merged commit 7e6569e into bitcoin:master Jun 10, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Jun 10, 2015
7e6569e [squashme] improve/corrects prune mode detection test for required wallet rescans (Jonas Schnelli)
7a12119 [RPC] disable import functions in pruned mode (Jonas Schnelli)
3201035 [autoprune] allow wallet in pruned mode (Jonas Schnelli)
@laanwj laanwj mentioned this pull request Jun 15, 2015
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 28, 2015
7e6569e [squashme] improve/corrects prune mode detection test for required wallet rescans (Jonas Schnelli)
7a12119 [RPC] disable import functions in pruned mode (Jonas Schnelli)
3201035 [autoprune] allow wallet in pruned mode (Jonas Schnelli)
zkbot added a commit to zcash/zcash that referenced this pull request Dec 3, 2019
Bitcoin 0.12 wallet PRs 1

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6057
- bitcoin/bitcoin#6415

Part of #2074. Closes #3700.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.