Skip to content

Commit

Permalink
fix: default call transactions would not get prepared when sender is …
Browse files Browse the repository at this point in the history
…a contract (#2508)
  • Loading branch information
antazoey authored Feb 16, 2025
1 parent 2a365bc commit ced89ab
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 46 deletions.
40 changes: 1 addition & 39 deletions src/ape/api/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def call(
if sign:
prepared_txn = self.sign_transaction(txn, **signer_options)
if not prepared_txn:
raise SignatureError("The transaction was not signed.")
raise SignatureError("The transaction was not signed.", transaction=txn)

else:
prepared_txn = txn
Expand Down Expand Up @@ -401,44 +401,6 @@ def check_signature(
else:
raise AccountsError(f"Unsupported message type: {type(data)}.")

def prepare_transaction(self, txn: TransactionAPI) -> TransactionAPI:
"""
Set default values on a transaction.
Raises:
:class:`~ape.exceptions.AccountsError`: When the account cannot afford the transaction
or the nonce is invalid.
:class:`~ape.exceptions.TransactionError`: When given negative required confirmations.
Args:
txn (:class:`~ape.api.transactions.TransactionAPI`): The transaction to prepare.
Returns:
:class:`~ape.api.transactions.TransactionAPI`
"""

# NOTE: Allow overriding nonce, assume user understands what this does
if txn.nonce is None:
txn.nonce = self.nonce
elif txn.nonce < self.nonce:
raise AccountsError("Invalid nonce, will not publish.")

txn = self.provider.prepare_transaction(txn)

if (
txn.sender not in self.account_manager.test_accounts._impersonated_accounts
and txn.total_transfer_value > self.balance
):
raise AccountsError(
f"Transfer value meets or exceeds account balance "
f"for account '{self.address}' on chain '{self.provider.chain_id}' "
f"using provider '{self.provider.name}'.\n"
"Are you using the correct account / chain / provider combination?\n"
f"(transfer_value={txn.total_transfer_value}, balance={self.balance})."
)

return txn

def get_deployment_address(self, nonce: Optional[int] = None) -> AddressType:
"""
Get a contract address before it is deployed. This is useful
Expand Down
54 changes: 50 additions & 4 deletions src/ape/api/address.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from eth_pydantic_types import HexBytes

from ape.exceptions import ConversionError
from ape.exceptions import AccountsError, ConversionError
from ape.types.address import AddressType
from ape.types.units import CurrencyValue
from ape.utils.basemodel import BaseInterface
Expand Down Expand Up @@ -103,9 +103,17 @@ def __call__(self, **kwargs) -> "ReceiptAPI":
"""

txn = self.as_transaction(**kwargs)
if "sender" in kwargs and hasattr(kwargs["sender"], "call"):
sender = kwargs["sender"]
return sender.call(txn, **kwargs)
if "sender" in kwargs:
if hasattr(kwargs["sender"], "call"):
# AccountAPI
sender = kwargs["sender"]
return sender.call(txn, **kwargs)

elif hasattr(kwargs["sender"], "prepare_transaction"):
# BaseAddress (likely, a ContractInstance)
prepare_transaction = kwargs["sender"].prepare_transaction(txn)
return self.provider.send_transaction(prepare_transaction)

elif "sender" not in kwargs and self.account_manager.default_sender is not None:
return self.account_manager.default_sender.call(txn, **kwargs)

Expand Down Expand Up @@ -186,6 +194,44 @@ def estimate_gas_cost(self, **kwargs) -> int:
txn = self.as_transaction(**kwargs)
return self.provider.estimate_gas_cost(txn)

def prepare_transaction(self, txn: "TransactionAPI") -> "TransactionAPI":
"""
Set default values on a transaction.
Raises:
:class:`~ape.exceptions.AccountsError`: When the account cannot afford the transaction
or the nonce is invalid.
:class:`~ape.exceptions.TransactionError`: When given negative required confirmations.
Args:
txn (:class:`~ape.api.transactions.TransactionAPI`): The transaction to prepare.
Returns:
:class:`~ape.api.transactions.TransactionAPI`
"""

# NOTE: Allow overriding nonce, assume user understands what this does
if txn.nonce is None:
txn.nonce = self.nonce
elif txn.nonce < self.nonce:
raise AccountsError("Invalid nonce, will not publish.")

txn = self.provider.prepare_transaction(txn)

if (
txn.sender not in self.account_manager.test_accounts._impersonated_accounts
and txn.total_transfer_value > self.balance
):
raise AccountsError(
f"Transfer value meets or exceeds account balance "
f"for account '{self.address}' on chain '{self.provider.chain_id}' "
f"using provider '{self.provider.name}'.\n"
"Are you using the correct account / chain / provider combination?\n"
f"(transfer_value={txn.total_transfer_value}, balance={self.balance})."
)

return txn


class Address(BaseAddress):
"""
Expand Down
4 changes: 4 additions & 0 deletions src/ape/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class SignatureError(AccountsError):
Raised when there are issues with signing.
"""

def __init__(self, message: str, transaction: Optional["TransactionAPI"] = None):
self.transaction = transaction
super().__init__(message)


class ContractDataError(ApeException):
"""
Expand Down
5 changes: 3 additions & 2 deletions src/ape_ethereum/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def serialize_transaction(self) -> bytes:
"Did you forget to add the `sender=` kwarg to the transaction function call?"
)

raise SignatureError(message)
raise SignatureError(message, transaction=self)

txn_data = self.model_dump(by_alias=True, exclude={"sender", "type"})

Expand Down Expand Up @@ -107,7 +107,8 @@ def serialize_transaction(self) -> bytes:
recovered_signer = EthAccount.recover_transaction(signed_txn)
if recovered_signer != self.sender:
raise SignatureError(
f"Recovered signer '{recovered_signer}' doesn't match sender {self.sender}!"
f"Recovered signer '{recovered_signer}' doesn't match sender {self.sender}!",
transaction=self,
)

return signed_txn
Expand Down
2 changes: 1 addition & 1 deletion src/ape_test/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def sign_transaction(
) = sign_transaction_dict(private_key, tx_data)
except TypeError as err:
# Occurs when missing properties on the txn that are needed to sign.
raise SignatureError(str(err)) from err
raise SignatureError(str(err), transaction=txn) from err

# NOTE: Using `to_bytes(hexstr=to_hex(sig_r))` instead of `to_bytes(sig_r)` as
# a performance optimization.
Expand Down
18 changes: 18 additions & 0 deletions tests/functional/test_contract_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -1027,3 +1027,21 @@ def test_calldata_arg(calldata, expected, contract_instance, owner):
tx = contract_instance.functionWithCalldata(calldata, sender=owner)
assert not tx.failed
assert HexBytes(expected) in tx.data


def test_call(fallback_contract, owner):
"""
Fallback contract call test.
"""
tx = fallback_contract(sender=owner, value="1 wei")
assert not tx.failed


def test_call_contract_as_sender(fallback_contract, owner, vyper_contract_instance):
owner.transfer(fallback_contract, "1 ETH")
with pytest.raises(SignatureError) as info:
fallback_contract(sender=fallback_contract, value="1 wei")

transaction = info.value.transaction
assert transaction is not None
assert transaction.nonce is not None # Proves it was prepared.

0 comments on commit ced89ab

Please sign in to comment.