From 40a298a996505735fa0d61affd310b6541f49e6b Mon Sep 17 00:00:00 2001 From: Daniel Schiavini Date: Wed, 22 Nov 2023 16:55:29 +0100 Subject: [PATCH] Fix pre-commit and more tests --- scripts/print_missing.py | 4 +- scripts/setup_metaregistry.py | 5 +- tests/conftest.py | 2 +- tests/fixtures/contracts.py | 9 --- .../api/test_get_admin_balances.py | 24 +++++-- .../metaregistry/api/test_get_balances.py | 2 +- .../metaregistry/api/test_get_gauge.py | 2 - .../metaregistry/api/test_get_pool_name.py | 36 +++++----- .../api/test_get_underlying_balances.py | 10 +-- .../api/test_get_underlying_decimals.py | 7 +- .../api/test_get_virtual_price.py | 7 +- .../registries/test_add_remove_basepool.py | 12 +++- .../registries/test_add_remove_metapool.py | 18 +++-- .../registries/test_add_remove_pool.py | 18 +++-- tests/utils.py | 69 ++++++++++++++----- 15 files changed, 144 insertions(+), 81 deletions(-) delete mode 100644 tests/fixtures/contracts.py diff --git a/scripts/print_missing.py b/scripts/print_missing.py index 82d75e5..3fbb4a1 100644 --- a/scripts/print_missing.py +++ b/scripts/print_missing.py @@ -1,3 +1,4 @@ +import boa from tabulate import tabulate MISSING = "\033[33m✖\033[0m" @@ -20,8 +21,9 @@ def get_non_indexed_view_functions( if registry_selectors[k] in view_fns } + metaregistry = boa.load_partial("contracts/mainnet/MetaRegistry.vy") function_index = get_non_indexed_view_functions( - MetaRegistry.selectors, MetaRegistry.abi, {} + metaregistry.selectors, metaregistry.abi, {} ) registry_coverage = [[PRESENT] * len(function_index)] registry_names = [ diff --git a/scripts/setup_metaregistry.py b/scripts/setup_metaregistry.py index b86af5c..b0f8a6d 100644 --- a/scripts/setup_metaregistry.py +++ b/scripts/setup_metaregistry.py @@ -133,9 +133,6 @@ def cli(): pass -@cli.command(cls=NetworkBoundCommand) -@network_option() -@account_option() def main(network: str, account: str): # admin only: only admin of ADDRESSPROVIDER's proxy admin can do the following: address_provider = get_deployed_contract( @@ -146,7 +143,7 @@ def main(network: str, account: str): if network == "ethereum:mainnet-fork": RICH_CONSOLE.log("Simulation mode.") - account = accounts[proxy_admin.admins(1)] + account = proxy_admin.admins(1) # deployed contracts: base_pool_registry = get_deployed_contract( diff --git a/tests/conftest.py b/tests/conftest.py index 2874ba5..40fea7b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -31,7 +31,7 @@ def pytest_sessionstart(): # connect to the network. TODO: use Drpc-Key header instead of GET param boa.env.fork(url=environ["RPC_ETHEREUM"]) - # store instance of registries globally, so we don't have to recreate multiple times when generating tests. + # store registries globally, so we don't have to recreate multiple times when generating tests. # TODO: Can we move these to fixtures? STABLE_REGISTRY_POOLS = get_contract_pools( "StableRegistry", "0x90E00ACe148ca3b23Ac1bC8C240C2a7Dd9c2d7f5" diff --git a/tests/fixtures/contracts.py b/tests/fixtures/contracts.py deleted file mode 100644 index f0a2522..0000000 --- a/tests/fixtures/contracts.py +++ /dev/null @@ -1,9 +0,0 @@ -import pytest - - -@pytest.fixture(scope="module") -def address_provider_contract(accounts): - address_provider = get_deployed_contract( - "AddressProvider", "0x0000000022D53366457F9d5E68Ec105046FC4383" - ) - return accounts[address_provider.admin()] diff --git a/tests/mainnet/metaregistry/api/test_get_admin_balances.py b/tests/mainnet/metaregistry/api/test_get_admin_balances.py index 2c52fbc..c35ecb8 100644 --- a/tests/mainnet/metaregistry/api/test_get_admin_balances.py +++ b/tests/mainnet/metaregistry/api/test_get_admin_balances.py @@ -2,7 +2,7 @@ import pytest from eth.codecs.abi.exceptions import DecodeError -from tests.utils import get_lp_contract +from tests.utils import get_deployed_token_contract def pre_test_checks(metaregistry, pool): @@ -10,13 +10,20 @@ def pre_test_checks(metaregistry, pool): pytest.skip("empty pool: skipping") try: - if get_lp_contract(metaregistry.get_lp_token(pool)).totalSupply() == 0: + if ( + get_deployed_token_contract( + metaregistry.get_lp_token(pool) + ).totalSupply() + == 0 + ): pytest.skip("LP token supply is zero") except DecodeError as err: # TODO: Document why this happens pytest.skip( - f"{type(err).__name__} for token {metaregistry.get_lp_token(pool)}: Skipping because of {err.msg}" + f"{type(err).__name__} for token {metaregistry.get_lp_token(pool)}: " + f"Skipping because of {err.msg}" ) + def test_stable_registry_pools( populated_metaregistry, stable_registry_pool, stable_registry ): @@ -30,7 +37,9 @@ def test_stable_registry_pools( assert output == metaregistry_output[i] -def test_stable_factory_pools(populated_metaregistry, stable_factory_pool, curve_pool): +def test_stable_factory_pools( + populated_metaregistry, stable_factory_pool, curve_pool +): pre_test_checks(populated_metaregistry, stable_factory_pool) pool = curve_pool(stable_factory_pool) @@ -47,14 +56,17 @@ def test_stable_factory_pools(populated_metaregistry, stable_factory_pool, curve def _get_crypto_pool_admin_fees( populated_metaregistry, pool, fee_receiver, alice_address ): - lp_token = get_lp_contract(populated_metaregistry.get_lp_token(pool)) + lp_token = get_deployed_token_contract( + populated_metaregistry.get_lp_token(pool) + ) fee_receiver_token_balance_before = lp_token.balanceOf(fee_receiver) with boa.env.anchor(): pool.claim_admin_fees(sender=alice_address) claimed_lp_token_as_fee = ( - lp_token.balanceOf(fee_receiver) - fee_receiver_token_balance_before + lp_token.balanceOf(fee_receiver) + - fee_receiver_token_balance_before ) total_supply_lp_token = lp_token.totalSupply() frac_admin_fee = int( diff --git a/tests/mainnet/metaregistry/api/test_get_balances.py b/tests/mainnet/metaregistry/api/test_get_balances.py index e45b290..470aa70 100644 --- a/tests/mainnet/metaregistry/api/test_get_balances.py +++ b/tests/mainnet/metaregistry/api/test_get_balances.py @@ -28,7 +28,7 @@ def test_crypto_registry_pools( ): try: actual_output = crypto_registry.get_balances(crypto_registry_pool) - except ContractLogicError: + except KeyError: # TODO: Pick right exception actual_output = [] pool = curve_pool_v2(crypto_registry_pool) for i in range( diff --git a/tests/mainnet/metaregistry/api/test_get_gauge.py b/tests/mainnet/metaregistry/api/test_get_gauge.py index f0d2eed..cedbdba 100644 --- a/tests/mainnet/metaregistry/api/test_get_gauge.py +++ b/tests/mainnet/metaregistry/api/test_get_gauge.py @@ -1,5 +1,3 @@ -import boa - from tests.utils import ZERO_ADDRESS diff --git a/tests/mainnet/metaregistry/api/test_get_pool_name.py b/tests/mainnet/metaregistry/api/test_get_pool_name.py index 1e7df87..9eed3ab 100644 --- a/tests/mainnet/metaregistry/api/test_get_pool_name.py +++ b/tests/mainnet/metaregistry/api/test_get_pool_name.py @@ -1,6 +1,6 @@ import pytest -from tests.utils import ZERO_ADDRESS +from tests.utils import ZERO_ADDRESS, get_deployed_token_contract def test_stable_registry_pools( @@ -24,40 +24,38 @@ def test_stable_factory_pools(populated_metaregistry, stable_factory_pool): list(filter((ZERO_ADDRESS).__ne__, pool_registry_handlers)) ) + name = populated_metaregistry.get_pool_name(stable_factory_pool) if num_registry_handlers == 1: - assert ( - populated_metaregistry.get_pool_name(stable_factory_pool) - == VyperContract(stable_factory_pool).name() - ) - - elif num_registry_handlers == 2: + assert name == get_deployed_token_contract(stable_factory_pool).name() + else: with pytest.raises(AssertionError): assert ( - populated_metaregistry.get_pool_name(stable_factory_pool) - == VyperContract(stable_factory_pool).name() + name == get_deployed_token_contract(stable_factory_pool).name() ) + pool_name2 = populated_metaregistry.get_pool_name( + stable_factory_pool, 1 + ) assert ( - populated_metaregistry.get_pool_name(stable_factory_pool, 1) - == VyperContract(stable_factory_pool).name() + pool_name2 + == get_deployed_token_contract(stable_factory_pool).name() ) - else: - raise + assert num_registry_handlers in (1, 2) def test_crypto_registry_pools( populated_metaregistry, crypto_registry_pool, crypto_registry ): - assert populated_metaregistry.get_pool_name( - crypto_registry_pool - ) == crypto_registry.get_pool_name(crypto_registry_pool) + pool_name = populated_metaregistry.get_pool_name(crypto_registry_pool) + assert pool_name == crypto_registry.get_pool_name(crypto_registry_pool) def test_crypto_factory_pools( populated_metaregistry, crypto_factory_pool, crypto_factory ): - assert ( - populated_metaregistry.get_pool_name(crypto_factory_pool) - == VyperContract(crypto_factory.get_token(crypto_factory_pool)).name() + pool_name = populated_metaregistry.get_pool_name(crypto_factory_pool) + contract = get_deployed_token_contract( + crypto_factory.get_token(crypto_factory_pool) ) + assert pool_name == contract.name() diff --git a/tests/mainnet/metaregistry/api/test_get_underlying_balances.py b/tests/mainnet/metaregistry/api/test_get_underlying_balances.py index abfa92e..2255473 100644 --- a/tests/mainnet/metaregistry/api/test_get_underlying_balances.py +++ b/tests/mainnet/metaregistry/api/test_get_underlying_balances.py @@ -1,9 +1,8 @@ import warnings -import boa import pytest -from tests.utils import ZERO_ADDRESS +from tests.utils import ZERO_ADDRESS, get_deployed_token_contract EXCEPTION_POOLS = ["0x79a8C46DeA5aDa233ABaFFD40F3A0A2B1e5A4F27"] @@ -30,10 +29,13 @@ def _get_underlying_balances( if base_pool != ZERO_ADDRESS: basepool_lp_token_balance = balances[idx] - coin_contract = VyperContract(coin) + coin_contract = get_deployed_token_contract(coin) try: lp_token_supply = coin_contract.totalSupply() - except (KeyError, AttributeError): # TODO: Pick the right exception + except ( + KeyError, + AttributeError, + ): # TODO: Pick the right exception assert "totalSupply" not in [ i.name for i in coin_contract.contract_type.view_methods diff --git a/tests/mainnet/metaregistry/api/test_get_underlying_decimals.py b/tests/mainnet/metaregistry/api/test_get_underlying_decimals.py index 2f9c670..6c6b2b2 100644 --- a/tests/mainnet/metaregistry/api/test_get_underlying_decimals.py +++ b/tests/mainnet/metaregistry/api/test_get_underlying_decimals.py @@ -1,7 +1,6 @@ -import boa import pytest -from tests.utils import ZERO_ADDRESS +from tests.utils import ZERO_ADDRESS, get_deployed_token_contract EXCEPTIONS = { # eth: ankreth pool returns [18, 0] when it should return: @@ -32,7 +31,9 @@ def _test_underlying_decimals_getter(metaregistry, registry, pool): continue try: - token_contract = VyperContract(underlying_coins[i]) + token_contract = get_deployed_token_contract( + underlying_coins[i] + ) actual_output.append(token_contract.decimals()) except KeyError: # TODO: Pick the right exception pytest.skip("Unverified contract. Skipping test.") diff --git a/tests/mainnet/metaregistry/api/test_get_virtual_price.py b/tests/mainnet/metaregistry/api/test_get_virtual_price.py index 81ab84b..e52a765 100644 --- a/tests/mainnet/metaregistry/api/test_get_virtual_price.py +++ b/tests/mainnet/metaregistry/api/test_get_virtual_price.py @@ -3,7 +3,7 @@ import boa import pytest -from tests.utils import ZERO_ADDRESS +from tests.utils import ZERO_ADDRESS, get_deployed_token_contract # ---- sanity checks since vprice getters can revert for specific pools states ---- @@ -37,7 +37,10 @@ def _check_skem_tokens_with_weird_decimals( if ( coin_decimals[i] == 0 - and VyperContract(metaregistry.get_coins(pool)[0]).decimals() == 0 + and get_deployed_token_contract( + metaregistry.get_coins(pool)[0] + ).decimals() + == 0 ): with boa.env.anchor(): metaregistry.get_virtual_price_from_lp_token(lp_token) diff --git a/tests/mainnet/registries/test_add_remove_basepool.py b/tests/mainnet/registries/test_add_remove_basepool.py index eb2f1e6..6812be7 100644 --- a/tests/mainnet/registries/test_add_remove_basepool.py +++ b/tests/mainnet/registries/test_add_remove_basepool.py @@ -6,7 +6,9 @@ def test_revert_unauthorised_add_base_pool( owner, unauthorised_address, base_pools ): - base_pool_registry = deploy_contract("BasePoolRegistry", directory="registries", sender=owner) + base_pool_registry = deploy_contract( + "BasePoolRegistry", directory="registries", sender=owner + ) base_pool_data = base_pools["tripool"] with boa.reverts(): base_pool_registry.add_base_pool( @@ -20,7 +22,9 @@ def test_revert_unauthorised_add_base_pool( def test_add_basepool(owner, base_pools, tokens): - base_pool_registry = deploy_contract("BasePoolRegistry", directory="registries", sender=owner) + base_pool_registry = deploy_contract( + "BasePoolRegistry", directory="registries", sender=owner + ) base_pool_count = base_pool_registry.base_pool_count() base_pool_data = base_pools["tripool"] tripool = base_pool_data["pool"] @@ -60,7 +64,9 @@ def test_add_basepool(owner, base_pools, tokens): def test_add_basepool_with_legacy_abi(owner, base_pools, tokens): - base_pool_registry = deploy_contract("BasePoolRegistry", directory="registries", sender=owner) + base_pool_registry = deploy_contract( + "BasePoolRegistry", directory="registries", sender=owner + ) base_pool_data = base_pools["sbtc"] assert base_pool_data["is_legacy"] diff --git a/tests/mainnet/registries/test_add_remove_metapool.py b/tests/mainnet/registries/test_add_remove_metapool.py index db56752..fa3b5ee 100644 --- a/tests/mainnet/registries/test_add_remove_metapool.py +++ b/tests/mainnet/registries/test_add_remove_metapool.py @@ -12,8 +12,13 @@ def test_add_metapool( base_pools, tokens, ): - crypto_registry = deploy_contract("CryptoRegistryV1", address_provider, populated_base_pool_registry, - directory="registries", sender=owner) + crypto_registry = deploy_contract( + "CryptoRegistryV1", + address_provider, + populated_base_pool_registry, + directory="registries", + sender=owner, + ) pool_count = crypto_registry.pool_count() assert pool_count == 0 @@ -205,8 +210,13 @@ def test_remove_metapool( base_pools, tokens, ): - crypto_registry = deploy_contract("CryptoRegistryV1", address_provider, populated_base_pool_registry, - directory="registries", sender=owner) + crypto_registry = deploy_contract( + "CryptoRegistryV1", + address_provider, + populated_base_pool_registry, + directory="registries", + sender=owner, + ) # add EURT3CRV pool eurt3crv = crypto_registry_pools["eurt3crv"] diff --git a/tests/mainnet/registries/test_add_remove_pool.py b/tests/mainnet/registries/test_add_remove_pool.py index 4bc4021..ce70a49 100644 --- a/tests/mainnet/registries/test_add_remove_pool.py +++ b/tests/mainnet/registries/test_add_remove_pool.py @@ -48,8 +48,13 @@ def test_add_pool( owner, tokens, ): - crypto_registry = deploy_contract("CryptoRegistryV1", address_provider, populated_base_pool_registry, - directory="registries", sender=owner) + crypto_registry = deploy_contract( + "CryptoRegistryV1", + address_provider, + populated_base_pool_registry, + directory="registries", + sender=owner, + ) pool_count = crypto_registry.pool_count() assert pool_count == 0 @@ -152,8 +157,13 @@ def test_remove_pool( max_coins, tokens, ): - crypto_registry = deploy_contract("CryptoRegistryV1", address_provider, populated_base_pool_registry, - directory="registries", sender=owner) + crypto_registry = deploy_contract( + "CryptoRegistryV1", + address_provider, + populated_base_pool_registry, + directory="registries", + sender=owner, + ) # add pool to be removed: tricrypto2 = crypto_registry_pools["tricrypto2"] diff --git a/tests/utils.py b/tests/utils.py index f54655d..e888a45 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -27,7 +27,9 @@ def get_deployed_contract(contract_name: str, address: str) -> VyperContract: :param contract_name: The name of the contract ABI to load. :param address: The address of the deployed contract. """ - file_name = path.join(BASE_DIR, f"contracts/interfaces/{contract_name}.json") + file_name = path.join( + BASE_DIR, f"contracts/interfaces/{contract_name}.json" + ) return boa.load_abi(file_name).at(address) @@ -45,24 +47,55 @@ def deploy_contract( return boa.load(file_name, *args, **kwargs) -def get_lp_contract(address: str) -> VyperContract: +def get_deployed_token_contract(address: str) -> VyperContract: """ - Gets the LP token contract at the given address. + Gets the LP token contract at the given address. This uses a subset of the ERC20 ABI. :param address: The address of the LP token contract. """ - abi = json.dumps([{ - "name": "totalSupply", - "constant": True, - "inputs": [], - "outputs": [{"name": "", "type": "uint256"}], - "payable": False, - "stateMutability": "view", - "type": "function" - }, { - "inputs": [{ "internalType": "address", "name": "account", "type": "address" }], - "name": "balanceOf", - "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], - "stateMutability": "view", - "type": "function" - }]) + abi = json.dumps( + [ + { + "constant": True, + "inputs": [], + "name": "name", + "outputs": [{"name": "", "type": "string"}], + "payable": False, + "stateMutability": "view", + "type": "function", + }, + { + "constant": True, + "inputs": [], + "name": "decimals", + "outputs": [{"name": "", "type": "uint8"}], + "payable": False, + "stateMutability": "view", + "type": "function", + }, + { + "name": "totalSupply", + "constant": True, + "inputs": [], + "outputs": [{"name": "", "type": "uint256"}], + "payable": False, + "stateMutability": "view", + "type": "function", + }, + { + "inputs": [ + { + "internalType": "address", + "name": "account", + "type": "address", + } + ], + "name": "balanceOf", + "outputs": [ + {"internalType": "uint256", "name": "", "type": "uint256"} + ], + "stateMutability": "view", + "type": "function", + }, + ] + ) return boa.loads_abi(abi).at(address)