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

Update methods to return type Hash32 where appropriate #192

Open
carver opened this issue May 14, 2020 · 2 comments
Open

Update methods to return type Hash32 where appropriate #192

carver opened this issue May 14, 2020 · 2 comments
Labels
breaking Breaking change. Before the next major release, consider going through these issues and merging. Good First Issue p3

Comments

@carver
Copy link
Collaborator

carver commented May 14, 2020

What was wrong?

A number of methods return a value from keccak which is guaranteed to be 32 bytes. For example, see:

def event_signature_to_log_topic(event_signature: str) -> bytes:
return keccak(text=event_signature.replace(" ", ""))
def event_abi_to_log_topic(event_abi: Dict[str, Any]) -> bytes:
event_signature = _abi_to_signature(event_abi)
return event_signature_to_log_topic(event_signature)

Other methods that require the 32-byte input type Hash32 currently require a cast when output from something like event_signature_to_log_topic() is already guaranteed.

How can it be fixed?

Update the return type signature to use Hash32 for all methods that apply keccak before returning.

@pratik-vii
Copy link

@carver as you mentioned in fix, it wont be true for the method function_signature_to_4byte_selector as it applies keccak before return but only returns 4-Bytes. Is my understanding correct or there is other reason to it.

@carver
Copy link
Collaborator Author

carver commented Oct 5, 2020

Ah yes, that's a typo. Thanks for pointing it out! 👍 I just fixed it above.

Note that I did not do a thorough search for all methods that should be updated, so part of the task is to identify all functions that should be updated.

@fselmo fselmo added the breaking Breaking change. Before the next major release, consider going through these issues and merging. label May 11, 2023
@reedsa reedsa added the p3 label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change. Before the next major release, consider going through these issues and merging. Good First Issue p3
Projects
None yet
Development

No branches or pull requests

4 participants