Web3.py: raise validation error if trying to send ether to a contract function with payable=False

Created on 13 Jun 2018 · 13Comments · Source: ethereum/web3.py

What was wrong?

When methods are marked with payable=False in the ABI, a transaction sending ether to them will (or should) fail. Web3 isn't doing anything to intercept these calls, leaving callers to debug the failing transaction.

How can it be fixed?

When running a call, transact, estimateGas, or buildTransaction, inspect the transaction's value field. If it is present, and non-0, check the function ABI. If payable is present and False, raise a validation error explaining that the user must not send ether to functions that are not payable.

Alternative approach: when searching for matching functions in the ABI, filter out options that are not payable if the transaction has non-0 value. Not sure that this really buys anything besides fitting into a framework we already have.

Good First Issue

Source

carver

Most helpful comment

Hi, I'm taking a look at this as a way to get started with issues.

dogweather on 23 Jun 2018

👍3

All 13 comments

Hi, I'm taking a look at this as a way to get started with issues.

dogweather on 23 Jun 2018

👍3

Update: I'm sorry about the delay. I'm setting up the development and booting it up in Docker Compose and jumping in.

dogweather on 2 Aug 2018

...and all tests pass. Nice to see.

dogweather on 2 Aug 2018

👍1

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

__This issue now has a funding of 165.0 DAI (165.0 USD @ $1.0/DAI) attached to it.__

If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $17,264.45 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot on 23 Aug 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

__Work has been started__.

These users each claimed they can complete the work by 11 months from now.
Please review their action plans below:

1) jong86 has applied to start work _(Funders only: approve worker | reject worker)_.

Hi I'm going to check this out.
Plan is:

find related code
write tests for this additional feature
ensure these tests pass

thanks

Learn more on the Gitcoin Issue Details page.

2) ankit-maverick has been approved to start work.

Raise ValidationError in transact, estimate_gas & build_transaction functions
Get the approach & code reviewed by @carver
Write appropriate tests for this change.
Get the PR merged after all everything is green from CircleCI

Learn more on the Gitcoin Issue Details page.

gitcoinbot on 23 Aug 2018

These users each claimed they can complete the work by 12 months from now.

This timeline seems unnecessarily long. If it is taking longer than 6-8 weeks, I would expect to pass it on to the next person who wants to take a shot. I think that's plenty.

carver on 23 Aug 2018

@carver I will take a shot at this. Should take less than a week.

ankit-maverick on 24 Aug 2018

@carver that timeline issue is on me... should have set a lower timeline when creating the issue. Accepting @ankit-maverick, thanks for providing a timeline.

vs77bb on 24 Aug 2018

👍2

@ankit-maverick Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!