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: Change ScriptPubKeyMan::Upgrade default to True #22461

Merged
merged 2 commits into from Jul 18, 2021

Conversation

achow101
Copy link
Member

When adding a new ScriptPubKeyMan, it's likely that there will be nothing for Upgrade to do. If it is called (via upgradewallet), then it should do nothing, successfully. This PR changes the default ScriptPubKeyMan::Upgrade function so that it returns a success instead of failure when doing nothing.

Fixes #22460

If a ScriptPubKeyMan does not implement Upgrade, then using upgraewallet
will fail unexpectedly. By changing the default to return true, then
this error can be avoided. This is still correct because a successful
upgrade can be that nothing happened.
@maflcko maflcko added this to the 22.0 milestone Jul 15, 2021
@GeneFerneau
Copy link

Concept + review ACK 4ff4dc0

Copy link
Contributor

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 5012a79

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 5012a79

@meshcollider meshcollider merged commit 5341c3b into bitcoin:master Jul 18, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 23, 2021
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 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.

upgradewallet on a descriptor wallet ends with error
6 participants