Skip to content

Conversation

@szemate
Copy link
Contributor

@szemate szemate commented Aug 16, 2024

Complete rework of the autonity library.

  • Remove most of the old code
  • Add Makefile for auto-generating contract wrappers with pyabigen
  • Add the generated contract wrappers for the following contracts:
    • Accountability
    • ACU
    • Autonity
    • Inflation Controller
    • Liquid
    • Non-Stakable Vesting
    • Oracle
    • Stabilization
    • Supply Control
    • Upgrade Manager
  • Add autonity.networks module with parameters for Piccadilly and Bakerloo testnets
  • Add contract factories that retrieve contract address from the Autonity contract and check the current contract version & protocol version against the supported versions
  • Add test generator for very basic sanity checks

The new library doesn't contain any of the Web3 helper functions of the old library because they mostly re-implemented existing Web3.py functionality and are out of scope of autonity.py.

All code of the autonity.contracts module have been auto-generated. Note that the contract ABIs are now included in the generated Python files as Python dictionaries (similarly to the Go wrappers).

Closes #73.

Closes #43 because Web3.py >=6.19.0 depends on eth-rlp 1.0.1 that includes ethereum/eth-rlp#19 i.e. the typing-extensions dependency could be removed.


TODO:

  • Finalize excluded contract functions
  • Do not add doc comments with the @dev NatSpec tag to docstrings

@szemate szemate force-pushed the rework branch 9 times, most recently from c900482 to cb117be Compare August 19, 2024 14:42
@szemate szemate marked this pull request as ready for review August 19, 2024 14:43
@szemate szemate requested a review from aiman August 19, 2024 14:43
@szemate szemate force-pushed the rework branch 3 times, most recently from 4901a95 to 890a49b Compare August 19, 2024 15:33
@szemate szemate changed the base branch from master to stable August 19, 2024 15:33
@szemate szemate changed the base branch from stable to master August 19, 2024 15:41
@szemate szemate force-pushed the rework branch 2 times, most recently from 5e800c1 to 6fda9d9 Compare August 20, 2024 08:09
@szemate szemate marked this pull request as draft August 20, 2024 08:40
@szemate szemate force-pushed the rework branch 2 times, most recently from 25cecbc to ac0ed28 Compare August 20, 2024 10:09
@szemate szemate marked this pull request as ready for review August 20, 2024 10:11
@szemate szemate force-pushed the rework branch 3 times, most recently from 033534a to 1434319 Compare August 23, 2024 13:27
@szemate
Copy link
Contributor Author

szemate commented Aug 23, 2024

@aiman I'm done with the contract wrapper factories as discussed, this is now ready for review.

@szemate
Copy link
Contributor Author

szemate commented Sep 25, 2024

I've addressed the comments from #59 (comment) in e287055

@aiman
Copy link
Contributor

aiman commented Sep 30, 2024

I'm reviewing a final time, this time from the point of view of a user, and also thinking about what the Changelog entry/entries should be... Any ideas?

@szemate
Copy link
Contributor Author

szemate commented Oct 1, 2024

what the Changelog entry/entries should be... Any ideas?

  • Add bindings for the Accountability, ACU, Inflation Controller, Non-Stakable Vesting, Oracle, Stabilization, Supply Control and Upgrade Manager protocol contracts
  • Replace bindings for the Autonity and Liquid protocol contracts, which involves breaking changes in the API
  • Add constants for parameters of Autonity networks
  • Create bindings via factory functions that fetch the actual protocol contract addresses on the given Autonity network
  • Update Web3.py to 7.2.0

@aiman
Copy link
Contributor

aiman commented Oct 8, 2024

I'm having trouble running the tests for this branch inside of a devenv:

$ git checkout master
git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.

$ git clean -x -d -i

$ hatch
bash: hatch: command not found

$ which python
/usr/bin/python

$ /usr/bin/python --version
Python 3.11.6

$ which python3.12
which: no python3.12 in ...

$ devenv shell
• Building shell ...
• Using Cachix: nixpkgs-python, devenv
warning: Performing inefficient double copy of path '«github:cachix/devenv-nixpkgs/4267e705586473d3e5c8d50299e71503f16a6fb6»/' to the store. This can typically be avoided by rewriting an attribute like `src = ./.` to `src = builtins.path { path = ./.; name = "source"; }`.
✔ Building shell in 6.3s.
• Entering shell

$ which python
~/.local/share/hatch/env/virtual/autonity/H65cO8gT/autonity/bin/python

$ python --version
Python 3.11.8

$ which hatch
/nix/store/87gcqzg7cra5nrfvfilrffmv9cgp7bad-hatch-1.9.0/bin/hatch

$ hatch run +py=3.12 test:python --version
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────── test.py3.12 ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Python 3.12.4

$ hatch run +py=3.12 test:all
...
Success: no issues found in 27 source files

$ exit
exit

$ git checkout rework
Switched to branch 'rework'
Your branch is up to date with 'origin/rework'.

$ devenv shell
• Building shell ...
• Using Cachix: nixpkgs-python, devenv
warning: Performing inefficient double copy of path '«github:cachix/devenv-nixpkgs/4267e705586473d3e5c8d50299e71503f16a6fb6»/' to the store. This can typically be avoided by rewriting an attribute like `src = ./.` to `src = builtins.path { path = ./.; name = "source"; }`.
✔ Building shell in 6.1s.
• Entering shell
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
autonity 4.0.0 requires web3==6.19.0, but you have web3 7.2.0 which is incompatible.

$ hatch run +py=3.12 test:python --version
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────── test.py3.12 ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Python 3.12.4

$ hatch run +py=3.12 test:all
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────── test.py3.12 ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
autonity 4.0.0 requires web3==6.19.0, but you have web3 7.2.0 which is incompatible.
Traceback (most recent call last):
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/bin/pytest", line 5, in <module>
    from pytest import console_main
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/pytest/__init__.py", line 9, in <module>
    from _pytest.assertion import register_assert_rewrite
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/_pytest/assertion/__init__.py", line 11, in <module>
    from _pytest.assertion import rewrite
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/_pytest/assertion/rewrite.py", line 32, in <module>
    from _pytest.assertion import util
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/_pytest/assertion/util.py", line 24, in <module>
    from _pytest.config import Config
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/_pytest/config/__init__.py", line 58, in <module>
    import _pytest.hookspec
  File "/home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/_pytest/hookspec.py", line 305, in <module>
    @hookspec(
     ^^^^^^^^^
TypeError: HookspecMarker.__call__() got an unexpected keyword argument 'warn_on_impl_args'

Notice the error line:

autonity 4.0.0 requires web3==6.19.0, but you have web3 7.2.0 which is incompatible.

This appears both on entering the devenv shell, and also when running the tests.

@szemate
Copy link
Contributor Author

szemate commented Oct 8, 2024

Notice the error line:

autonity 4.0.0 requires web3==6.19.0, but you have web3 7.2.0 which is incompatible.

This appears both on entering the devenv shell, and also when running the tests.

This error doesn't seem to be related to devenv. I got the same without entering the devenv shell, after switching branches:

git checkout master
hatch run true
git checkout rework
hatch run true

and it prints

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
autonity 4.0.0 requires web3==7.2.0, but you have web3 6.19.0 which is incompatible.

The error disappears after running

hatch run pip install .

Could it be that Hatch doesn't update the dependencies properly after switching branches?

@aiman
Copy link
Contributor

aiman commented Oct 9, 2024

It may be better to drop the devenv support for the moment, as I notice Hatch in the repo devenv-nixpkgs/rolling seems to be at version 1.9.0 from Dec 2023, which may be related to these issues, and in future it may be worth considering a different Python framework than Hatch since the landscape is changing quite fast.

@szemate
Copy link
Contributor Author

szemate commented Oct 9, 2024

TypeError: HookspecMarker.__call__() got an unexpected keyword argument 'warn_on_impl_args'

This error appears to be a result of conflicting dependencies. As Hatch doesn't generate lockfiles, I've added the hatch-pip-compile plugin to inspect dependencies, however adding the lockfile itself resolved the error.

It may be better to drop the devenv support for the moment, as I notice Hatch in the repo devenv-nixpkgs/rolling seems to be at version 1.9.0

Yes this might be a Hatch bug that has been fixed, I only got the error with Hatch 1.9.0 but not with 1.12.0.

@aiman
Copy link
Contributor

aiman commented Oct 9, 2024

Can we address these test warnings?

tests/test_sanity.py::test_bindings_with_arbitrary_inputs[Stabilization.debt_amount]
  /home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/plum/signature.py:361: UserWarning: Could not resolve the type hint of `eth_typing.evm.ChecksumAddress`. I have ended the resolution here to not make your code break, but some types might not be working correctly. Please open an issue at https://github.com/wesselb/plum.
    annotation = resolve_type_hint(p.annotation)

tests/test_sanity.py::test_bindings_with_arbitrary_inputs[Stabilization.debt_amount]
  /home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/plum/type.py:265: UserWarning: Could not resolve the type hint of `eth_typing.evm.ChecksumAddress`. I have ended the resolution here to not make your code break, but some types might not be working correctly. Please open an issue at https://github.com/wesselb/plum.
    return _is_faithful(resolve_type_hint(x))

tests/test_sanity.py::test_bindings_with_arbitrary_inputs[Stabilization.debt_amount]
  /home/test/.local/share/hatch/env/virtual/autonity/H65cO8gT/test.py3.12/lib/python3.12/site-packages/plum/type.py:265: UserWarning: Could not determine whether `eth_typing.evm.ChecksumAddress` is faithful or not. I have concluded that the type is not faithful, so your code might run with subpar performance. Please open an issue at https://github.com/wesselb/plum.
    return _is_faithful(resolve_type_hint(x))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

Generate a test case for each method when there are multiple methods
with the same name but different signatures.
The `plum` library raises warnings about not being able to resolve
`eth_typing.ChecksumAddress` to dispatch the `stabilization.debt_amount`
multimethod; nevertheless the dispatch seems to be working as expected.
@szemate
Copy link
Contributor Author

szemate commented Oct 9, 2024

Can we address these test warnings?

I've suppressed these warnings in 636a39a and opened https://github.com/clearmatics/pyabigen/issues/7 to investigate the issue further.

These warnings are raised by the plum library that has been added to handle overloaded contract functions, of which there is only one in autonity.py and it seems to be working as expected.

Copy link
Contributor

@aiman aiman left a comment

Choose a reason for hiding this comment

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

Thanks!

@aiman aiman merged commit e4994e0 into master Oct 10, 2024
@aiman aiman deleted the rework branch October 10, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate all contact bindings automatically Drop temporary dependency on typing-extensions

2 participants