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

Refactors to support global IP asset identification #129

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

leeren
Copy link
Contributor

@leeren leeren commented Oct 16, 2023

Closes #128

To support our migration to a global IP Asset registry, we start with a set of changes that does the following:

  • A huge renaming refactor to support new conventions around IPAssetOrgs (previously IPAssetRegistry), IPAssets, and an IPAssetOrgFactory (previously FranchiseRegistry)
  • Changes the existing FranchiseRegistry to IPAssetOrgFactory, which will be in charge of creation of future IPAssetGroups (previously named IPAssetRegistry)
  • Changes the existing IPAssetRegistry to IPAssetOrg, which will be in charge of authorization and creation of IP assets through a particular IPAssetGroup (previously referred to as franchise)
  • Creates a new IPAssetRegistry, which will act as the global registry / SoT for all IP assets (whether they are created through an IPAssetGroup or not)
  • Deprecates use of BeaconProxies in the prior IPAssetRegistries (now IPAssetOrg) in favor of immutable clones

This PR has intentionally not yet decided to refactor all existing modules to support the new IPAssetRegistry directly, as it makes sense to break this into a subsequent PR - instead, these changes were made to allow ALL existing tests to continue passing.

The following changes are not in scope for this PR, and should be updated in the future:

  • Migrating all modules to utilizing the global IP Asset registry IPAssetRegistry
  • Creating a standardized authorization layer for IPAsset registry enrollment
  • Creating a pathway for IPAssets not belonging to any specific IPAssetGroup (for this, we should either enable this functionality in the IPAssetController itself or create a default generic IPAssetGroup, similar to the prior franchise 0)

Copy link
Contributor

@Ramarti Ramarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the hard work in this refactor, it brings us closer to the new architecture.
However, as we talked about last week, we don't have to fear a more drastic approach, since at this stage we can break things, stability of previous versions is not a concern yet and we have a tight schedule.
My proposal is to be more specific on the changes we expect on issues.

contracts/IPAssetRegistry.sol Show resolved Hide resolved
Copy link
Contributor

@kingster-will kingster-will left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

contracts/IPAssetOrgFactory.sol Outdated Show resolved Hide resolved
contracts/ip-assets/IPAssetOrg.sol Show resolved Hide resolved
@Ramarti Ramarti added the P0 Most urgent priority label Oct 25, 2023
@Ramarti
Copy link
Contributor

Ramarti commented Oct 25, 2023

Please @leeren resolve the conversations and merge this even the fork test fails. There is a different workstream to fix that one, and it may even be ignored for now

@Ramarti Ramarti mentioned this pull request Oct 25, 2023
@leeren leeren merged commit a7c3db7 into storyprotocol:main Oct 30, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Most urgent priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor to support global IP Registry
3 participants