dev-protocol / DIPs

📋The Dev Proposal repository
MIT License
11 stars 7 forks source link

Property Grouping #31

Open aggre opened 4 years ago

aggre commented 4 years ago

This proposal is a new implementation proposal of DIP5 (#5) for efficient handling of multiple Properties.

"Property Grouping" feature allows creators to combine multiple properties they have tokenized into one or more groups. This behavior is similar to the relationship between "folders" and "files" in a file system.

Directory Tokens

An ERC20 contract creating to group multiple Property Tokens (a.k.a. Creator Tokens). The total supply is fixed at 10,000,000. There are no restrictions on the transfer destination.

Directory Tokens holders can earn creator rewards for all grouped Property Tokens.

Once Directory Tokens are generated, no one can self-destruct them.

Technical requirements

I propose the main technical requirements and interfaces for Directory Tokens as follows.

Also, Directory Tokens should be implemented as an upgradeable contract. I think it is desirable to implement it as an external contract rather than incorporating it into the Dev Protocol core. However, Dev Kit and GraphQL should support Property Grouping.

Association

Creators can associate up to 9 Property Tokens with Directory Tokens (this limit works to keep low gas fees). The operation is required the Directory holder and the Property holder to be the same.

The association between Directory Tokens and Property Tokens is done by sending Property Tokens to Directory Tokens (Contract).

The interface is:

function associate(address _property, uint _amount) external;

Disassociation

Note: This feature should not be implemented initially. It should be implemented after sufficient security verification.

Transfer the Creator Tokens deposited in a Directory Token to another address. This operation requires agreement by all Directory Tokens holders. This is because disassociation means a reduction in rewards for Directory Tokens holders.

The interface is:

function disassociate(address _recipient, address _property, uint _amount) external;

Reward withdrawal

Withdraw rewards for associated Property Tokens. With the total reward amount of all associated Property Tokens as the maximum value, the reward is withdrawn according to the share ratio of Directory Tokens held by the account.

Calling Withdraw.withdraw each grouped Properties means running ERC20.mint and Lockup.update each grouped Properties, which means higher gas fees. Should add the bulkWithdraw function to the Withdraw contract.

The interface is:

function withdraw(address _directory) external;

Transfer Hook

A hook that updates the withdrawable rewards for the source and destination when transferring Directory Tokens. Withdraw.beforeBalanceChange is the reference model.

The interface is:

function beforeBalanceChange(address _directory, address _from, address _to) external;
aggre commented 4 years ago

It's mostly from a software engineering standpoint that I thought Property Grouping shouldn't be built into the protocol core, but it also causes the gas fees to skyrocket. If we're UX is a priority, I think it should still be built into the protocol core. I'm still thinking. Please let the opinion of everyone.

Akira-Taniguchi commented 4 years ago

What should be the second argument of the associate function? The associate and disassociate functions are aware that only the author of DirectoryTokens can execute If the author of the associated property is changed after associating, consider it forced disassociate when executing withdraw.

aggre commented 4 years ago

The argument named _amount is the number of tokens to transfer. This means that users can transfer "part" or "all" of the Property Tokens.

Also, Directory Tokens can be associated by "holders" not "author". This means that Alice, who has 3000 Directory Tokens and 5000 Property Tokens, can associate up to 3000 Property Tokens with Directory Tokens.

aggre commented 4 years ago

Even if Property Grouping is built into the core, contracts will need to update the staked amount for each Property when the user runs bulkStake. Otherwise, there will be no consistency between the stakers for the grouped properties and the other stakers. Trying to do everything with a small amount of computation to reduce gas costs lead to a hotbed for bugs and seems to be contrary to Ethereum philosophy.

Also, the strong consistency between staking via grouped properties and direct staking to Property is a security advantage that outweighs gas costs.

defi-er commented 4 years ago

What is the conceptual difference between DIP #27 ?

aggre commented 4 years ago

What is the conceptual difference between DIP #27 ?

@defi-er Property Grouping can be seen as a new form of Property. It has the disposition that a static asset set, with convenience for creators in mind. Therefore, it has a different purpose than, for example, dynamic sets for maximizing ROI.

Why the protocol needs Property Grouping:

defi-er commented 4 years ago

What happens if for example, Sindre takes advantage of the Creator token to grow some of his projects which then get passed on to a new maintainer. Is the Group reversible? If we start to scale projects then current creators could lose affiliation.

defi-er commented 4 years ago

I see so Sets is more beneficial for Foundations and users while Grouping is more beneficial for Creators.

aggre commented 4 years ago

What happens if for example, Sindre takes advantage of the Creator token to grow some of his projects which then get passed on to a new maintainer. Is the Group reversible? If we start to scale projects then current creators could lose affiliation.

@defi-er Creators deposit their Property into Property Grouping for Grouping. This means delegating property management to the group. If the user wants to cancel the delegation, the user can release part of the group's deposited amount.

I see so Sets is more beneficial for Foundations and users while Grouping is more beneficial for Creators.

Yes, I think of them the same as you said.

aggre commented 4 years ago

Since Property Grouping aims to make the protocol more convenient for creators, I decided to remove bulkStake and bulkUnstake from the specs. Not only are they gas inefficient, but they also leave a technical debt as the same functionality can be achieved by Sets.

I also suggest adding the bulkWithdraw function to the Withdraw contract to improve the withdraw function's poor gas efficiency. There is no need to call every time ERC20.mint, Lockup.update, or any other interface sufficient to run once.

Akira-Taniguchi commented 3 years ago

Since Property Grouping aims to make the protocol more convenient for creators, I decided to remove bulkStake and bulkUnstake from the specs. Not only are they gas inefficient, but they also leave a technical debt as the same functionality can be achieved by Sets.

I think so, too.

I also suggest adding the bulkWithdraw function to the Withdraw contract to improve the withdraw function's poor gas efficiency. There is no need to call every time ERC20.mint, Lockup.update, or any other interface sufficient to run once.

I think so, too.

@aggre

Akira-Taniguchi commented 3 years ago

It's mostly from a software engineering standpoint that I thought Property Grouping shouldn't be built into the protocol core, but it also causes the gas fees to skyrocket. If we're UX is a priority, I think it should still be built into the protocol core. I'm still thinking. Please let the opinion of everyone.

I don't think it's a good idea to incorporate Directory contracts into the Dev Protocol itself. The reason is that Directory Contracts is not a required feature, but a helpful feature!

I think it would be a good idea to create and operate it as an external contract first, and then consider incorporating it into the DEV Protocol if it causes problems in terms of gas and UX.

@aggre