Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

feat: staking #112

Closed
wants to merge 15 commits into from
Closed

feat: staking #112

wants to merge 15 commits into from

Conversation

Eknir
Copy link
Contributor

@Eknir Eknir commented Aug 12, 2020

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.

@julianmrodri julianmrodri self-requested a review August 13, 2020 19:00
Copy link
Contributor

@julianmrodri julianmrodri left a 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

contracts/Staking.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@julianmrodri julianmrodri left a 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

contracts/Staking.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@julianmrodri julianmrodri left a 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 Show resolved Hide resolved
@@ -0,0 +1,45 @@
pragma solidity 0.6.2;

import "./vendor/SafeMath.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

Include the OpenZeppelin library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

function totalStaked() public view returns (uint256) {
return totalStakedFor(msg.sender);
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@vojtechsimetka vojtechsimetka self-assigned this Aug 21, 2020
@Eknir Eknir requested a review from vojtechsimetka August 28, 2020 10:48
@Eknir Eknir changed the title Staking feat: staking Aug 28, 2020
@julianmrodri julianmrodri linked an issue Aug 31, 2020 that may be closed by this pull request
* feat: add ERC20 mock contract
move all tests related contracts to contract/test
check if we get un-stake event

* chore: fix linter
@julianmrodri julianmrodri requested review from julianmrodri and removed request for vojtechsimetka September 2, 2020 14:09
Copy link
Contributor

@julianmrodri julianmrodri left a 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

nduchak and others added 2 commits September 2, 2020 19:50
* feat: add support of multi-currency for staking contract
adjust tests

* feat: fix pr comments

* feat: add tests for native toke white list
@nduchak nduchak closed this Sep 9, 2020
@nduchak nduchak reopened this Sep 9, 2020
@nduchak nduchak closed this Sep 9, 2020
@nduchak nduchak reopened this Sep 9, 2020
@AuHau
Copy link
Contributor

AuHau commented Sep 17, 2020

Closed in favor of #132

@AuHau AuHau closed this Sep 17, 2020
@AuHau AuHau deleted the staking branch September 17, 2020 06:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Funds Depositing contract
5 participants