Differential D14804

[electrum] only increment the aux key index if we know it has been used
ClosedPublic
Actions

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

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC1fbafada93e7: [electrum] only increment the aux key index if we know it has been used

Summary

Instead of incrementing the index everytime a key is displayed, only increment it when a proof is signed or a delegation is generated with the suggested key.
This will avoid burning keys when using the proof editor just to inspect some proof and make the key detection more reliable (less likely to cross the gap limit) when loading a proof generated by this wallet.

Depends on D14803

Test Plan

Open the "Auxiliary keys" dialog multiple times, check that the key is no longer incremented each time.
Open the Proof editor, check that the key is the same as the one currently shown by default in the keys dialog. Close and reopen the Proof editor, nothing changes.
Generate a proof with the suggested key, check that now the index has been incremented.
Open the delegation editor, generate a delegated key using the suggested one. Check again that this does not affect the index. Generate a delegation with the suggested key, check that the index is now incremented.

Load a proof whose master key is not from this wallet, check that generating the (unsigned) proof does not increment the index.
Generate a proof with a WIF key that does not belong to this wallet, check that the index is not incremented.

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Nov 16 2023, 15:39

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 16 2023, 15:39

PiRK requested review of this revision.Nov 16 2023, 15:39

PiRK edited the test plan for this revision. (Show Details)Nov 16 2023, 15:43

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:42:39] "GET /invoice HTTP/1.1" 503 -
.127.0.0.1 - - [16/Nov/2023 15:42:39] "GET / HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:42:39] "GET /invoice HTTP/1.1" 200 -
127.0.0.1 - - [16/Nov/2023 15:42:39] "POST /pay HTTP/1.1" 200 -
.127.0.0.1 - - [16/Nov/2023 15:42:39] "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 19.027s

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 B25646: Diff 43131.Nov 16 2023, 15:43

PiRK planned changes to this revision.Nov 16 2023, 15:43

PiRK edited the summary of this revision. (Show Details)

rebase

Harbormaster completed remote builds in B25653: Diff 43139.Nov 17 2023, 07:28

Fabien accepted this revision.Nov 17 2023, 10:30

Fabien added a subscriber: Fabien.

Fabien added inline comments.

electrum/electrumabc/avalanche/proof.py
55 ↗	(On Diff #43139)	Note: this is by consensus an invalid signature, so no risk of collision

This revision is now accepted and ready to land.Nov 17 2023, 10:30

PiRK added inline comments.Nov 17 2023, 10:32

electrum/electrumabc/avalanche/proof.py
55 ↗	(On Diff #43139)	Good to know. I was hesitating between just hoping it would never collide or making it None and handling the special case in the serialization code.