diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index f46ac543e1..c6e578ca50 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -42,19 +42,24 @@ contract MixinStorage is address internal stakingContract; // mapping from Owner to Amount of Active Stake - mapping (address => IStructs.DelayedBalance) internal activeStakeByOwner; + // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + mapping (address => IStructs.StoredBalance) internal activeStakeByOwner; // mapping from Owner to Amount of Inactive Stake - mapping (address => IStructs.DelayedBalance) internal inactiveStakeByOwner; + // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + mapping (address => IStructs.StoredBalance) internal inactiveStakeByOwner; // mapping from Owner to Amount Delegated - mapping (address => IStructs.DelayedBalance) internal delegatedStakeByOwner; + // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + mapping (address => IStructs.StoredBalance) internal delegatedStakeByOwner; // mapping from Owner to Pool Id to Amount Delegated - mapping (address => mapping (bytes32 => IStructs.DelayedBalance)) internal delegatedStakeToPoolByOwner; + // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + mapping (address => mapping (bytes32 => IStructs.StoredBalance)) internal delegatedStakeToPoolByOwner; // mapping from Pool Id to Amount Delegated - mapping (bytes32 => IStructs.DelayedBalance) internal delegatedStakeByPoolId; + // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + mapping (bytes32 => IStructs.StoredBalance) internal delegatedStakeByPoolId; // mapping from Owner to Amount of Withdrawable Stake mapping (address => uint256) internal withdrawableStakeByOwner; diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index ff8b06eef7..ab88860950 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -25,9 +25,9 @@ interface IStakingEvents { event MoveStake( address indexed owner, uint256 amount, - uint8 fromState, + uint8 fromStatus, bytes32 indexed fromPool, - uint8 toState, + uint8 toStatus, bytes32 indexed toProol ); diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index 1a81552a27..bacb4a89c0 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -39,7 +39,7 @@ interface IStructs { address makerAddress; } - /// @dev State for Staking Pools (see MixinStakingPool). + /// @dev Status for Staking Pools (see MixinStakingPool). /// @param operatorAddress Address of pool operator. /// @param operatorShare Portion of pool rewards owned by operator. struct Pool { @@ -47,7 +47,7 @@ interface IStructs { uint8 operatorShare; } - /// @dev State for a pool that actively traded during the current epoch. + /// @dev Status for a pool that actively traded during the current epoch. /// (see MixinExchangeFees). /// @param poolId Unique Id of staking pool. /// @param feesCollected Fees collected in ETH by this pool in the current epoch. @@ -59,11 +59,17 @@ interface IStructs { uint256 delegatedStake; } - /// @dev A delayed balance allows values to be computed - struct DelayedBalance { + /// @dev Encapsulates a balance for the current and next epochs. + /// Note that these balances may be stale if the current epoch + /// is greater than `storedAtEpoch`. + /// Always load this struct using _loadAndSyncBalance or _loadUnsyncedBalance. + /// @param current balance in the current epoch. + /// @param next balance in the next epoch. + /// @param storedAtEpoch the reference epoch for this struct. + struct StoredBalance { uint96 current; uint96 next; - uint64 lastStored; + uint64 storedAtEpoch; } /// @dev Balance struct for stake. @@ -74,18 +80,18 @@ interface IStructs { uint256 next; } - /// @dev States that stake can exist in. - enum StakeState { + /// @dev Statuses that stake can exist in. + enum StakeStatus { ACTIVE, INACTIVE, DELEGATED } - /// @dev Info used to describe a state. - /// @param state of the stake. - /// @param poolId Unique Id of pool. This is set when state=DELEGATED. - struct StakeStateInfo { - StakeState state; + /// @dev Info used to describe a status. + /// @param status of the stake. + /// @param poolId Unique Id of pool. This is set when status=DELEGATED. + struct StakeInfo { + StakeStatus status; bytes32 poolId; } diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 6c9eaa7505..b9b32f59be 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -118,17 +118,17 @@ library LibStakingRichErrors { bytes4 internal constant POOL_ALREADY_EXISTS_ERROR_SELECTOR = 0x2a5e4dcf; - // bytes4(keccak256("EthVaultNotSet()")) - bytes4 internal constant ETH_VAULT_NOT_SET = - 0xdb3f0be8; + // bytes4(keccak256("EthVaultNotSetError()")) + bytes4 internal constant ETH_VAULT_NOT_SET_ERROR_SELECTOR = + 0xa067f596; - // bytes4(keccak256("RewardVaultNotSet()")) - bytes4 internal constant REWARD_VAULT_NOT_SET = - 0xfcb260f7; + // bytes4(keccak256("RewardVaultNotSetError()")) + bytes4 internal constant REWARD_VAULT_NOT_SET_ERROR_SELECTOR = + 0xe6976d70; - // bytes4(keccak256("InvalidStakeState(uint256)")) - bytes4 internal constant INVALID_STAKE_STATE = - 0xe6586728; + // bytes4(keccak256("InvalidStakeStatusError(uint256)")) + bytes4 internal constant INVALID_STAKE_STATUS_ERROR_SELECTOR = + 0xb7161acd; // solhint-disable func-name-mixedcase function MiscalculatedRewardsError( @@ -456,34 +456,34 @@ library LibStakingRichErrors { ); } - function EthVaultNotSet() + function EthVaultNotSetError() internal pure returns (bytes memory) { return abi.encodeWithSelector( - ETH_VAULT_NOT_SET + ETH_VAULT_NOT_SET_ERROR_SELECTOR ); } - function RewardVaultNotSet() + function RewardVaultNotSetError() internal pure returns (bytes memory) { return abi.encodeWithSelector( - REWARD_VAULT_NOT_SET + REWARD_VAULT_NOT_SET_ERROR_SELECTOR ); } - function InvalidStakeState(uint256 state) + function InvalidStakeStatusError(uint256 status) internal pure returns (bytes memory) { return abi.encodeWithSelector( - INVALID_STAKE_STATE, - state + INVALID_STAKE_STATUS_ERROR_SELECTOR, + status ); } } diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index ffdd9d3f9e..761a73fcc2 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -44,7 +44,7 @@ contract MixinStake is using LibSafeMath for uint256; /// @dev Stake ZRX tokens. Tokens are deposited into the ZRX Vault. Unstake to retrieve the ZRX. - /// Stake is in the 'Active' state. + /// Stake is in the 'Active' status. /// @param amount of ZRX to stake. function stake(uint256 amount) external @@ -55,7 +55,7 @@ contract MixinStake is _depositFromOwnerIntoZrxVault(owner, amount); // mint stake - _mintBalance(activeStakeByOwner[owner], amount); + _incrementCurrentAndNextBalance(activeStakeByOwner[owner], amount); // notify emit Stake( @@ -65,7 +65,7 @@ contract MixinStake is } /// @dev Unstake. Tokens are withdrawn from the ZRX Vault and returned to the owner. - /// Stake must be in the 'inactive' state for at least one full epoch to unstake. + /// Stake must be in the 'inactive' status for at least one full epoch to unstake. /// @param amount of ZRX to unstake. function unstake(uint256 amount) external @@ -84,7 +84,7 @@ contract MixinStake is } // burn inactive stake - _burnBalance(inactiveStakeByOwner[owner], amount); + _decrementCurrentAndNextBalance(inactiveStakeByOwner[owner], amount); // update withdrawable field withdrawableStakeByOwner[owner] = currentWithdrawableStake.safeSub(amount); @@ -99,30 +99,30 @@ contract MixinStake is ); } - /// @dev Moves stake between states: 'active', 'inactive' or 'delegated'. + /// @dev Moves stake between statuses: 'active', 'inactive' or 'delegated'. /// This change comes into effect next epoch. - /// @param from state to move stake out of. - /// @param to state to move stake into. + /// @param from status to move stake out of. + /// @param to status to move stake into. /// @param amount of stake to move. function moveStake( - IStructs.StakeStateInfo calldata from, - IStructs.StakeStateInfo calldata to, + IStructs.StakeInfo calldata from, + IStructs.StakeInfo calldata to, uint256 amount ) external { - // sanity check - do nothing if moving stake between the same state - if (from.state != IStructs.StakeState.DELEGATED && from.state == to.state) { + // sanity check - do nothing if moving stake between the same status + if (from.status != IStructs.StakeStatus.DELEGATED && from.status == to.status) { return; - } else if (from.state == IStructs.StakeState.DELEGATED && from.poolId == to.poolId) { + } else if (from.status == IStructs.StakeStatus.DELEGATED && from.poolId == to.poolId) { return; } address payable owner = msg.sender; // handle delegation; this must be done before moving stake as the current - // (out-of-sync) state is used during delegation. - if (from.state == IStructs.StakeState.DELEGATED) { + // (out-of-sync) status is used during delegation. + if (from.status == IStructs.StakeStatus.DELEGATED) { _undelegateStake( from.poolId, owner, @@ -130,7 +130,7 @@ contract MixinStake is ); } - if (to.state == IStructs.StakeState.DELEGATED) { + if (to.status == IStructs.StakeStatus.DELEGATED) { _delegateStake( to.poolId, owner, @@ -138,28 +138,28 @@ contract MixinStake is ); } - // cache the current withdrawal state if we're moving out of the inactive state. - uint256 cachedWithdrawableStakeByOwner = (from.state == IStructs.StakeState.INACTIVE) + // cache the current withdrawal amoiunt, which may change if we're moving out of the inactive status. + uint256 withdrawableStake = (from.status == IStructs.StakeStatus.INACTIVE) ? getWithdrawableStake(owner) : 0; // execute move - IStructs.DelayedBalance storage fromPtr = _getBalancePtrFromState(from.state); - IStructs.DelayedBalance storage toPtr = _getBalancePtrFromState(to.state); + IStructs.StoredBalance storage fromPtr = _getBalancePtrFromStatus(owner, from.status); + IStructs.StoredBalance storage toPtr = _getBalancePtrFromStatus(owner, to.status); _moveStake(fromPtr, toPtr, amount); // update withdrawable field, if necessary - if (from.state == IStructs.StakeState.INACTIVE) { - withdrawableStakeByOwner[owner] = _computeWithdrawableStake(owner, cachedWithdrawableStakeByOwner); + if (from.status == IStructs.StakeStatus.INACTIVE) { + withdrawableStakeByOwner[owner] = _computeWithdrawableStake(owner, withdrawableStake); } // notify emit MoveStake( owner, amount, - uint8(from.state), + uint8(from.status), from.poolId, - uint8(to.state), + uint8(to.status), to.poolId ); } @@ -183,10 +183,10 @@ contract MixinStake is _syncCumulativeRewardsNeededByDelegator(poolId, currentEpoch); // increment how much stake the owner has delegated to the input pool - _incrementBalance(delegatedStakeToPoolByOwner[owner][poolId], amount); + _incrementNextBalance(delegatedStakeToPoolByOwner[owner][poolId], amount); // increment how much stake has been delegated to pool - _incrementBalance(delegatedStakeByPoolId[poolId], amount); + _incrementNextBalance(delegatedStakeByPoolId[poolId], amount); } /// @dev Un-Delegates a owners stake from a staking pool. @@ -208,33 +208,33 @@ contract MixinStake is _syncCumulativeRewardsNeededByDelegator(poolId, currentEpoch); // decrement how much stake the owner has delegated to the input pool - _decrementBalance(delegatedStakeToPoolByOwner[owner][poolId], amount); + _decrementNextBalance(delegatedStakeToPoolByOwner[owner][poolId], amount); // decrement how much stake has been delegated to pool - _decrementBalance(delegatedStakeByPoolId[poolId], amount); + _decrementNextBalance(delegatedStakeByPoolId[poolId], amount); } - /// @dev Returns a storage pointer to a user's stake in a given state. - /// @param state of user's stake to lookup. + /// @dev Returns a storage pointer to a user's stake in a given status. + /// @param owner of stake to query. + /// @param status of user's stake to lookup. /// @return a storage pointer to the corresponding stake stake - function _getBalancePtrFromState(IStructs.StakeState state) + function _getBalancePtrFromStatus(address owner, IStructs.StakeStatus status) private view - returns (IStructs.DelayedBalance storage) + returns (IStructs.StoredBalance storage) { - // lookup state - address owner = msg.sender; - if (state == IStructs.StakeState.ACTIVE) { + // lookup status + if (status == IStructs.StakeStatus.ACTIVE) { return activeStakeByOwner[owner]; - } else if (state == IStructs.StakeState.INACTIVE) { + } else if (status == IStructs.StakeStatus.INACTIVE) { return inactiveStakeByOwner[owner]; - } else if (state == IStructs.StakeState.DELEGATED) { + } else if (status == IStructs.StakeStatus.DELEGATED) { return delegatedStakeByOwner[owner]; } - // invalid state + // invalid status LibRichErrors.rrevert( - LibStakingRichErrors.InvalidStakeState(uint256(state)) + LibStakingRichErrors.InvalidStakeStatusError(uint256(status)) ); // required to compile ~ we should never hit this. diff --git a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol index 7bd82f8d5c..c063743312 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol @@ -59,7 +59,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.DelayedBalance memory storedBalance = _syncBalanceDestructive(activeStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(activeStakeByOwner[owner]); return IStructs.StakeBalance({ current: storedBalance.current, next: storedBalance.next @@ -74,7 +74,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.DelayedBalance memory storedBalance = _syncBalanceDestructive(inactiveStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(inactiveStakeByOwner[owner]); return IStructs.StakeBalance({ current: storedBalance.current, next: storedBalance.next @@ -89,8 +89,7 @@ contract MixinStakeBalances is view returns (uint256) { - uint256 cachedWithdrawableStakeByOwner = withdrawableStakeByOwner[owner]; - return _computeWithdrawableStake(owner, cachedWithdrawableStakeByOwner); + return _computeWithdrawableStake(owner, withdrawableStakeByOwner[owner]); } /// @dev Returns the stake delegated by a given owner. @@ -101,7 +100,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.DelayedBalance memory storedBalance = _syncBalanceDestructive(delegatedStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(delegatedStakeByOwner[owner]); return IStructs.StakeBalance({ current: storedBalance.current, next: storedBalance.next @@ -117,7 +116,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.DelayedBalance memory storedBalance = _syncBalanceDestructive(delegatedStakeToPoolByOwner[owner][poolId]); + IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(delegatedStakeToPoolByOwner[owner][poolId]); return IStructs.StakeBalance({ current: storedBalance.current, next: storedBalance.next @@ -132,7 +131,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.DelayedBalance memory storedBalance = _syncBalanceDestructive(delegatedStakeByPoolId[poolId]); + IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(delegatedStakeByPoolId[poolId]); return IStructs.StakeBalance({ current: storedBalance.current, next: storedBalance.next @@ -140,21 +139,21 @@ contract MixinStakeBalances is } /// @dev Returns the stake that can be withdrawn for a given owner. - /// This stake is in the "Deactive & Withdrawable" state. /// @param owner to query. + /// @param lastStoredWithdrawableStake The amount of withdrawable stake that was last stored. /// @return Withdrawable stake for owner. - function _computeWithdrawableStake(address owner, uint256 cachedWithdrawableStakeByOwner) + function _computeWithdrawableStake(address owner, uint256 lastStoredWithdrawableStake) internal view returns (uint256) { // stake cannot be withdrawn if it has been reallocated for the `next` epoch; // so the upper bound of withdrawable stake is always limited by the value of `next`. - IStructs.DelayedBalance memory storedBalance = inactiveStakeByOwner[owner]; - if (storedBalance.lastStored == currentEpoch) { - return storedBalance.next < cachedWithdrawableStakeByOwner ? storedBalance.next : cachedWithdrawableStakeByOwner; - } else if (uint256(storedBalance.lastStored).safeAdd(1) == currentEpoch) { - return storedBalance.next < storedBalance.current ? storedBalance.next : storedBalance.current; + IStructs.StoredBalance memory storedBalance = _loadUnsyncedBalance(inactiveStakeByOwner[owner]); + if (storedBalance.storedAtEpoch == currentEpoch) { + return LibSafeMath.min256(storedBalance.next, lastStoredWithdrawableStake); + } else if (uint256(storedBalance.storedAtEpoch).safeAdd(1) == currentEpoch) { + return LibSafeMath.min256(storedBalance.next, storedBalance.current); } else { return storedBalance.next; } diff --git a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol index 3b757b85c1..9500c1085d 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol @@ -36,6 +36,7 @@ contract MixinStakeStorage is { using LibSafeMath for uint256; + using LibSafeDowncast for uint256; /// @dev Moves stake between states: 'active', 'inactive' or 'delegated'. /// This change comes into effect next epoch. @@ -43,8 +44,8 @@ contract MixinStakeStorage is /// @param toPtr pointer to storage location of `to` stake. /// @param amount of stake to move. function _moveStake( - IStructs.DelayedBalance storage fromPtr, - IStructs.DelayedBalance storage toPtr, + IStructs.StoredBalance storage fromPtr, + IStructs.StoredBalance storage toPtr, uint256 amount ) internal @@ -55,8 +56,8 @@ contract MixinStakeStorage is } // load balance from storage and synchronize it - IStructs.DelayedBalance memory from = _syncBalanceDestructive(fromPtr); - IStructs.DelayedBalance memory to = _syncBalanceDestructive(toPtr); + IStructs.StoredBalance memory from = _loadAndSyncBalance(fromPtr); + IStructs.StoredBalance memory to = _loadAndSyncBalance(toPtr); // sanity check on balance if (amount > from.next) { @@ -69,90 +70,104 @@ contract MixinStakeStorage is } // move stake for next epoch - from.next = LibSafeDowncast.downcastToUint96(uint256(from.next).safeSub(amount)); - to.next = LibSafeDowncast.downcastToUint96(uint256(to.next).safeAdd(amount)); + from.next = uint256(from.next).safeSub(amount).downcastToUint96(); + to.next = uint256(to.next).safeAdd(amount).downcastToUint96(); // update state in storage _storeBalance(fromPtr, from); _storeBalance(toPtr, to); } - /// @dev Synchronizes the fields of a stored balance. + /// @dev Loads a balance from storage and synchronizes its current/next fields. /// The structs `current` field is set to `next` if the /// current epoch is greater than the epoch in which the struct /// was stored. - /// @param balance to update. This will be equal to the return value after calling. + /// @param balancePtr to load and sync. /// @return synchronized balance. - function _syncBalanceDestructive(IStructs.DelayedBalance memory balance) + function _loadAndSyncBalance(IStructs.StoredBalance storage balancePtr) internal view - returns (IStructs.DelayedBalance memory) + returns (IStructs.StoredBalance memory balance) { + // load from storage + balance = balancePtr; + // sync uint256 currentEpoch = getCurrentEpoch(); - if (currentEpoch > balance.lastStored) { - balance.lastStored = LibSafeDowncast.downcastToUint64(currentEpoch); + if (currentEpoch > balance.storedAtEpoch) { + balance.storedAtEpoch = currentEpoch.downcastToUint64(); balance.current = balance.next; } return balance; } - /// @dev Mints new value in a balance. - /// This causes both the `current` and `next` fields to increase immediately. + /// @dev Loads a balance from storage without synchronizing its fields. + /// This function exists so that developers will have to explicitly + /// communicate that they're loading a synchronized or unsynchronized balance. + /// These balances should never be accessed directly. + /// @param balancePtr to load. + /// @return unsynchronized balance. + function _loadUnsyncedBalance(IStructs.StoredBalance storage balancePtr) + internal + view + returns (IStructs.StoredBalance memory balance) + { + balance = balancePtr; + return balance; + } + + /// @dev Increments both the `current` and `next` fields. /// @param balancePtr storage pointer to balance. /// @param amount to mint. - function _mintBalance(IStructs.DelayedBalance storage balancePtr, uint256 amount) + function _incrementCurrentAndNextBalance(IStructs.StoredBalance storage balancePtr, uint256 amount) internal { // Remove stake from balance - IStructs.DelayedBalance memory balance = _syncBalanceDestructive(balancePtr); - balance.next = LibSafeDowncast.downcastToUint96(uint256(balance.next).safeAdd(amount)); - balance.current = LibSafeDowncast.downcastToUint96(uint256(balance.current).safeAdd(amount)); + IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + balance.next = uint256(balance.next).safeAdd(amount).downcastToUint96(); + balance.current = uint256(balance.current).safeAdd(amount).downcastToUint96(); // update state _storeBalance(balancePtr, balance); } - /// @dev Burns existing value in a balance. - /// This causes both the `current` and `next` fields to decrease immediately. + /// @dev Decrements both the `current` and `next` fields. /// @param balancePtr storage pointer to balance. /// @param amount to mint. - function _burnBalance(IStructs.DelayedBalance storage balancePtr, uint256 amount) + function _decrementCurrentAndNextBalance(IStructs.StoredBalance storage balancePtr, uint256 amount) internal { // Remove stake from balance - IStructs.DelayedBalance memory balance = _syncBalanceDestructive(balancePtr); - balance.next = LibSafeDowncast.downcastToUint96(uint256(balance.next).safeSub(amount)); - balance.current = LibSafeDowncast.downcastToUint96(uint256(balance.current).safeSub(amount)); + IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + balance.next = uint256(balance.next).safeSub(amount).downcastToUint96(); + balance.current = uint256(balance.current).safeSub(amount).downcastToUint96(); // update state _storeBalance(balancePtr, balance); } - /// @dev Increments a balance. - /// Ths updates the `next` field but not the `current` field. + /// @dev Increments the `next` field (but not the `current` field). /// @param balancePtr storage pointer to balance. /// @param amount to increment by. - function _incrementBalance(IStructs.DelayedBalance storage balancePtr, uint256 amount) + function _incrementNextBalance(IStructs.StoredBalance storage balancePtr, uint256 amount) internal { // Add stake to balance - IStructs.DelayedBalance memory balance = _syncBalanceDestructive(balancePtr); - balance.next = LibSafeDowncast.downcastToUint96(uint256(balance.next).safeAdd(amount)); + IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + balance.next = uint256(balance.next).safeAdd(amount).downcastToUint96(); // update state _storeBalance(balancePtr, balance); } - /// @dev Decrements a balance. - /// Ths updates the `next` field but not the `current` field. + /// @dev Decrements the `next` field (but not the `current` field). /// @param balancePtr storage pointer to balance. /// @param amount to decrement by. - function _decrementBalance(IStructs.DelayedBalance storage balancePtr, uint256 amount) + function _decrementNextBalance(IStructs.StoredBalance storage balancePtr, uint256 amount) internal { // Remove stake from balance - IStructs.DelayedBalance memory balance = _syncBalanceDestructive(balancePtr); - balance.next = LibSafeDowncast.downcastToUint96(uint256(balance.next).safeSub(amount)); + IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + balance.next = uint256(balance.next).safeSub(amount).downcastToUint96(); // update state _storeBalance(balancePtr, balance); @@ -162,14 +177,14 @@ contract MixinStakeStorage is /// @param balancePtr points to where `balance` will be stored. /// @param balance to save to storage. function _storeBalance( - IStructs.DelayedBalance storage balancePtr, - IStructs.DelayedBalance memory balance + IStructs.StoredBalance storage balancePtr, + IStructs.StoredBalance memory balance ) private { // note - this compresses into a single `sstore` when optimizations are enabled, // since the StakeBalance struct occupies a single word of storage. - balancePtr.lastStored = balance.lastStored; + balancePtr.storedAtEpoch = balance.storedAtEpoch; balancePtr.next = balance.next; balancePtr.current = balance.current; } @@ -180,9 +195,9 @@ contract MixinStakeStorage is /// @return true iff pointers are equal. function _arePointersEqual( // solhint-disable-next-line no-unused-vars - IStructs.DelayedBalance storage balancePtrA, + IStructs.StoredBalance storage balancePtrA, // solhint-disable-next-line no-unused-vars - IStructs.DelayedBalance storage balancePtrB + IStructs.StoredBalance storage balancePtrB ) private pure diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol index 1b483df935..e1afd462ca 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol @@ -73,7 +73,7 @@ contract MixinStakingPoolRewardVault is address payable rewardVaultAddress = address(uint160(address(rewardVault))); if (rewardVaultAddress == NIL_ADDRESS) { LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSet() + LibStakingRichErrors.RewardVaultNotSetError() ); } @@ -96,7 +96,7 @@ contract MixinStakingPoolRewardVault is IStakingPoolRewardVault _rewardVault = rewardVault; if (address(_rewardVault) == NIL_ADDRESS) { LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSet() + LibStakingRichErrors.RewardVaultNotSetError() ); } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index c90170e6bb..26c5c2c93a 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -48,33 +48,33 @@ contract MixinStakingPoolRewards is returns (uint256 totalReward) { // cache some values to reduce sloads - IStructs.DelayedBalance memory delegatedStake = delegatedStakeToPoolByOwner[member][poolId]; + IStructs.StoredBalance memory delegatedStake = _loadUnsyncedBalance(delegatedStakeToPoolByOwner[member][poolId]); uint256 currentEpoch = getCurrentEpoch(); // value is always zero in these two scenarios: - // 1. The current epoch is zero: delegation begins at epoch 1 - // 2. The owner's delegated is current as of this epoch: their rewards have been moved to the ETH vault. - if (currentEpoch == 0 || delegatedStake.lastStored == currentEpoch) return 0; + // 1. The owner's delegated is current as of this epoch: their rewards have been moved to the ETH vault. + // 2. The current epoch is zero: delegation begins at epoch 1 + if (delegatedStake.storedAtEpoch == currentEpoch || currentEpoch == 0) return 0; - // compute reward accumulated during `lastStored` epoch; - // the `current` balance describes how much stake was collecting rewards when `lastStored` was set. + // compute reward accumulated during `storedAtEpoch` epoch; + // the `current` balance describes how much stake was collecting rewards when `storedAtEpoch` was set. uint256 rewardsAccumulatedDuringLastStoredEpoch = (delegatedStake.current != 0) ? _computeMemberRewardOverInterval( poolId, delegatedStake.current, - delegatedStake.lastStored - 1, - delegatedStake.lastStored + delegatedStake.storedAtEpoch - 1, + delegatedStake.storedAtEpoch ) : 0; // compute the rewards accumulated by the `next` balance; - // this starts at `lastStored + 1` and goes up until the last epoch, during which + // this starts at `storedAtEpoch + 1` and goes up until the last epoch, during which // rewards were accumulated. This is at most the most recently finalized epoch (current epoch - 1). - uint256 rewardsAccumulatedAfterLastStoredEpoch = (cumulativeRewardsByPoolLastStored[poolId] > delegatedStake.lastStored) + uint256 rewardsAccumulatedAfterLastStoredEpoch = (cumulativeRewardsByPoolLastStored[poolId] > delegatedStake.storedAtEpoch) ? _computeMemberRewardOverInterval( poolId, delegatedStake.next, - delegatedStake.lastStored, + delegatedStake.storedAtEpoch, cumulativeRewardsByPoolLastStored[poolId] ) : 0; diff --git a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol index a50f578d9e..3c4a5d6dfa 100644 --- a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol +++ b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol @@ -34,7 +34,7 @@ import "../interfaces/IVaultCore.sol"; /// recoverable flaw/bug/vulnerability, simply detach the staking contract /// by setting its address to `address(0)`. Once in Catastrophic Failure Mode, /// a vault cannot be reset to normal mode; this prevents corruption of related -/// state in the staking contract. +/// status in the staking contract. contract MixinVaultCore is Authorizable, IVaultCore diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index f922585721..2a5cde28e4 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -280,7 +280,7 @@ contract StakingPoolRewardVault is IEthVault _ethVault = ethVault; if (address(_ethVault) == address(0)) { LibRichErrors.rrevert( - LibStakingRichErrors.EthVaultNotSet() + LibStakingRichErrors.EthVaultNotSetError() ); } diff --git a/contracts/staking/test/utils/Simulation.ts b/contracts/staking/test/utils/Simulation.ts index b9e4c59148..58112cdb9f 100644 --- a/contracts/staking/test/utils/Simulation.ts +++ b/contracts/staking/test/utils/Simulation.ts @@ -59,7 +59,7 @@ export class Simulation { await this._withdrawRewardForStakingPoolMemberForDelegatorsByUndelegatingAsync(this._p); } - // @TODO cleanup state and verify the staking contract is empty + // @TODO cleanup status and verify the staking contract is empty } private async _withdrawRewardForStakingPoolMemberForDelegatorsByUndelegatingAsync( diff --git a/contracts/staking/test/utils/staking_wrapper.ts b/contracts/staking/test/utils/staking_wrapper.ts index 7d3b3c2375..8d13088387 100644 --- a/contracts/staking/test/utils/staking_wrapper.ts +++ b/contracts/staking/test/utils/staking_wrapper.ts @@ -216,27 +216,27 @@ export class StakingWrapper { } public async moveStakeAsync( owner: string, - _fromState: { - state: number, + _fromStatus: { + status: number, poolId?: string }, - _toState: { - state: number, + _toStatus: { + status: number, poolId?: string }, amount: BigNumber, ): Promise { - const fromState = { - state: _fromState.state, - poolId: _fromState.poolId !== undefined ? _fromState.poolId : constants.NIL_POOL_ID + const fromStatus = { + status: _fromStatus.status, + poolId: _fromStatus.poolId !== undefined ? _fromStatus.poolId : constants.NIL_POOL_ID }; - const toState = { - state: _toState.state, - poolId: _toState.poolId !== undefined ? _toState.poolId : constants.NIL_POOL_ID + const toStatus = { + status: _toStatus.status, + poolId: _toStatus.poolId !== undefined ? _toStatus.poolId : constants.NIL_POOL_ID }; const calldata = this.getStakingContract().moveStake.getABIEncodedTransactionData( - fromState, - toState, + fromStatus, + toStatus, amount, ); const txReceipt = await this._executeTransactionAsync(calldata, owner); diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index afe3526617..3fa047fdc3 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -54,7 +54,7 @@ export interface StakeBalance { next: BigNumber, } -export enum StakeStateId { +export enum StakeStatus { ACTIVE, INACTIVE, DELEGATED diff --git a/contracts/utils/contracts/src/LibSafeMath.sol b/contracts/utils/contracts/src/LibSafeMath.sol index 4dea8bf10a..407e02eb45 100644 --- a/contracts/utils/contracts/src/LibSafeMath.sol +++ b/contracts/utils/contracts/src/LibSafeMath.sol @@ -6,8 +6,6 @@ import "./LibSafeMathRichErrors.sol"; library LibSafeMath { - using LibSafeMath for uint256; - function safeMul(uint256 a, uint256 b) internal pure diff --git a/packages/order-utils/src/exchange_revert_errors.ts b/packages/order-utils/src/exchange_revert_errors.ts index 939a56e8bb..7c61ce9cb9 100644 --- a/packages/order-utils/src/exchange_revert_errors.ts +++ b/packages/order-utils/src/exchange_revert_errors.ts @@ -260,6 +260,17 @@ export class IncompleteFillError extends RevertError { } } +export class ProxyDestinationCannotBeNilError extends RevertError { + constructor() + { + super( + 'ProxyDestinationCannotBeNilError', + 'ProxyDestinationCannotBeNilError()', + ); + } +} + + const types = [ BatchMatchOrdersError, OrderStatusError, @@ -280,6 +291,7 @@ const types = [ TransactionSignatureError, TransactionExecutionError, IncompleteFillError, + ProxyDestinationCannotBeNilError ]; // Register the types we've defined. diff --git a/packages/order-utils/src/staking_revert_errors.ts b/packages/order-utils/src/staking_revert_errors.ts index 7888c5d0d4..8f2980f5de 100644 --- a/packages/order-utils/src/staking_revert_errors.ts +++ b/packages/order-utils/src/staking_revert_errors.ts @@ -195,21 +195,21 @@ export class PoolAlreadyExistsError extends RevertError { } } -export class EthVaultNotSet extends RevertError { +export class EthVaultNotSetError extends RevertError { constructor() { - super('EthVaultNotSet', 'EthVaultNotSet()'); + super('EthVaultNotSetError', 'EthVaultNotSetError()'); } } -export class RewardVaultNotSet extends RevertError { +export class RewardVaultNotSetError extends RevertError { constructor() { - super('RewardVaultNotSet', 'RewardVaultNotSet()'); + super('RewardVaultNotSetError', 'RewardVaultNotSetError()'); } } -export class InvalidStakeState extends RevertError { - constructor(state?: BigNumber) { - super('InvalidStakeState', 'InvalidStakeState(uint256 state)', { state }); +export class InvalidStakeStatusError extends RevertError { + constructor(status?: BigNumber) { + super('InvalidStakeStatusError', 'InvalidStakeStatusError(uint256 status)', { status }); } } @@ -238,9 +238,9 @@ const types = [ AmountExceedsBalanceOfPoolError, OperatorShareMustBeBetween0And100Error, PoolAlreadyExistsError, - EthVaultNotSet, - RewardVaultNotSet, - InvalidStakeState + EthVaultNotSetError, + RewardVaultNotSetError, + InvalidStakeStatusError ]; // Register the types we've defined.