Contract caller #1157

kclowes · 2018-12-11T00:00:02Z

What was wrong?

Want to deprecate ConciseContract and ImplicitContract in favor of a ContractCaller. There are more details in the issue linked below.

Todo:

better testing
ENS module breakage
Update docs

Related to Issue #
#1025

How was it fixed?

Added a new ContractCaller class and added deprecation methods to ConciseContract and ImplicitContract.

Cute Animal Picture

carver

Just some early thoughts, and another API improvement possibility.

web3/contract.py

carver · 2018-12-11T00:23:01Z

web3/contract.py

+
+    def __call__(self, *args, **kwargs):
+        # TODO: Remove this and consolidate into ContractCaller.
+        # I don't know why this is working but it is.


🤣 (I've been there)

web3/contract.py

carver · 2018-12-11T00:35:30Z

tests/core/contracts/test_contract_call_interface.py

+
+
+def test_caller_with_parens_and_transaction_dict(math_contract):
+    result = math_contract.caller({'from': 'notarealaddress.eth'}).add(2, 3)


Another potentially interesting API would be to support sending these transaction fields in by keyword. from is problematic since it's a keyword, but we could go the pyethereum route and use sender.

Here that would mean:

math_contract.caller(sender='notarealaddress.eth').add(2, 3)

which is kind of nice.

Sounds good to me. So we would support both the sender kwarg and a transaction dict with from as a key, right?

@carver I'd like to introduce that in a subsequent PR since it would make sense to make it available globally.

web3.eth.sendTransaction(sender=..., gas=...) contract.functions.transfer(...).transact(sender=...) contract.caller(sender=...).add(...)

It'd be nice to know that the from address was properly set. For this reason, maybe we should test this using a different contract that had a method similar to.

contract CallerTester { function returnMeta() returns (address, bytes, uint256, uint256, uint256) { return (msg.sender, msg.data, msg.gasPrice, msg.value, gasLeft()) } }

This would allow you to test at least 4 of the transaction parameters (with gasLeft() only allowing you to do a approximate test for gas allowance).

carver · 2018-12-11T00:36:28Z

web3/contract.py

+
+
+class ContractCaller:
+    def __init__(self, abi, web3, address, *args, **kwargs):


Instead of *args, **kwargs I think it would be sufficient to have a single transaction_dict=None.

I was thinking that you could optionally pass in a plain old dictionary without the transaction keyword, in which case I'd need to keep *args. Here is an example: https://web3py.readthedocs.io/en/stable/contracts.html#web3.contract.ContractFunction.call. Am I reading those docs right or does a transaction dict always need the transaction keyword in front of it?

I can't dive in right this second to make sure it works, but what I was thinking in general is that you can use the fact that positional args can fill in keyword args, like:

def fn(kw1=None, **kwargs): return kw1 assert fn(1) == 1

carver · 2018-12-11T00:39:48Z

web3/contract.py

+                setattr(self, func['name'], caller_method)
+
+    def __call__(self, *args, **kwargs):
+        return type(self)(self.abi, self.web3, self.address, *args, **kwargs)


This is where the kwargs could be consolidated into transaction_dict. I'm not sure what args would be here, besides the transaction dict.

Something like this could work:

def __call__(self, transaction_dict=None, **kwargs): assert transaction_dict is None or len(kwargs) == 0 # but not actually an assert return type(self)(self.abi, self.web3, self.address, transaction_dict=(transaction_dict or kwargs))

IIUC the **kwargs portion is to support keyword based transaction parameters? If so my previous comment stands about holding off on introducing this option into the API.

It looks like you can use keyword args for the transaction parameters and block_identifier

I'm not sure I'm keen on having the block_identifier be something that you set once in the caller and keep around. At the very least, it makes me less excited about the earlier idea (break up the transaction dict into keywords), because then you have a mess of transaction fields and other stuff like the block_identifier.

But... if we don't have the block_identifier in the caller, then is it just impossible to set the block identifier using the ContractCaller? (That might be okay, maybe the "long form" call is required to do that kind of request)

tests/core/contracts/test_contract_call_interface.py

pipermerriam · 2019-01-14T17:54:34Z

tests/core/contracts/test_contract_call_interface.py

+
+
+def test_caller_with_parens_and_transaction_dict(math_contract):
+    result = math_contract.caller({'from': 'notarealaddress.eth'}).add(2, 3)


It'd be nice to know that the from address was properly set. For this reason, maybe we should test this using a different contract that had a method similar to.

contract CallerTester { function returnMeta() returns (address, bytes, uint256, uint256, uint256) { return (msg.sender, msg.data, msg.gasPrice, msg.value, gasLeft()) } }

This would allow you to test at least 4 of the transaction parameters (with gasLeft() only allowing you to do a approximate test for gas allowance).

tests/core/contracts/test_contract_call_interface.py

tests/ens/test_setup_address.py

web3/contract.py

pipermerriam · 2019-01-24T16:36:58Z

tests/core/contracts/conftest.py

@@ -527,6 +527,52 @@ def FallballFunctionContract(web3, FALLBACK_FUNCTION_CONTRACT):
    return web3.eth.contract(**FALLBACK_FUNCTION_CONTRACT)


+CONTRACT_RETURN_ARGS_SOURCE = """
+contract CallerTester {
+  function returnMeta() public payable returns (address, bytes memory, uint256, uint256) {


you might return block number too which would make it easy to test calls at specific block numbers.

rename transaction_dict for consistency

Use shorthand syntax for

Test a contract with no_abi

…ller

kclowes · 2019-01-25T19:42:18Z

@pipermerriam @carver I think this is pretty close if you want to give it another once-over 🙏. The only outstanding question is: do we want to allow block_identifier to be passed in as a kwarg? I don't have a strong opinion. The functionality is there now, but I'm happy to take it out if someone has strong feelings. Also, let me know if you have any feedback on the docs. Thanks!

carver · 2019-01-25T22:00:05Z

Can you rebase away the merge from master so those changes don't show up in the diff?

kclowes · 2019-01-30T22:49:19Z

@carver Since this branch was open so long and a bunch of people were touching the same files, git history got really ugly. I couldn't figure out a graceful and somewhat quick way to rebase away the merge from master. So, I grabbed the relevant commits from here and opened a new branch off of the current master – see #1227. I'm much happier with how the history looks over there. Closing this PR in favor of #1227.

pipermerriam · 2019-01-30T23:04:11Z

Lemme know if you'd like a crash course in git rebasing

kclowes · 2019-01-30T23:19:59Z

I'd love a crash course in git rebasing 😆 It occurred to me while I was trying to rebase this PR that I actually need to learn about what it does, rather than guessing like I've done in the past.

carver reviewed Dec 11, 2018

View reviewed changes

kclowes force-pushed the contract-caller branch from 64e95bb to a25abbd Compare January 12, 2019 17:55

pipermerriam reviewed Jan 14, 2019

View reviewed changes

carver reviewed Jan 15, 2019

View reviewed changes

web3/contract.py Outdated Show resolved Hide resolved

pipermerriam approved these changes Jan 24, 2019

View reviewed changes

kclowes and others added 24 commits January 24, 2019 13:52

First pass on Contract Caller

e4586bd

ContractReader works with parentheses option

e25f3ad

Allow reader to take a transaction_dict

f5042ee

Change all 'reader' references to 'caller'

f3cc8cb

Change 'reader' to 'caller' in the tests

918b4ba

Add test to make sure a contract without an ABI doesn't return an error

4f12850

Add deprecation decorators to Concise and Implicit Contract

b00e1a1

Change deprecation message

f8c75bb

Get rid of ContractMethod, raise error if no ABI

9433e66

Pass around args in a way that makes tests pass

6e4b6c1

Change ImplicitContract deprecation method,

65f02d0

rename transaction_dict for consistency

Tests passing for ENS setup_name

a4bde5b

ENS tests passing

0c2f8fb

Fix linting

e8d1824

ConciseContract tests passing

ceee3c8

Allow no ABI until a function is called

bd38eb2

Use shorthand syntax for

Rename middleware_stack to middleware_onion

1f92ef1

Change CircleCI badge link to the CircleCI project

1fd2658

Fix syntax error

cd5cb32

Move ContractCaller tests to it's own file.

a8cf234

Test a contract with no_abi

Test that block_identifier is set correctly

07ae1d9

Can pass transaction_dict with or without keyword

9d80d27

Test that address gets set correctly

95dc88e

Whitespace

1b3c129

kclowes force-pushed the contract-caller branch from 369c593 to 1b3c129 Compare January 24, 2019 20:54

kclowes added 5 commits January 24, 2019 14:52

Can pass transaction_dict with or without keyword

02b42d1

Add block number to CallerTester contract

17b3749

Fix merge conflicts

7047329

Merge branch 'master' of github.com:ethereum/web3.py into contract-ca…

6415cdd

…ller

Remove unneeded *args and **kwargs

0185da9

kclowes changed the title ~~[WIP] Contract caller~~ Contract caller Jan 24, 2019

kclowes added 2 commits January 25, 2019 11:44

Refactor out none_or_zero_address function

497a2eb

Add documentation to ContractCaller Class

d8e05a7

kclowes mentioned this pull request Jan 30, 2019

Contract Caller Cleaned up #1227

Merged

kclowes closed this Jan 30, 2019

kclowes deleted the contract-caller branch February 21, 2019 23:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contract caller #1157

Contract caller #1157

kclowes commented Dec 11, 2018 •

edited

Loading

carver left a comment

carver Dec 11, 2018 •

edited

Loading

carver Dec 11, 2018

kclowes Dec 12, 2018

pipermerriam Dec 12, 2018

pipermerriam Jan 14, 2019

carver Dec 11, 2018

kclowes Dec 12, 2018

carver Dec 13, 2018

carver Dec 11, 2018

pipermerriam Dec 12, 2018

kclowes Dec 12, 2018

carver Dec 13, 2018

pipermerriam Jan 14, 2019

pipermerriam Jan 24, 2019

kclowes commented Jan 25, 2019

carver commented Jan 25, 2019

kclowes commented Jan 30, 2019

pipermerriam commented Jan 30, 2019

kclowes commented Jan 30, 2019



		def test_caller_with_parens_and_transaction_dict(math_contract):
		result = math_contract.caller({'from': 'notarealaddress.eth'}).add(2, 3)



		class ContractCaller:
		def __init__(self, abi, web3, address, args, *kwargs):

Contract caller #1157

Contract caller #1157

Conversation

kclowes commented Dec 11, 2018 • edited Loading

What was wrong?

How was it fixed?

Cute Animal Picture

carver left a comment

Choose a reason for hiding this comment

carver Dec 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kclowes commented Jan 25, 2019

carver commented Jan 25, 2019

kclowes commented Jan 30, 2019

pipermerriam commented Jan 30, 2019

kclowes commented Jan 30, 2019

kclowes commented Dec 11, 2018 •

edited

Loading

carver Dec 11, 2018 •

edited

Loading