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: default call transactions would not get prepared when sender is a contract #2508

Merged
merged 4 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
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.
Loading