wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet#18671
Merged
meshcollider merged 1 commit intobitcoin:masterfrom Apr 23, 2020
Merged
wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet#18671meshcollider merged 1 commit intobitcoin:masterfrom
meshcollider merged 1 commit intobitcoin:masterfrom
Conversation
maflcko
force-pushed
the
2004-walletDumpChain
branch
from
fa4532a to
fa9f4be
Compare
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
promag
reviewed
Apr 16, 2020
Contributor
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
Concept ACK. Need to fix test since lock is held before: https://github.com/bitcoin/bitcoin/blob/fa9f4be785dbc62f3a713e437c29d281b140c9d9/src/wallet/test/wallet_tests.cpp#L230
maflcko
force-pushed
the
2004-walletDumpChain
branch
from
fa9f4be to
faaf3b2
Compare
maflcko
force-pushed
the
2004-walletDumpChain
branch
from
faaf3b2 to
fa60afc
Compare
This was referenced Apr 17, 2020
Contributor
|
Code review ACK fa60afc. |
This was referenced Apr 18, 2020
Member
|
I don't see why we should ever hold a fix back from backporting... especially a simple one. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 24, 2020
…mpwallet fa60afc wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet (MarcoFalke) Pull request description: dumpwallet includes the block hash in the output, so this method depends on the chainstate. According to the developer notes https://github.com/bitcoin/bitcoin/blame/e84a5f000493fe39adb2a5f22b43c3848dcd0a4f/doc/developer-notes.md#L1095 it must include a `BlockUntilSyncedToCurrentChain`. This is a minor fix and does not need backport, I think. It fixes test failures such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/675487097#L2657 , which can only happen in master because the test was not backported. ACKs for top commit: promag: Code review ACK fa60afc. ryanofsky: Code review ACK fa60afc meshcollider: utACK fa60afc Tree-SHA512: 8df70b06b226b2cdf880dec9264adb72d66fd81b09b404fd1665a79e5f5236d26122eebf15df00fe71ee292b5c91b2dc23a0a42b2aa50a8d690604b23832723f
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
May 5, 2020
Just the actual fix Github-Pull: bitcoin#18671 Rebased-From: fa60afc (partial)
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 11, 2020
Summary: This is a backport of Core [[bitcoin/bitcoin#18671 | PR18671]] Depends on D7868 Test Plan: ninja all check-all Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7876
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
dumpwallet includes the block hash in the output, so this method depends on the chainstate. According to the developer notes https://github.com/bitcoin/bitcoin/blame/e84a5f000493fe39adb2a5f22b43c3848dcd0a4f/doc/developer-notes.md#L1095 it must include a
BlockUntilSyncedToCurrentChain.It fixes test failures such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/675487097#L2657 , which can only happen in master because #17954 is only in master.