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

wallet: have prune error take precedence over assumedvalid #26282

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

jamesob
Copy link
Contributor

@jamesob jamesob commented Oct 7, 2022

Fixes #23997 (comment).

From Russ Yanofsky:

Agree with all of Marco's points here and think this should be updated

If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}"

From Russ Yanofsky:

"Agree with all of Marco's points here and think this should be updated

If havePrune and hasAssumedValidChain are both true, better to show
havePrune error message.  Assumed-valid error message is vague and not
very actionable.  Would suggest "Error loading wallet. Wallet requires
blocks to be downloaded, and software does not currently support loading
wallets while blocks are being downloaded out of order though assumeutxo
snapshots. Wallet should be able to load successfully after node sync
reaches height {block_height}"

Co-authored-by: MacroFake <MarcoFalke@gmail.com>
Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
@jamesob jamesob force-pushed the 2022-10-fix-au-wallet branch from 7077e7a to 1c36baf Compare October 7, 2022 19:19
@DrahtBot DrahtBot added the Wallet label Oct 7, 2022
@maflcko
Copy link
Member

maflcko commented Oct 10, 2022

ACK 1c36baf

Copy link
Contributor

@aureleoules aureleoules left a comment

Choose a reason for hiding this comment

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

ACK 1c36baf

@fanquake fanquake merged commit 869342f into bitcoin:master Oct 10, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 10, 2022
…sumedvalid

1c36baf wallet: have prune error take precedence over assumedvalid (James O'Beirne)

Pull request description:

  Fixes bitcoin#23997 (comment).

  From Russ Yanofsky:

  > Agree with all of Marco's points here and think this should be updated
  >
  > If havePrune and hasAssumedValidChain are both true, better to show havePrune error message.  Assumed-valid error message is vague and not very actionable.  Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}"

ACKs for top commit:
  MarcoFalke:
    ACK 1c36baf
  aureleoules:
    ACK 1c36baf

Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
@jamesob jamesob mentioned this pull request Oct 13, 2022
18 tasks
@bitcoin bitcoin locked and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants