Skip to content

Conversation

@S3RK
Copy link
Contributor

@S3RK S3RK commented Oct 15, 2020

Currently importdescriptor command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart.

@fanquake fanquake requested a review from achow101 October 15, 2020 11:15
@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, 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.

Copy link
Member

@achow101 achow101 left a comment

Choose a reason for hiding this comment

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

ACK 538be42

@achow101
Copy link
Member

This should probably be added to the 0.21 milestone.

@maflcko maflcko added this to the 0.21.0 milestone Oct 21, 2020
@luke-jr
Copy link
Member

luke-jr commented Oct 24, 2020

Concept ACK. Looked over the code too, and it looks fine to me, but I didn't dig in deep enough to be comfortable with a full utACK.

Copy link
Contributor

@meshcollider meshcollider left a comment

Choose a reason for hiding this comment

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

utACK 538be42

@laanwj
Copy link
Member

laanwj commented Nov 9, 2020

Code review ACK 538be42

@laanwj laanwj merged commit 1dfe19e into bitcoin:master Nov 9, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 10, 2020
… derivations into a watch-only wallet

538be42 wallet: fix importdescriptor silent fail (Ivan Metlushko)

Pull request description:

  Currently `importdescriptor` command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart.

ACKs for top commit:
  laanwj:
    Code review ACK 538be42
  achow101:
    ACK 538be42
  meshcollider:
    utACK 538be42

Tree-SHA512: 4bdd0ab4437d55b3f1a79c3a300a0b186089155c020fe220a73d0cce274de47d90371d88918d39fd795f9fccf8db328f1e322d29a6062f9ce94a1c254398f004
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 22, 2021
Summary:
Currently `importdescriptor` command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart.

This is a backport of [[bitcoin/bitcoin#20153 | core#20153]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10721
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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.

8 participants