diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index 6c3f2336f..feb5ca1e7 100644 --- a/.github/workflows/node.js.yml +++ b/.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 diff --git a/contracts/governance/Staking/interfaces/IStaking.sol b/contracts/governance/Staking/interfaces/IStaking.sol index 21b54ce74..b6abffb71 100644 --- a/contracts/governance/Staking/interfaces/IStaking.sol +++ b/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. diff --git a/contracts/governance/Staking/modules/StakingAdminModule.sol b/contracts/governance/Staking/modules/StakingAdminModule.sol index b3ba25ce9..d8688724a 100644 --- a/contracts/governance/Staking/modules/StakingAdminModule.sol +++ b/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; } } diff --git a/contracts/governance/Staking/modules/StakingStorageModule.sol b/contracts/governance/Staking/modules/StakingStorageModule.sol index 6dc468854..babbe22cd 100644 --- a/contracts/governance/Staking/modules/StakingStorageModule.sol +++ b/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; } diff --git a/contracts/governance/Staking/modules/StakingWithdrawModule.sol b/contracts/governance/Staking/modules/StakingWithdrawModule.sol index 0abb1708c..93610bf95 100644 --- a/contracts/governance/Staking/modules/StakingWithdrawModule.sol +++ b/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); } } diff --git a/contracts/governance/Staking/modules/shared/StakingStorageShared.sol b/contracts/governance/Staking/modules/shared/StakingStorageShared.sol index 7476ca1cb..4886efcd6 100644 --- a/contracts/governance/Staking/modules/shared/StakingStorageShared.sol +++ b/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. diff --git a/contracts/mockup/modules/IStakingModuleBlockMockup.sol b/contracts/mockup/modules/IStakingModuleBlockMockup.sol index 1c9f6c40e..1af531b67 100644 --- a/contracts/mockup/modules/IStakingModuleBlockMockup.sol +++ b/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. diff --git a/hardhat.config.js b/hardhat.config.js index e248775db..bf879255b 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -132,7 +132,7 @@ module.exports = { abiExporter: { clear: true, runOnCompile: true, - flat: true, + flat: false, spacing: 4, }, contractSizer: { diff --git a/tests-foundry/staking/StakingStake.t.sol b/tests-foundry/staking/StakingStake.t.sol index dffcc2eaf..d33161ee7 100644 --- a/tests-foundry/staking/StakingStake.t.sol +++ b/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) { diff --git a/tests-foundry/staking/interfaces/IStaking.sol b/tests-foundry/staking/interfaces/IStaking.sol index c50cb749b..fe6551d5b 100644 --- a/tests-foundry/staking/interfaces/IStaking.sol +++ b/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. diff --git a/tests/protocol/CloseDepositTestToken.test.js b/tests/protocol/CloseDepositTestToken.test.js index 5d7e1ba83..9d6eebdb4 100644 --- a/tests/protocol/CloseDepositTestToken.test.js +++ b/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(); diff --git a/tests/staking/ExtendedStakingTest.js b/tests/staking/ExtendedStakingTest.js index 5512bf781..567c8652f 100644 --- a/tests/staking/ExtendedStakingTest.js +++ b/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()); } }); diff --git a/tests/staking/PauseStaking.test.js b/tests/staking/PauseStaking.test.js index 7139bfdba..51c86aaea 100644 --- a/tests/staking/PauseStaking.test.js +++ b/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); diff --git a/tests/staking/StakingTest.js b/tests/staking/StakingTest.js index 7f851ff30..bb1fd6c4c 100644 --- a/tests/staking/StakingTest.js +++ b/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; diff --git a/tests/stakingRewards/StakingRewards.js b/tests/stakingRewards/StakingRewards.js index 9bcbe23a2..68b6913f4 100644 --- a/tests/stakingRewards/StakingRewards.js +++ b/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();