Using `@functools.lru_cache` on an instance method behaves in interesting ways.
The cache kept a ref around for `self`, so in effect we were never GC-ing keystore objects. Effectively there was a single global cache for derive_pubkey, with keys `(keystore, for_change, n)`.
This PR now changes the caching to be per-keystore:
each ks has a cache, keyed `(for_change, n)`.
GC-ing individual keystores should now be possible, which should result in cleaning up just their own cache.
This also enables the corresponding previously silence flake8-bugbear check for `@functools.lru_cache`. (note that the check can selectively be disabled by adding a comment on the relevant line: `# noqa: B019`)
Stops the running regtest if one test fails (using the --failfast
option) and makes the wallet data directories of alice, bob and carol
available for debugging. This seems helpful to fix issues only happening
on the CI.
- previously we had the CI spawn us inside a docker container as per our Dockerfile,
and we ran make_appimage.sh inside that
- now we also need to run make_type2_runtime.sh, which builds and runs another docker container, so the old approach does not work
- follow-up https://github.com/spesmilo/electrum/pull/10019
- looks easier to just try to run build.sh, the outer script, which does all that
- makes the CI task more similar to what dev machines run
- at the cost of somewhat more compute
I think it makes sense to run the tests with both the "latest" dependencies,
and with the pinned dependencies that we package for releases.
Testing with latest can reveal changes/issues with new dep versions,
while testing with pinned is testing what users will actually run.
Previously we were only testing with "latest".
- it was originally added in https://github.com/spesmilo/electrum/pull/1334,
with the goal of simplifying running the tests on local dev machines.
However this usage is not documented anywhere, and AFAIK regular contributors
don't use it either.
- tox just adds another layer of abstraction that is not that useful IMO
- I want more control over which electrum-deps are installed, which tox is
(in this case) unhelpfully abstracting away
It seems like the OS version of the CI image has been bumped, the regtest of PR https://github.com/spesmilo/electrum/pull/10073 fails for me with this error:
```
(Reading database ... 55%
(Reading database ... 60%
(Reading database ... 65%
(Reading database ... 70%
(Reading database ... 75%
(Reading database ... 80%
(Reading database ... 85%
(Reading database ... 90%
(Reading database ... 95%
(Reading database ... 100%
(Reading database ... 111702 files and directories currently installed.)
Preparing to unpack .../jq_1.7.1-3ubuntu0.24.04.1_amd64.deb ...
Unpacking jq (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ...
Preparing to unpack .../libjq1_1.7.1-3ubuntu0.24.04.1_amd64.deb ...
Unpacking libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ...
Setting up libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) ...
Setting up jq (1.7.1-3ubuntu0.24.04.1) ...
Processing triggers for man-db (2.12.0-4build2) ...
Processing triggers for libc-bin (2.39-0ubuntu8.5) ...
Running kernel seems to be up-to-date.
No services need to be restarted.
No containers need to be restarted.
No user sessions are running outdated binaries.
No VM guests are running outdated hypervisor (qemu) binaries on this host.
python3 -m pip install --user --upgrade pip
error: externally-managed-environment
× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
python3-xyz, where xyz is the package you are trying to
install.
If you wish to install a non-Debian-packaged Python package,
create a virtual environment using python3 -m venv path/to/venv.
Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
sure you have python3-full installed.
If you wish to install a non-Debian packaged Python application,
it may be easiest to use pipx install xyz, which will manage a
virtual environment for you. Make sure you have pipx installed.
See /usr/share/doc/python3.12/README.venv for more information.
note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
```
This should make the pip installs work again, however not sure how to test it *on* the CI? i it locally and it seems to work on Ubuntu 24.04 with this patch.
This script scans the whole codebase for unicode characters and
errors if it finds any, unless the character is specifically whitelisted.
The motivation is to protect against homoglyph attacks, invisible unicode characters,
bidirectional and other control characters, and other malicious unicode usage.
Given that we mostly expect to use ASCII characters in the source code,
the most robust and generic fix seems to be to just ban all unicode usage.
see https://trojansource.codes/ :
> Compilers, interpreters, and build pipelines supporting Unicode should throw errors or warnings
> for unterminated bidirectional control characters in comments or string literals,
> and for identifiers with mixed-script confusable characters.
> Language specifications should formally disallow unterminated bidirectional
> control characters in comments and string literals.
> Code editors and repository frontends should make bidirectional control characters
> and mixed-script confusable characters perceptible with visual symbols or warnings.
also https://github.com/maltfield/detect-malicious-unicode
```
$ export ELECTRUM_LINTERS=E9,E101,E129,E273,E274,E703,E71,E722,F5,F6,F7,F8,W191,W29,B
$ export ELECTRUM_LINTERS_IGNORE=B007,B009,B010,B019,B036,F541,F841
$ flake8 . --count --select="$ELECTRUM_LINTERS" --ignore="$ELECTRUM_LINTERS_IGNORE" --show-source --statistics --exclude "*_pb2.py,electrum/_vendor/"
./electrum/commands.py:98:1: F811 redefinition of unused 'format_satoshis' from line 48
def format_satoshis(x):
^
./electrum/commands.py:437:9: F811 redefinition of unused 'Mnemonic' from line 62
from .mnemonic import Mnemonic
^
./electrum/gui/qt/wizard/wallet.py:37:5: F811 redefinition of unused 'Daemon' from line 14
from electrum.daemon import Daemon
^
./electrum/lntransport.py:14:1: F811 redefinition of unused 'Optional' from line 12
from typing import NamedTuple, List, Tuple, Mapping, Optional, TYPE_CHECKING, Union, Dict, Set, Sequence
^
./electrum/lntransport.py:14:1: F811 redefinition of unused 'TYPE_CHECKING' from line 12
from typing import NamedTuple, List, Tuple, Mapping, Optional, TYPE_CHECKING, Union, Dict, Set, Sequence
^
./electrum/plugin.py:966:13: F811 redefinition of unused 'hid' from line 593
import hid
^
./electrum/plugin.py:1040:13: F811 redefinition of unused 'hid' from line 593
import hid
^
./electrum/util.py:44:1: F811 redefinition of unused 'json' from line 26
import json
^
./electrum/util.py:46:1: F811 redefinition of unused 'NamedTuple' from line 29
from typing import NamedTuple, Optional
^
./electrum/util.py:46:1: F811 redefinition of unused 'Optional' from line 29
from typing import NamedTuple, Optional
^
./electrum/util.py:1456:56: F811 redefinition of unused 'traceback' from line 34
async def __aexit__(self, exc_type, exc_value, traceback):
^
./electrum/wallet_db.py:536:9: F811 redefinition of unused 'LOCAL' from line 46
LOCAL = 1
^
./electrum/wallet_db.py:537:9: F811 redefinition of unused 'REMOTE' from line 46
REMOTE = -1
^
./tests/test_bitcoin.py:28:1: F811 redefinition of unused 'bitcoin' from line 9
from electrum import crypto, constants, bitcoin
^
./tests/test_txbatcher.py:11:1: F811 redefinition of unused 'Transaction' from line 7
from electrum.transaction import Transaction, PartialTxInput, PartialTxOutput, TxOutpoint
^
./tests/test_wallet_vertical.py:20:1: F811 redefinition of unused 'Transaction' from line 10
from electrum.transaction import Transaction, PartialTxOutput, tx_from_any, Sighash
^
16 F811 redefinition of unused 'format_satoshis' from line 48
16
```
```
./electrum/commands.py:144:9: F824 `global known_commands` is unused: name is never assigned in scope
global known_commands
^
./electrum/commands.py:1916:9: F824 `global known_commands` is unused: name is never assigned in scope
global known_commands
^
./electrum/gui/qt/main_window.py:2405:13: F824 `nonlocal done` is unused: name is never assigned in scope
nonlocal done
^
./electrum/i18n.py:52:5: F824 `global language` is unused: name is never assigned in scope
global language
^
./electrum/plugin.py:189:9: F824 `global _root_permission_cache` is unused: name is never assigned in scope
global _root_permission_cache
^
5 F824 `global known_commands` is unused: name is never assigned in scope
5
```
pip was failing to install "electrum_aionostr>=0.0.7".
This was when using "python3-pip" from ubuntu 22.04 apt.
```
Discarding 95a16d58c8/electrum_aionostr-0.0.7.tar.gz (sha256)=e0b45d608977891c3bb4f6a4dee9a4e843fa661ceca31f81e7b992e65117070a (from https://pypi.org/simple/electrum-aionostr/) (requires-python:>=3.8): Requested unknown from 95a16d58c8/electrum_aionostr-0.0.7.tar.gz (sha256)=e0b45d608977891c3bb4f6a4dee9a4e843fa661ceca31f81e7b992e65117070a (from Electrum==4.5.8) has inconsistent name: filename has 'electrum-aionostr', but metadata has 'unknown'
```
Updating to newer pip directly using pip seems to fix this:
```
$ python3 -m pip install --user --upgrade pip
Requirement already satisfied: pip in /usr/lib/python3/dist-packages (22.0.2)
Collecting pip
Downloading pip-25.0-py3-none-any.whl (1.8 MB)
```
follow-up fcc4e1d387
error msg was:
```
SHA for cache folders (/opt/wine64/drive_c/electrum/contrib/build-wine/.cache/win32/wine_pip_cache, /opt/wine64/drive_c/electrum/contrib/build-wine/.cache/win64/wine_pip_cache) is 'be33ad1b0598b1733992e36659bb71406f8fcfaa3a442166ecbe26e2db9a65c2'
Failed to tar caches for Upload 'pip' cache with error walking folder /opt/wine64/drive_c/electrum/contrib/build-wine/.cache/win32/wine_pip_cache: lstat /opt/wine64/drive_c/electrum/contrib/build-wine/.cache/win32/wine_pip_cache: no such file or directory!
```
- new version of flake8-bugbear (24.1.16) introduced new tests (B036), which we were failing
- that's fine, but our CI should not suddenly break because of this
- better to do explicit manual version bumps of the linter
These linter warnings were all fixed in
312f2641e7, so making CI enforce them will
reduce the risk of regressions.
Co-authored-by: Jeremy Rand <jeremyrand@danwin1210.de>
Python 3.12 does not work with current aiohttp, see
https://github.com/aio-libs/aiohttp/issues/7229
It is currently possible to build a wheel using aiohttp==3.9.0b0.
However, unit tests fail in that case, because TestLNTransport::test_loop stalls.
We never really supported pypy - the CI was running the tests there
just to see compatibility.
A few of the unit tests consistently hang, resulting in the whole
test runner timing out and wasting compute resources. Someone could
debug the hangs and try to fix them, but I don't have the time.
The "python:rc" tag on dockerhub has not been updated for 2+ years
and is still at 3.10.0rc2. Even 3.11 has been released for many months now.
see https://hub.docker.com/_/python/tags
