Refactor for duplicate code in contract internal implementations#3579
Conversation
darwintree
force-pushed
the
improve-contract
branch
from
11b15fa to
02d1a5e
Compare
darwintree
changed the title
darwintree
changed the title
reedsa
left a comment
reedsa
left a comment
There was a problem hiding this comment.
These updates are looking great, thanks for your contributions. I've mentioned a few changes to additional cleanup and suggested exception messages. Should be good to go after that!
| ] | ||
|
|
||
| # Check that arguments in call match a function ABI | ||
| num_attempts = 0 |
There was a problem hiding this comment.
I believe these num_attempts variables can be removed. They were mistakenly left in from a previous change.
| except ABIFunctionNotFound: | ||
| return False | ||
|
|
||
| def __iter__(self) -> Iterable[TContractFn]: |
There was a problem hiding this comment.
The corresponding function __iter__ in ContractFunction still exists and should be removed.
| for func in self._functions: | ||
| yield self[abi_to_signature(func)] | ||
|
|
||
| def __getattr__(self, function_name: str) -> TContractFn: |
There was a problem hiding this comment.
The corresponding function __getattr__ in ContractFunction still exists and should be removed.
| function_name, | ||
| ) | ||
|
|
||
| def __getitem__(self, function_name: str) -> TContractFn: |
There was a problem hiding this comment.
The corresponding function __getitem__ in ContractFunction still exists and should be removed.
| ccip_read_enabled: Optional[bool] = None, | ||
| ) -> Any: | ||
| """ | ||
| Should be implemented by child class. |
There was a problem hiding this comment.
| Should be implemented by child class. | |
| Implementation of ``call`` should create a callable contract function and execute it using the `eth_call` interface. |
| # mypy types | ||
| w3: "AsyncWeb3" | ||
|
|
||
| def __call__(self, *args: Any, **kwargs: Any) -> "AsyncContractEvent": |
There was a problem hiding this comment.
I'm inclined to leave this as is since the returned object is a new copy of the event.
| def __call__(self, *args: Any, **kwargs: Any) -> "AsyncContractEvent": | |
| def __call__(self, *args: Any, **kwargs: Any) -> "AsyncContractEvent": |
There was a problem hiding this comment.
I think they are nearly the same and Self can keep consistency. Self is used to indicate type hints, but did not imply if it is self or a copy of it
There was a problem hiding this comment.
That's fine with me. Should this __call__ and the one in the ContractEvent class move to base_contract? If not, I noticed the ContractEvent.__call__ has the type "ContractEvent" rather than self, probably best to make them consistent.
There was a problem hiding this comment.
Done. Moved into BaseContractEvent
Co-authored-by: Stuart Reed <stucan@gmail.com>
Co-authored-by: Stuart Reed <stucan@gmail.com>
|
@reedsa Thanks for reviewing. Changes already made except for |
| contract_function = None | ||
| for abi in function_abis_with_arg_count: | ||
| try: | ||
| pass |
|
@reedsa I think the above mentioned issues are resolved now |
3 participants
What was wrong?
Closes #3561
In internal contract implementations, there might be duplicate implementations repeating same code for async and non-async functions, for example,
ContractFunctionsandAsyncContractFunctions.How was it fixed?
This case is because of:
As for previous one, they can be resolved using self.class to other techiniques to dynamically get the right class.
As for latter one,
Selfcombined withGenericwould help solve the issueTodo:
Cute Animal Picture