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

feat: add gas limit configuration #1009

Merged
merged 25 commits into from Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
63f9d16
feat: add gas limit configuration
Aug 23, 2022
930c86d
fix: Improve NetworkAPI.gas_limit type hint
Aug 25, 2022
2c88d18
fix: use ConfigError as NetworkAPI.gas_limit error
Aug 25, 2022
b96572f
fix: comment clarity in estimate_gas_cost()
Aug 25, 2022
2e8b2a1
test: add provider gas estimation test
Sep 12, 2022
80e3a21
fix: import sorting
Sep 12, 2022
5a350f5
fix: remove 'if' fallthrough
Sep 14, 2022
320c165
fix: make gas config case insensitive
Sep 14, 2022
56dd5a7
test: parametrize network gas limit config test
Sep 14, 2022
29a917a
test: use pytest-mocker
Sep 14, 2022
4c502af
feat: update supported values for gas_limit config
Sep 14, 2022
88688b1
chore: include gas_limit in config docs
Sep 14, 2022
a2c219e
chore: fix spacing in doc
Sep 14, 2022
dc90483
chore: add missing "the" in doc
Sep 14, 2022
7685ffc
fix: coerce auto/max gas config to lowercase
Sep 14, 2022
f2aba77
chore: remove extra quotes in doc
Sep 14, 2022
324d04c
fix: remove validation from NetworkAPI.gas_limit
Sep 14, 2022
e002111
test: add config helper and fix doc
Sep 14, 2022
a065de5
test: fix flakey test
antazoey Sep 14, 2022
46e51aa
test: try mkdir
antazoey Sep 15, 2022
df6616d
Merge branch 'main' into feat/network-gas-limit-config
antazoey Sep 15, 2022
27302b6
fix: use finally to change back on fail
antazoey Sep 15, 2022
3d9690d
feat: GasLimit type
antazoey Sep 15, 2022
a7c8a09
chore: Merge branch 'main' into feat/network-gas-limit-config
antazoey Sep 15, 2022
d0f687c
fix: add pre=True to gas limit validator
antazoey Sep 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion docs/userguides/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@ ethereum:
default_provider: hardhat
```

Set the gas limit for a given network:

```yaml
ethereum:
default_network: mainnet-fork
gas_limit: max
```

You may use one of:
This conversation was marked as resolved.
Show resolved Hide resolved

- `"auto"` - gas limit is estimated for each transaction
- `"max"` - the maximum block gas limit is used
- A number or numeric string, base 10 or 16 (e.g. `1234`, `"1234"`, `0x1234`, `"0x1234"`)

## Plugins

Set which plugins you want to always use:
Expand All @@ -157,4 +171,4 @@ Configure your test accounts:
test:
mnemonic: test test test test test test test test test test test junk
number_of_accounts: 5
```
```
14 changes: 14 additions & 0 deletions docs/userguides/networks.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ default_ecosystem: <ecosystem-name>
default_provider: <provider-name>
```

You may also configure a specific gas limit for a given network:

```yaml
<ecosystem-name>:
default_network: <network-name>
<network-name>:
gas_limit: "max"
```

You may use one of:
- `"auto"` - gas limit is estimated for each transaction
- `"max"` - the maximum block gas limit is used
- A number or numeric string, base 10 or 16 (e.g. `1234`, `"1234"`, `0x1234`, `"0x1234"`)

## Local Network

The default network in Ape is the local network (keyword `"local"`).
Expand Down
6 changes: 5 additions & 1 deletion src/ape/api/networks.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from functools import partial
from pathlib import Path
from tempfile import mkdtemp
from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Tuple, Type, Union
from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Literal, Optional, Tuple, Type, Union

from eth_account import Account as EthAccount
from eth_account._utils.legacy_transactions import (
Expand Down Expand Up @@ -607,6 +607,10 @@ def config(self) -> PluginConfig:
def _network_config(self) -> Dict:
return self.config.get(self.name, {})

@cached_property
def gas_limit(self) -> Union[Literal["auto", "max"], int]:
return self._network_config.get("gas_limit", "auto")

@property
def chain_id(self) -> int:
"""
Expand Down
14 changes: 12 additions & 2 deletions src/ape/api/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,18 @@ def estimate_gas_cost(self, txn: TransactionAPI, **kwargs) -> int:

Returns:
int: The estimated cost of gas to execute the transaction
reported in the fee-currency's smallest unit, e.g. Wei.
"""
reported in the fee-currency's smallest unit, e.g. Wei. If the
provider's network has been configured with a gas limit override, it
will be returned. If the gas limit configuration is "max" this will
return the block maximum gas limit.
"""
if isinstance(self.network.gas_limit, int):
return self.network.gas_limit

if self.network.gas_limit == "max":
This conversation was marked as resolved.
Show resolved Hide resolved
block = self.web3.eth.get_block("latest")
return block["gasLimit"]
# else: Handle "auto" gas limit via estimation

This conversation was marked as resolved.
Show resolved Hide resolved
txn_dict = txn.dict()
try:
Expand Down
19 changes: 10 additions & 9 deletions src/ape/managers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,13 @@ def using_project(
self.project_manager.path = project_folder
os.chdir(project_folder)

self.load(force_reload=True)
yield self.project_manager

self.PROJECT_FOLDER = initial_project_folder
self.contracts_folder = initial_contracts_folder
self.project_manager.path = initial_project_folder

if initial_project_folder.is_dir():
os.chdir(initial_project_folder)
try:
self.load(force_reload=True)
yield self.project_manager
finally:
self.PROJECT_FOLDER = initial_project_folder
self.contracts_folder = initial_contracts_folder
self.project_manager.path = initial_project_folder

if initial_project_folder.is_dir():
os.chdir(initial_project_folder)
9 changes: 9 additions & 0 deletions src/ape/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@
"""


GasLimit = Union[Literal["auto", "max"], int, str]
"""
A value you can give to Ape for handling gas-limit calculations.
``"auto"`` refers to automatically figuring out the gas,
``"max"`` refers to using the maximum block gas limit,
and otherwise you can provide a numeric value.
"""


TopicFilter = List[Union[Optional[HexStr], List[Optional[HexStr]]]]


Expand Down
35 changes: 32 additions & 3 deletions src/ape_ethereum/ecosystem.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import re
from enum import IntEnum
from typing import Any, Dict, Iterator, List, Optional, Tuple, Union
from typing import Any, Dict, Iterator, List, Literal, Optional, Tuple, Union

from eth_abi import decode, encode
from eth_abi.exceptions import InsufficientDataBytes
from eth_typing import HexStr
from eth_utils import add_0x_prefix, encode_hex, keccak, to_bytes, to_checksum_address
from ethpm_types.abi import ABIType, ConstructorABI, EventABI, MethodABI
from hexbytes import HexBytes
from pydantic import Field
from pydantic import Field, validator

from ape.api import BlockAPI, EcosystemAPI, PluginConfig, ReceiptAPI, TransactionAPI
from ape.api.networks import LOCAL_NETWORK_NAME, ProxyInfoAPI
from ape.contracts.base import ContractCall
from ape.exceptions import APINotImplementedError, DecodingError, TransactionError
from ape.logging import logger
from ape.types import AddressType, ContractLog, RawAddress, TransactionSignature
from ape.types import AddressType, ContractLog, GasLimit, RawAddress, TransactionSignature
from ape.utils import (
DEFAULT_LOCAL_TRANSACTION_ACCEPTANCE_TIMEOUT,
DEFAULT_TRANSACTION_ACCEPTANCE_TIMEOUT,
Expand Down Expand Up @@ -76,6 +76,35 @@ class NetworkConfig(PluginConfig):
block_time: int = 0
transaction_acceptance_timeout: int = DEFAULT_TRANSACTION_ACCEPTANCE_TIMEOUT

gas_limit: GasLimit = "auto"
"""
The gas limit override to use for the network. If set to ``"auto"``, ape will
estimate gas limits based on the transaction. If set to ``"max"`` the gas limit
will be set to the maximum block gas limit for the network. Otherwise an ``int``
can be used to specify an explicit gas limit amount (either base 10 or 16).
"""

class Config:
smart_union = True

@validator("gas_limit", pre=True)
def validate_gas_limit(cls, value: GasLimit) -> Union[Literal["auto", "max"], int]:
This conversation was marked as resolved.
Show resolved Hide resolved
if isinstance(value, str):
if value.lower() in ("auto", "max"):
return value.lower() # type: ignore

# Value could be an integer string
if value.isdigit():
return int(value)
# Enforce "0x" prefix on base 16 integer strings
elif value.lower().startswith("0x"):
return int(value, 16)
else:
raise ValueError("Invalid gas_limit, must be 'auto', 'max', or a number")

# Value is an integer literal
return value


class EthereumConfig(PluginConfig):
mainnet: NetworkConfig = NetworkConfig(required_confirmations=7, block_time=13) # type: ignore
Expand Down
7 changes: 7 additions & 0 deletions src/ape_test/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ def update_settings(self, new_settings: dict):
pass

def estimate_gas_cost(self, txn: TransactionAPI, **kwargs) -> int:
if isinstance(self.network.gas_limit, int):
return self.network.gas_limit

elif self.network.gas_limit == "max":
block = self.web3.eth.get_block("latest")
return block["gasLimit"]

block_id = kwargs.pop("block_identifier", None)
estimate_gas = self.web3.eth.estimate_gas

Expand Down
1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def project_folder(config):

@pytest.fixture(scope="session")
def project(config):
config.PROJECT_FOLDER.mkdir(parents=True, exist_ok=True)
yield ape.Project(config.PROJECT_FOLDER)


Expand Down
58 changes: 57 additions & 1 deletion tests/functional/test_config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import Dict
from typing import Dict, Literal, Union

import pytest

Expand Down Expand Up @@ -54,6 +54,62 @@ def test_ethereum_network_configs(config, temp_config):
assert actual.rinkeby.block_time == 15


def test_network_gas_limit_default(config):
eth_config = config.get_config("ethereum")

assert eth_config.rinkeby.gas_limit == "auto"
assert eth_config.local.gas_limit == "auto"
Copy link
Contributor

Choose a reason for hiding this comment

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

In 0.6 or any future release, we may want to consider making max the default for local.
I think it will give speed improvements to running test suites.

Copy link
Author

Choose a reason for hiding this comment

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

Good point, and what's funny is I thought previously I had local set to max but I can't seem to find trace of that now 🤔 it was before that pre-audit work so I think I lost a bit of track with the gear switching

I may need to revisit this for #957 but I'll see about including such changes in that PR if need be 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to make it use max now, that is probably fine. It should not affect anything really so maybe not a real breaking change



def _rinkeby_with_gas_limit(gas_limit: Union[Literal["auto", "max"], int]) -> dict:
return {
"ethereum": {
"rinkeby": {
"default_provider": "test",
"gas_limit": gas_limit,
}
}
}


@pytest.mark.parametrize("gas_limit", ("auto", "max"))
def test_network_gas_limit_string_config(gas_limit, config, temp_config):
eth_config = _rinkeby_with_gas_limit(gas_limit)

with temp_config(eth_config):
actual = config.get_config("ethereum")

assert actual.rinkeby.gas_limit == gas_limit

# Local configuration is unaffected
assert actual.local.gas_limit == "auto"


@pytest.mark.parametrize("gas_limit", (1234, "1234", 0x4D2, "0x4D2"))
def test_network_gas_limit_numeric_config(gas_limit, config, temp_config):
eth_config = _rinkeby_with_gas_limit(gas_limit)

with temp_config(eth_config):
actual = config.get_config("ethereum")

assert actual.rinkeby.gas_limit == 1234

# Local configuration is unaffected
assert actual.local.gas_limit == "auto"


def test_network_gas_limit_invalid_numeric_string(config, temp_config):
"""
Test that using hex strings for a network's gas_limit config must be
prefixed with '0x'
"""
eth_config = _rinkeby_with_gas_limit("4D2")

with pytest.raises(ValueError, match="Invalid gas_limit, must be 'auto', 'max', or a number"):
with temp_config(eth_config):
pass


def test_dependencies(dependency_config, config):
assert len(config.dependencies) == 1
assert config.dependencies[0].name == "testdependency"
Expand Down
5 changes: 1 addition & 4 deletions tests/functional/test_contract_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ def test_deploy_and_publish_local_network(owner, contract_container):
contract_container.deploy(sender=owner, publish=True)


def test_deploy_and_publish_live_network_no_explorer(
owner, project, contract_container, dummy_live_network
):
_ = project # Ensure active project for `track_deployment` to work
def test_deploy_and_publish_live_network_no_explorer(owner, contract_container, dummy_live_network):
dummy_live_network.__dict__["explorer"] = None
expected_message = "Unable to publish contract - no explorer plugin installed."
with pytest.raises(NetworkError, match=expected_message):
Expand Down
8 changes: 8 additions & 0 deletions tests/functional/test_networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,11 @@ def test_ecosystems_when_default_provider_not_exists(temp_config, caplog, networ
f"Failed setting default provider: "
f"Provider '{bad_provider}' not found in network 'kovan'."
)


def test_gas_limits(ethereum):
"""
Test the default gas limit configurations for local and live networks.
"""
assert ethereum.rinkeby.gas_limit == "auto"
assert ethereum.local.gas_limit == "auto"
11 changes: 11 additions & 0 deletions tests/functional/test_provider.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest
from eth_typing import HexStr

Expand All @@ -16,6 +18,15 @@ def test_get_block(eth_tester_provider, block_id):
assert latest_block.gas_used == 0


def test_estimate_gas_with_max_value_from_block(mocker, eth_tester_provider):
mock_txn = mocker.patch("ape.api.networks.NetworkAPI.gas_limit", new_callable=mock.PropertyMock)
mock_txn.return_value = "max"
gas_cost = eth_tester_provider.estimate_gas_cost(mock_txn)
latest_block = eth_tester_provider.get_block("latest")

assert gas_cost == latest_block.gas_limit


def test_chain_id(eth_tester_provider):
chain_id = eth_tester_provider.chain_id
assert chain_id == CHAIN_ID
Expand Down