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

Staking app #101

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 208 additions & 0 deletions future-apps/staking/contracts/Staking.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
pragma solidity 0.4.18;

import "@aragon/os/contracts/apps/AragonApp.sol";
import "@aragon/os/contracts/lib/zeppelin/token/ERC20.sol";


contract Staking is AragonApp {
struct Account {
uint256 amount;
Lock[] locks;
}

struct Lock {
uint256 amount;
Timespan timespan;
address unlocker;
bytes32 metadata;
}

struct Timespan {
//uint64 start; // exclusive
uint64 end; // inclusive
TimeUnit unit;
}

enum TimeUnit { Blocks, Seconds }

ERC20 public token;
bytes public stakeScript;
bytes public unstakeScript;
bytes public lockScript;

mapping (address => Account) accounts;

// TODO: figure out a way to get lock from metadata, given changing lock ids
// mapping (bytes32 => LockLocation) lockByMetadata;

event Stake(address indexed account, uint256 amount);
event Unstake(address indexed account, uint256 amount);

event Lock(address indexed account, uint256 lockId, bytes32 metadata);
event Unlock(address indexed account, address indexed unlocker, uint256 oldLockId);

event MoveTokens(address indexed from, address indexed to, uint256 amount);

bytes32 constant STAKE_ROLE = keccak256("STAKE_ROLE");
bytes32 constant UNSTAKE_ROLE = keccak256("UNSTAKE_ROLE");
bytes32 constant LOCK_ROLE = keccak256("LOCK_ROLE");
bytes32 constant GOD_ROLE = keccak256("GOD_ROLE");

modifier checkUnlocked(uint256 amount) {
require(unlockedBalanceOf(msg.sender) >= amount);
_;
}

// TODO: Implement forwarder interface

function initialize(ERC20 _token, bytes _stakeScript, bytes _unstakeScript, bytes _lockScript) onlyInit external {
token = _token;
stakeScript = _stakeScript;
unstakeScript = _unstakeScript;
lockScript = _lockScript;

initialized();
}

function stake(uint256 amount, bytes data) authP(STAKE_ROLE, arr(amount)) public {
stakeFor(msg.sender, amount, data);
}

function stakeFor(address acct, uint256 amount, bytes data) authP(STAKE_ROLE, arr(amount)) public {
// From needs to be msg.sender to avoid token stealing by front-running
require(token.transferFrom(msg.sender, this, amount));
processStake(acct, amount, data);

if (stakeScript.length > 0) {
runScript(stakeScript, data, new address[](0));
}
}

function processStake(address acct, uint256 amount, bytes data) internal {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are using another function here? It's kind of short and only called once.

accounts[acct].amount += amount; // overflow check in token contract
assert(accounts[acct].amount >= amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean with the comment one line above? Aren't you checking overflow here?
Shouldn't we use SafeMath lib like the rest of the apps?
Do we allow amount = 0?


Stake(acct, amount);
}

function unstake(uint256 amount, bytes data) authP(UNSTAKE_ROLE, arr(amount)) checkUnlocked(amount) public {
require(accounts[msg.sender].amount >= amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

SafeMath?

accounts[msg.sender].amount -= amount;

require(token.transfer(msg.sender, amount));

Unstake(msg.sender, amount);

if (unstakeScript.length > 0) {
runScript(unstakeScript, data, new address[](0));
}
}

function lock(
uint256 amount,
uint8 lockUnit,
uint64 lockEnds,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't be useful a helper/wrapper to indefinitely lock tokens without forcing the user to pass MAX UINT64?

address unlocker,
bytes32 metadata,
bytes data
)
authP(LOCK_ROLE, arr(amount, uint256(lockUnit), uint256(lockEnds)))
checkUnlocked(amount)
public
{
Lock memory newLock = Lock(amount, Timespan(lockEnds, TimeUnit(lockUnit)), unlocker, metadata);
uint256 lockId = accounts[msg.sender].locks.push(newLock) - 1;

Lock(msg.sender, lockId);

if (lockScript.length > 0) {
runScript(lockScript, data, new address[](0));
}
}

function stakeAndLock(
uint256 amount,
uint8 lockUnit,
uint64 lockEnds,
address unlocker,
bytes32 metadata,
bytes stakeData,
bytes lockData
)
authP(STAKE_ROLE, arr(amount))
authP(LOCK_ROLE, arr(amount, uint256(lockUnit), uint256(lockEnds)))
public
{
stake(amount, stakeData);
lock(amount, lockUnit, lockEnds, unlocker, metadata, lockData);
}

function unlockAll(address acct) external {
while (accounts[acct].locks.length > 0) {
if (canUnlock(acct, 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something here, but:

  • Why unlock head of array first, instead of tail? With tail there would be no need to move elements of array.
  • If the first element of the array can't be unlocked, how would the loop be exited?

unlock(acct, 0);
}
}
}

function unlock(address acct, uint256 lockId) public {
require(canUnlock(acct, lockId));

uint256 lastAccountLock = accounts[acct].locks.length - 1;
if (lockId < lastAccountLock) {
accounts[acct].locks[lockId] = accounts[acct].locks[lastAccountLock];
}

accounts[acct].locks.length -= 1;

Unlock(acct, msg.sender, lockId);
}

function moveTokens(address from, address to, uint256 amount) authP(GOD_ROLE, arr(from, to, amount)) external {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we check if moved tokens are unlocked? Otherwise an account could end up having more locked tokens than staked.

require(accounts[from].amount >= amount);

accounts[from].amount -= amount;
accounts[to].amount += amount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, with SafeMath we could avoid require and assert here.


assert(accounts[to].amount >= amount);

MoveTokens(from, to, amount);
}

function unlockedBalanceOf(address acct) public view returns (uint256) {
uint256 unlockedTokens = accounts[acct].amount;

Lock[] storage locks = accounts[acct].locks;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can be memory

for (uint256 i = 0; i < locks.length; i++) {
if (!canUnlock(acct, i)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this. I think I understand the rationale, but I see a potential problem: If unstake function is called, it will call this function to check available staked but not locked tokens. If there are potentially unlocked, but actually locked tokens, they may be unstaked, leaving a bigger total locked balance than the staked balance.

unlockedTokens -= locks[i].amount;
}
}

return unlockedTokens;
}

function locksCount(address acct) public view returns (uint256) {
return accounts[acct].locks.length;
}

function getLock(address acct, uint256 lockId) public view returns (Lock) {
return accounts[acct].locks[lockId];
}

function lastLock(address acct) public view returns (Lock) {
return accounts[acct].locks[locksCount(acct) - 1];
}

function canUnlock(address acct, uint256 lockId) public view returns (bool) {
Lock memory l = accounts[acct].locks[lockId];

return timespanEnded(l.timespan) || msg.sender == l.unlocker;
}

function timespanEnded(Timespan memory timespan) internal view returns (bool) {
uint256 comparingValue = timespan.unit == TimeUnit.Blocks ? block.number : block.timestamp;

return uint64(comparingValue) > timespan.end;
}
}
5 changes: 5 additions & 0 deletions future-apps/staking/migrations/1_initial_migration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
var Migrations = artifacts.require("./Migrations.sol");

module.exports = function(deployer, n, accounts) {
deployer.deploy(Migrations)
}
Loading