-
Notifications
You must be signed in to change notification settings - Fork 66
Task 1183 refactor identity contract #127
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
Task 1183 refactor identity contract #127
Conversation
pgonday
left a comment
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.
With the storage update, when a contract is updated and already have claims or keys, the indexes storage variables will have zero value which will lead to an invalid state...
Add a function like initializeUpgradeV2 with keys and claims array parameters (get offchain from events for example) which will initialize the indexes maps.
agreed with Philippe, we can probably make it a function with no access control, so that we can run a script ourselves on all existing identities no matter if we have management key or not, to make the upgrade clean. Once it is done we can upgrade the contracts again, with a clean version of the contracts that doesn't contain the upgrade-related function |
Bug in IdentityUtilities
contracts/Identity.sol
Outdated
| * @return The version string | ||
| */ | ||
| function version() external view returns (string memory) { | ||
| return _getClaimStorage().version; |
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.
please hardcode, otherwise you need to update the storage on each proxy individually when the version is upgraded, it just doesn't scale if you add the value in storage.
This feature includes refactoring:
Identity contract includes key management and claim management systems.
I created Key Manager and moved the key management functionality from Identity to Key Manager, the the Identity inherits the Key Manager as well.
Applied the ERC7201 for Identity and claim issuer
Applied the 1967 proxy pattern to the claim issuer