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

Un-stake tokens when they are transferred / redeemed #9

Closed
lkngtn opened this issue Sep 16, 2019 · 7 comments · Fixed by #57
Closed

Un-stake tokens when they are transferred / redeemed #9

lkngtn opened this issue Sep 16, 2019 · 7 comments · Fixed by #57
Assignees
Labels
product Discussion about integration of new features
Milestone

Comments

@lkngtn
Copy link
Member

lkngtn commented Sep 16, 2019

When a user stakes to a proposal we assume that their token balance will not change while they are still staked to a proposal.

To make this a sound assumption we need to determine a way to prevent transfers or redemptions (eg via redemptions app or the fundraising app). As it stands we would only support non-transferrable tokens and make the assumption that users would not have a way to burn/transfer those tokens (eg not have redemptions or fundraising installed)...

However, it is likely that orgs will want one or both of redemptions/fundraising installed, and it would also be nice to be able to support transferrable tokens.

Autark has done some work to make a version of the token manager that has a notion of a transferability oracle which may be a good way to handle this: https://github.com/AutarkLabs/aragon-apps/blob/token-manager-oracle/apps/token-manager/contracts/TokenManager.sol

We could create an oracle as part of the conviction voting app and have that hooked into this modified token manager. (though its not clear how this would work in cases where there are other oracles in use, or even in the case where we have multiple instances of conviction voting installed).

@lkngtn lkngtn self-assigned this Sep 16, 2019
@sembrestels sembrestels added the product Discussion about integration of new features label Oct 15, 2019
@sembrestels
Copy link
Member

Folks at Commons Stack may have already thought on a solution, as they post on their blog:

When a member joins the community, they will be requested to assert percentages of their preference towards existing proposals, adding up to 100% in total. Using something like the ERC-888 token standard, we can allow tokens to be automatically asserted towards proposals, without affecting token liquidity with staking locks. Since your preferences are expressed as a percentage of your token holdings, any change to your token holdings (through buying or selling) automatically updates the weight of your preference.

I still have to check out ERC-888: The Multidimensional Token Standard, and see how we can use it in this case.

@lkngtn
Copy link
Member Author

lkngtn commented Oct 16, 2019

I think this solution won't work (or will have significant trade-offs) because even if the weight represents a percentage, you are forced to tabulate conviction all at once when executing the proposal since you don't have any guarantees about the balances staked to a proposal over time.

if you tabulate conviction all at once at the end, then you force the person doing the tabulation to pay for all of the gas and also introduce issues where either we can only take the top staked amounts and include that (to limit the number of addresses we evaluate) or potentially hit out of gas errors.

Atleast that is my understanding of the issue at this time, is that not the case with the approach you are referencing?

@sembrestels sembrestels modified the milestones: Milestone 2, Milestone 3 Mar 24, 2020
@sembrestels
Copy link
Member

Instead of preventing transferring or burning tokens, I think that we should un-stake those tokens if they are not going to be possessed anymore by the holder.

The rationale for preventing transfers or redemptions is that we should prevent somebody to stake, transfer, and stake again from another account. Preventing them to transfer only works when they are the only ones that can transfer or burn their own tokens, and this is not usually the case.

In Aragon, Token Managers are Token Controllers of Minime Tokens. They can mint and burn tokens using the Minime generateTokens and destroyTokens respectively. Minime tokens can also be transferred by other parties in regard to somebody else using the transferFrom function. This is why I would not recommend to prevent transfers if tokens are staked, because it gives power to the holder to prevent transfers and burns that previously maybe haven't.

Otherwise, what I recommend is to un-stake tokens when they are being transferred or burned by the user or any other party. In order to do that, I propose to use a hook similar to ERC777TokensSender. The alternative proposed in https://github.com/AutarkLabs/token-manager-custom would be to use an oracle, but that would only prevent those transactions to happen.

So, we would create a customized TokenManager that calls the tokensToSend function of all registered contracts when tokens are transferred or burnt. Each Conviction Voting instance will be registered (they will implement an ERC777TokenSender interface), and when some tokens are being transferred or burned by the Token Manager, Conviction Voting instances will internally un-stake as much tokens as necessary in order to keep the congruence in the application.

The first tokens to be un-staked will be the ones that have been staked for more time. We will also put a maximum amount of staked proposals per address to avoid an out-of-gas attack in which a user stakes small amounts in a tremendous amount of proposals in order to avoid their tokens to be transferred or burnt by another party.

@lkngtn
Copy link
Member Author

lkngtn commented Apr 9, 2020

Interesting. It seems to make sense to me and also seems like it would be general enough that it could be merged up-stream eventually if there was interest and other use-cases for the hook appeared. I would build off of the customization that autark did as the oracle functionality is very useful as well!

@sembrestels
Copy link
Member

Following up with this issue, I've implemented the following interface for the Token Manager hooks. I finally did not used ERC777 interfaces because I preferred to hook the Token Manager itself. In the future we could implement a MiniMe token with support for ERC777, and set up Aragon apps as token operators (contracts with permission to transfer your tokens), but in the meanwhile, a TokenManagerHook as defined in the PR is enough.

@sembrestels sembrestels changed the title Preventing transfers / redemptions while "staked" Un-stake tokens when they are transferred / redeemed Apr 11, 2020
@GriffGreen
Copy link

@sembrestels
Copy link
Member

This is very cool @GriffGreen.

If I understand well, Yoga Token is an upgrade to MiniMe that supports token operators. This means that when transfering, we not only call the token controller's onTransfer/onSend controller function, but also call the receiver's tokenFallback function in case it is implementing that interface.

In our case, we need to hook the token controller's onSend function in order to un-stake those tokens for conviction voting, which is already covered by MiniMe, and we only need to change it's controller (aka the Hooked Token Manager). We will not need the features of ERC777 anymore either, because they work in the operator level, and we need that in the controller level.

Thanks for sharing anyway, it let me understand much better the work you have been doing at giveth and we may use it in the future if we need to work with token operators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product Discussion about integration of new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants