From 64bfb004e4bc38e682959c5979892f59acf934fd Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Mon, 23 Dec 2024 13:42:53 -0600 Subject: [PATCH 1/2] fix: handle arg mixup --- src/ape/api/accounts.py | 6 ++++++ tests/functional/test_accounts.py | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/ape/api/accounts.py b/src/ape/api/accounts.py index 6400a865b7..b449287b1f 100644 --- a/src/ape/api/accounts.py +++ b/src/ape/api/accounts.py @@ -220,6 +220,12 @@ def transfer( Returns: :class:`~ape.api.transactions.ReceiptAPI` """ + if isinstance(account, int): + raise TypeError( + "Cannot use integer-type for the `receiver` argument in the " + "`.transfer()` method (this protects against accidentally passing " + "the `value` as the `receiver`)." + ) receiver = self.conversion_manager.convert(account, AddressType) txn = self.provider.network.ecosystem.create_transaction( diff --git a/tests/functional/test_accounts.py b/tests/functional/test_accounts.py index 7fed67ca04..d0eea058a8 100644 --- a/tests/functional/test_accounts.py +++ b/tests/functional/test_accounts.py @@ -240,6 +240,22 @@ def test_transfer_value_of_0(sender, receiver): assert receiver.balance == initial_balance +def test_transfer_mixed_up_sender_and_value(sender, receiver): + """ + Testing the case where the user mixes up the argument order, + it should show a nicer error than it was previously, as this is + a common and easy mistake. + """ + expected = ( + r"Cannot use integer-type for the `receiver` " + r"argument in the `\.transfer\(\)` method \(this " + r"protects against accidentally passing the " + r"`value` as the `receiver`\)." + ) + with pytest.raises(TypeError, match=expected): + sender.transfer(123, receiver) + + def test_deploy(owner, contract_container, clean_contract_caches): contract = owner.deploy(contract_container, 0) assert contract.address From 077f0649ee4b1dd9dbdb800f07dc491d1a332de7 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Mon, 23 Dec 2024 13:46:10 -0600 Subject: [PATCH 2/2] fix: improve and handle errors in transfer mixup arg --- src/ape/api/accounts.py | 9 +++++++-- tests/functional/test_accounts.py | 7 ++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/ape/api/accounts.py b/src/ape/api/accounts.py index b449287b1f..223e9c3aaf 100644 --- a/src/ape/api/accounts.py +++ b/src/ape/api/accounts.py @@ -18,6 +18,7 @@ from ape.exceptions import ( AccountsError, AliasAlreadyInUseError, + ConversionError, MethodNonPayableError, MissingDeploymentBytecodeError, SignatureError, @@ -221,13 +222,17 @@ def transfer( :class:`~ape.api.transactions.ReceiptAPI` """ if isinstance(account, int): - raise TypeError( + raise AccountsError( "Cannot use integer-type for the `receiver` argument in the " "`.transfer()` method (this protects against accidentally passing " "the `value` as the `receiver`)." ) - receiver = self.conversion_manager.convert(account, AddressType) + try: + receiver = self.conversion_manager.convert(account, AddressType) + except ConversionError as err: + raise AccountsError(f"Invalid `receiver` value: '{account}'.") from err + txn = self.provider.network.ecosystem.create_transaction( sender=self.address, receiver=receiver, **kwargs ) diff --git a/tests/functional/test_accounts.py b/tests/functional/test_accounts.py index d0eea058a8..9f330de27f 100644 --- a/tests/functional/test_accounts.py +++ b/tests/functional/test_accounts.py @@ -252,9 +252,14 @@ def test_transfer_mixed_up_sender_and_value(sender, receiver): r"protects against accidentally passing the " r"`value` as the `receiver`\)." ) - with pytest.raises(TypeError, match=expected): + with pytest.raises(AccountsError, match=expected): sender.transfer(123, receiver) + # Similarly show using currency-str (may fail for different error). + expected = r"Invalid `receiver` value: '123 wei'\." + with pytest.raises(AccountsError, match=expected): + sender.transfer("123 wei", receiver) + def test_deploy(owner, contract_container, clean_contract_caches): contract = owner.deploy(contract_container, 0)