From 55c8853384eb014f2e1194f7346f516015981bd0 Mon Sep 17 00:00:00 2001 From: antazoey Date: Tue, 27 Feb 2024 14:49:35 -0600 Subject: [PATCH] fix: estimate gas (#37) Co-authored-by: fubuloubu <3859395+fubuloubu@users.noreply.github.com> --- ape_safe/accounts.py | 13 +++++++++++ ape_safe/client/__init__.py | 14 +++++++++++ ape_safe/client/base.py | 5 ++++ ape_safe/client/mock.py | 7 +++++- tests/functional/test_account.py | 40 ++++++++++++++++++-------------- 5 files changed, 60 insertions(+), 19 deletions(-) diff --git a/ape_safe/accounts.py b/ape_safe/accounts.py index 21f357f..4770d00 100644 --- a/ape_safe/accounts.py +++ b/ape_safe/accounts.py @@ -459,8 +459,21 @@ def load_submitter( def prepare_transaction(self, txn: TransactionAPI) -> TransactionAPI: # NOTE: Need to override `AccountAPI` behavior for balance checks + if txn.gas_limit is None: + txn.gas_limit = self.estimate_gas_cost(txn=txn) + return self.provider.prepare_transaction(txn) + def estimate_gas_cost(self, **kwargs) -> int: + operation = kwargs.pop("operation", 0) + txn = kwargs.pop("txn", self.as_transaction(**kwargs)) + return ( + self.client.estimate_gas_cost( + txn.receiver or ZERO_ADDRESS, txn.value, txn.data, operation=operation + ) + or 0 + ) + def _preapproved_signature( self, signer: Union[AddressType, BaseAddress, str] ) -> MessageSignature: diff --git a/ape_safe/client/__init__.py b/ape_safe/client/__init__.py index 511a0e5..6c89797 100644 --- a/ape_safe/client/__init__.py +++ b/ape_safe/client/__init__.py @@ -162,6 +162,20 @@ def post_signatures( raise # The error from BaseClient we are already raising (no changes) + def estimate_gas_cost( + self, receiver: AddressType, value: int, data: bytes, operation: int = 0 + ) -> Optional[int]: + url = f"safes/{self.address}/multisig-transactions/estimations" + request: Dict = { + "to": receiver, + "value": value, + "data": HexBytes(data).hex(), + "operation": operation, + } + result = self._post(url, json=request).json() + gas = result.get("safeTxGas") + return int(HexBytes(gas).hex(), 16) + __all__ = [ "ExecutedTxData", diff --git a/ape_safe/client/base.py b/ape_safe/client/base.py index 3863510..31a29d9 100644 --- a/ape_safe/client/base.py +++ b/ape_safe/client/base.py @@ -57,6 +57,11 @@ def post_signatures( signatures: Dict[AddressType, MessageSignature], ): ... + @abstractmethod + def estimate_gas_cost( + self, receiver: AddressType, value: int, data: bytes, operation: int = 0 + ) -> Optional[int]: ... + """Shared methods""" def get_transactions( diff --git a/ape_safe/client/mock.py b/ape_safe/client/mock.py index 6abd614..d1f5e32 100644 --- a/ape_safe/client/mock.py +++ b/ape_safe/client/mock.py @@ -1,5 +1,5 @@ from datetime import datetime, timezone -from typing import Dict, Iterator, List, Union, cast +from typing import Dict, Iterator, List, Optional, Union, cast from ape.contracts import ContractInstance from ape.types import AddressType, MessageSignature @@ -109,3 +109,8 @@ def post_signatures( signatureType=SignatureType.EOA, ) ) + + def estimate_gas_cost( + self, receiver: AddressType, value: int, data: bytes, operation: int = 0 + ) -> Optional[int]: + return None # Estimate gas normally diff --git a/tests/functional/test_account.py b/tests/functional/test_account.py index 5378354..dd134f3 100644 --- a/tests/functional/test_account.py +++ b/tests/functional/test_account.py @@ -19,7 +19,7 @@ def test_init(safe, OWNERS, THRESHOLD, safe_contract): assert safe.next_nonce == 0 -@pytest.mark.parametrize("mode", ["impersonate", "api", "sign"]) +@pytest.mark.parametrize("mode", ("impersonate", "api", "sign")) def test_swap_owner(safe, accounts, OWNERS, mode): impersonate = mode == "impersonate" submit = mode != "api" @@ -30,21 +30,24 @@ def test_swap_owner(safe, accounts, OWNERS, mode): # NOTE: Since the signers are processed in order, we replace the last account prev_owner = safe.compute_prev_signer(old_owner) - exec_transaction = lambda: safe.contract.swapOwner( # noqa: E731 - prev_owner, - old_owner, - new_owner, - sender=safe, - impersonate=impersonate, - submit=submit, - ) + + def exec_transaction(): + return safe.contract.swapOwner( + prev_owner, + old_owner, + new_owner, + sender=safe, + impersonate=impersonate, + submit=submit, + ) if submit: receipt = exec_transaction() else: # Attempting to execute should raise `SignatureError` and push `safe_tx` to mock client - assert len(list(safe.client.get_transactions(confirmed=False))) == 0 + size = len(list(safe.client.get_transactions(confirmed=False))) + assert size == 0 with pytest.raises(SignatureError): exec_transaction() @@ -78,7 +81,7 @@ def test_swap_owner(safe, accounts, OWNERS, mode): assert new_owner.address in safe.signers -@pytest.mark.parametrize("mode", ["impersonate", "api", "sign"]) +@pytest.mark.parametrize("mode", ("impersonate", "api", "sign")) def test_add_owner(safe, accounts, OWNERS, mode): impersonate = mode == "impersonate" submit = mode != "api" @@ -86,13 +89,14 @@ def test_add_owner(safe, accounts, OWNERS, mode): new_owner = accounts[len(OWNERS)] # replace owner 1 with account N + 1 assert new_owner.address not in safe.signers - exec_transaction = lambda: safe.contract.addOwnerWithThreshold( # noqa: E731 - new_owner, - safe.confirmations_required, - sender=safe, - impersonate=impersonate, - submit=submit, - ) + def exec_transaction(): + return safe.contract.addOwnerWithThreshold( + new_owner, + safe.confirmations_required, + sender=safe, + impersonate=impersonate, + submit=submit, + ) if submit: receipt = exec_transaction()