ethereum / staking-deposit-cli

Secure key generation for deposits
Creative Commons Zero v1.0 Universal
523 stars 319 forks source link

Python 3.11 requirements #375

Open yorickdowne opened 10 months ago

yorickdowne commented 10 months ago

Updating cytoolz and toolz allows staking-deposit-cli to build with Python 3.11 / 3.12.

The hashes in this PR cover cytoolz on all architectures from Python 3.8 through Python 3.12. They were fetched with the helper script in https://github.com/ethereum/staking-deposit-cli/pull/377 , with a minimum Python version of 3.8.

CI for Python 3.11 and 3.12 has been added, which required typed-ast to be removed from requirements_test.txt. As typed-ast is obsolete from Python 3.8, that should be fine.

Closes #363

yorickdowne commented 10 months ago

keystore.py had a small zoo of these:

  File "/usr/local/lib/python3.11/dataclasses.py", line 815, in _get_field
    raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'staking_deposit.key_handling.keystore.KeystoreModule'> for field kdf is not allowed: use default_factory

I've changed all instances of this error to use a default_factory.

This is due to this change in 3.11:

* Change field default mutability check, allowing only defaults which are
  :term:`hashable` instead of any object which is not an instance of
  :class:`dict`, :class:`list` or :class:`set`. (Contributed by Eric V. Smith in
  :issue:`44674`.)
eth2353 commented 9 months ago

I ran into this error (and similar ones) on Python 3.11 too:

ValueError: mutable default <class 'staking_deposit.key_handling.keystore.KeystoreModule'> for field kdf is not allowed: use default_factory

I fixed it in my own fork before looking if there was a PR addressing it. Anyway, I came up with pretty much the same solution, except I used a lambda instead of the partial object and it seems to preserve more readability to me, see here . So just adding my 2 cents, consider using the lambda?

Also, an unnecessary extra @dataclass decorator seems to have sneaked in at line 63 🙂

yorickdowne commented 9 months ago

Your solution is way more elegant @eth2353 ! Thank you, adopted.

yorickdowne commented 9 months ago

@holiman Can I pull you in for review? This changes requirements.txt and some of the code so it works with Python 3.11.

holiman commented 9 months ago

Me??

Better leave this for @CarlBeek