-
Notifications
You must be signed in to change notification settings - Fork 211
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
Staking app #101
Changes from 8 commits
dcebfc5
a513d62
fad9be6
c254d05
73bddb9
e9390fd
57ff719
6514851
f70da64
ae92945
4e1e07e
02a5b6c
358b461
dd6eaaf
bfa9fa3
1ddd278
a1c7fd3
ea1ce83
7877e47
99f507f
b8604b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
|
||
interface ERCStaking { | ||
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 token() public view returns (address); | ||
|
||
function supportsHistory() public pure returns (bool); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,221 @@ | ||
pragma solidity 0.4.18; | ||
|
||
import "./ERCStaking.sol"; | ||
|
||
import "@aragon/os/contracts/apps/AragonApp.sol"; | ||
import "@aragon/os/contracts/lib/zeppelin/token/ERC20.sol"; | ||
|
||
|
||
contract Staking is ERCStaking, 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 stakingToken; | ||
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 Locked(address indexed account, uint256 lockId, bytes32 metadata); | ||
event Unlocked(address indexed account, address indexed unlocker, uint256 oldLockId); | ||
|
||
event MovedTokens(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 _stakingToken, bytes _stakeScript, bytes _unstakeScript, bytes _lockScript) onlyInit external { | ||
stakingToken = _stakingToken; | ||
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(stakingToken.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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
|
||
Staked(acct, amount, totalStakedFor(acct), data); | ||
} | ||
|
||
function unstake(uint256 amount, bytes data) authP(UNSTAKE_ROLE, arr(amount)) checkUnlocked(amount) public { | ||
require(accounts[msg.sender].amount >= amount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SafeMath? |
||
accounts[msg.sender].amount -= amount; | ||
|
||
require(stakingToken.transfer(msg.sender, amount)); | ||
|
||
Unstaked(msg.sender, amount, totalStakedFor(msg.sender), data); | ||
|
||
if (unstakeScript.length > 0) { | ||
runScript(unstakeScript, data, new address[](0)); | ||
} | ||
} | ||
|
||
function lock( | ||
uint256 amount, | ||
uint8 lockUnit, | ||
uint64 lockEnds, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
||
Locked(msg.sender, lockId, metadata); | ||
|
||
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)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm missing something here, but:
|
||
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; | ||
|
||
Unlocked(acct, msg.sender, lockId); | ||
} | ||
|
||
function moveTokens(address from, address to, uint256 amount) authP(GOD_ROLE, arr(from, to, amount)) external { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
||
MovedTokens(from, to, amount); | ||
} | ||
|
||
// ERCStaking | ||
|
||
function token() public view returns (address) { | ||
return stakingToken; | ||
} | ||
|
||
function supportsHistory() public pure returns (bool) { | ||
return false; | ||
} | ||
|
||
function totalStakedFor(address addr) public view returns (uint256) { | ||
return accounts[addr].amount; | ||
} | ||
|
||
function unlockedBalanceOf(address acct) public view returns (uint256) { | ||
uint256 unlockedTokens = accounts[acct].amount; | ||
|
||
Lock[] storage locks = accounts[acct].locks; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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; | ||
} | ||
} |
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) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing
totalStaked
? I saw discussion here and your response, but it seems it's part of the interface definition.