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.

Sign up for GitHub

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

raise ValidationError if ether is sent to non payable functions #1017

Merged
merged 1 commit into from
Sep 19, 2018
Merged

raise ValidationError if ether is sent to non payable functions #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 71 additions & 23 deletions tests/core/contracts/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,51 +387,52 @@ def ArraysContract(web3, ARRAYS_CONTRACT):
return web3.eth.contract(**ARRAYS_CONTRACT)


CONTRACT_FALLBACK_FUNCTION_SOURCE = """
contract A {
uint data;
function A() public payable { data = 0; }
function getData() returns (uint r) { return data; }
function() { data = 1; }
CONTRACT_PAYABLE_TESTER_SOURCE = """
contract PayableTester {
bool public wasCalled;

function doNoValueCall() public {
wasCalled = true;
}
}
"""

CONTRACT_FALLBACK_FUNCTION_CODE = "60606040526000808190555060ae806100196000396000f300606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680633bc5de30146053575b3415604957600080fd5b6001600081905550005b3415605d57600080fd5b60636079565b6040518082815260200191505060405180910390f35b600080549050905600a165627a7a72305820045439389e4742569ec078687e6a0c81997709778a0097adbe07ccfd9f7b1a330029" # noqa: E501
CONTRACT_PAYABLE_TESTER_CODE = "608060405234801561001057600080fd5b5060e88061001f6000396000f3006080604052600436106049576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063c680362214604e578063e4cb8f5c14607a575b600080fd5b348015605957600080fd5b506060608e565b604051808215151515815260200191505060405180910390f35b348015608557600080fd5b50608c60a0565b005b6000809054906101000a900460ff1681565b60016000806101000a81548160ff0219169083151502179055505600a165627a7a723058205362c7376eda918b0dc3a75d0ffab904a241c9b10b68d5268af6ca405242303e0029" # noqa: E501

CONTRACT_FALLBACK_FUNCTION_RUNTIME = "606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680633bc5de30146053575b3415604957600080fd5b6001600081905550005b3415605d57600080fd5b60636079565b6040518082815260200191505060405180910390f35b600080549050905600a165627a7a72305820045439389e4742569ec078687e6a0c81997709778a0097adbe07ccfd9f7b1a330029" # noqa: E501
CONTRACT_PAYABLE_TESTER_RUNTIME = "6080604052600436106049576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063c680362214604e578063e4cb8f5c14607a575b600080fd5b348015605957600080fd5b506060608e565b604051808215151515815260200191505060405180910390f35b348015608557600080fd5b50608c60a0565b005b6000809054906101000a900460ff1681565b60016000806101000a81548160ff0219169083151502179055505600a165627a7a723058205362c7376eda918b0dc3a75d0ffab904a241c9b10b68d5268af6ca405242303e0029" # noqa: E501

CONTRACT_FALLBACK_FUNCTION_ABI = json.loads('[{"constant": false, "inputs": [], "name": "getData", "outputs": [{"name": "r", "type": "uint256"}], "payable": false, "stateMutability": "nonpayable", "type": "function"}, {"inputs": [], "payable": true, "stateMutability": "payable", "type": "constructor"}, {"payable": false, "stateMutability": "nonpayable", "type": "fallback"}]') # noqa: E501
CONTRACT_PAYABLE_TESTER_ABI = json.loads('[{"constant": true, "inputs": [], "name": "wasCalled", "outputs": [{"name": "", "type": "bool"}], "payable": false, "stateMutability": "view", "type": "function"}, {"constant": false, "inputs": [], "name": "doNoValueCall", "outputs": [], "payable": false, "stateMutability": "nonpayable", "type": "function"}]') # noqa: E501


@pytest.fixture()
def FALLBACK_FUNCTION_CODE():
return CONTRACT_FALLBACK_FUNCTION_CODE
def PAYABLE_TESTER_CODE():
return CONTRACT_PAYABLE_TESTER_CODE


@pytest.fixture()
def FALLBACK_FUNCTION_RUNTIME():
return CONTRACT_FALLBACK_FUNCTION_RUNTIME
def PAYABLE_TESTER_RUNTIME():
return CONTRACT_PAYABLE_TESTER_RUNTIME


@pytest.fixture()
def FALLBACK_FUNCTION_ABI():
return CONTRACT_FALLBACK_FUNCTION_ABI
def PAYABLE_TESTER_ABI():
return CONTRACT_PAYABLE_TESTER_ABI


@pytest.fixture()
def FALLBACK_FUNCTION_CONTRACT(FALLBACK_FUNCTION_CODE,
FALLBACK_FUNCTION_RUNTIME,
FALLBACK_FUNCTION_ABI):
def PAYABLE_TESTER_CONTRACT(PAYABLE_TESTER_CODE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @njgheorghita I was looking at this stuff and it might be a good test-bed for seeing how py-ethpm and maybe pytest-ethereum could clean up these test fixtures.

PAYABLE_TESTER_RUNTIME,
PAYABLE_TESTER_ABI):
return {
'bytecode': FALLBACK_FUNCTION_CODE,
'bytecode_runtime': FALLBACK_FUNCTION_RUNTIME,
'abi': FALLBACK_FUNCTION_ABI,
'bytecode': PAYABLE_TESTER_CODE,
'bytecode_runtime': PAYABLE_TESTER_RUNTIME,
'abi': PAYABLE_TESTER_ABI,
}


@pytest.fixture()
def FallballFunctionContract(web3, FALLBACK_FUNCTION_CONTRACT):
return web3.eth.contract(**FALLBACK_FUNCTION_CONTRACT)
def PayableTesterContract(web3, PAYABLE_TESTER_CONTRACT):
return web3.eth.contract(**PAYABLE_TESTER_CONTRACT)


# no matter the function selector, this will return back the 32 bytes of data supplied
Expand Down Expand Up @@ -479,6 +480,53 @@ def FixedReflectionContract(web3):
return web3.eth.contract(abi=CONTRACT_FIXED_ABI, bytecode=CONTRACT_REFLECTION_CODE)


CONTRACT_FALLBACK_FUNCTION_SOURCE = """
contract A {
uint data;
function A() public payable { data = 0; }
function getData() returns (uint r) { return data; }
function() { data = 1; }
}
"""

CONTRACT_FALLBACK_FUNCTION_CODE = "60606040526000808190555060ae806100196000396000f300606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680633bc5de30146053575b3415604957600080fd5b6001600081905550005b3415605d57600080fd5b60636079565b6040518082815260200191505060405180910390f35b600080549050905600a165627a7a72305820045439389e4742569ec078687e6a0c81997709778a0097adbe07ccfd9f7b1a330029" # noqa: E501

CONTRACT_FALLBACK_FUNCTION_RUNTIME = "606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680633bc5de30146053575b3415604957600080fd5b6001600081905550005b3415605d57600080fd5b60636079565b6040518082815260200191505060405180910390f35b600080549050905600a165627a7a72305820045439389e4742569ec078687e6a0c81997709778a0097adbe07ccfd9f7b1a330029" # noqa: E501

CONTRACT_FALLBACK_FUNCTION_ABI = json.loads('[{"constant": false, "inputs": [], "name": "getData", "outputs": [{"name": "r", "type": "uint256"}], "payable": false, "stateMutability": "nonpayable", "type": "function"}, {"inputs": [], "payable": true, "stateMutability": "payable", "type": "constructor"}, {"payable": false, "stateMutability": "nonpayable", "type": "fallback"}]') # noqa: E501


@pytest.fixture()
def FALLBACK_FUNCTION_CODE():
return CONTRACT_FALLBACK_FUNCTION_CODE


@pytest.fixture()
def FALLBACK_FUNCTION_RUNTIME():
return CONTRACT_FALLBACK_FUNCTION_RUNTIME


@pytest.fixture()
def FALLBACK_FUNCTION_ABI():
return CONTRACT_FALLBACK_FUNCTION_ABI


@pytest.fixture()
def FALLBACK_FUNCTION_CONTRACT(FALLBACK_FUNCTION_CODE,
FALLBACK_FUNCTION_RUNTIME,
FALLBACK_FUNCTION_ABI):
return {
'bytecode': FALLBACK_FUNCTION_CODE,
'bytecode_runtime': FALLBACK_FUNCTION_RUNTIME,
'abi': FALLBACK_FUNCTION_ABI,
}


@pytest.fixture()
def FallballFunctionContract(web3, FALLBACK_FUNCTION_CONTRACT):
return web3.eth.contract(**FALLBACK_FUNCTION_CONTRACT)


class LogFunctions:
LogAnonymous = 0
LogNoArguments = 1
Expand Down
39 changes: 39 additions & 0 deletions tests/core/contracts/test_contract_buildTransaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from web3._utils.toolz import (
dissoc,
)
from web3.exceptions import (
ValidationError,
)

# Ignore warning in pyethereum 1.6 - will go away with the upgrade
pytestmark = pytest.mark.filterwarnings("ignore:implicit cast from 'char *'")
Expand Down Expand Up @@ -32,6 +35,42 @@ def fallback_function_contract(web3, FallballFunctionContract, address_conversio
return _fallback_contract


@pytest.fixture()
def payable_tester_contract(web3, PayableTesterContract, address_conversion_func):
deploy_txn = PayableTesterContract.constructor().transact()
deploy_receipt = web3.eth.waitForTransactionReceipt(deploy_txn)
assert deploy_receipt is not None
payable_tester_address = address_conversion_func(deploy_receipt['contractAddress'])
_payable_tester = PayableTesterContract(address=payable_tester_address)
assert _payable_tester.address == payable_tester_address
return _payable_tester


def test_build_transaction_not_paying_to_nonpayable_function(
web3,
payable_tester_contract,
buildTransaction):
txn = buildTransaction(contract=payable_tester_contract,
contract_function='doNoValueCall')
assert dissoc(txn, 'gas') == {
'to': payable_tester_contract.address,
'data': '0xe4cb8f5c',
'value': 0,
'gasPrice': 1,
'chainId': None,
}


def test_build_transaction_paying_to_nonpayable_function(
web3,
payable_tester_contract,
buildTransaction):
with pytest.raises(ValidationError):
buildTransaction(contract=payable_tester_contract,
contract_function='doNoValueCall',
tx_params={'value': 1})


def test_build_transaction_with_contract_no_arguments(web3, math_contract, buildTransaction):
txn = buildTransaction(contract=math_contract, contract_function='increment')
assert dissoc(txn, 'gas') == {
Expand Down
18 changes: 18 additions & 0 deletions tests/core/contracts/test_contract_call_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ def fixed_reflection_contract(web3, FixedReflectionContract, address_conversion_
return deploy(web3, FixedReflectionContract, address_conversion_func)


@pytest.fixture()
def payable_tester_contract(web3, PayableTesterContract, address_conversion_func):
return deploy(web3, PayableTesterContract, address_conversion_func)


@pytest.fixture()
def call_transaction():
return {
Expand Down Expand Up @@ -532,6 +537,19 @@ def test_call_abi_no_functions(web3):
contract.functions.thisFunctionDoesNotExist().call()


def test_call_not_sending_ether_to_nonpayable_function(payable_tester_contract, call):
result = call(contract=payable_tester_contract,
contract_function='doNoValueCall')
assert result == []


def test_call_sending_ether_to_nonpayable_function(payable_tester_contract, call):
with pytest.raises(ValidationError):
call(contract=payable_tester_contract,
contract_function='doNoValueCall',
tx_params={'value': 1})


@pytest.mark.parametrize(
'function, value',
(
Expand Down
45 changes: 45 additions & 0 deletions tests/core/contracts/test_contract_estimateGas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import pytest

from web3.exceptions import (
ValidationError,
)


@pytest.fixture(autouse=True)
def wait_for_first_block(web3, wait_for_block):
Expand Down Expand Up @@ -54,6 +58,19 @@ def fallback_function_contract(web3,
return _fallback_function_contract


@pytest.fixture()
def payable_tester_contract(web3, PayableTesterContract, address_conversion_func):
deploy_txn = PayableTesterContract.constructor().transact({'from': web3.eth.coinbase})
deploy_receipt = web3.eth.waitForTransactionReceipt(deploy_txn)

assert deploy_receipt is not None
payable_tester_address = address_conversion_func(deploy_receipt['contractAddress'])

_payable_tester = PayableTesterContract(address=payable_tester_address)
assert _payable_tester.address == payable_tester_address
return _payable_tester


def test_contract_estimateGas(web3, math_contract, estimateGas, transact):
gas_estimate = estimateGas(contract=math_contract,
contract_function='increment')
Expand Down Expand Up @@ -92,3 +109,31 @@ def test_contract_estimateGas_with_arguments(web3, math_contract, estimateGas, t
gas_used = txn_receipt.get('gasUsed')

assert abs(gas_estimate - gas_used) < 21000


def test_estimateGas_not_sending_ether_to_nonpayable_function(
web3,
payable_tester_contract,
estimateGas,
transact):
gas_estimate = estimateGas(contract=payable_tester_contract,
contract_function='doNoValueCall')

txn_hash = transact(
contract=payable_tester_contract,
contract_function='doNoValueCall')

txn_receipt = web3.eth.waitForTransactionReceipt(txn_hash)
gas_used = txn_receipt.get('gasUsed')

assert abs(gas_estimate - gas_used) < 21000


def test_estimateGas_sending_ether_to_nonpayable_function(
web3,
payable_tester_contract,
estimateGas):
with pytest.raises(ValidationError):
estimateGas(contract=payable_tester_contract,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would focus on why this is failing. Many of the other functions will fail if this fails, because calls like buildTransaction will call estimateGas under the hood, if no gas limit is supplied (which is common).

Can you reproduce outside of testing, with payable_tester_contract.estimateGas(dict(value=1)).doNoValueCall()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could reproduce it. Will dig deeper into it.

>>> contract_instance.estimateGas(dict(value=1)).doNoValueCall()
API Exception: {'type': 'TransactionFailed', 'args': (), 'message': ''}
Traceback (most recent call last):
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/jsonrpc/manager.py", line 112, in _get_responses
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/server.py", line 34, in inner
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/rpc.py", line 138, in eth_estimateGas
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/client/client.py", line 253, in estimate_gas
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/client/client.py", line 263, in send_transaction
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/client/client.py", line 58, in inner
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/client/utils.py", line 104, in inner
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/testrpc/client/client.py", line 225, in _send_transaction
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/ethereum/tester.py", line 338, in send
  File "/Users/ankit/web3.py/venv/lib/python3.7/site-packages/ethereum/tester.py", line 296, in _send
ethereum.tester.TransactionFailed
127.0.0.1 - - [13/Sep/2018 08:08:42] "POST / HTTP/1.1" 200 147
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ankit/web3.py/web3/contract.py", line 1506, in estimate_gas_for_function
    gas_estimate = web3.eth.estimateGas(estimate_transaction)
  File "/Users/ankit/web3.py/web3/eth.py", line 304, in estimateGas
    [transaction],
  File "/Users/ankit/web3.py/web3/manager.py", line 112, in request_blocking
    raise ValueError(response["error"])
ValueError: {'code': -32000, 'message': 'Server error', 'data': {'type': 'TransactionFailed', 'args': [], 'message': ''}}

contract_function='doNoValueCall',
tx_params={'value': 1})
56 changes: 56 additions & 0 deletions tests/core/contracts/test_contract_transact_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
from web3._utils.empty import (
empty,
)
from web3.exceptions import (
ValidationError,
)

# Ignore warning in pyethereum 1.6 - will go away with the upgrade
pytestmark = pytest.mark.filterwarnings("ignore:implicit cast from 'char *'")
Expand Down Expand Up @@ -63,6 +66,17 @@ def arrays_contract(web3, ArraysContract, address_conversion_func):
return _arrays_contract


@pytest.fixture()
def payable_tester_contract(web3, PayableTesterContract, address_conversion_func):
deploy_txn = PayableTesterContract.constructor().transact()
deploy_receipt = web3.eth.waitForTransactionReceipt(deploy_txn)
assert deploy_receipt is not None
address = address_conversion_func(deploy_receipt['contractAddress'])
_payable_tester = PayableTesterContract(address=address)
assert _payable_tester.address == address
return _payable_tester


def test_transacting_with_contract_no_arguments(web3, math_contract, transact, call):
initial_value = call(contract=math_contract,
contract_function='counter')
Expand All @@ -78,6 +92,48 @@ def test_transacting_with_contract_no_arguments(web3, math_contract, transact, c
assert final_value - initial_value == 1


def test_transact_not_sending_ether_to_nonpayable_function(
web3,
payable_tester_contract,
transact,
call):
initial_value = call(contract=payable_tester_contract,
contract_function='wasCalled')

assert initial_value is False
txn_hash = transact(contract=payable_tester_contract,
contract_function='doNoValueCall')
txn_receipt = web3.eth.waitForTransactionReceipt(txn_hash)
assert txn_receipt is not None

final_value = call(contract=payable_tester_contract,
contract_function='wasCalled')

assert final_value is True


def test_transact_sending_ether_to_nonpayable_function(
web3,
payable_tester_contract,
transact,
call):
initial_value = call(contract=payable_tester_contract,
contract_function='wasCalled')

assert initial_value is False
with pytest.raises(ValidationError):
txn_hash = transact(contract=payable_tester_contract,
contract_function='doNoValueCall',
tx_params={'value': 1})
txn_receipt = web3.eth.waitForTransactionReceipt(txn_hash)
assert txn_receipt is not None

final_value = call(contract=payable_tester_contract,
contract_function='wasCalled')

assert final_value is False


@pytest.mark.parametrize(
'transact_args,transact_kwargs',
(
Expand Down
19 changes: 19 additions & 0 deletions web3/_utils/contracts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ def prepare_transaction(
TODO: make this a public API
TODO: add new prepare_deploy_transaction API
"""
if fn_abi is None:
fn_abi = find_matching_fn_abi(contract_abi, fn_identifier, fn_args, fn_kwargs)

validate_payable(transaction, fn_abi)

if transaction is None:
prepared_transaction = {}
else:
Expand Down Expand Up @@ -245,3 +250,17 @@ def get_function_info(fn_name, contract_abi=None, fn_abi=None, args=None, kwargs
fn_arguments = merge_args_and_kwargs(fn_abi, args, kwargs)

return fn_abi, fn_selector, fn_arguments


def validate_payable(transaction, abi):
"""Raise ValidationError if non-zero ether
is sent to a non payable function.
"""
if 'value' in transaction:
if transaction['value'] != 0:
if "payable" in abi and not abi["payable"]:
raise ValidationError(
"Sending non-zero ether to a contract function "
"with payable=False. Please ensure that "
"transaction's value is 0."
)
1 change: 1 addition & 0 deletions web3/contract.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,7 @@ def call(self, transaction=None, block_identifier='latest'):
raise ValueError(
"Please ensure that this contract instance has an address."
)

block_id = parse_block_identifier(self.web3, block_identifier)

return call_contract_function(
Expand Down