Page MenuHomePhabricator
Differential D14803

[electrum] fix auxiliary key dialog initialization on first use
ClosedPublic
Actions

Authored by PiRK on Nov 16 2023, 15:25.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC201b71123c21: [electrum] fix auxiliary key dialog initialization on first use
Summary

When no key was ever used, the index is still 0. The default value of a QSpinBox is 0, so the valueChanged signal is not emitted when setting the index, and the keys are not filled until the user increments the spinbox manually.

Fix this by calling the slot manually when the index is 0. Also rename the slot to something more descriptive than onIndexChanged.

Also remove some unneeded synchronization code. The only thing calling this slot is the signal emitted by the spinbox or the new call added in this diff (when the spinbox's value is already 0), so no need to update the spinbox.

Test Plan

Open the "Show auxiliary keys" tool in the Wallet menu in a wallet that never generated a proof or a delegation and never displayed this tool. Check that the keys are properly shown for index 0.

Diff Detail

Event Timeline

PiRK created this revision.Nov 16 2023, 15:25
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 16 2023, 15:25
PiRK requested review of this revision.Nov 16 2023, 15:25
PiRK updated this revision to Diff 43130.Nov 16 2023, 15:28
PiRK edited the summary of this revision. (Show Details)

remove unnecessary synchronization code. This was needed in an old version of the code when the index could be set via some API.

PiRK added a child revision: D14804: [electrum] only increment the aux key index if we know it has been used.Nov 16 2023, 15:39
Harbormaster completed remote builds in B25644: Diff 43129.Nov 16 2023, 15:39
teamcity added a comment.Nov 16 2023, 15:39
Build Bitcoin ABC Diffs / Diff Testing (electrum-tests) failed.

Tail of the build log:

-- Found Event: /usr/include (found suitable version "2.1.12-stable", minimum required is "2.0.22") found components: event 
-- Found Boost: /usr/lib/x86_64-linux-gnu/cmake/Boost-1.74.0/BoostConfig.cmake (found suitable version "1.74.0", minimum required is "1.64")  
-- Found Event component pthreads: /usr/lib/x86_64-linux-gnu/libevent_pthreads.so
-- Found Event: /usr/include (found suitable version "2.1.12-stable", minimum required is "2.0.22") found components: pthreads 
-- Found MiniUPnPc component miniupnpc: /usr/lib/x86_64-linux-gnu/libminiupnpc.so
-- Found MiniUPnPc: /usr/include/miniupnpc (found suitable version "2.2.1", minimum required is "1.9")  
-- Found NATPMP component natpmp: /usr/lib/x86_64-linux-gnu/libnatpmp.so
-- Found NATPMP: /usr/include   
-- Found BerkeleyDB component CXX: /usr/lib/x86_64-linux-gnu/libdb_cxx-5.3.so
-- Found BerkeleyDB: /usr/include (found suitable version "5.3.28", minimum required is "5.3") found components: CXX 
-- Found SQLite3: /usr/include (found suitable version "3.34.1", minimum required is "3.7.17") 
-- Found ZeroMQ component zmq: /usr/lib/x86_64-linux-gnu/libzmq.so
-- Found ZeroMQ: /usr/include (found suitable version "4.3.4", minimum required is "4.1.5")  
-- Could NOT find Protobuf (missing: Protobuf_DIR)
-- Found Protobuf: /usr/lib/x86_64-linux-gnu/libprotobuf.so;-pthread (found version "3.12.4") 
-- Found OpenSSL: /usr/lib/x86_64-linux-gnu/libcrypto.so (found version "1.1.1w")  
-- Looking for EVP_MD_CTX_new
-- Looking for EVP_MD_CTX_new - found
-- Found QREncode component qrencode: /usr/lib/x86_64-linux-gnu/libqrencode.so
-- Found QREncode: /usr/include   
-- Configuring native build in /work/abc-ci-builds/electrum-tests/native
-- Configuring done
-- Generating done
-- Build files have been written to: /work/abc-ci-builds/electrum-tests
[1/2] link test_runner.py
[2/2] Run Electrum ABC unit tests...
FAILED: electrum/CMakeFiles/check-electrum 
cd /work/abc-ci-builds/electrum-tests/electrum && /usr/bin/python3.9 ./test_runner.py
[secp256k1] warning: libsecp256k1 library failed to load
[secp256k1] warning: loading from /work/electrum/electrumabc/libsecp256k1.so.0 failed with: /work/electrum/electrumabc/libsecp256k1.so.0: cannot open shared object file: No such file or directory
[secp256k1] warning: loading from libsecp256k1.so.0 failed with: libsecp256k1.so.0: cannot open shared object file: No such file or directory
Testing `setup.py --version`: OK

[ecc] info: libsecp256k1 library not available, falling back to python-ecdsa. This means signing operations will be slower. Try running:

  $  contrib/make_secp

(You need to be running from the git sources for contrib/make_secp to be available)
..................................................................................................................127.0.0.1 - - [16/Nov/2023 15:39:35] "GET /invoice HTTP/1.1" 503 -
.127.0.0.1 - - [16/Nov/2023 15:39:35] "GET / HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:39:35] "GET /invoice HTTP/1.1" 200 -
127.0.0.1 - - [16/Nov/2023 15:39:35] "POST /pay HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:39:36] "GET / HTTP/1.1" 200 -
.s.........................................................................................................................................s.........sss...................................F.....s.s....
======================================================================
FAIL: test_bip39_seed_bip44_standard (electrumabc.tests.test_wallet_vertical.TestWalletKeystoreAddressIntegrity)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.9/unittest/mock.py", line 1337, in patched
    return func(*newargs, **newkeywargs)
  File "/work/electrum/electrumabc/tests/test_wallet_vertical.py", line 192, in test_bip39_seed_bip44_standard
    self.assertEqual(
AssertionError: None != 1

----------------------------------------------------------------------
Ran 313 tests in 20.506s

FAILED (failures=1, skipped=7)
ninja: build stopped: cannot make progress due to previous errors.
Build electrum-tests failed with exit code 1
Harbormaster completed remote builds in B25645: Diff 43130.Nov 16 2023, 15:40
teamcity added a comment.Nov 16 2023, 15:40
Build Bitcoin ABC Diffs / Diff Testing (electrum-tests) failed.

Tail of the build log:

-- Found Event: /usr/include (found suitable version "2.1.12-stable", minimum required is "2.0.22") found components: event 
-- Found Boost: /usr/lib/x86_64-linux-gnu/cmake/Boost-1.74.0/BoostConfig.cmake (found suitable version "1.74.0", minimum required is "1.64")  
-- Found Event component pthreads: /usr/lib/x86_64-linux-gnu/libevent_pthreads.so
-- Found Event: /usr/include (found suitable version "2.1.12-stable", minimum required is "2.0.22") found components: pthreads 
-- Found MiniUPnPc component miniupnpc: /usr/lib/x86_64-linux-gnu/libminiupnpc.so
-- Found MiniUPnPc: /usr/include/miniupnpc (found suitable version "2.2.1", minimum required is "1.9")  
-- Found NATPMP component natpmp: /usr/lib/x86_64-linux-gnu/libnatpmp.so
-- Found NATPMP: /usr/include   
-- Found BerkeleyDB component CXX: /usr/lib/x86_64-linux-gnu/libdb_cxx-5.3.so
-- Found BerkeleyDB: /usr/include (found suitable version "5.3.28", minimum required is "5.3") found components: CXX 
-- Found SQLite3: /usr/include (found suitable version "3.34.1", minimum required is "3.7.17") 
-- Found ZeroMQ component zmq: /usr/lib/x86_64-linux-gnu/libzmq.so
-- Found ZeroMQ: /usr/include (found suitable version "4.3.4", minimum required is "4.1.5")  
-- Could NOT find Protobuf (missing: Protobuf_DIR)
-- Found Protobuf: /usr/lib/x86_64-linux-gnu/libprotobuf.so;-pthread (found version "3.12.4") 
-- Found OpenSSL: /usr/lib/x86_64-linux-gnu/libcrypto.so (found version "1.1.1w")  
-- Looking for EVP_MD_CTX_new
-- Looking for EVP_MD_CTX_new - found
-- Found QREncode component qrencode: /usr/lib/x86_64-linux-gnu/libqrencode.so
-- Found QREncode: /usr/include   
-- Configuring native build in /work/abc-ci-builds/electrum-tests/native
-- Configuring done
-- Generating done
-- Build files have been written to: /work/abc-ci-builds/electrum-tests
[1/2] link test_runner.py
[2/2] Run Electrum ABC unit tests...
FAILED: electrum/CMakeFiles/check-electrum 
cd /work/abc-ci-builds/electrum-tests/electrum && /usr/bin/python3.9 ./test_runner.py
[secp256k1] warning: libsecp256k1 library failed to load
[secp256k1] warning: loading from /work/electrum/electrumabc/libsecp256k1.so.0 failed with: /work/electrum/electrumabc/libsecp256k1.so.0: cannot open shared object file: No such file or directory
[secp256k1] warning: loading from libsecp256k1.so.0 failed with: libsecp256k1.so.0: cannot open shared object file: No such file or directory
Testing `setup.py --version`: OK

[ecc] info: libsecp256k1 library not available, falling back to python-ecdsa. This means signing operations will be slower. Try running:

  $  contrib/make_secp

(You need to be running from the git sources for contrib/make_secp to be available)
..................................................................................................................127.0.0.1 - - [16/Nov/2023 15:39:43] "GET /invoice HTTP/1.1" 503 -
.127.0.0.1 - - [16/Nov/2023 15:39:43] "GET / HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:39:44] "GET /invoice HTTP/1.1" 200 -
127.0.0.1 - - [16/Nov/2023 15:39:44] "POST /pay HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:39:44] "GET / HTTP/1.1" 200 -
.s.........................................................................................................................................s.........sss...................................F.....s.s....
======================================================================
FAIL: test_bip39_seed_bip44_standard (electrumabc.tests.test_wallet_vertical.TestWalletKeystoreAddressIntegrity)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.9/unittest/mock.py", line 1337, in patched
    return func(*newargs, **newkeywargs)
  File "/work/electrum/electrumabc/tests/test_wallet_vertical.py", line 192, in test_bip39_seed_bip44_standard
    self.assertEqual(
AssertionError: None != 1

----------------------------------------------------------------------
Ran 313 tests in 17.167s

FAILED (failures=1, skipped=7)
ninja: build stopped: cannot make progress due to previous errors.
Build electrum-tests failed with exit code 1
PiRK planned changes to this revision.Nov 16 2023, 15:44
PiRK updated this revision to Diff 43138.Nov 17 2023, 07:25

rebase

Harbormaster completed remote builds in B25652: Diff 43138.Nov 17 2023, 07:27
Fabien added a subscriber: Fabien.Nov 17 2023, 10:25
Fabien added inline comments.
electrum/electrumabc_gui/qt/avalanche/util.py
156–168 ↗(On Diff #43138)

Could this be simplified ?

PiRK added inline comments.Nov 17 2023, 10:32
electrum/electrumabc_gui/qt/avalanche/util.py
156–168 ↗(On Diff #43138)

that would work. It adds a tiny bit of computation, but probably not significant

PiRK updated this revision to Diff 43143.Nov 17 2023, 10:39

remove some branching in the code by unconditionally initializing the keys for index 0 before setting the index

Harbormaster completed remote builds in B25657: Diff 43143.Nov 17 2023, 10:40
Fabien accepted this revision.Nov 17 2023, 10:42
This revision is now accepted and ready to land.Nov 17 2023, 10:42
Closed by commit rABC201b71123c21: [electrum] fix auxiliary key dialog initialization on first use (authored by PiRK). · Explain WhyNov 17 2023, 12:44
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC201b71123c21: [electrum] fix auxiliary key dialog initialization on first use.

Revision Contents
Changeset List

PathSize
electrum/
electrumabc_gui/
qt/
avalanche/
17 lines

Diff 43130

View Options

electrum/electrumabc_gui/qt/avalanche/util.py

Loading...