Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: David Núñez <[email protected]>
Co-authored-by: Michalina <[email protected]>
  • Loading branch information
3 people committed Nov 28, 2023
1 parent b181f2a commit 5fb485e
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 40 deletions.
7 changes: 0 additions & 7 deletions .github/workflows/contracts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ jobs:
- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Resolve latest contracts
run: yarn upgrade @keep-network/keep-core@${{ github.event.inputs.environment }}

- name: Configure tenderly
env:
TENDERLY_TOKEN: ${{ secrets.TENDERLY_TOKEN }}
Expand Down Expand Up @@ -197,7 +194,6 @@ jobs:
# `etherscan-verify` plugins tries to verify them, which is not desired.
- name: Prepare for verification on Etherscan
run: |
rm -rf ./node_modules/@keep-network/keep-core
rm -rf ./external/npm
- name: Verify contracts on Etherscan
Expand Down Expand Up @@ -240,9 +236,6 @@ jobs:
- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Resolve latest contracts
run: yarn upgrade @keep-network/keep-core@${{ github.event.inputs.environment }}

- name: Deploy contracts
env:
# Using fake ternary expressions to decide which credentials to use,
Expand Down
5 changes: 0 additions & 5 deletions .github/workflows/npm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ jobs:
registry-url: "https://registry.npmjs.org"
cache: "yarn"

- name: Resolve latest contracts
run: |
yarn upgrade --exact \
@keep-network/keep-core
# Deploy contracts to a local network to generate deployment artifacts that
# are required by dashboard compilation.
- name: Deploy contracts
Expand Down
11 changes: 7 additions & 4 deletions contracts/staking/IStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ interface IStaking {
//
//

/// @notice Reduces the liquid T stake amount by the provided amount and
/// @notice Reduces the T stake amount by the provided amount and
/// withdraws T to the owner. Reverts if there is at least one
/// authorization higher than the remaining liquid T stake or
/// if the unstake amount is higher than the liquid T stake amount.
/// authorization higher than the remaining T stake or
/// if the unstake amount is higher than the T stake amount.
/// Can be called only by the delegation owner or the staking
/// provider.
function unstakeT(address stakingProvider, uint96 amount) external;
Expand Down Expand Up @@ -232,7 +232,10 @@ interface IStaking {
returns (uint96);

/// @notice Returns staked amount of T for the specified staking provider.
function tStake(address stakingProvider) external view returns (uint96);
function stakeAmount(address stakingProvider)
external
view
returns (uint96);

/// @notice Returns start staking timestamp.
/// @dev This value is set at most once.
Expand Down
25 changes: 17 additions & 8 deletions contracts/staking/TokenStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";

/// @notice TokenStaking is the main staking contract of the Threshold Network.
/// Additionally, it serves as application manager for the apps
/// that run on the Threshold Network.
/// It serves as application manager for the apps that run on
/// the Threshold Network.
/// @dev TokenStaking is upgradeable, using OpenZeppelin's Upgradeability
/// framework. As such, it is required to satisfy OZ's guidelines, like
/// restrictions on constructors, immutable variables, base contracts and
Expand All @@ -39,6 +39,13 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
using PercentUtils for uint256;
using SafeCastUpgradeable for uint256;

// enum is used for Staked event to have backward compatibility
enum StakeType {
NU,
KEEP,
T
}

enum ApplicationStatus {
NOT_APPROVED,
APPROVED,
Expand All @@ -47,9 +54,9 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
}

struct StakingProviderInfo {
uint96 legacyNuInTStake;
uint96 nuInTStake;
address owner;
uint96 legacyKeepInTStake;
uint96 keepInTStake;
address payable beneficiary;
uint96 tStake;
address authorizer;
Expand Down Expand Up @@ -101,6 +108,7 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
uint256 public slashingQueueIndex;

event Staked(
StakeType indexed stakeType,
address indexed owner,
address indexed stakingProvider,
address beneficiary,
Expand Down Expand Up @@ -263,6 +271,7 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
increaseStakeCheckpoint(stakingProvider, amount);

emit Staked(
StakeType.T,
msg.sender,
stakingProvider,
beneficiary,
Expand Down Expand Up @@ -641,10 +650,10 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
//
//

/// @notice Reduces the liquid T stake amount by the provided amount and
/// @notice Reduces the T stake amount by the provided amount and
/// withdraws T to the owner. Reverts if there is at least one
/// authorization higher than the remaining liquid T stake or
/// if the unstake amount is higher than the liquid T stake amount.
/// authorization higher than the remaining T stake or
/// if the unstake amount is higher than the T stake amount.
/// Can be called only by the delegation owner or the staking
/// provider.
function unstakeT(address stakingProvider, uint96 amount)
Expand Down Expand Up @@ -814,7 +823,7 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
}

/// @notice Returns staked amount of T for the specified staking provider.
function tStake(address stakingProvider)
function stakeAmount(address stakingProvider)
external
view
override
Expand Down
6 changes: 3 additions & 3 deletions docs/rfc-1-staking-contract.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ will be added to already authorized applications.

==== `unstakeT(address stakingProvider, uint96 amount) external onlyOwnerOrStakingProvider(stakingProvider)`

Reduces the liquid T stake amount by `amount` and withdraws `amount` of T
Reduces the T stake amount by `amount` and withdraws `amount` of T
to the owner. Reverts if there is at least one authorization higher than the
remaining liquid T stake or if the `amount` is higher than the liquid T stake amount.
remaining T stake or if the `amount` is higher than the T stake amount.
Can be called only by the owner or the staking provider.

=== Keeping information in sync
Expand Down Expand Up @@ -342,7 +342,7 @@ each affected application.

Returns the authorized stake amount of the staking provider for the application.

==== `tStake(address stakingProvider) external view returns (uint96)`
==== `stakeAmount(address stakingProvider) external view returns (uint96)`

Returns staked amount of T for the specified staking provider.

Expand Down
4 changes: 0 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,8 @@
"typescript": "^4.4.4"
},
"dependencies": {
"@keep-network/keep-core": ">1.8.1-dev <1.8.1-goerli",
"@openzeppelin/contracts": "~4.5.0",
"@openzeppelin/contracts-upgradeable": "~4.5.2",
"@thesis/solidity-contracts": "github:thesis/solidity-contracts#4985bcf"
},
"peerDependencies": {
"@keep-network/keep-core": ">1.8.1-dev <1.8.1-goerli"
}
}
9 changes: 8 additions & 1 deletion test/staking/TokenStaking.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ const { to1e18 } = helpers.number

const { AddressZero, Zero } = ethers.constants

const StakeTypes = {
NU: 0,
KEEP: 1,
T: 2,
}

const ApplicationStatus = {
NOT_APPROVED: 0,
APPROVED: 1,
Expand Down Expand Up @@ -288,6 +294,7 @@ describe("TokenStaking", () => {
await expect(tx)
.to.emit(tokenStaking, "Staked")
.withArgs(
StakeTypes.T,
staker.address,
stakingProvider.address,
beneficiary.address,
Expand Down Expand Up @@ -3520,7 +3527,7 @@ describe("TokenStaking", () => {
})

async function assertStake(address, expectedTStake) {
expect(await tokenStaking.tStake(address), "invalid tStake").to.equal(
expect(await tokenStaking.stakeAmount(address), "invalid tStake").to.equal(
expectedTStake
)
}
Expand Down
8 changes: 0 additions & 8 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1269,14 +1269,6 @@
resolved "https://registry.yarnpkg.com/@keep-network/hardhat-helpers/-/hardhat-helpers-0.6.0-pre.8.tgz#6e0722889a0132dabed5182fb32f6424ff4a77d0"
integrity sha512-51oLHceBubutBYxfVk2pLjgyhvpcDC1ahKM3V9lOiTa9lbWyY18Dza7rnM9V04kq+8DbweQRM0M9/f+K26nl9g==

"@keep-network/keep-core@>1.8.1-dev <1.8.1-goerli":
version "1.8.1-dev.0"
resolved "https://registry.yarnpkg.com/@keep-network/keep-core/-/keep-core-1.8.1-dev.0.tgz#d95864b25800214de43d8840376a68336cb12055"
integrity sha512-gFXkgN4PYOYCZ14AskL7fZHEFW5mu3BDd+TJKBuKZc1q9CgRMOK+dxpJnSctxmSH1tV+Ln9v9yqlSkfPCoiBHw==
dependencies:
"@openzeppelin/upgrades" "^2.7.2"
openzeppelin-solidity "2.4.0"

"@keep-network/prettier-config-keep@github:keep-network/prettier-config-keep":
version "0.0.1"
resolved "https://codeload.github.com/keep-network/prettier-config-keep/tar.gz/a1a333e7ac49928a0f6ed39421906dd1e46ab0f3"
Expand Down

0 comments on commit 5fb485e

Please sign in to comment.