Fix/staking epoch finalization (#221)

* Patch staking and recover state in constructor

* Add ganache mainnet fork test

* Add ganache mainnet fork test

* update changelog

* hardcode last pool ID

* Separate patch contract to unbreak tests

contracts/staking/CHANGELOG.json

@@ -1,4 +1,13 @@
 [
+    {
+        "version": "2.0.37",
+        "changes": [
+            {
+                "note": "Patch epoch finalization issue",
+                "pr": 221
+            }
+        ]
+    },
     {
         "timestamp": 1619596077,
         "version": "2.0.36",

contracts/staking/contracts/src/StakingPatch.sol

@@ -0,0 +1,55 @@
+/*
+
+  Copyright 2019 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.5.9;
+pragma experimental ABIEncoderV2;
+
+import "./interfaces/IStaking.sol";
+import "./sys/MixinParams.sol";
+import "./stake/MixinStake.sol";
+import "./fees/MixinExchangeFees.sol";
+
+
+contract StakingPatch is
+    IStaking,
+    MixinParams,
+    MixinStake,
+    MixinExchangeFees
+{
+    /// @dev Initialize storage owned by this contract.
+    ///      This function should not be called directly.
+    ///      The StakingProxy contract will call it in `attachStakingContract()`.
+    function init()
+        public
+        onlyAuthorized
+    {
+        uint256 currentEpoch_ = currentEpoch;
+        uint256 prevEpoch = currentEpoch_.safeSub(1);
+
+        // Patch corrupted state
+        aggregatedStatsByEpoch[prevEpoch].numPoolsToFinalize = 0;
+        this.endEpoch();
+
+        uint256 lastPoolId_ = 57;
+        for (uint256 i = 1; i <= lastPoolId_; i++) {
+            this.finalizePool(bytes32(i));
+        }
+        // Ensure that current epoch's state is not corrupted
+        aggregatedStatsByEpoch[currentEpoch_].numPoolsToFinalize = 0;
+    }
+}

contracts/staking/contracts/src/fees/MixinExchangeFees.sol

@@ -53,6 +53,10 @@ contract MixinExchangeFees is
     {
         _assertValidProtocolFee(protocolFee);
 
+        if (protocolFee == 0) {
+            return;
+        }
+
         // Transfer the protocol fee to this address if it should be paid in
         // WETH.
         if (msg.value == 0) {

contracts/staking/package.json

@@ -41,7 +41,7 @@
     "config": {
         "publicInterfaceContracts": "IStaking,IStakingEvents,IStakingProxy,IZrxVault,LibStakingRichErrors,Staking,StakingProxy,ZrxVault,TestStaking",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
-        "abis": "./test/generated-artifacts/@(IStaking|IStakingEvents|IStakingProxy|IStorage|IStorageInit|IStructs|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewards|MixinStorage|Staking|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestExchangeManager|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibSafeDowncast|TestMixinCumulativeRewards|TestMixinParams|TestMixinScheduler|TestMixinStake|TestMixinStakeBalances|TestMixinStakeStorage|TestMixinStakingPool|TestMixinStakingPoolRewards|TestProtocolFees|TestProxyDestination|TestStaking|TestStakingNoWETH|TestStakingProxy|TestStakingProxyUnit|TestStorageLayoutAndConstants|ZrxVault).json"
+        "abis": "./test/generated-artifacts/@(IStaking|IStakingEvents|IStakingProxy|IStorage|IStorageInit|IStructs|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewards|MixinStorage|Staking|StakingPatch|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestExchangeManager|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibSafeDowncast|TestMixinCumulativeRewards|TestMixinParams|TestMixinScheduler|TestMixinStake|TestMixinStakeBalances|TestMixinStakeStorage|TestMixinStakingPool|TestMixinStakingPoolRewards|TestProtocolFees|TestProxyDestination|TestStaking|TestStakingNoWETH|TestStakingProxy|TestStakingProxyUnit|TestStorageLayoutAndConstants|ZrxVault).json"
     },
     "repository": {
         "type": "git",

contracts/staking/test/artifacts.ts

@@ -33,6 +33,7 @@ import * as MixinStakingPool from '../test/generated-artifacts/MixinStakingPool.
 import * as MixinStakingPoolRewards from '../test/generated-artifacts/MixinStakingPoolRewards.json';
 import * as MixinStorage from '../test/generated-artifacts/MixinStorage.json';
 import * as Staking from '../test/generated-artifacts/Staking.json';
+import * as StakingPatch from '../test/generated-artifacts/StakingPatch.json';
 import * as StakingProxy from '../test/generated-artifacts/StakingProxy.json';
 import * as TestAssertStorageParams from '../test/generated-artifacts/TestAssertStorageParams.json';
 import * as TestCobbDouglas from '../test/generated-artifacts/TestCobbDouglas.json';
@@ -61,6 +62,7 @@ import * as TestStorageLayoutAndConstants from '../test/generated-artifacts/Test
 import * as ZrxVault from '../test/generated-artifacts/ZrxVault.json';
 export const artifacts = {
     Staking: Staking as ContractArtifact,
+    StakingPatch: StakingPatch as ContractArtifact,
     StakingProxy: StakingProxy as ContractArtifact,
     ZrxVault: ZrxVault as ContractArtifact,
     MixinExchangeFees: MixinExchangeFees as ContractArtifact,

contracts/staking/test/patch_mainnet_test.ts

@@ -0,0 +1,66 @@
+import { blockchainTests, constants, expect, filterLogsToArguments } from '@0x/contracts-test-utils';
+import { BigNumber, logUtils } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { artifacts } from './artifacts';
+import { StakingEvents, StakingPatchContract, StakingProxyContract, StakingProxyEvents } from './wrappers';
+
+const abis = _.mapValues(artifacts, v => v.compilerOutput.abi);
+const STAKING_PROXY = '0xa26e80e7dea86279c6d778d702cc413e6cffa777';
+const STAKING_OWNER = '0x7d3455421bbc5ed534a83c88fd80387dc8271392';
+const EXCHANGE_PROXY = '0xdef1c0ded9bec7f1a1670819833240f027b25eff';
+blockchainTests.configure({
+    fork: {
+        unlockedAccounts: [STAKING_OWNER, EXCHANGE_PROXY],
+    },
+});
+
+blockchainTests.fork('Staking patch mainnet fork tests', env => {
+    let stakingProxyContract: StakingProxyContract;
+    let patchedStakingPatchContract: StakingPatchContract;
+
+    before(async () => {
+        stakingProxyContract = new StakingProxyContract(STAKING_PROXY, env.provider, undefined, abis);
+        patchedStakingPatchContract = await StakingPatchContract.deployFrom0xArtifactAsync(
+            artifacts.Staking,
+            env.provider,
+            env.txDefaults,
+            artifacts,
+        );
+    });
+
+    it('Staking proxy successfully attaches to patched logic', async () => {
+        const tx = await stakingProxyContract
+            .attachStakingContract(patchedStakingPatchContract.address)
+            .awaitTransactionSuccessAsync({ from: STAKING_OWNER, gasPrice: 0 }, { shouldValidate: false });
+        expect(filterLogsToArguments(tx.logs, StakingProxyEvents.StakingContractAttachedToProxy)).to.deep.equal([
+            {
+                newStakingPatchContractAddress: patchedStakingPatchContract.address,
+            },
+        ]);
+        expect(filterLogsToArguments(tx.logs, StakingEvents.EpochEnded).length).to.equal(1);
+        expect(filterLogsToArguments(tx.logs, StakingEvents.EpochFinalized).length).to.equal(1);
+        logUtils.log(`${tx.gasUsed} gas used`);
+    });
+
+    it('Patched staking handles 0 gas protocol fees', async () => {
+        const staking = new StakingPatchContract(STAKING_PROXY, env.provider, undefined, abis);
+        const maker = '0x7b1886e49ab5433bb46f7258548092dc8cdca28b';
+        const zeroFeeTx = await staking
+            .payProtocolFee(maker, constants.NULL_ADDRESS, constants.ZERO_AMOUNT)
+            .awaitTransactionSuccessAsync({ from: EXCHANGE_PROXY, gasPrice: 0 }, { shouldValidate: false });
+        // StakingPoolEarnedRewardsInEpoch should _not_ be emitted for a zero protocol fee.
+        // tslint:disable-next-line:no-unused-expression
+        expect(filterLogsToArguments(zeroFeeTx.logs, StakingEvents.StakingPoolEarnedRewardsInEpoch)).to.be.empty;
+
+        // Coincidentally there's some ETH in the ExchangeProxy
+        const nonZeroFeeTx = await staking
+            .payProtocolFee(maker, constants.NULL_ADDRESS, new BigNumber(1))
+            .awaitTransactionSuccessAsync({ from: EXCHANGE_PROXY, gasPrice: 0, value: 1 }, { shouldValidate: false });
+        // StakingPoolEarnedRewardsInEpoch _should_ be emitted for a non-zero protocol fee.
+        expect(
+            filterLogsToArguments(nonZeroFeeTx.logs, StakingEvents.StakingPoolEarnedRewardsInEpoch),
+        ).to.have.lengthOf(1);
+    });
+});
+// tslint:enable:no-unnecessary-type-assertion

contracts/staking/test/wrappers.ts

@@ -31,6 +31,7 @@ export * from '../test/generated-wrappers/mixin_staking_pool';
 export * from '../test/generated-wrappers/mixin_staking_pool_rewards';
 export * from '../test/generated-wrappers/mixin_storage';
 export * from '../test/generated-wrappers/staking';
+export * from '../test/generated-wrappers/staking_patch';
 export * from '../test/generated-wrappers/staking_proxy';
 export * from '../test/generated-wrappers/test_assert_storage_params';
 export * from '../test/generated-wrappers/test_cobb_douglas';

contracts/staking/tsconfig.json

@@ -40,6 +40,7 @@
         "test/generated-artifacts/MixinStakingPoolRewards.json",
         "test/generated-artifacts/MixinStorage.json",
         "test/generated-artifacts/Staking.json",
+        "test/generated-artifacts/StakingPatch.json",
         "test/generated-artifacts/StakingProxy.json",
         "test/generated-artifacts/TestAssertStorageParams.json",
         "test/generated-artifacts/TestCobbDouglas.json",