-
Notifications
You must be signed in to change notification settings - Fork 5
feat: staking #112
feat: staking #112
Conversation
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.
Looks nice! Good job. Added a comment about SafeMath
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.
New comment about totalStaked
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.
Looks good so far. Added some comments!
contracts/Staking.sol
Outdated
@@ -0,0 +1,45 @@ | |||
pragma solidity 0.6.2; | |||
|
|||
import "./vendor/SafeMath.sol"; |
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.
Include the OpenZeppelin library
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.
Done
contracts/Staking.sol
Outdated
} | ||
|
||
function totalStaked() public view returns (uint256) { | ||
return totalStakedFor(msg.sender); |
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.
I think the totalStaked
without parameter should return the TOTAL staked in the complete contract, according to the ERC900. So we may beed an attribute to store this and add or subtract when corresponds. I think its good to have that indicator in the contract directly.
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.
Done
* feat: add ERC20 mock contract move all tests related contracts to contract/test check if we get un-stake event * chore: fix linter
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.
LGTM - Merge when ready @nduchak
* feat: add support of multi-currency for staking contract adjust tests * feat: fix pr comments * feat: add tests for native toke white list
Closed in favor of #132 |
This adds the Solidity code and unit tests for a staking functionality in a separate contract.
I follows the guidelines from ERC900, but could not implement the interface to the letter, as it only worked with tokens and required some functions which we won't use.
The contract allows for staking in the token which is identified in the constructor (where address(0) is the native currency). Multi-currency is achieved by having multiple instances of this contract.
The contract only allows for unstaking when there is no capacity utilized from the StorageManager contract.