chore: bump Vyper version to 0.4.0 #18
Conversation
AlbertoCentonze
changed the title
There was a problem hiding this comment.
I went through the codebase changes, and observed minor syntax adjustments to make contracts compile with vyper 4.0. Notably, contract names were simplified, and now look much better.
As long as all previously passing tests are passing now, i'd say we're safe from introducing some bugs.
I left some comments here and there, let's iterate a bit on that before accepting this PR.
There was a problem hiding this comment.
I see in the logs that cache@v2 is outdated, may be bump to v3
There was a problem hiding this comment.
all changes seem ok, i did not go through contract to check if anything else should be changed. I guess if contract compiles - no static(ext)calls are missing and syntax is correct.
There was a problem hiding this comment.
the changes are 99% syntax: reentrancy flag, uint division or function calls. However, there's one @pure that became @view, any good reason for that?
| @@ -250,10 +244,10 @@ def get_y( | |||
| gamma2: int256 = unsafe_mul(gamma, gamma) | |||
|
|
|||
| # savediv by x_j done here: | |||
| y: int256 = D**2 / (x_j * N_COINS**2) | |||
| y: int256 = D**2 // (x_j * convert(N_COINS, int256)**2) | |||
There was a problem hiding this comment.
what's up with N_COINS convert? why is it introduced?
| D*ann_gamma2/400000000/x_j | ||
| - convert(unsafe_mul(10**32, 3), int256) | ||
| D*ann_gamma2//400000000//x_j | ||
| - (3 * 10**32) | ||
| - unsafe_mul(unsafe_mul(2, gamma), 10**14) |
| amm_contract_obj = boa.load_partial( | ||
| f"./contracts/{contract_folder}/CurveTwocryptoOptimized.vy" | ||
| ) | ||
| amm_contract_obj = boa.load_partial(f"./contracts/{contract_folder}/CurveTwocryptoOptimized.vy") |
There was a problem hiding this comment.
ctrl-f old names across the whole repo, see if they are hidden where formatter didn't find them
| factory = boa.load_partial( | ||
| "contracts/main/CurveTwoCryptoFactory.vy" | ||
| ).at(deployments[network]["factory"]) | ||
| factory = boa.load_partial("contracts/main/CurveTwoCryptoFactory.vy").at( |
| with boa.swap_env(env): | ||
| yield | ||
| assert rpc_url is not None, "Provider url is not set, add RPC_ETHEREUM param to env" | ||
| boa.fork(rpc_url) |
There was a problem hiding this comment.
shouldn't we use syntax like
with boa.swap_env(boa.Env()):
boa.fork(rpc_url)
There was a problem hiding this comment.
some legacy tests are just rm'ed. were they not used/not working?
There was a problem hiding this comment.
we have one! test failing, i believe because this test was rm'ed
==================================== ERRORS ====================================
__________ ERROR collecting tests/unitary/factory/test_deploy_pool.py __________
ImportError while importing test module '/home/runner/work/twocrypto-ng/twocrypto-ng/tests/unitary/factory/test_deploy_pool.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.local/share/uv/python/cpython-3.12.6-linux-x86_64-gnu/lib/python3.12/importlib/init.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/unitary/factory/test_deploy_pool.py:4: in
from tests.unitary.pool.token.test_permit import ZERO_ADDRESS
E ModuleNotFoundError: No module named 'tests.unitary.pool.token.test_permit'
List of Changes