From 5fb485ec28ae6abc8544b2ad0ede1fc83b663537 Mon Sep 17 00:00:00 2001 From: Victoria Zotova Date: Tue, 28 Nov 2023 10:38:06 -0500 Subject: [PATCH] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Núñez Co-authored-by: Michalina --- .github/workflows/contracts.yml | 7 ------- .github/workflows/npm.yml | 5 ----- contracts/staking/IStaking.sol | 11 +++++++---- contracts/staking/TokenStaking.sol | 25 +++++++++++++++++-------- docs/rfc-1-staking-contract.adoc | 6 +++--- package.json | 4 ---- test/staking/TokenStaking.test.js | 9 ++++++++- yarn.lock | 8 -------- 8 files changed, 35 insertions(+), 40 deletions(-) diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index 61bdeb4c..109bdf9a 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -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 }} @@ -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 @@ -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, diff --git a/.github/workflows/npm.yml b/.github/workflows/npm.yml index 3c054b81..8c414694 100644 --- a/.github/workflows/npm.yml +++ b/.github/workflows/npm.yml @@ -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 diff --git a/contracts/staking/IStaking.sol b/contracts/staking/IStaking.sol index 8857b5ce..e15f1126 100644 --- a/contracts/staking/IStaking.sol +++ b/contracts/staking/IStaking.sol @@ -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; @@ -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. diff --git a/contracts/staking/TokenStaking.sol b/contracts/staking/TokenStaking.sol index f7aefc4a..31212028 100644 --- a/contracts/staking/TokenStaking.sol +++ b/contracts/staking/TokenStaking.sol @@ -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 @@ -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, @@ -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; @@ -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, @@ -263,6 +271,7 @@ contract TokenStaking is Initializable, IStaking, Checkpoints { increaseStakeCheckpoint(stakingProvider, amount); emit Staked( + StakeType.T, msg.sender, stakingProvider, beneficiary, @@ -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) @@ -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 diff --git a/docs/rfc-1-staking-contract.adoc b/docs/rfc-1-staking-contract.adoc index e1b944c7..fda9adc1 100644 --- a/docs/rfc-1-staking-contract.adoc +++ b/docs/rfc-1-staking-contract.adoc @@ -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 @@ -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. diff --git a/package.json b/package.json index dd7f6ff9..b21ae812 100644 --- a/package.json +++ b/package.json @@ -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" } } diff --git a/test/staking/TokenStaking.test.js b/test/staking/TokenStaking.test.js index abfe7891..bf91e4ee 100644 --- a/test/staking/TokenStaking.test.js +++ b/test/staking/TokenStaking.test.js @@ -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, @@ -288,6 +294,7 @@ describe("TokenStaking", () => { await expect(tx) .to.emit(tokenStaking, "Staked") .withArgs( + StakeTypes.T, staker.address, stakingProvider.address, beneficiary.address, @@ -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 ) } diff --git a/yarn.lock b/yarn.lock index 0f302329..e488deef 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"