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

Clean up old eth abi references #2668

Merged
merged 3 commits into from
Oct 12, 2022

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Oct 6, 2022

What was wrong?

Related to Issue ethereum/eth-abi#161

How was it fixed?

  • Use abi.decode() over the deprecated abi.decode_single()
  • Since "encode" and "decode" are common across many libraries, import abi and use abi.encode() and abi.decode() so a reader can have more context when reading the code.

Todo:

Cute Animal Picture

20221006_210324

@fselmo fselmo force-pushed the clean-up-old-eth-abi-references branch 2 times, most recently from 64a2711 to ae49967 Compare October 6, 2022 16:31
@fselmo fselmo force-pushed the clean-up-old-eth-abi-references branch 3 times, most recently from 88416d2 to cee2275 Compare October 7, 2022 22:00
- With commonly used words like ``encode`` and ``decode``, it is better to import the library so a reader knows what exactly is encoding / decoding. Here, it is better to import ``abi`` and use ``abi.encode()`` / ``abi.decode()`` for better context.
@fselmo fselmo force-pushed the clean-up-old-eth-abi-references branch from cee2275 to 0b76cde Compare October 7, 2022 22:02
@fselmo fselmo requested review from kclowes and pacrob October 7, 2022 22:03
- This updates the ``towncrier`` types. Seems like we were missing a few.
@fselmo fselmo force-pushed the clean-up-old-eth-abi-references branch from ab89b2a to c1acffe Compare October 7, 2022 22:14
@fselmo fselmo marked this pull request as ready for review October 7, 2022 22:14
* `misc`
* `breaking-change`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kclowes any reason we'd want to keep these different for web3.py? I needed the internal and so I just copy / pasted from the template project. Makes things easier.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, just hadn't gotten around to changing it. Thanks!

@fselmo fselmo mentioned this pull request Oct 8, 2022
1 task
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

LGTM. I like the changes from encode to abi.encode!

* `misc`
* `breaking-change`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, just hadn't gotten around to changing it. Thanks!

@fselmo fselmo merged commit 56a7831 into ethereum:master Oct 12, 2022
@fselmo fselmo deleted the clean-up-old-eth-abi-references branch April 3, 2024 20:50
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.

2 participants