chore: migrate GPv2Signing domain separator tests to Foundry #202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mfw78
reviewed
Aug 12, 2024
Comment on lines
+133
to
+134
| keccak256(bytes(domain.name)), | ||
| keccak256(bytes(domain.version)), |
There was a problem hiding this comment.
nit: is there any explicit need for bytes wrapping?
There was a problem hiding this comment.
I was also surprised but the wrapping is indeed needed:
Error (7556): Invalid type for argument in function call. Invalid implicit conversion from string memory to bytes memory requested. This function requires a single bytes argument. Use abi.encodePacked(...) to obtain the pre-0.5.0 behaviour or abi.encode(...) to use ABI encoding.
By the way, the compiler's suggestion is quite off the mark here, even if I see where it's coming from.
mfw78
approved these changes
Aug 12, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Starts migrating the GPv2Signing tests.
Some unused helper functions have been removed.
Test Plan
CI.
Related Issues
#120