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

web3.datastructures.AttributeDict.__hash__ fails if event logs contain list objects #2908

Closed
JustCallMeDavid opened this issue Apr 10, 2023 · 3 comments · Fixed by #2939
Closed

Comments

@JustCallMeDavid
Copy link
Contributor

JustCallMeDavid commented Apr 10, 2023

  • Version: 5.31.1
  • Python: 3.8
  • OS: linux
  • pip freeze output omitted since not a package specific issue

What was wrong?

Note: Not exactly an issue but something I had to circumvent myself when using the library and thinking it may be helpful if web3.py handled it instead directly.

I was trying to use functions requiring __hash__ on event logs and the code failed saying lists are not hashable here. This happens if the event logs contain list objects nested inside them (which some smart contracts seem to return), as lists by default are mutable and thus not hashable in Python.

How can it be fixed?

Would it make sense to (possibly recursively) cast any present lists to tuples inside the hash function to avoid crashes? Would some detection mechanism inside these pseudo-immutable classes be helpful to make sure they are indeed hashable (e.g., raising an error if a list is added)?

Any thoughts?

@pacrob
Copy link
Contributor

pacrob commented Apr 11, 2023

I think that since it's already casting it to a tuple, recursively tupelizing the whole thing would be fine. We'd also need to preserve the sorting.

If you have the time to put in a PR, that would be great! Otherwise I'll take further look later.

@JustCallMeDavid
Copy link
Contributor Author

Thank you @pacrob, I agree.

Was thinking of adding an additional class simulating immutable lists similar to the already present AttributeDict logic, but I believe that would bring unneeded complexity and little practical benefit. I think the simplest approach would be to leave the lists as is, but only convert them recursively to tuples inside the hash checks, with preserved sorting, so that those will not crash anymore.
If you have any thoughts feel free to let me know, otherwise I would proceed with implementation.

Give me some time (~2 weeks), I will put in a small PR.

@JustCallMeDavid
Copy link
Contributor Author

Hi @pacrob, created the PR here. Please feel free to have a look.

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 a pull request may close this issue.

2 participants