Skip to content

re-enable wallet in autoprune #6057

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

Jump to bottom
Merged
laanwj merged 3 commits into from
Jun 10, 2015
Merged

re-enable wallet in autoprune #6057

laanwj merged 3 commits into from
Jun 10, 2015

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Apr 24, 2015

No description provided.

@morcos
Copy link
Contributor

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
Contributor 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)

PRabahy
PRabahy reviewed Apr 24, 2015
View reviewed changes
src/init.cpp Outdated
Copy link
Contributor

@PRabahy PRabahy Apr 24, 2015

Choose a reason for hiding this comment

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

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."?

Copy link
Contributor Author

@jonasschnelli jonasschnelli Apr 25, 2015

Choose a reason for hiding this comment

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

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 2015/04/autoprune_wallet branch from 37cf0f4 to 9c28a03 Compare April 26, 2015 12:51
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Apr 26, 2015

Fixed pindexRescan check mentioned by @morcos.

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

sipa commented Apr 27, 2015

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

@jonasschnelli
Copy link
Contributor 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 2015/04/autoprune_wallet branch 2 times, most recently from 3942591 to 3774c90 Compare April 30, 2015 15:44
@laanwj laanwj added the Wallet label May 6, 2015
@jonasschnelli jonasschnelli force-pushed the 2015/04/autoprune_wallet branch from 3774c90 to d36bee5 Compare May 8, 2015 12:45
@PRabahy PRabahy mentioned this pull request May 27, 2015
Merged
@jonasschnelli jonasschnelli force-pushed the 2015/04/autoprune_wallet branch from d36bee5 to 7e6569e Compare May 28, 2015 07:00
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented May 28, 2015

Rebased.

luke-jr
luke-jr reviewed Jun 2, 2015
View reviewed changes
src/init.cpp
Copy link
Member

@luke-jr luke-jr Jun 2, 2015

Choose a reason for hiding this comment

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

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
laanwj added a commit that referenced this pull request Jun 10, 2015
@laanwj
Merge pull request #6057
ac5476e 
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
Closed
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 28, 2015
@luke-jr
Merge pull request bitcoin#6057: re-enable wallet in autoprune
3bb96b2 
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)
@str4d str4d mentioned this pull request Mar 29, 2017
Draft
@jnewbery jnewbery mentioned this pull request May 7, 2018
Merged
@daira daira mentioned this pull request Jul 31, 2019
Open
@str4d str4d mentioned this pull request Nov 14, 2019
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Dec 3, 2019
@zkbot
Auto merge of #4226 - str4d:2074-wallet-1, r=mdr0id
5e26d48 
Bitcoin 0.12 wallet PRs 1

Cherry-picked from the following upstream PRs:

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

Part of #2074. Closes #3700.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Wallet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jonasschnelli @morcos @sipa @laanwj @PRabahy @luke-jr