Skip to content

Conversation

@Saibato
Copy link
Contributor

@Saibato Saibato commented Aug 23, 2021

the result of IsHDEnabled() was initialized with true.

But in case of no keys or a blank hd wallet the iterator would be skipped
and not set to false.
What had resulted in a wrong return and subsequent false hd and watch only icon display
in gui when reloading a wallet after closing.

.. PR has move to main repro bitcoin/bitcoin#22781

…key size

the result of the call was initialized with true.
But in case of no keys, the iterator would be skipped
and not set to false.
What had resulted in a wrong return and subsequent false icon display
in gui.
Comment on lines 1366 to 1375
// All Active ScriptPubKeyMans must be HD for this to be true
bool result = true;
bool result = false;
for (const auto& spk_man : GetActiveScriptPubKeyMans()) {
result &= spk_man->IsHDEnabled();
if (spk_man->IsHDEnabled()) {
result = true;
} else {
return false;
}
}
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

easier would be:

for ...
  if ! enabled: return false;
return true;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not quite, since we need to return false of the init value in case the for loop is never entered. afaics.

@maflcko
Copy link
Contributor

maflcko commented Aug 23, 2021

This is the wrong repo. To modify wallet code, you will need to submit to the main repo.

@maflcko maflcko closed this Aug 23, 2021
@Saibato
Copy link
Contributor Author

Saibato commented Aug 23, 2021

This is the wrong repo. To modify wallet code, you will need to submit to the main repo.

fwics it effects only the gui, but you obviously correct, i will file it too under main,

@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants