SOV-3666 burn penalties staking

.github/workflows/node.js.yml

@@ -49,7 +49,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Installing Packages
-        run: npm ci
+        run: npm ci --parallel=1
       - name: Checking Formatting
         run: npm run lint &&  npm run prettier && npm run prettier-check
       - name: Running Test

contracts/governance/Staking/interfaces/IStaking.sol

@@ -46,13 +46,6 @@ interface IStaking {
      * */
     function freezeUnfreeze(bool _freeze) external;
 
-    /**
-     * @notice Allows the owner to set a fee sharing proxy contract.
-     * We need it for unstaking with slashing.
-     * @param _feeSharing The address of FeeSharingCollectorProxy contract.
-     * */
-    function setFeeSharing(address _feeSharing) external;
-
     /**
      * @notice Allow the owner to set weight scaling.
      * We need it for unstaking with slashing.

contracts/governance/Staking/modules/StakingAdminModule.sol

@@ -90,16 +90,6 @@ contract StakingAdminModule is IFunctionsList, StakingShared {
         emit StakingFrozen(_freeze);
     }
 
-    /**
-     * @notice Allow the owner to set a fee sharing proxy contract.
-     * We need it for unstaking with slashing.
-     * @param _feeSharing The address of FeeSharingCollectorProxy contract.
-     * */
-    function setFeeSharing(address _feeSharing) external onlyOwner whenNotFrozen {
-        require(_feeSharing != address(0), "FeeSharing address shouldn't be 0"); // S17
-        feeSharing = IFeeSharingCollector(_feeSharing);
-    }
-
     /**
      * @notice Allow the owner to set weight scaling.
      * We need it for unstaking with slashing.
@@ -143,20 +133,19 @@ contract StakingAdminModule is IFunctionsList, StakingShared {
     }
 
     function getFunctionsList() external pure returns (bytes4[] memory) {
-        bytes4[] memory functionsList = new bytes4[](13);
+        bytes4[] memory functionsList = new bytes4[](12);
         functionsList[0] = this.addAdmin.selector;
         functionsList[1] = this.removeAdmin.selector;
         functionsList[2] = this.addPauser.selector;
         functionsList[3] = this.removePauser.selector;
         functionsList[4] = this.pauseUnpause.selector;
         functionsList[5] = this.freezeUnfreeze.selector;
-        functionsList[6] = this.setFeeSharing.selector;
-        functionsList[7] = this.setWeightScaling.selector;
-        functionsList[8] = this.setNewStakingContract.selector;
-        functionsList[9] = this.owner.selector;
-        functionsList[10] = this.isOwner.selector;
-        functionsList[11] = this.transferOwnership.selector;
-        functionsList[12] = this.migrateToNewStakingContract.selector;
+        functionsList[6] = this.setWeightScaling.selector;
+        functionsList[7] = this.setNewStakingContract.selector;
+        functionsList[8] = this.owner.selector;
+        functionsList[9] = this.isOwner.selector;
+        functionsList[10] = this.transferOwnership.selector;
+        functionsList[11] = this.migrateToNewStakingContract.selector;
         return functionsList;
     }
 }

contracts/governance/Staking/modules/StakingStorageModule.sol

@@ -67,7 +67,7 @@ contract StakingStorageModule is IFunctionsList, StakingStorageShared {
     }
 
     function getFunctionsList() external pure returns (bytes4[] memory) {
-        bytes4[] memory functionsList = new bytes4[](32);
+        bytes4[] memory functionsList = new bytes4[](31);
         functionsList[0] = this.getStorageMaxDurationToStakeTokens.selector;
         functionsList[1] = this.getStorageMaxVotingWeight.selector;
         functionsList[2] = this.getStorageWeightFactor.selector;
@@ -88,18 +88,17 @@ contract StakingStorageModule is IFunctionsList, StakingStorageShared {
         functionsList[17] = this.userStakingCheckpoints.selector;
         functionsList[18] = this.numUserStakingCheckpoints.selector;
         functionsList[19] = this.nonces.selector;
-        functionsList[20] = this.feeSharing.selector;
-        functionsList[21] = this.weightScaling.selector;
-        functionsList[22] = this.vestingWhitelist.selector;
-        functionsList[23] = this.admins.selector;
-        functionsList[24] = this.vestingCodeHashes.selector;
-        functionsList[25] = this.vestingCheckpoints.selector;
-        functionsList[26] = this.numVestingCheckpoints.selector;
-        functionsList[27] = this.vestingRegistryLogic.selector;
-        functionsList[28] = this.pausers.selector;
-        functionsList[29] = this.paused.selector;
-        functionsList[30] = this.frozen.selector;
-        functionsList[31] = this.getMaxVestingWithdrawIterations.selector;
+        functionsList[20] = this.weightScaling.selector;
+        functionsList[21] = this.vestingWhitelist.selector;
+        functionsList[22] = this.admins.selector;
+        functionsList[23] = this.vestingCodeHashes.selector;
+        functionsList[24] = this.vestingCheckpoints.selector;
+        functionsList[25] = this.numVestingCheckpoints.selector;
+        functionsList[26] = this.vestingRegistryLogic.selector;
+        functionsList[27] = this.pausers.selector;
+        functionsList[28] = this.paused.selector;
+        functionsList[29] = this.frozen.selector;
+        functionsList[30] = this.getMaxVestingWithdrawIterations.selector;
 
         return functionsList;
     }

contracts/governance/Staking/modules/StakingWithdrawModule.sol

@@ -189,11 +189,10 @@ contract StakingWithdrawModule is IFunctionsList, StakingShared, CheckpointsShar
 
             /// @dev punishedAmount can be 0 if block.timestamp are very close to 'until'
             if (punishedAmount > 0) {
-                require(address(feeSharing) != address(0), "FeeSharing address wasn't set"); // S08
-                /// @dev Move punished amount to fee sharing.
-                /// @dev Approve transfer here and let feeSharing do transfer and write checkpoint.
-                SOVToken.approve(address(feeSharing), punishedAmount);
-                feeSharing.transferTokens(address(SOVToken), punishedAmount);
+                /** Burn the punished amount by sending it to 0x1 address
+                 * @note SOV token is not allowed to be transferred to zero address
+                 */
+                SOVToken.transfer(address(0x1), punishedAmount);
             }
         }
 

contracts/governance/Staking/modules/shared/StakingStorageShared.sol

@@ -113,6 +113,8 @@ contract StakingStorageShared is Ownable {
     /*************************** Slashing *******************************/
 
     /// @notice the address of FeeSharingCollectorProxy contract, we need it for unstaking with slashing.
+    /// @notice this storage variable is no longer used since the penalties for unstaking early will be burned.
+    /// @notice we keep this storage variable here to keep the backward compatibility & prevent storage collision.
     IFeeSharingCollector public feeSharing;
 
     /// @notice used for weight scaling when unstaking with slashing.

contracts/mockup/modules/IStakingModuleBlockMockup.sol

@@ -46,13 +46,6 @@ interface IStakingModuleBlockMockup {
      * */
     function freezeUnfreeze(bool _freeze) external;
 
-    /**
-     * @notice Allows the owner to set a fee sharing proxy contract.
-     * We need it for unstaking with slashing.
-     * @param _feeSharing The address of FeeSharingCollectorProxy contract.
-     * */
-    function setFeeSharing(address _feeSharing) external;
-
     /**
      * @notice Allow the owner to set weight scaling.
      * We need it for unstaking with slashing.

hardhat.config.js

@@ -132,7 +132,7 @@ module.exports = {
     abiExporter: {
         clear: true,
         runOnCompile: true,
-        flat: true,
+        flat: false,
         spacing: 4,
     },
     contractSizer: {

tests-foundry/staking/StakingStake.t.sol

@@ -109,11 +109,8 @@ contract StakingFuzzTest is Test {
         feeSharingCollectorProxy.setImplementation(address(feeSharingCollector));
         feeSharingCollector = IFeeSharingCollector(address(feeSharingCollectorProxy));
 
-        // Set FeeSharingCollectorProxy as the fee sharing address in the staking contract
-        staking.setFeeSharing(address(feeSharingCollector));
-
         amount = 1000;
-        user = address(1);
+        user = address(100);
         user2 = address(2);
         delegatee = address(3);
 
@@ -944,7 +941,8 @@ contract StakingFuzzTest is Test {
 
             emit log_named_uint("test case 2: block.timestamp after withdraw", block.timestamp);
 
-            assertEq(sov.balanceOf(address(feeSharingCollector)), expectedSlashAmount);
+            assertEq(sov.balanceOf(address(feeSharingCollector)), 0); // we burn penalties amount rather than distributing it
+            assertEq(sov.balanceOf(address(0x1)), expectedSlashAmount); // burn by sending to 0x1 address
 
             //@test-case 3) _withdrawTS is in (lockDate, ∞) - should withdraw with no penalties
             if (block.timestamp >= lockDate) {

tests-foundry/staking/interfaces/IStaking.sol

@@ -46,13 +46,6 @@ interface IStaking {
      * */
     function freezeUnfreeze(bool _freeze) external;
 
-    /**
-     * @notice Allows the owner to set a fee sharing proxy contract.
-     * We need it for unstaking with slashing.
-     * @param _feeSharing The address of FeeSharingCollectorProxy contract.
-     * */
-    function setFeeSharing(address _feeSharing) external;
-
     /**
      * @notice Allow the owner to set weight scaling.
      * We need it for unstaking with slashing.

tests/protocol/CloseDepositTestToken.test.js

@@ -19,6 +19,7 @@ const FeesEvents = artifacts.require("FeesEvents");
 const LoanClosingsEvents = artifacts.require("LoanClosingsEvents");
 const IERC20 = artifacts.require("IERC20");
 const LockedSOVMockup = artifacts.require("LockedSOVMockup");
+const mutexUtils = require("../../deployment/helpers/reentrancy/utils");
 
 const {
     getSUSD,
@@ -59,6 +60,7 @@ contract("ProtocolCloseDeposit", (accounts) => {
     let borrower, receiver;
 
     async function deploymentAndInitFixture(_wallets, _provider) {
+        await mutexUtils.getOrDeployMutex();
         // Deploying sovrynProtocol w/ generic function from initializer.js
         SUSD = await getSUSD();
         RBTC = await getRBTC();

tests/staking/ExtendedStakingTest.js

@@ -34,6 +34,7 @@ const {
     getSovryn,
     decodeLogs,
     getSOV,
+    CONSTANTS,
 } = require("../Utils/initializer.js");
 const {
     deployAndGetIStaking,
@@ -169,7 +170,6 @@ contract("Staking", (accounts) => {
             feeSharingCollectorProxyObj.address
         );
         await sovryn.setFeesController(feeSharingCollectorProxy.address);
-        await staking.setFeeSharing(feeSharingCollectorProxy.address);
 
         await token.transfer(account1, 1000);
         await token.approve(staking.address, TOTAL_SUPPLY);
@@ -1303,8 +1303,12 @@ contract("Staking", (accounts) => {
             weight = weight * weightScaling;
             let punishedAmount = weight;
 
-            await expect(feeSharingBalance).to.be.bignumber.equal(new BN(punishedAmount));
+            /** no longer has punished amount in feeSharing */
+            await expect(feeSharingBalance).to.be.bignumber.equal(new BN(0));
             await expect(userBalance).to.be.bignumber.equal(new BN(amount - punishedAmount));
+
+            const burnRecipientBalance = await token.balanceOf(CONSTANTS.ONE_ADDRESS);
+            await expect(burnRecipientBalance.toString()).to.equal(punishedAmount.toString());
         });
 
         it("Should be able to withdraw second time", async () => {
@@ -1384,6 +1388,7 @@ contract("Staking", (accounts) => {
                     continue;
                 }
 
+                const initialBurnRecipientBalance = await token.balanceOf(CONSTANTS.ONE_ADDRESS);
                 // FeeSharingCollectorProxy
                 let feeSharingCollector = await FeeSharingCollector.new();
                 feeSharingCollectorProxyObj = await FeeSharingCollectorProxy.new(
@@ -1395,7 +1400,6 @@ contract("Staking", (accounts) => {
                     feeSharingCollectorProxyObj.address
                 );
                 await sovryn.setFeesController(feeSharingCollectorProxy.address);
-                await staking.setFeeSharing(feeSharingCollectorProxy.address);
 
                 let duration = new BN(i * TWO_WEEKS);
                 let lockedTS = await getTimeFromKickoff(duration);
@@ -1436,12 +1440,17 @@ contract("Staking", (accounts) => {
 
                 let weeks = i * 2;
 
-                expect(feeSharingBalance).to.be.bignumber.equal(new BN(punishedAmount));
+                expect(feeSharingBalance).to.be.bignumber.equal(new BN(0));
 
                 expect(returnedPunishedAmount).to.be.bignumber.equal(new BN(punishedAmount));
                 expect(returnedAvailableAmount).to.be.bignumber.equal(
                     new BN(amount).sub(returnedPunishedAmount)
                 );
+
+                const latestBurnRecipientBalance = await token.balanceOf(CONSTANTS.ONE_ADDRESS);
+                await expect(
+                    latestBurnRecipientBalance.sub(initialBurnRecipientBalance).toString()
+                ).to.equal(punishedAmount.toString());
             }
         });
 

tests/staking/PauseStaking.test.js

@@ -136,7 +136,6 @@ contract("Staking", (accounts) => {
             feeSharingCollectorProxyObj.address
         );
         await sovryn.setFeesController(feeSharingCollectorProxy.address);
-        await staking.setFeeSharing(feeSharingCollectorProxy.address);
 
         await token.transfer(account1, 1000);
         await token.approve(staking.address, TOTAL_SUPPLY);

tests/staking/StakingTest.js

@@ -116,7 +116,6 @@ contract("Staking", (accounts) => {
         feeSharingCollectorProxy = await FeeSharingCollector.at(
             feeSharingCollectorProxyObj.address
         );
-        await staking.setFeeSharing(feeSharingCollectorProxy.address);
 
         await staking.setVestingRegistry(vesting.address);
 
@@ -2428,42 +2427,6 @@ contract("Staking", (accounts) => {
         });
     });
 
-    describe("setFeeSharing", () => {
-        it("the owner may set the fee sharing contract if the contract is not frozen", async () => {
-            expect(await staking.frozen()).to.be.false; // sanity check
-
-            await staking.setFeeSharing(a2);
-            expect(await staking.feeSharing()).to.equal(a2);
-        });
-
-        it("the owner may not set the fee sharing contract if the contract is frozen", async () => {
-            await staking.freezeUnfreeze(true);
-            await expectRevert(staking.setFeeSharing(a2), "paused");
-        });
-
-        it("the owner may set the fee sharing contract if the contract is paused", async () => {
-            await staking.pauseUnpause(true);
-            await staking.setFeeSharing(a2);
-            expect(await staking.feeSharing()).to.equal(a2);
-        });
-
-        it("any other address may not set the fee sharing contract", async () => {
-            await expectRevert(staking.setFeeSharing(a2, { from: a2 }), "unauthorized");
-        });
-
-        it("it is not allowed to set the fee sharing contract to the 0 address", async () => {
-            await expectRevert(
-                staking.setFeeSharing(ZERO_ADDRESS),
-                "FeeSharing address shouldn't be 0"
-            );
-        });
-
-        it("calling feeSharing returns _feeSharing", async () => {
-            await staking.setFeeSharing(a2);
-            expect(await staking.feeSharing()).to.equal(a2);
-        });
-    });
-
     describe("setWeightScaling", () => {
         let MIN_WEIGHT_SCALING;
         let MAX_WEIGHT_SCALING;

tests/stakingRewards/StakingRewards.js

@@ -80,7 +80,6 @@ contract("StakingRewards", (accounts) => {
         feeSharingCollectorProxy = await FeeSharingCollector.at(
             feeSharingCollectorProxyObj.address
         );
-        await staking.setFeeSharing(feeSharingCollectorProxy.address);
 
         //Upgradable Vesting Registry
         vestingRegistryLogic = await VestingRegistryLogic.new();