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: handle when log address is not checksummed when decoding #2429

Conversation

antazoey
Copy link
Member

What I did

providers should not be expected to checksum addresses the user doesnt need to deal with yet
(for performance reasons)
raw log data seems acceptable to not require checksumming

How I did it

use lower for decoding logs match checks

How to verify it

my thing in ape-titanoboa works now
see test as well...

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

Copy link
Contributor

@NotPeopling2day NotPeopling2day left a comment

Choose a reason for hiding this comment

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

Sorry to muddy this, but are there any downstream implications associated with this change? As in, if you are attempting to pass this address to something else that is expecting checksummed? Considerations for informational logging or something?

@antazoey
Copy link
Member Author

Sorry to muddy this

Two things

  1. if the address is already checksummed, it won't matter - this doesn't modify the .logs on a receipt at all
  2. none of this lowering-of-address is seen or really touched by the user, it is just some internal handling when it comes to decoding the logs. Nothing saved

@antazoey antazoey enabled auto-merge (squash) December 20, 2024 14:58
@antazoey antazoey changed the title fix: handle when log address is not checksummed fix: handle when log address is not checksummed when decoding Dec 20, 2024
@antazoey antazoey merged commit 2f9d83f into ApeWorX:main Dec 20, 2024
19 checks passed
@antazoey antazoey deleted the fix/handle-non-checksummed-events-in-logs-decode branch December 20, 2024 15:55
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.

3 participants