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

ERC900: Simple Staking Interface #900

Closed
decanus opened this issue Feb 23, 2018 · 38 comments
Closed

ERC900: Simple Staking Interface #900

decanus opened this issue Feb 23, 2018 · 38 comments
Labels

Comments

@decanus
Copy link
Contributor

decanus commented Feb 23, 2018

Preamble

EIP: 900
Title: Simple Staking Interface
Author: Dean Eigenmann <dean@tokenate.io>, Jorge Izquierdo <jorge@aragon.one>
Type: Standard Track
Category: ERC
Status: Draft
Created: 2018-02-22

Abstract

The following standard describes a common staking interface allowing for easy to use staking systems. The interface is kept simple allowing for various use cases to be implemented. This standard describes the common functionality for staking as well as providing information on stakes.

Motivation

As we move to more token models, having a common staking interface which is familiar to users can be useful. The common interface can be used by a variety of applications, this common interface could be beneficial especially to things like Token curated registries which have recently gained popularity.

Specification

interface Staking {

    event Staked(address indexed user, uint256 amount, uint256 total, bytes data);
    event Unstaked(address indexed user, uint256 amount, uint256 total, bytes data);

    function stake(uint256 amount, bytes data) public;
    function stakeFor(address user, uint256 amount, bytes data) public;
    function unstake(uint256 amount, bytes data) public;
    function totalStakedFor(address addr) public view returns (uint256);
    function totalStaked() public view returns (uint256);
    function token() public view returns (address);
    function supportsHistory() public pure returns (bool);

    // optional
    function lastStakedFor(address addr) public view returns (uint256);
    function totalStakedForAt(address addr, uint256 blockNumber) public view returns (uint256);
    function totalStakedAt(uint256 blockNumber) public view returns (uint256);
}

stake

Stakes a certain amount of tokens, this MUST transfer the given amount from the user.

The data field can be used to add signalling information in more complex staking applications

MUST trigger Staked event.

stakeFor

Stakes a certain amount of tokens, this MUST transfer the given amount from the caller.

The data field can be used to add signalling information in more complex staking applications

MUST trigger Staked event.

unstake

Unstakes a certain amount of tokens, this SHOULD return the given amount of tokens to the user, if unstaking is currently not possible the function MUST revert.

The data field can be used to remove signalling information in more complex staking applications

MUST trigger Unstaked event.

totalStakedFor

Returns the current total of tokens staked for an address.

totalStaked

Returns the current total of tokens staked.

token

Address of the token being used by the staking interface.

supportsHistory

MUST return true if the optional history functions are implemented, otherwise false.

lastStakedFor

OPTIONAL: As not all staking systems require a complete history, this function is optional.

Returns last block address staked at.

totalStakedForAt

OPTIONAL: As not all staking systems require a complete history, this function is optional.

Returns total amount of tokens staked at block for address.

totalStakedAt

OPTIONAL: As not all staking systems require a complete history, this function is optional.

Returns the total tokens staked at block.

Implementation

Copyright

Copyright and related rights waived via CC0.

@decanus decanus changed the title Simple Staking Interface ERC900 Simple Staking Interface Feb 23, 2018
@decanus decanus changed the title ERC900 Simple Staking Interface ERC900: Simple Staking Interface Feb 23, 2018
@shrugs
Copy link

shrugs commented Feb 23, 2018

Do we want a totalStaked()? I see we have totalStakedAt(blockNumber) already.

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

@shrugs good point, seems like it could be important. I’ll add it

@izqui
Copy link
Contributor

izqui commented Feb 23, 2018

It would just be token.balanceOf(this) in most cases, right?

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

@izqui good point, but maybe for consistency sake it makes sense

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

@shrugs @izqui I updated

@shrugs
Copy link

shrugs commented Feb 23, 2018

Is it worth using Interface Detection instead of supportsHistory?

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

I feel like in this scenario a simple boolean function is more readable, than an interface detection function.

@AtLeastSignificant
Copy link

Is there a staking protocol you can link to that is in a state where all the variables and rules are completely fleshed out? I just want to make sure this interface covers all the bases / actually applies to where it may be used.

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

@AtLeastSignificant This standard grew out of something built for both Harbour and Aragon, we've tried to build it as generically as possible. The previous implementation from aragon has been reworked to fit to this standard.

@AtLeastSignificant
Copy link

@decanus Gotcha. Unfortunately, I'm not familiar enough with this to have very valuable input on your interface without a more concrete protocol to reference.

One thing I noticed though - the unstake function requires that tokens be returned to the user, but it's my understanding that this is not possible in staking systems where unstaking is a process that happens over a period of time (STEEM power as an example). Is the period of time where the return happens assumed to be instant, or is the wording to imply that it can happen instantly or over time?

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

@AtLeastSignificant the wording I used in the unstake function is probably a little wrong. Tokens must be returned if an unstake is successful, however if you aren't at a time yet where you can unstake this function can of course trigger a revert.

Updated the text a bit

@AtLeastSignificant
Copy link

@decanus hmm, then perhaps an initUnstake() function is more appropriate. In systems where unstaking is instant, a successful execution would return the tokens, but in systems where unstaking is something you do over time, a successful return would be confirmation that the status of the account has changed from staked to unstaked/unstaking.

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

Well a user would still need to call unstake, splitting initUnstake and unstake into 2 separate functions seems to be a bit of an over complexity.

@AtLeastSignificant
Copy link

I was more suggesting that initUnstake would replace unstake. That, or unstake should be worded to more generally reflect that unstaking could be a single instant process or multiple processes over time.

It could just be that unstake is used and the protocol knows whether or not it's time to allow tokens to be returned, but that means checking both the current staked balance and other requirements like the time since last unstake and amount eligible to be unstaked right now.

@decanus
Copy link
Contributor Author

decanus commented Feb 23, 2018

I wouldn't call it initUnstake as that usually means initialize, which definately then shouldn't run the unstake process. I think that unstake here is probably a better name which is generic enough. Also I would expect that an application knows some of the rules for unstaking, if they need to they could implement a function called canUnstake which they can use. However this being part of the standard seems a little off.

Would like to hear @izqui's opinion on this.

@christoph2806
Copy link
Contributor

We have a very similar staking contract; see here for reference.
https://github.com/etherisc/tokensale/blob/develop/contracts/protocol/TokenStake.sol

@cbruguera
Copy link

Cool initiative. I'm just thinking that for the stake function to actually transfer user tokens to the contract (assuming these are ERC20 tokens), first there should be an approve call made on the token contract. This should most probably be manually invoked by the user first, correct? Or is there a way to perform these two steps programmatically? Would a delegatecall be able to approve token transfers on behalf of the sender?

@decanus
Copy link
Contributor Author

decanus commented Feb 24, 2018

@cbruguera No that's not possible, a user needs to execute that manually.

@decanus
Copy link
Contributor Author

decanus commented Feb 24, 2018

@christoph2806 Looks interesting, looks like a lot of your code work with the interface we have defined.

I've seen a bunch of contracts that have this awfully similar interface.

@ngrinkevich
Copy link

first there should be an approve call made on the token contract. This should most probably be manually invoked by the user first, correct?

@cbruguera a solution that might help, there's so called approveAndCall pattern. Example can be found here https://www.ethereum.org/token

Staker just calls this function on the token contract providing the address of the staking contract as _spender, it approves the amount and calls receiveApproval() on the staking contract. Based on zeppelin Standard token it can be combined into something like this:

StandardToken contract

// Interface of recipient contract for approveAndCall pattern.
interface tokenRecipient { function receiveApproval(address _from, uint256 _value, address _token, bytes _extraData) public; }

function approveAndCall(address _spender, uint256 _value, bytes _extraData) public returns (bool success) {
     tokenRecipient spender = tokenRecipient(_spender);
     if (approve(_spender, _value)) {
         spender.receiveApproval(msg.sender, _value, this, _extraData);
         return true;
     }
}

Staking contract

// Staking contract constructor, we must know what token it will accept for staking
function TokenStaking(address _tokenAddress) {
    require(_tokenAddress != address(0x0));
    token = StandardToken(_tokenAddress);
}

function receiveApproval(address _from, uint256 _value, address _token, bytes _extraData) {
     ...
     // Make sure provided token contract is the same one linked to this contract.
     require(StandardToken(_token) == token);
     // at this point amount is approved and we can do transfers, i.e.
     token.transferFrom(_from, this, _value);
     ...
}

We're using this pattern at Keep network and it works nicely 😀 Let me know if you need any further info, happy to help

@izqui
Copy link
Contributor

izqui commented Feb 26, 2018

@AtLeastSignificant @decanus I really like the idea of having asynchronous unstake(), however I think the function that initiates the action should still be called unstake() and maybe a subprotocol or a extension of this one with some optional methods, can be written to support the notion that a unstake action might take some time (some methods to get these timespans would be great too). Then the user can be expected to call to finalizeUnstake() to finally get their tokens.

@ngrinkevich The problem with approveAndCall is that a broken implementation was spread around too much and many tokens have a broken approveAndCall which is fine since no one uses the pattern in a generalized way (you can make sure the particular token you are using works well) but you cannot assume every token with the function will work properly.

I would be however happy to extend this standard in a way that allows you to stake by doing a ERC777 or send() or a similar standard.

@mhluongo
Copy link
Contributor

mhluongo commented Apr 8, 2018

How do you expect this to work in systems that have eg minimum staking periods? I'd expect something like startUnstake and withdraw.

Many staking schemes I've seen need to be able to extend how long funds are at risk after certain staker behavior. If we build infrastructure around this I'm not sure how tools would cope without another standard.

@decanus
Copy link
Contributor Author

decanus commented Apr 8, 2018

@mhluongo I might be missunderstanding but unstake could just revert if you haven’t staked for a minimum amount of time.

@mhluongo
Copy link
Contributor

mhluongo commented Apr 8, 2018

I'm not doing a great job explaining 😊

Our unstake process has a delay. Once you've claimed you want to unstake, you can't fully withdraw from the staking contract for 2 weeks. It's to prevent certain attacks where triggering slashing conditions are delayed- if stakers can quickly unstake, they can avoid getting slashed.

@johnhforrest
Copy link
Contributor

Is it intentional that stakeFor essentially transfers your tokens to another party? I know the standard calls out SHOULD, but both the Stakebank and Aragon implementations do this. If I stake on behalf of someone else or on behalf of another application there's no way for me to get those tokens back.

Maybe something like unstakeFrom is needed where users can dictate which stake they are trying to withdraw if they have staked for an address other than their own?

@decanus
Copy link
Contributor Author

decanus commented May 27, 2018

@johnhforrest Our definition is currently that method, I find it dangerous if you can stake for someone but then also unstake those tokens you have staked. Could lead to some weird issues, I think. Of course if you need it differently you could do so in your own contract. Additionally you could limit stakeFor to only allow the user == msg.sender. There is a Stakebank implementation that currently enforces this, see: https://github.com/HarbourProject/stakebank/pull/6

@johnhforrest
Copy link
Contributor

I don't think having an unstakeFrom implies you could stake the same tokens for multiple accounts—once you've staked tokens they are transferred to the staking contract.

That being said, I do agree that something didn't feel quite right in the implementation I was writing where users could stake multiple times. I've added some logic now that each address staking tokens can only have a single stake. This stake can be for themselves, or for another address. unstake then becomes a withdrawal against that stake (i.e., their own tokens) as opposed to withdrawing against the total staked tokens for that address, so unstakeFrom isn't necessary.

@decanus
Copy link
Contributor Author

decanus commented May 27, 2018

Sorry, there was an error in my response. I meant unstake.

@johnhforrest
Copy link
Contributor

@decanus Just finished my interpretation of the standard here: https://github.com/codex-protocol/contract.codex-registry/blob/master/contracts/ERC900/ERC900BasicStakeContainer.sol

With a battery of tests here: https://github.com/codex-protocol/contract.codex-registry/blob/master/test/core/CodexStakeContainer.test.js

Would love to get your feedback & potentially include it in the implementations section if it's worth sharing. We took a different approach where staked tokens (even if staked for another address) always belong to the originator. Users can create multiple stakes, and can receive annualized interest on the perceived value of their stakes by pinging the contract after sufficient time has passed. The intent is that these would be long-term stakes that provide benefits to the user based on the amount of perceived tokens staked.

@decanus
Copy link
Contributor Author

decanus commented Jun 14, 2018

@johnhforrest This sounds interesting, I will need to have a full look a little later. Feel free to add your implementation to the EIP, as it is merged you can add a PR.

@whiteyhat
Copy link

Hey @decanus!

I was wondering how useful this simple staking interface can be applied for the creation of masternodos through this standard?

@ptrwtts
Copy link

ptrwtts commented Jul 10, 2018

I agree with @johnhforrest that staked tokens should always belong to the originator, even if they are staked to another user (stakeFor). This enables a couple of important use cases:

  1. Delegated Voting - Users can stake to a more informed representative, who votes on their behalf, while maintaining ownership of their tokens.

  2. Key Separation - Users can stake to a second address they control, which is used for voting. This way, they aren't putting the key that can spend funds at risk each time they vote.

What are the use-cases where someone would use stakeFor, and want the recipient to own their tokens when they unstake?

This doesn't feel an implementation detail. There should be consistency over what unstake() will do. If there are legitimate reasons for transferring ownership when tokens are staked to another user, it should probably have a different function to when they are just delegated.

@tavakyan
Copy link

tavakyan commented Jul 12, 2018

I keep coming across this need to get a list of staked addresses and their corresponding balances for a staking contract I'm working on. Perhaps we can have an optional method that does one of the following:

function allStakers() public constant returns (address[]);

and/or

function allStakerBalances() public constant returns (address[], uint[]);

When another contract is using this standard, as say an owner, it's likely such a aggregate will have to be constructed to be consumed. With the current standard there isn't a way to do this except if you know the address beforehand. This doesn't imply you must implement this of course if you don't want anyone to know the stakers in a direct way so I don't think it'll break the current design.

@johnhforrest
Copy link
Contributor

FYI I sent a PR to add another reference implementation here: #1212

Reference implementation is located here:
https://github.com/codex-protocol/contract.erc-900

@decanus
Copy link
Contributor Author

decanus commented Jul 13, 2018

@tavakyan I am not sure whether this should be defined in the ERC itself, however it does not prevent you from implementing something like this on your own as it doesn’t really break compatibility.

@tavakyan
Copy link

@decanus In my case I was trying to come up with an interface that a validator set contract can use to interact with all the stakers for the purposes of a PoS-like consensus engine. Perhaps this is better left for a future standard since this is supposed to be a 'simple' interface. I got this idea when looking at some code in the melonproject project: https://github.com/melonproject/smart-contracts/blob/develop/src/system/OperatorStaking.sol <-- see getStakersAndAmounts()

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Dec 18, 2021
@github-actions
Copy link

github-actions bot commented Jan 1, 2022

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

@github-actions github-actions bot closed this as completed Jan 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

13 participants