Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issue with custom errors on estimate gas and static fee txns [APE-1421] #1680

Merged
merged 15 commits into from
Sep 29, 2023
35 changes: 30 additions & 5 deletions src/ape/api/networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class EcosystemAPI(BaseInterfaceModel):
fee_token_decimals: int = 18
"""The number of the decimals the fee token has."""

_default_network: str = LOCAL_NETWORK_NAME
_default_network: Optional[str] = None

def __repr__(self) -> str:
return f"<{self.name}>"
Expand Down Expand Up @@ -254,7 +254,25 @@ def default_network(self) -> str:
Returns:
str
"""
return self._default_network

if network := self._default_network:
# Was set programatically.
return network

elif network := self.config.get("default_network"):
# Default found in config.
return network

elif LOCAL_NETWORK_NAME in self.networks:
# Default to the LOCAL_NETWORK_NAME, at last resort.
return LOCAL_NETWORK_NAME

elif len(self.networks) >= 1:
# Use the first network.
return self.networks[0]

# Very unlikely scenario.
raise ValueError("No networks found.")

def set_default_network(self, network_name: str):
"""
Expand Down Expand Up @@ -906,12 +924,19 @@ def default_provider(self) -> Optional[str]:
Optional[str]
"""

if self._default_provider:
return self._default_provider
if provider := self._default_provider:
# Was set programatically.
return provider

elif provider_from_config := self._network_config.get("default_provider"):
# The default is found in the Network's config class.
return provider_from_config

if len(self.providers) > 0:
elif len(self.providers) > 0:
# No default set anywhere - use the first installed.
return list(self.providers)[0]

# There are no providers at all for this network.
return None

@property
Expand Down
15 changes: 13 additions & 2 deletions src/ape/api/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1420,15 +1420,26 @@ def prepare_transaction(self, txn: TransactionAPI) -> TransactionAPI:
txn.max_fee = int(self.base_fee * multiplier + txn.max_priority_fee)
# else: Assume user specified the correct amount or txn will fail and waste gas

if txn.gas_limit is None:
multiplier = self.network.auto_gas_multiplier
gas_limit = self.network.gas_limit if txn.gas_limit is None else txn.gas_limit
if gas_limit in (None, "auto") or isinstance(gas_limit, AutoGasLimit):
multiplier = (
gas_limit.multiplier
if isinstance(gas_limit, AutoGasLimit)
else self.network.auto_gas_multiplier
)
if multiplier != 1.0:
gas = min(int(self.estimate_gas_cost(txn) * multiplier), self.max_gas)
else:
gas = self.estimate_gas_cost(txn)

txn.gas_limit = gas

elif gas_limit == "max":
txn.gas_limit = self.max_gas
antazoey marked this conversation as resolved.
Show resolved Hide resolved

elif gas_limit is not None and isinstance(gas_limit, int):
txn.gas_limit = gas_limit

if txn.required_confirmations is None:
txn.required_confirmations = self.network.required_confirmations
elif not isinstance(txn.required_confirmations, int) or txn.required_confirmations < 0:
Expand Down
7 changes: 1 addition & 6 deletions src/ape/managers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from ape._pydantic_compat import root_validator
from ape.api import ConfigDict, DependencyAPI, PluginConfig
from ape.exceptions import ConfigError, NetworkError
from ape.exceptions import ConfigError
from ape.logging import logger
from ape.utils import BaseInterfaceModel, load_config

Expand Down Expand Up @@ -182,11 +182,6 @@ def _plugin_configs(self) -> Dict[str, PluginConfig]:
configs["compiler"] = compiler_dict
self.compiler = CompilerConfig(**compiler_dict)

try:
self.network_manager.set_default_ecosystem(self.default_ecosystem)
except NetworkError as err:
logger.warning(str(err))

dependencies = user_config.pop("dependencies", []) or []
if not isinstance(dependencies, list):
raise ConfigError("'dependencies' config item must be a list of dicts.")
Expand Down
62 changes: 15 additions & 47 deletions src/ape/managers/networks.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from functools import cached_property
from typing import Dict, Iterator, List, Optional, Set, Union

import yaml

from ape.api import EcosystemAPI, ProviderAPI, ProviderContextManager
from ape.api.networks import LOCAL_NETWORK_NAME, NetworkAPI
from ape.api.networks import NetworkAPI
from ape.exceptions import ApeAttributeError, NetworkError
from ape.logging import logger

from .base import BaseManager
from ape.managers.base import BaseManager


class NetworkManager(BaseManager):
Expand All @@ -27,7 +26,6 @@ class NetworkManager(BaseManager):

_active_provider: Optional[ProviderAPI] = None
_default: Optional[str] = None
_ecosystems_by_project: Dict[str, Dict[str, EcosystemAPI]] = {}

def __repr__(self):
provider = self.active_provider
Expand Down Expand Up @@ -137,54 +135,21 @@ def provider_names(self) -> Set[str]:

return names

@property
@cached_property
def ecosystems(self) -> Dict[str, EcosystemAPI]:
"""
All the registered ecosystems in ``ape``, such as ``ethereum``.
"""

project_name = self.config_manager.PROJECT_FOLDER.stem
if project_name in self._ecosystems_by_project:
return self._ecosystems_by_project[project_name]
def to_kwargs(name: str) -> Dict:
return {
"name": name,
"data_folder": self.config_manager.DATA_FOLDER / name,
"request_header": self.config_manager.REQUEST_HEADER,
}

ecosystem_dict = {}
for plugin_name, ecosystem_class in self.plugin_manager.ecosystems:
ecosystem = ecosystem_class( # type: ignore
name=plugin_name,
data_folder=self.config_manager.DATA_FOLDER / plugin_name,
request_header=self.config_manager.REQUEST_HEADER,
)
ecosystem_config = self.config_manager.get_config(plugin_name).dict()
default_network = ecosystem_config.get("default_network", LOCAL_NETWORK_NAME)

try:
ecosystem.set_default_network(default_network)
except NetworkError as err:
message = f"Failed setting default network: {err}"
logger.error(message)

if ecosystem_config:
for network_name, network in ecosystem.networks.items():
network_name = network_name.replace("-", "_")
if network_name not in ecosystem_config:
continue

network_config = ecosystem_config[network_name]
if "default_provider" not in network_config:
continue

default_provider = network_config["default_provider"]
if default_provider:
try:
network.set_default_provider(default_provider)
except NetworkError as err:
message = f"Failed setting default provider: {err}"
logger.error(message)

ecosystem_dict[plugin_name] = ecosystem

self._ecosystems_by_project[project_name] = ecosystem_dict
return ecosystem_dict
ecosystems = self.plugin_manager.ecosystems
return {n: cls(**to_kwargs(n)) for n, cls in ecosystems} # type: ignore

def create_adhoc_geth_provider(self, uri: str) -> ProviderAPI:
"""
Expand Down Expand Up @@ -485,6 +450,9 @@ def default_ecosystem(self) -> EcosystemAPI:
if self._default:
return ecosystems[self._default]

elif self.config_manager.default_ecosystem:
return ecosystems[self.config_manager.default_ecosystem]

# If explicit default is not set, use first registered ecosystem
elif len(ecosystems) > 0:
return list(ecosystems.values())[0]
Expand Down
9 changes: 6 additions & 3 deletions src/ape_ethereum/ecosystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,10 +602,13 @@ def create_transaction(self, **kwargs) -> TransactionAPI:
if "type" in kwargs:
if kwargs["type"] is None:
version = TransactionType.DYNAMIC
elif not isinstance(kwargs["type"], int):
version = TransactionType(self.conversion_manager.convert(kwargs["type"], int))
else:
elif isinstance(kwargs["type"], TransactionType):
version = kwargs["type"]
elif isinstance(kwargs["type"], int):
version = TransactionType(kwargs["type"])
else:
# Using hex values or alike.
version = TransactionType(self.conversion_manager.convert(kwargs["type"], int))

elif "gas_price" in kwargs:
version = TransactionType.STATIC
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ def ethereum(networks):


@pytest.fixture(autouse=True)
def eth_tester_provider():
def eth_tester_provider(ethereum):
if not ape.networks.active_provider or ape.networks.provider.name != "test":
with ape.networks.ethereum.local.use_provider("test") as provider:
with ethereum.local.use_provider("test") as provider:
yield provider
else:
yield ape.networks.provider
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import ape
from ape.api import EcosystemAPI, NetworkAPI, TransactionAPI
from ape.api.networks import LOCAL_NETWORK_NAME
from ape.contracts import ContractContainer, ContractInstance
from ape.exceptions import ChainError, ContractLogicError
from ape.logging import LogLevel
Expand Down Expand Up @@ -416,9 +415,10 @@ def use_debug(logger):

@pytest.fixture
def dummy_live_network(chain):
original_network = chain.provider.network.name
chain.provider.network.name = "goerli"
yield chain.provider.network
chain.provider.network.name = LOCAL_NETWORK_NAME
chain.provider.network.name = original_network


@pytest.fixture(scope="session")
Expand Down
48 changes: 32 additions & 16 deletions tests/functional/test_accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ape.types.signatures import recover_signer
from ape.utils.testing import DEFAULT_NUMBER_OF_TEST_ACCOUNTS
from ape_ethereum.ecosystem import ProxyType
from ape_ethereum.transactions import TransactionType
from ape_test.accounts import TestAccount
from tests.conftest import explorer_test

Expand Down Expand Up @@ -499,28 +500,33 @@ def test_declare(contract_container, sender):
assert not receipt.failed


@pytest.mark.parametrize(
"tx_type,params", [(0, ["gas_price"]), (2, ["max_fee", "max_priority_fee"])]
)
def test_prepare_transaction(sender, ethereum, tx_type, params):
@pytest.mark.parametrize("tx_type", (TransactionType.STATIC, TransactionType.DYNAMIC))
def test_prepare_transaction_using_auto_gas(sender, ethereum, tx_type):
params = (
("gas_price",) if tx_type is TransactionType.STATIC else ("max_fee", "max_priority_fee")
)

def clear_network_property_cached():
for field in ("gas_limit", "auto_gas_multiplier"):
if field in tx.provider.network.__dict__:
del tx.provider.network.__dict__[field]
if field in ethereum.local.__dict__:
del ethereum.local.__dict__[field]

tx = ethereum.create_transaction(type=tx_type, gas_limit="auto")
auto_gas = AutoGasLimit(multiplier=1.0)
original_limit = tx.provider.network.config.local.gas_limit
original_limit = ethereum.config.local.gas_limit

try:
tx.provider.network.config.local.gas_limit = auto_gas
ethereum.config.local.gas_limit = auto_gas
clear_network_property_cached()
assert ethereum.local.gas_limit == auto_gas, "Setup failed - auto gas not set."

# NOTE: Must create tx _after_ setting network gas value.
tx = ethereum.create_transaction(type=tx_type)

# Show tx doesn't have these by default.
assert tx.nonce is None
for param in params:
# Custom fields depending on type.
assert getattr(tx, param) is None
assert getattr(tx, param) is None, f"'{param}' unexpectedly set."

# Gas should NOT yet be estimated, as that happens closer to sending.
assert tx.gas_limit is None
Expand All @@ -530,24 +536,34 @@ def clear_network_property_cached():

# We expect these fields to have been set.
assert tx.nonce is not None
assert tx.gas_limit is not None # Gas was estimated (using eth_estimateGas).
assert tx.gas_limit is not None

# Show multipliers work. First, reset network to use one (hack).
gas_smaller = tx.gas_limit

clear_network_property_cached()
auto_gas.multiplier = 1.1
tx.provider.network.config.local.gas_limit = auto_gas
ethereum.config.local.gas_limit = auto_gas
clear_network_property_cached()
assert ethereum.local.gas_limit == auto_gas, "Setup failed - auto gas multiplier not set."

tx2 = ethereum.create_transaction(type=tx_type, gas_limit="auto")
tx2 = ethereum.create_transaction(type=tx_type)
tx2 = sender.prepare_transaction(tx2)
gas_bigger = tx2.gas_limit

assert gas_smaller < gas_bigger

for param in params:
assert getattr(tx, param) is not None

finally:
tx.provider.network.config.local.gas_limit = original_limit
ethereum.config.local.gas_limit = original_limit
clear_network_property_cached()


@pytest.mark.parametrize("type_", (TransactionType.STATIC, TransactionType.DYNAMIC))
def test_prepare_transaction_and_call_using_max_gas(type_, ethereum, sender, eth_tester_provider):
tx = ethereum.create_transaction(type=type_.value)
tx = sender.prepare_transaction(tx)
assert tx.gas_limit == eth_tester_provider.max_gas, "Test setup failed - gas limit unexpected."

actual = sender.call(tx)
assert not actual.failed
10 changes: 6 additions & 4 deletions tests/functional/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ def test_integer_deployment_addresses(networks):


@pytest.mark.parametrize(
"ecosystems,networks,err_part",
"ecosystem_names,network_names,err_part",
[(["ERRORS"], ["mainnet"], "ecosystem"), (["ethereum"], ["ERRORS"], "network")],
)
def test_bad_value_in_deployments(ecosystems, networks, err_part, ape_caplog, plugin_manager):
def test_bad_value_in_deployments(
ecosystem_names, network_names, err_part, ape_caplog, plugin_manager
):
deployments = _create_deployments()
all_ecosystems = dict(plugin_manager.ecosystems)
ecosystem_dict = {e: all_ecosystems[e] for e in ecosystems if e in all_ecosystems}
data = {**deployments, "valid_ecosystems": ecosystem_dict, "valid_networks": networks}
ecosystem_dict = {e: all_ecosystems[e] for e in ecosystem_names if e in all_ecosystems}
data = {**deployments, "valid_ecosystems": ecosystem_dict, "valid_networks": network_names}
ape_caplog.assert_last_log_with_retries(
lambda: DeploymentConfigCollection(__root__=data),
f"Invalid {err_part}",
Expand Down
8 changes: 7 additions & 1 deletion tests/functional/test_contract_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
MethodNonPayableError,
)
from ape.types import AddressType
from ape_ethereum.transactions import TransactionStatusEnum
from ape_ethereum.transactions import TransactionStatusEnum, TransactionType

MATCH_TEST_CONTRACT = re.compile(r"<TestContract((Sol)|(Vy))")

Expand Down Expand Up @@ -871,3 +871,9 @@ def test_sending_funds_to_non_payable_constructor_by_accountDeploy(
match="Sending funds to a non-payable constructor.",
):
owner.deploy(solidity_contract_container, 1, value="1 ether")


@pytest.mark.parametrize("type_", TransactionType)
def test_as_transaction(type_, vyper_contract_instance, owner, eth_tester_provider):
tx = vyper_contract_instance.setNumber.as_transaction(987, sender=owner, type=type_.value)
assert tx.gas_limit == eth_tester_provider.max_gas
Loading
Loading