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

IPN-21 remove concept of "sales cycle" IPTs #159

Merged
merged 10 commits into from
Jul 11, 2024
Merged

IPN-21 remove concept of "sales cycle" IPTs #159

merged 10 commits into from
Jul 11, 2024

Conversation

elmariachi111
Copy link
Member

@elmariachi111 elmariachi111 commented Jul 4, 2024 •
edited
Loading

  • Tokenizer indexes IPTs by IPNFT ids
  • drops "hashes" completely
  • Tokenizer:reinit adds existing IPTs to the known ipt mapping
  • IPToken asks its owner (Tokenizer) if senders hold the underlying IPNFT
  • even is upgradeable on top of the previous 1.1 -> 1.2 chain state
  • loosens SalesShareDistributor's cap rules since there's only 1 ipt instance
  • updates test cases where necessary

@elmariachi111
IPSeed: the tokenizer is the only account that controls token issuance
4173cb4 
Tokenizer ensures that the sender is the IPNFT holder
loosens SalesShareDistributor's cap rules since there's only 1 ipt instance
updates test cases where necessary

Signed-off-by: stadolf <[email protected]>
@openzeppelin-code OpenZeppelin Code
Copy link

openzeppelin-code bot commented Jul 4, 2024 •
edited
Loading

IPN-21 remove concept of "sales cycle" IPTs

Generated at commit: 653474af8f2e188e17ab3166b248160908846fd7

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total		 1
1
0
5
30
37
Dependencies Critical
High
Medium
Low
Note
Total		 0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@elmariachi111
follows the original concept of self sufficient IPToken contracts
c4c15c0 
Tokenizer indexes IPTs by IPNFT ids
drops "hashes" completely
reinit adds existing IPTs to the known ipt mapping
IPToken asks its owner (Tokenizer) if senders hold the underlying IPNFT
even works on top of the previous 1.1 -> 1.2 chain state

Signed-off-by: stadolf <[email protected]>
@elmariachi111 elmariachi111 force-pushed the feature/ipn-21-use-tokenizer-contract-to-control-ipt-issuance branch from cdda55f to c4c15c0 Compare July 4, 2024 19:18
@elmariachi111
IPTs: save a storage slot for not storing the tokenizer explicitly
b58a912 
tests IPTs control after IPNFT transfers

Signed-off-by: stadolf <[email protected]>
@elmariachi111
happy first throw at subgraphs
16a696b 
regnerated ABIs & tested this with a local setup

Signed-off-by: stadolf <[email protected]>
@elmariachi111
full mainnet fork test case for new Tokenizer
653474a 
Signed-off-by: stadolf <[email protected]>
@elmariachi111 elmariachi111 requested a review from nour-karoui July 5, 2024 14:44
@elmariachi111
updates subgraph dependencies & config
78638e5 
uses latest foundry image
uses latest graph specs
replaces graph templating with network feature
adds network string replacements mainnet

Signed-off-by: stadolf <[email protected]>
nour-karoui
nour-karoui reviewed Jul 8, 2024
View reviewed changes
Copy link
Contributor

@nour-karoui nour-karoui left a comment

Choose a reason for hiding this comment

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

After implementing the commented todos (//todos), will we also remove the originalOwner in the metadata? because I don't understand why we need to save it?

src/IPToken.sol Outdated Show resolved Hide resolved
src/SalesShareDistributor.sol Outdated Show resolved Hide resolved
src/SalesShareDistributor.sol Outdated Show resolved Hide resolved
src/IPToken.sol Show resolved Hide resolved
@elmariachi111 elmariachi111 marked this pull request as ready for review July 8, 2024 16:38
@elmariachi111
rolled back reduced event signature for subgraph compatibility
8566a19 
new deployment preparation script

Signed-off-by: stadolf <[email protected]>
@elmariachi111
Copy link
Member Author

the originalOwner property is part of the storage layout of the permissioner contract that can't be upgraded. Hence we cannot remove it without breaking the already existing contracts. (see Permissioner.sol:63 -> the old IPTs use that memory layout. Even if I change it on IPToken, it will still be laid out like that)

@elmariachi111
IPT/Tokenizer: "controller" terminology leaves room for interpretation
df9f676 
updates sales distributor to acknowledge IPT controllers

Signed-off-by: stadolf <[email protected]>
@elmariachi111 elmariachi111 force-pushed the feature/ipn-21-use-tokenizer-contract-to-control-ipt-issuance branch from daeedba to df9f676 Compare July 9, 2024 11:19
@elmariachi111
abstracts the controllable aspect of tokenizer
d240fb9 
demonstrates how decisions could be handed off to a governor later

Signed-off-by: stadolf <[email protected]>
nour-karoui
nour-karoui approved these changes Jul 9, 2024
View reviewed changes
src/Tokenizer.sol Show resolved Hide resolved
@elmariachi111
rollout sepolia subgraph version
3c84806 
uses latest satsuma supported spec version 1.0.0
updates initial block heights

Signed-off-by: stadolf <[email protected]>
@elmariachi111 elmariachi111 force-pushed the feature/ipn-21-use-tokenizer-contract-to-control-ipt-issuance branch from 288f2dc to 3c84806 Compare July 11, 2024 13:33
@elmariachi111 elmariachi111 merged commit 10a42a1 into main Jul 11, 2024
1 check passed
@elmariachi111 elmariachi111 deleted the feature/ipn-21-use-tokenizer-contract-to-control-ipt-issuance branch July 11, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nour-karoui nour-karoui nour-karoui approved these changes

@DorianWilhelm DorianWilhelm Awaiting requested review from DorianWilhelm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elmariachi111 @nour-karoui