Do not initialize stakingProxy in vault constructors

This commit is contained in:
Amir Bandeali
2019-09-16 09:21:08 -07:00
parent 16ebdfad9a
commit de9527ce2f
10 changed files with 93 additions and 106 deletions

View File

@@ -43,13 +43,27 @@ contract Staking is
/// @dev Initialize storage owned by this contract.
/// This function should not be called directly.
/// The StakingProxy contract will call it in `attachStakingContract()`.
function init()
/// @param _wethProxyAddress The address that can transfer WETH for fees.
/// @param _ethVaultAddress Address of the EthVault contract.
/// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract.
/// @param _zrxVaultAddress Address of the ZrxVault contract.
function init(
address _wethProxyAddress,
address _ethVaultAddress,
address payable _rewardVaultAddress,
address _zrxVaultAddress
)
external
onlyOwner
{
// DANGER! When performing upgrades, take care to modify this logic
// to prevent accidentally clearing prior state.
_initMixinScheduler();
_initMixinParams();
_initMixinParams(
_wethProxyAddress,
_ethVaultAddress,
_rewardVaultAddress,
_zrxVaultAddress
);
}
}

View File

@@ -50,7 +50,7 @@ contract StakingProxy is
MixinStorage()
{
readOnlyProxy = _readOnlyProxy;
_attachStakingContract(
_init(
_stakingContract,
_wethProxyAddress,
_ethVaultAddress,
@@ -74,27 +74,11 @@ contract StakingProxy is
/// @dev Attach a staking contract; future calls will be delegated to the staking contract.
/// Note that this is callable only by this contract's owner.
/// @param _stakingContract Address of staking contract.
/// @param _wethProxyAddress The address that can transfer WETH for fees.
/// @param _ethVaultAddress Address of the EthVault contract.
/// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract.
/// @param _zrxVaultAddress Address of the ZrxVault contract.
function attachStakingContract(
address _stakingContract,
address _wethProxyAddress,
address _ethVaultAddress,
address _rewardVaultAddress,
address _zrxVaultAddress
)
function attachStakingContract(address _stakingContract)
external
onlyOwner
{
_attachStakingContract(
_stakingContract,
_wethProxyAddress,
_ethVaultAddress,
_rewardVaultAddress,
_zrxVaultAddress
);
_attachStakingContract(_stakingContract);
}
/// @dev Detach the current staking contract.
@@ -117,7 +101,6 @@ contract StakingProxy is
} else {
stakingContract = readOnlyProxyCallee;
}
emit ReadOnlyModeSet(readOnlyMode);
}
@@ -163,11 +146,20 @@ contract StakingProxy is
/// @dev Attach a staking contract; future calls will be delegated to the staking contract.
/// @param _stakingContract Address of staking contract.
function _attachStakingContract(address _stakingContract)
private
{
stakingContract = readOnlyProxyCallee = _stakingContract;
emit StakingContractAttachedToProxy(_stakingContract);
}
/// @dev Initializes Staking contract specific state.
/// @param _stakingContract Address of staking contract.
/// @param _wethProxyAddress The address that can transfer WETH for fees.
/// @param _ethVaultAddress Address of the EthVault contract.
/// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract.
/// @param _zrxVaultAddress Address of the ZrxVault contract.
function _attachStakingContract(
function _init(
address _stakingContract,
address _wethProxyAddress,
address _ethVaultAddress,
@@ -176,9 +168,11 @@ contract StakingProxy is
)
private
{
stakingContract = readOnlyProxyCallee = _stakingContract;
// Attach the Staking contract
_attachStakingContract(_stakingContract);
// Call `init()` on the staking contract to initialize storage.
(bool didInitSucceed, bytes memory initReturnData) = _stakingContract.delegatecall(
(bool didInitSucceed, bytes memory initReturnData) = stakingContract.delegatecall(
abi.encodeWithSelector(
IStorageInit(0).init.selector,
_wethProxyAddress,
@@ -188,8 +182,9 @@ contract StakingProxy is
)
);
if (!didInitSucceed) {
assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) }
assembly {
revert(add(initReturnData, 0x20), mload(initReturnData))
}
}
emit StakingContractAttachedToProxy(_stakingContract);
}
}

View File

@@ -60,6 +60,7 @@ interface IStakingEvents {
/// @param maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool.
/// @param cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor.
/// @param cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor.
/// @param wethProxyAddress The address that can transfer WETH for fees.
/// @param ethVaultAddress Address of the EthVault contract.
/// @param rewardVaultAddress Address of the StakingPoolRewardVault contract.
/// @param zrxVaultAddress Address of the ZrxVault contract.
@@ -70,6 +71,7 @@ interface IStakingEvents {
uint256 maximumMakersInPool,
uint256 cobbDouglasAlphaNumerator,
uint256 cobbDouglasAlphaDenomintor,
address wethProxyAddress,
address ethVaultAddress,
address rewardVaultAddress,
address zrxVaultAddress

View File

@@ -41,7 +41,7 @@ interface IVaultCore {
/// @dev Sets the address of the StakingProxy contract.
/// Note that only the contract owner can call this function.
/// @param _stakingContractAddress Address of Staking proxy contract.
/// @param _stakingProxyAddress Address of Staking proxy contract.
function setStakingProxy(address payable _stakingProxyAddress)
external;

View File

@@ -52,7 +52,7 @@ contract MixinParams is
uint32 _cobbDouglasAlphaDenominator,
address _wethProxyAddress,
address _ethVaultAddress,
address _rewardVaultAddress,
address payable _rewardVaultAddress,
address _zrxVaultAddress
)
external
@@ -111,31 +111,6 @@ contract MixinParams is
_zrxVaultAddress = address(_zrxVaultAddress);
}
/// @dev Assert param values before initializing them.
/// This must be updated for each migration.
function _assertMixinParamsBeforeInit()
internal
{
// Ensure state is uninitialized.
if (epochDurationInSeconds != 0 &&
rewardDelegatedStakeWeight != 0 &&
minimumPoolStake != 0 &&
maximumMakersInPool != 0 &&
cobbDouglasAlphaNumerator != 0 &&
cobbDouglasAlphaDenomintor != 0 &&
address(wethAssetProxy) != NIL_ADDRESS &&
address(ethVault) != NIL_ADDRESS &&
address(rewardVault) != NIL_ADDRESS &&
address(zrxVault) != NIL_ADDRESS
) {
LibRichErrors.rrevert(
LibStakingRichErrors.InitializationError(
LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized
)
);
}
}
/// @dev Initialzize storage belonging to this mixin.
/// @param _wethProxyAddress The address that can transfer WETH for fees.
/// @param _ethVaultAddress Address of the EthVault contract.
@@ -144,28 +119,23 @@ contract MixinParams is
function _initMixinParams(
address _wethProxyAddress,
address _ethVaultAddress,
address _rewardVaultAddress,
address payable _rewardVaultAddress,
address _zrxVaultAddress
)
internal
{
// assert the current values before overwriting them.
_assertMixinParamsBeforeInit();
// Ensure state is uninitialized.
_assertStorageNotInitialized();
// Set up defaults.
_epochDurationInSeconds = 2 weeks;
_rewardDelegatedStakeWeight = (90 * PPM_DENOMINATOR) / 100; // 90%
_minimumPoolStake = 100 * MIN_TOKEN_VALUE; // 100 ZRX
_maximumMakersInPool = 10;
_cobbDouglasAlphaNumerator = 1;
_cobbDouglasAlphaDenomintor = 2;
// These cannot be set to variables, or we go over the stack variable limit.
_setParams(
_epochDurationInSeconds,
_rewardDelegatedStakeWeight,
_minimumPoolStake,
_maximumMakersInPool,
_cobbDouglasAlphaNumerator,
_cobbDouglasAlphaDenomintor,
2 weeks, // epochDurationInSeconds
(90 * PPM_DENOMINATOR) / 100, // rewardDelegatedStakeWeight
100 * MIN_TOKEN_VALUE, // minimumPoolStake
10, // maximumMakersInPool
1, // cobbDouglasAlphaNumerator
2, // cobbDouglasAlphaDenomintor
_wethProxyAddress,
_ethVaultAddress,
_rewardVaultAddress,
@@ -193,10 +163,10 @@ contract MixinParams is
uint32 _cobbDouglasAlphaDenominator,
address _wethProxyAddress,
address _ethVaultAddress,
address _rewardVaultAddress,
address payable _rewardVaultAddress,
address _zrxVaultAddress
)
internal
private
{
_assertValidRewardDelegatedStakeWeight(_rewardDelegatedStakeWeight);
_assertValidMaximumMakersInPool(_maximumMakersInPool);
@@ -237,6 +207,29 @@ contract MixinParams is
);
}
function _assertStorageNotInitialized()
private
view
{
if (epochDurationInSeconds != 0 &&
rewardDelegatedStakeWeight != 0 &&
minimumPoolStake != 0 &&
maximumMakersInPool != 0 &&
cobbDouglasAlphaNumerator != 0 &&
cobbDouglasAlphaDenomintor != 0 &&
address(wethAssetProxy) != NIL_ADDRESS &&
address(ethVault) != NIL_ADDRESS &&
address(rewardVault) != NIL_ADDRESS &&
address(zrxVault) != NIL_ADDRESS
) {
LibRichErrors.rrevert(
LibStakingRichErrors.InitializationError(
LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized
)
);
}
}
/// @dev Asserts that cobb douglas alpha values are valid.
/// @param numerator Numerator for cobb douglas alpha factor.
/// @param denominator Denominator for cobb douglas alpha factor.
@@ -293,7 +286,7 @@ contract MixinParams is
/// @param _ethVaultAddress Address of the EthVault contract.
/// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract.
/// @param _zrxVaultAddress Address of the ZrxVault contract.
function _assertValidVaultAddresses(
function _assertValidAddresses(
address _wethProxyAddress,
address _ethVaultAddress,
address _rewardVaultAddress,

View File

@@ -33,12 +33,6 @@ contract EthVault is
// mapping from Owner to ETH balance
mapping (address => uint256) internal _balances;
/// @dev Constructor.
constructor(address _stakingProxyAddress)
public
MixinVaultCore(_stakingProxyAddress)
{} // solhint-disable-line no-empty-blocks
/// @dev Deposit an `amount` of ETH from `owner` into the vault.
/// Note that only the Staking contract can call this.
/// Note that this can only be called when *not* in Catostrophic Failure mode.

View File

@@ -45,13 +45,6 @@ contract MixinVaultCore is
// True iff vault has been set to Catastrophic Failure Mode
bool public isInCatastrophicFailure;
/// @dev Constructor.
constructor(address _stakingProxyAddress)
public
{
_setStakingProxy(_stakingProxyAddress);
}
/// @dev Asserts that the sender (`msg.sender`) is the staking contract.
modifier onlyStakingProxy {
if (msg.sender != stakingProxyAddress) {
@@ -80,12 +73,13 @@ contract MixinVaultCore is
/// @dev Sets the address of the StakingProxy contract.
/// Note that only the contract owner can call this function.
/// @param _stakingContractAddress Address of Staking proxy contract.
/// @param _stakingProxyAddress Address of Staking proxy contract.
function setStakingProxy(address payable _stakingProxyAddress)
external
onlyOwner
{
_setStakingProxy(_stakingProxyContract);
stakingProxyAddress = _stakingProxyAddress;
emit StakingProxySet(_stakingProxyAddress);
}
/// @dev Vault enters into Catastrophic Failure Mode.
@@ -98,13 +92,4 @@ contract MixinVaultCore is
isInCatastrophicFailure = true;
emit InCatastrophicFailureMode(msg.sender);
}
/// @dev Sets the address of the StakingProxy contract.
/// @param _stakingContractAddress Address of Staking proxy contract.
function _setStakingProxy(address payable _stakingProxyAddress)
internal
{
stakingProxyAddress = _stakingProxyAddress;
emit StakingProxySet(_stakingProxyAddress);
}
}

View File

@@ -52,16 +52,13 @@ contract ZrxVault is
bytes internal _zrxAssetData;
/// @dev Constructor.
/// @param _stakingProxyAddress Address of StakingProxy contract.
/// @param _zrxProxyAddress Address of the 0x Zrx Proxy.
/// @param _zrxTokenAddress Address of the Zrx Token.
constructor(
address _stakingProxyAddress,
address _zrxProxyAddress,
address _zrxTokenAddress
)
public
MixinVaultCore(_stakingProxyAddress)
{
zrxAssetProxy = IAssetProxy(_zrxProxyAddress);
_zrxToken = IERC20Token(_zrxTokenAddress);

View File

@@ -36,8 +36,12 @@ contract TestProtocolFees is
constructor(address exchangeAddress, address wethProxyAddress) public {
validExchanges[exchangeAddress] = true;
wethAssetProxy = IAssetProxy(wethProxyAddress);
_initMixinParams();
_initMixinParams(
wethProxyAddress,
NIL_ADDRESS,
address(0),
NIL_ADDRESS
);
}
function addMakerToPool(bytes32 poolId, address makerAddress)

View File

@@ -28,10 +28,13 @@ contract TestStakingProxy is
// solhint-disable no-empty-blocks
constructor(address _stakingContract)
public
StakingProxy(_stakingContract, address(0), address(0))
StakingProxy(
_stakingContract,
NIL_ADDRESS,
NIL_ADDRESS,
NIL_ADDRESS,
NIL_ADDRESS,
NIL_ADDRESS
)
{}
function getAttachedContract() external view returns (address) {
return stakingContract;
}
}