-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: include shape in merkle contracts #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feedback below
Co-authored-by: Paul Razvan Berg <prberg@proton.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally LGTM 👍
refactor: rename var to campaignName refactor: store the campaign name as string test: update tests accordingly
pushed a commit to address the feedback, lmk if it looks good @smol-ninja |
Yes. Thanks for the commit. Looks good to me. Can you please approve the PR if there are no more suggestions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #1.
Changelog:
salt
by encodingbaseParams
instead encoding parameters separately. This helps to prevent "stack too deep error" caused after addingshape
inkeccak256(abi.encodePacked(..))
of salt.asset
totoken
as a result of Refactor asset to token v2-core#1097.shape
of typestring
. Instead of reverting on longer than 32 chars length, it truncates the name to 32 chars.NAME
toCAMPAIGN_NAME
inDefaults
contract.@andreivladbrg the idea of storing shape as
bytes32
did not work because when the string of length less than 32 characters is stored asbytes32
, it adds0
to make it fit into 32 bytes slot which changes the length of the string upon conversion back to string. On the other hand, storing it as a string datatype preserves the string length as well in the storage. Therefore I decided to store it as a string and truncates its length if the length exceeds 32 characters.