From c68b5d7844048568c6793876614ccf2ef6e0301d Mon Sep 17 00:00:00 2001 From: mzhu25 Date: Tue, 4 May 2021 11:29:16 -0700 Subject: [PATCH] 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 | 9 +++ .../staking/contracts/src/StakingPatch.sol | 55 ++++++++++++++++ .../contracts/src/fees/MixinExchangeFees.sol | 4 ++ contracts/staking/package.json | 2 +- contracts/staking/test/artifacts.ts | 2 + contracts/staking/test/patch_mainnet_test.ts | 66 +++++++++++++++++++ contracts/staking/test/wrappers.ts | 1 + contracts/staking/tsconfig.json | 1 + 8 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 contracts/staking/contracts/src/StakingPatch.sol create mode 100644 contracts/staking/test/patch_mainnet_test.ts diff --git a/contracts/staking/CHANGELOG.json b/contracts/staking/CHANGELOG.json index 09d35151cd..f92d7bc9f7 100644 --- a/contracts/staking/CHANGELOG.json +++ b/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", diff --git a/contracts/staking/contracts/src/StakingPatch.sol b/contracts/staking/contracts/src/StakingPatch.sol new file mode 100644 index 0000000000..34cedfdf50 --- /dev/null +++ b/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; + } +} diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 63f3aacf82..6eba6d1996 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/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) { diff --git a/contracts/staking/package.json b/contracts/staking/package.json index 51c5762691..b6e3dc0dce 100644 --- a/contracts/staking/package.json +++ b/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", diff --git a/contracts/staking/test/artifacts.ts b/contracts/staking/test/artifacts.ts index 9064620477..3cfedb0547 100644 --- a/contracts/staking/test/artifacts.ts +++ b/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, diff --git a/contracts/staking/test/patch_mainnet_test.ts b/contracts/staking/test/patch_mainnet_test.ts new file mode 100644 index 0000000000..4f4e31ae51 --- /dev/null +++ b/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 diff --git a/contracts/staking/test/wrappers.ts b/contracts/staking/test/wrappers.ts index 4c8dd0c7b7..02a2aa183a 100644 --- a/contracts/staking/test/wrappers.ts +++ b/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'; diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index 5f0be618eb..fb170dab85 100644 --- a/contracts/staking/tsconfig.json +++ b/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",