-
Notifications
You must be signed in to change notification settings - Fork 119
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
HOLO-1536 save init payload (WIP) #318
base: develop
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,218 @@ | |||
// SPDX-License-Identifier: MIT | |||
pragma solidity 0.8.13; |
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.
Nice. Thanks for adding this fixture. I think it'll be great if we can make all the tests relatively consistent
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.
Yes you are right, I'd like to improve/refactor the entire foundry test base to standardize everything and make it consistent.
But we'll have to wait until everything has been migrated from hardhat to foundry I think.
wdyt ?
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.
yeah probably. we can do what we can on the tests we're writing but we probably shouldn't touch what the Think and Dev team are done until they reach a good stopping point
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.
Nice. This looks good!
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.
Can this also be applied to the ERC20 contracts?
Describe Changes
I updated all the ERC721 sources contracts to make them store the payload used in their initialization.
For each source contracts
Note
The initPayload could be directly storaged ass a struct type for each contracts, but in the
CountdownERC721
and theCustomERC721
contracts, build failed with a stack too deep error in the constructors. So to create a "standard" in all contracts, I opted for a private storage variable of typebytes
and a getter function to decode the bytes. This also made it possible to have a getter function named according to each contract.Checklist before requesting a review