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.

Sign up for GitHub

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

Jump to bottom

Relationship Module #37

Merged
merged 35 commits into from
Jul 26, 2023
Merged

Relationship Module #37

merged 35 commits into from
Jul 26, 2023

Conversation

Ramarti
Copy link
Contributor

@Ramarti Ramarti commented Jul 14, 2023 •
edited
Loading

  • link setting and config
  • time based links
  • link processors
  • processors can be pending (enabling request/response and similar flows)
  • refactor from Linking to Relationship
  • added a per relationship config disputer address and removed related admin key
  • abstracted RelationshipModule to RelationshipModuleBase, then created ProtocolRelationshipModule with config gating with protocol wide admin keys. This allows to easily create RelationshipModules with other configurations.

Relates to #12

TODO in other PRs:

Raul added 25 commits June 29, 2023 20:50
simplify errors
2813cfa
added linking module POC
5e8ac2c
eip-7201 helper task
62dc0f9
removed __gaps in favour of namespaced storage
1d72fa4
Merge branch 'main' into namespaced_storage
f037d5b
refactor
9adc8a5
Merge branch 'refactor_story_block_to_ip_asset' into linking_poc
beb9abe
add directional linking
cb5182e
Merge branch 'main' into refactor_story_block_to_ip_asset
8057158
Merge branch 'refactor_story_block_to_ip_asset' into linking_poc
34f2352
franchise level intents
cb457e6
missing refactor files and method
a004a0f
remove franchise level links
f4cb48f
removed intent references in favour of link id
6f5bca2
link types mask checker
e12675d
moret ipAssetTypeTests
4a76f03
tests for linking
7a69484
check for link existance and unify errors
846ef3f
refactor regrouping
72cf670
test fixing
fe8fe28
refactor protocol out of link
40fd1c1
refactor link to relationship
01b0d5d
rel processor flow control
eff34d8
fix
0d58886
time links
04ebdf3
@Ramarti Ramarti changed the title Linking Module Relationship Module Jul 15, 2023
@Ramarti Ramarti marked this pull request as ready for review July 15, 2023 01:26
@Ramarti Ramarti assigned kingster-will and Ramarti and unassigned leeren and kingster-will Jul 15, 2023
@Ramarti Ramarti requested review from leeren and kingster-will and removed request for leeren and kingster-will July 15, 2023 01:27
@Ramarti Ramarti marked this pull request as draft July 15, 2023 01:34
@Ramarti Ramarti marked this pull request as ready for review July 21, 2023 14:47
kingster-will
kingster-will approved these changes Jul 24, 2023
View reviewed changes
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. a few comments.

contracts/modules/relationships/IRelationshipModule.sol Outdated Show resolved Hide resolved
contracts/modules/relationships/RelationshipModuleBase.sol Outdated Show resolved Hide resolved
contracts/modules/relationships/RelationshipModuleBase.sol Show resolved Hide resolved
@Ramarti
Copy link
Contributor Author

Ramarti commented Jul 24, 2023

@kingster-will I added Natspec to everything relationship related. Please let me know if it's clearer.

LeoHChen
LeoHChen reviewed Jul 24, 2023
View reviewed changes
Copy link
Member

@LeoHChen LeoHChen left a comment

Choose a reason for hiding this comment

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

nice job. Love the different processors we added.

so we need to add an allowlist of all valid processors in the protocol level, and franchise level. Anyway, we can add a github issue for this governance process.

contracts/modules/relationships/IRelationshipModule.sol Show resolved Hide resolved
contracts/ip-assets/IIPAssetRegistry.sol Show resolved Hide resolved
contracts/ip-assets/IPAssetRegistry.sol Outdated Show resolved Hide resolved
contracts/modules/relationships/RelationshipModuleBase.sol Outdated Show resolved Hide resolved
contracts/modules/relationships/RelationshipModuleBase.sol Show resolved Hide resolved
@Ramarti
Copy link
Contributor Author

Ramarti commented Jul 25, 2023

so we need to add an allowlist of all valid processors in the protocol level, and franchise level. Anyway, we can add a github issue for this governance process.

I added the issue to solve in other PR:
#42

@Ramarti Ramarti merged commit aacf89b into main Jul 26, 2023
@Ramarti Ramarti deleted the linking_poc branch July 26, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeoHChen LeoHChen LeoHChen left review comments

@kingster-will kingster-will kingster-will approved these changes

@leeren leeren Awaiting requested review from leeren

Assignees

@Ramarti Ramarti

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ramarti @LeoHChen @kingster-will @leeren