Merge branch 'main' into add-codeowners

pdyraga · web-flow · commit 9041a3b01d82 · 2022-08-26T11:37:54.000+02:00
diff --git a/.github/workflows/contracts.yaml b/.github/workflows/contracts.yaml
@@ -7,12 +7,25 @@ on:
     branches:
       - main
   pull_request:
+  # We intend to use `workflow dispatch` in two different situations/paths:
+  # 1. If a workflow will be manually dspatched from branch named
+  #    `dapp-development`, workflow will deploy the contracts on the selected
+  #    testnet and publish them to NPM registry with `dapp-dev-<environment>`
+  #    suffix and `dapp-development-<environment>` tag. Such packages are meant
+  #    to be used locally by the team developing Threshold Token dApp and may
+  #    contain contracts that have different values from the ones used on
+  #    mainnet.
+  # 2. If a workflow will be manually dspatched from a branch which name is not
+  #    `dapp-development`, the workflow will deploy the contracts on the
+  #    selected testnet and publish them to NPM registry with `<environment>`
+  #    suffix and tag. Such packages will be used later to deploy public
+  #    Threshold Token dApp on a testnet, with contracts resembling those used
+  #    on mainnet.
   workflow_dispatch:
     inputs:
       environment:
-        description: "Environment for workflow execution"
+        description: "Environment (network) for workflow execution, e.g. `goerli`"
         required: false
-        default: "dev"
       upstream_builds:
         description: "Upstream builds"
         required: false
@@ -27,7 +40,7 @@ jobs:
     outputs:
       system-tests: ${{ steps.filter.outputs.system-tests }}
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         if: github.event_name == 'pull_request'
 
       - uses: dorny/paths-filter@v2
@@ -42,9 +55,9 @@ jobs:
   contracts-build-and-test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           cache: "yarn"
@@ -56,16 +69,19 @@ jobs:
         run: yarn build
 
       - name: Run tests
+        if: github.ref != 'refs/heads/dapp-development'
         run: yarn test
 
   contracts-system-tests:
     needs: contracts-detect-changes
-    if: needs.contracts-detect-changes.outputs.system-tests == 'true'
+    if: |
+      needs.contracts-detect-changes.outputs.system-tests == 'true'
+        && github.ref != 'refs/heads/dapp-development'
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           cache: "yarn"
@@ -84,9 +100,9 @@ jobs:
   contracts-deployment-dry-run:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           cache: "yarn"
@@ -99,12 +115,14 @@ jobs:
 
   contracts-deployment-testnet:
     needs: [contracts-build-and-test]
-    if: github.event_name == 'workflow_dispatch'
+    if: |
+      github.event_name == 'workflow_dispatch'
+        && github.ref != 'refs/heads/dapp-development'
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           cache: "yarn"
@@ -154,7 +172,7 @@ jobs:
           version: ${{ steps.npm-version-bump.outputs.version }}
 
       - name: Upload files needed for etherscan verification
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v3
         with:
           name: Artifacts for etherscan verifcation
           path: |
@@ -166,14 +184,14 @@ jobs:
     needs: [contracts-deployment-testnet]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Download files needed for etherscan verification
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v3
         with:
           name: Artifacts for etherscan verifcation
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           cache: "yarn"
@@ -196,20 +214,80 @@ jobs:
           yarn run hardhat --network ${{ github.event.inputs.environment }} \
             etherscan-verify --license GPL-3.0 --force-license
 
+  # This job is responsible for publishing packages from `dapp-development`
+  # branch, which are slightly modified to help with the process of testing some
+  # features on the Threshold Token dApp. The job starts only if workflow gets
+  # triggered by the `workflow_dispatch` event on the branch `dapp-development`.
+  contracts-dapp-development-deployment-testnet:
+    needs: [contracts-build-and-test]
+    if: |
+      github.event_name == 'workflow_dispatch'
+        && github.ref == 'refs/heads/dapp-development'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-node@v3
+        with:
+          node-version: "14.x"
+          cache: "yarn"
+          registry-url: "https://registry.npmjs.org"
+
+      - 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:
+          CHAIN_API_URL: ${{ secrets.GOERLI_ETH_HOSTNAME_HTTP }}
+          CONTRACT_OWNER_ACCOUNT_PRIVATE_KEY: ${{ secrets.DAPP_DEV_GOERLI_ETH_CONTRACT_OWNER_PRIVATE_KEY }}
+          KEEP_CONTRACT_OWNER_ACCOUNT_PRIVATE_KEY: ${{ secrets.GOERLI_KEEP_ETH_CONTRACT_OWNER_PRIVATE_KEY }}
+        run: yarn deploy --network ${{ github.event.inputs.environment }}
+
+      - name: Bump up package version
+        id: npm-version-bump
+        uses: keep-network/npm-version-bump@v2
+        with:
+          environment: dapp-dev-${{ github.event.inputs.environment }}
+          branch: ${{ github.ref }}
+          commit: ${{ github.sha }}
+
+      - name: Publish to npm
+        env:
+          NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
+        run: |
+          npm publish --access=public \
+            --network=${{ github.event.inputs.environment }} \
+            --tag dapp-development-${{ github.event.inputs.environment }}
+
+      - name: Notify CI about completion of the workflow
+        uses: keep-network/ci/actions/notify-workflow-completed@v2
+        env:
+          GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}
+        with:
+          module: "github.com/threshold-network/solidity-contracts"
+          url: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}
+          environment: ${{ github.event.inputs.environment }}
+          upstream_builds: ${{ github.event.inputs.upstream_builds }}
+          upstream_ref: dapp-development
+          version: ${{ steps.npm-version-bump.outputs.version }}
+
   contracts-slither:
     runs-on: ubuntu-latest
     if: |
       github.event_name != 'workflow_dispatch'
         && github.event_name != 'schedule'
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14"
           cache: "yarn"
 
-      - uses: actions/setup-python@v2
+      - uses: actions/setup-python@v4
         with:
           python-version: 3.8.5
 
diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml
@@ -11,9 +11,9 @@ jobs:
   code-lint-and-format:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14"
           cache: "yarn"
diff --git a/.github/workflows/npm.yml b/.github/workflows/npm.yml
@@ -16,9 +16,9 @@ jobs:
   npm-compile-publish-contracts:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
           node-version: "14.x"
           registry-url: "https://registry.npmjs.org"
diff --git a/contracts/staking/IApplication.sol b/contracts/staking/IApplication.sol
@@ -48,6 +48,9 @@ interface IApplication {
     ///         pending decrease and respond to the staking contract with
     ///         `approveAuthorizationDecrease` at its discretion. It may
     ///         happen right away but it also may happen several months later.
+    ///         If there is already a pending authorization decrease request
+    ///         for the application, and the application does not agree for
+    ///         overwriting it, the function should revert.
     function authorizationDecreaseRequested(
         address stakingProvider,
         uint96 fromAmount,
diff --git a/contracts/staking/IStaking.sol b/contracts/staking/IStaking.sol
@@ -70,7 +70,8 @@ interface IStaking {
         address authorizer
     ) external;
 
-    /// @notice Refresh Keep stake owner. Can be called only by the old owner.
+    /// @notice Refresh Keep stake owner. Can be called only by the old owner
+    ///         or their staking provider.
     /// @dev The staking provider in T staking contract is the legacy KEEP
     ///      staking contract operator.
     function refreshKeepStakeOwner(address stakingProvider) external;
@@ -109,7 +110,9 @@ interface IStaking {
     ///         it happens depends on the application. Can only be called by the
     ///         given staking provider’s authorizer. Overwrites pending
     ///         authorization decrease for the given staking provider and
-    ///         application.
+    ///         application if the application agrees for that. If the
+    ///         application does not agree for overwriting, the function
+    ///         reverts.
     /// @dev Calls `authorizationDecreaseRequested(address stakingProvider, uint256 amount)`
     ///      on the given application. See `IApplication`.
     function requestAuthorizationDecrease(
@@ -180,7 +183,6 @@ interface IStaking {
     //
 
     /// @notice Increases the amount of the stake for the given staking provider.
-    ///         Can be called only by the owner or the staking provider.
     /// @dev The sender of this transaction needs to have the amount approved to
     ///      transfer to the staking contract.
     function topUp(address stakingProvider, uint96 amount) external;
diff --git a/contracts/staking/KeepStake.sol b/contracts/staking/KeepStake.sol
@@ -28,12 +28,13 @@ interface IManagedGrant {
 ///         KEEP stakers to use their stakes in T network and weights them based
 ///         on KEEP<>T token ratio. KEEP stake owner is cached in T staking
 ///         contract and used to restrict access to all functions only owner or
-///         operator should call. To cache KEEP staking contract in T staking
-///         contract, it fitst needs to resolve the owner. Resolving liquid
-///         KEEP stake owner is easy. Resolving token grant stake owner is
-///         complicated and not possible to do on-chain from a contract external
-///         to KEEP TokenStaking contract. Keep TokenStaking knows the grant ID
-///         but does not expose it externally.
+///         operator should call. To cache KEEP stake owner in T staking
+///         contract, T staking contract first needs to resolve the owner.
+///
+///         Resolving liquid KEEP stake owner is easy. Resolving token grant
+///         stake owner is complicated and not possible to do on-chain from
+///         a contract external to KEEP TokenStaking contract. Keep TokenStaking
+///         knows the grant ID but does not expose it externally.
 ///
 ///         KeepStake contract addresses this problem by exposing
 ///         operator-owner mappings snapshotted off-chain based on events and
diff --git a/contracts/staking/TokenStaking.sol b/contracts/staking/TokenStaking.sol
@@ -283,7 +283,10 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
             createdAt == 0 && stakingProviderStruct.owner == address(0),
             "Provider is already in use"
         );
-        require(amount > minTStakeAmount, "Amount is less than minimum");
+        require(
+            amount > 0 && amount >= minTStakeAmount,
+            "Amount is less than minimum"
+        );
         stakingProviderStruct.owner = msg.sender;
         stakingProviderStruct.authorizer = authorizer;
         stakingProviderStruct.beneficiary = beneficiary;
@@ -397,17 +400,18 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
         );
     }
 
-    /// @notice Refresh Keep stake owner. Can be called only by the old owner.
+    /// @notice Refresh Keep stake owner. Can be called only by the old owner
+    ///         or their staking provider.
     /// @dev The staking provider in T staking contract is the legacy KEEP
     ///      staking contract operator.
-    function refreshKeepStakeOwner(address stakingProvider) external override {
+    function refreshKeepStakeOwner(address stakingProvider)
+        external
+        override
+        onlyOwnerOrStakingProvider(stakingProvider)
+    {
         StakingProviderInfo storage stakingProviderStruct = stakingProviders[
             stakingProvider
         ];
-        require(
-            stakingProviderStruct.owner == msg.sender,
-            "Caller is not owner"
-        );
         address newOwner = keepStake.resolveOwner(stakingProvider);
 
         emit OwnerRefreshed(
@@ -713,14 +717,13 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
     //
 
     /// @notice Increases the amount of the stake for the given staking provider.
-    ///         Can be called only by the owner or the staking provider.
     /// @dev The sender of this transaction needs to have the amount approved to
     ///      transfer to the staking contract.
-    function topUp(address stakingProvider, uint96 amount)
-        external
-        override
-        onlyOwnerOrStakingProvider(stakingProvider)
-    {
+    function topUp(address stakingProvider, uint96 amount) external override {
+        require(
+            stakingProviders[stakingProvider].owner != address(0),
+            "Nothing to top-up"
+        );
         require(amount > 0, "Parameters must be specified");
         StakingProviderInfo storage stakingProviderStruct = stakingProviders[
             stakingProvider
@@ -1281,7 +1284,9 @@ contract TokenStaking is Initializable, IStaking, Checkpoints {
     ///         it happens depends on the application. Can only be called by the
     ///         given staking provider’s authorizer. Overwrites pending
     ///         authorization decrease for the given staking provider and
-    ///         application.
+    ///         application if the application agrees for that. If the
+    ///         application does not agree for overwriting, the function
+    ///         reverts.
     /// @dev Calls `authorizationDecreaseRequested` callback on the given
     ///      application. See `IApplication`.
     function requestAuthorizationDecrease(
diff --git a/docs/rfc-1-staking-contract.adoc b/docs/rfc-1-staking-contract.adoc
@@ -389,11 +389,11 @@ protect against DoSing slashing queue. Can only be called by the governance.
 
 === Stake top-up
 
-==== `topUp(address stakingProvider, uint96 amount) external onlyOwnerOrStakingProvider(stakingProvider)`
+==== `topUp(address stakingProvider, uint96 amount) external`
 
 Increases the amount of the stake for the given staking provider. The sender of this
 transaction needs to have the amount approved to transfer to the staking
-contract. Can be called only by the owner or staking provider.
+contract.
 
 ==== `topUpKeep(address stakingProvider) external onlyOwnerOrStakingProvider(stakingProvider)`
 
diff --git a/package.json b/package.json
@@ -45,11 +45,11 @@
     "eslint": "^7.27.0",
     "eslint-config-keep": "github:keep-network/eslint-config-keep#0c27ade",
     "ethereum-waffle": "^3.4.0",
-    "ethers": "^5.4.1",
-    "hardhat": "^2.6.8",
-    "hardhat-contract-sizer": "^2.1.1",
-    "hardhat-deploy": "^0.9.4",
-    "hardhat-gas-reporter": "^1.0.4",
+    "ethers": "^5.5.3",
+    "hardhat": "^2.8.3",
+    "hardhat-contract-sizer": "^2.5.0",
+    "hardhat-deploy": "^0.9.27",
+    "hardhat-gas-reporter": "^1.0.6",
     "prettier": "^2.3.2",
     "prettier-plugin-sh": "^0.7.1",
     "prettier-plugin-solidity": "^1.0.0-beta.14 ",
diff --git a/test/staking/TokenStaking.test.js b/test/staking/TokenStaking.test.js
diff --git a/yarn.lock b/yarn.lock
