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

Consider making Upgradeable CoreMetadataModule #99

Merged
merged 6 commits into from
May 22, 2024

Conversation

Ramarti
Copy link

@Ramarti Ramarti commented Apr 15, 2024

The module itself doesn't have storage, but ipAccountStorage.setXXX will take the address of the caller as key.
If we want to change something from CoreMetadataModule by deploying another contract, all previous metadata would need to be migrated (either by the new module setting it all with a script, or each ipAccount migrating)

With a proxy, the address would be the same, so logic can be changed. This introduces the need for the user to trust us with the metadata, and weakens the promise of metadata freezing

@jdubpark
Copy link

This introduces the need for the user to trust us with the metadata

Code looks good, but this is something to consider since metadata can be made "immutable" (freeze). With the upgradeability, it's not completely immutable as code logic can change.

@Ramarti Ramarti changed the title Upgradeable CoreMetadataModule Consider making Upgradeable CoreMetadataModule Apr 15, 2024
@Ramarti
Copy link
Author

Ramarti commented Apr 15, 2024

Very good point @jdubpark , updating the title.
Making IPAccounts migrate the metadata is perfectly fine too

@LeoHChen
Copy link

can you rebase the code? Also, @kingster-will can you review?

@jdubpark
Copy link

After some thoughts, I think we should make it upgradeable since the rest of the codebase is also upgradeable. We can renounce upgradeability for this module down the road (freezing from our side)

@Ramarti Ramarti force-pushed the upgrade-coremetadata branch from 59b52d3 to f379a8c Compare April 26, 2024 20:51
@Ramarti Ramarti requested review from LeoHChen and jdubpark April 26, 2024 21:12
@kingster-will
Copy link

Please kindly also get review from @LeoHChen to confirm whether we need to make CoreMetadataModule upgradeable.

@LeoHChen LeoHChen merged commit b2b1e82 into storyprotocol:main May 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants