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

Ensure external signer option remains disabled without signers #396

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Aug 6, 2021

When no external signers are available, the option to enable external signers should always be disabled. However the encrypt wallet checkbox can erroneously re-enable the external signer checkbox. To avoid this, CreateWalletDialog now stores whether signers were available during setSigners so that future calls to external_signer_checkbox->setEnabled can account for whether signers are available.

Fixes #395

When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.
@achow101
Copy link
Member Author

achow101 commented Aug 6, 2021

Needs backport to 22.0

@jarolrod jarolrod added Bug Something isn't working Wallet labels Aug 6, 2021
@maflcko maflcko added this to the 22.0 milestone Aug 6, 2021
@hebasto hebasto changed the title gui: ensure external signer option remains disabled without signers Ensure external signer option remains disabled without signers Aug 6, 2021
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK a9b9ca8, tested on Linux Mint 20.2 (Qt 5.12.8).

@@ -32,7 +32,7 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) :
// set to true, enable it when isEncryptWalletChecked is false.
ui->disable_privkeys_checkbox->setEnabled(!checked);
#ifdef ENABLE_EXTERNAL_SIGNER
ui->external_signer_checkbox->setEnabled(!checked);
ui->external_signer_checkbox->setEnabled(m_has_signers && !checked);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Suggested change
ui->external_signer_checkbox->setEnabled(m_has_signers && !checked);
if (m_has_signers) {
ui->external_signer_checkbox->setEnabled(!checked);
}

to make the same code structure as it is in CreateWalletDialog::setSigners for better readability?

Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

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

tACK a9b9ca8

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK a9b9ca8

Extensively tested on Ubuntu 20.04. Thank you for the speedy fix!

@hebasto hebasto merged commit 03826ae into bitcoin-core:master Aug 6, 2021
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 6, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
@hebasto
Copy link
Member

hebasto commented Aug 6, 2021

Backported in bitcoin/bitcoin#22629.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 6, 2021
…abled without signers

a9b9ca8 gui: ensure external signer option remains disabled without signers (Andrew Chow)

Pull request description:

  When no external signers are available, the option to enable external signers should always be disabled. However the encrypt wallet checkbox can erroneously re-enable the external signer checkbox. To avoid this, CreateWalletDialog now stores whether signers were available during setSigners so that future calls to external_signer_checkbox->setEnabled can account for whether signers are available.

  Fixes #395

ACKs for top commit:
  hebasto:
    ACK a9b9ca8, tested on Linux Mint 20.2 (Qt 5.12.8).
  Sjors:
    tACK a9b9ca8
  jarolrod:
    ACK a9b9ca8

Tree-SHA512: 98951bcadc23fce99a66ea2d367c44360989e888c253845a767e1f7085c594562d0f099de4130f4a078c5072aa7806294097d976ee6407291f3d3c5a4a608b44
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 7, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 9, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 9, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 19, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
laanwj added a commit to bitcoin/bitcoin that referenced this pull request Aug 26, 2021
32e1424 Fix build with Boost 1.77.0 (Rafael Sadowski)
cb34a0a qt: Handle new added plurals in bitcoin_en.ts (Hennadii Stepanov)
068985c doc: Mention the flat directory structure for uploads (Andrew Chow)
27d43e5 guix: Don't include directory name in SHA256SUMS (Andrew Chow)
88fb7e3 test: fix bug in 22686 (S3RK)
63fec7e clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE (Carl Dong)
dfaffbe test: Test for ApproximateBestSubset edge case with too little fees (Andrew Chow)
e86b023 wallet: Assert that enough was selected to cover the fees (Andrew Chow)
ffc81e2 wallet: Use GetSelectionAmount for target value calculations (Andrew Chow)
ce77b45 release: Release with separate SHA256SUMS and sig files (Carl Dong)
cb491bd guix-verify: Non-zero exit code when anything fails (Carl Dong)
6a611d2 gui: ensure external signer option remains disabled without signers (Andrew Chow)
e9b4487 qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov)
57fce06 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns)
e9d30fb ci: Run fuzzer task for the master branch only (Hennadii Stepanov)

Pull request description:

  Backported:

  1) #22730
  1) bitcoin-core/gui#393
  1) #22597
  1) bitcoin-core/gui#396
  1) #22643
  1) #22642
  1) #22685
  1) #22686
  1) #22654
  1) #22742
  1) bitcoin-core/gui#406
  1) #22713

ACKs for top commit:
  laanwj:
    Code list-of-backported-PRs review ACK 32e1424

Tree-SHA512: f5e2dd1be6cdcd39368eeb5d297b3ff4418d0bf2e70c90e59ab4ba1dbf16f773045d877b4997511de58c3aca75a978dcf043e338bad23951557e2a27ccc845f6
fujicoin pushed a commit to fujicoin/fujicoin-22.0 that referenced this pull request Aug 27, 2021
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca82daefc77ee3c884d3f250460d7cf734a5
gwillen pushed a commit to gwillen/elements that referenced this pull request Jul 27, 2022
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
gwillen pushed a commit to gwillen/elements that referenced this pull request Aug 1, 2022
When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca8
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Something isn't working Wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

External Signer can be enabled even when external signer script is not set
5 participants