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

Fix process_log for HexStr inputs #3293

Merged
merged 3 commits into from Mar 26, 2024
Merged

Conversation

reedsa
Copy link
Contributor

@reedsa reedsa commented Mar 18, 2024

What was wrong?

Related to Issue #3286

Realized the topics and data values had not been explicitly converted to bytes. The initial fix I made was to convert the log topics to a HexStr but since bytes are needed later on, it made sense to just ensure everything is coerced to bytes before handling them.

How was it fixed?

Explicit type coercion of entry topics and data values.

Todo:

Cute Animal Picture

Screen Shot 2024-03-18 at 12 24 54 PM

reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 18, 2024
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 18, 2024
@reedsa reedsa marked this pull request as ready for review March 18, 2024 19:29
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.

Just one nit which adds a bit of unnecessary overhead, wrapping with @curry where we don't need to. Looks good otherwise 👍🏼

web3/_utils/events.py Outdated Show resolved Hide resolved
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.

Agree with the @curry nit, otherwise lgtm!

`HexStr` values now converted to bytes for processing
reedsa added a commit to reedsa/web3.py that referenced this pull request Mar 25, 2024
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!

@reedsa reedsa merged commit 653c536 into ethereum:main Mar 26, 2024
81 checks passed
@reedsa reedsa deleted the event-data-coercion-v7 branch March 26, 2024 15:03
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.

None yet

3 participants