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: various issues preventing impersonated accounts and forks from working #80

Merged
merged 3 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 10 additions & 21 deletions ape_foundry/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
TransactionAPI,
)
from ape.exceptions import (
APINotImplementedError,
ContractLogicError,
OutOfGasError,
SubprocessError,
Expand Down Expand Up @@ -90,6 +89,7 @@ class FoundryNetworkConfig(PluginConfig):
# RPC defaults
base_fee: int = 0
priority_fee: int = 0
gas_price: int = 0

# For setting the values in --fork and --fork-block-number command arguments.
# Used only in FoundryForkProvider.
Expand Down Expand Up @@ -225,6 +225,10 @@ def ws_uri(self) -> str:
def priority_fee(self) -> int:
return self.settings.priority_fee

@property
def gas_price(self) -> int:
return self.settings.gas_price

@property
def is_connected(self) -> bool:
if self._host in ("auto", None):
Expand Down Expand Up @@ -427,6 +431,8 @@ def build_command(self) -> List[str]:
"--steps-tracing",
"--block-base-fee-per-gas",
f"{self.settings.base_fee}",
"--gas-price",
f"{self.settings.gas_price}",
Copy link
Member

Choose a reason for hiding this comment

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

I believe you can't have both base fee and gas price at the same time

To get a more consistently nice testing environment, I think a gas price of zero works for local testing. For fork testing, I'm not sure if that'll work but it is better than using base fee because that always requires deducting gas

Copy link
Member Author

@antazoey antazoey Jan 5, 2024

Choose a reason for hiding this comment

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

i think it is just 1 setting applies to 1 type of tx and the other applies to another type of tx
also seems like brownie uses pre-1559 txns by default, which is how i got this bug... because i was trying to copy brownie in ape

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it went out of favor around the time EIP-1559 got popular

]

if not self.settings.auto_mine:
Expand Down Expand Up @@ -496,10 +502,6 @@ def send_transaction(self, txn: TransactionAPI) -> ReceiptAPI:
# Allow for an unsigned transaction
sender = cast(AddressType, sender) # We know it's checksummed at this point.
txn = self.prepare_transaction(txn)
original_code = HexBytes(self.get_code(sender))
if original_code:
self.set_code(sender, "")

txn_dict = txn.model_dump(mode="json", by_alias=True)
if isinstance(txn_dict.get("type"), int):
txn_dict["type"] = HexBytes(txn_dict["type"]).hex()
Expand All @@ -510,9 +512,6 @@ def send_transaction(self, txn: TransactionAPI) -> ReceiptAPI:
except ValueError as err:
raise self.get_virtual_machine_error(err, txn=txn) from err

finally:
if original_code:
self.set_code(sender, original_code.hex())
else:
try:
txn_hash = self.web3.eth.send_raw_transaction(txn.serialize_transaction())
Expand Down Expand Up @@ -921,9 +920,9 @@ def forked_network(self) -> ForkedNetworkAPI:
@property
def upstream_provider_name(self) -> str:
if upstream_name := self._fork_config.upstream_provider:
self.forked_network.network_config.upstream_provider = upstream_name
self._fork_config.upstream_provider = upstream_name

return self.forked_network.network_config.upstream_provider
return self.forked_network.upstream_provider.name

@property
def fork_url(self) -> str:
Expand Down Expand Up @@ -981,16 +980,6 @@ def reset_fork(self, block_number: Optional[int] = None):
if block_number is not None:
forking_params["blockNumber"] = block_number

# # Rest the fork
result = self._make_request("anvil_reset", [{"forking": forking_params}])

try:
base_fee = self.base_fee
except APINotImplementedError:
base_fee = None
logger.warning("base_fee not found in block - base fee may not be reset.")

# reset next block base fee to that of new chain head if can
if base_fee is not None:
self._make_request("anvil_setNextBlockBaseFeePerGas", [base_fee])

return result
2 changes: 1 addition & 1 deletion tests/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_connect_and_disconnect(disconnected_provider):

def test_gas_price(connected_provider):
gas_price = connected_provider.gas_price
assert gas_price > 1
assert gas_price == 0


def test_uri_disconnected(disconnected_provider):
Expand Down
Loading