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.

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

Bugfix: AsyncWeb3.solidity_keccak no longer throws #3034

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Jul 10, 2023

What was wrong?

AsyncWeb3.solidity_keccak was raising TypeError: 'cytoolz.functoolz.curry' object is not iterable. The root was the normalize_values function on the BaseWeb3 class.

Closes #3006

How was it fixed?

Took the normalize_values function from the Web3 class and moved it up to the BaseWeb3 class so that both AsyncWeb3 and Web3 use the same function now. Also added a test to

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes changed the title Standardize normalize_values between Web3 and AsyncWeb3 classes Bugfix: AsyncWeb3.solidity_keccak no longer throws Jul 12, 2023
@kclowes kclowes marked this pull request as ready for review July 12, 2023 17:06
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm 👍🏼

It looks like a test_async_solidity_keccak_ens() passes when I add it in locally... but should we add tests for that too? Although I think we could benefit from an AsyncWeb3ModuleTest class altogether, which would be beyond the scope of this PR. Maybe we go that route instead?

@kclowes
Copy link
Collaborator Author

kclowes commented Jul 12, 2023

It looks like a test_async_solidity_keccak_ens() passes when I add it in locally... but should we add tests for that too?

Good call. I added a parametrization for Web3 and AsyncWeb3 for the ENS test, and will make an issue (or just a quick PR) to add a AsyncWeb3ModuleTest.

Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

@kclowes kclowes merged commit a3a7597 into ethereum:main Jul 12, 2023
1 check passed
@kclowes kclowes deleted the fix-solidity-keccak branch July 12, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Call to AsyncWeb3.solidity_keccak raises TypeError: 'cytoolz.functoolz.curry' object is not iterable
3 participants