Revert when attempting to delegate to/undelegate from a pool that doesn't exist

contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol

@@ -28,12 +28,10 @@ pragma solidity ^0.5.9;
 interface IStakingPoolRewardVault {
 
     /// @dev Holds the balances and other data for a staking pool.
-    /// @param initialzed True iff the balance struct is initialized.
     /// @param operatorShare Fraction of the total balance owned by the operator, in ppm.
     /// @param operatorBalance Balance in ETH of the operator.
     /// @param membersBalance Balance in ETH co-owned by the pool members.
     struct Pool {
-        bool initialized;
         uint32 operatorShare;
         uint96 operatorBalance;
         uint96 membersBalance;

contracts/staking/contracts/src/libs/LibStakingRichErrors.sol

@@ -117,9 +117,9 @@ library LibStakingRichErrors {
     bytes4 internal constant OPERATOR_SHARE_ERROR_SELECTOR =
         0x22df9597;
 
-    // bytes4(keccak256("PoolAlreadyExistsError(bytes32)"))
-    bytes4 internal constant POOL_ALREADY_EXISTS_ERROR_SELECTOR =
-        0x2a5e4dcf;
+    // bytes4(keccak256("PoolExistenceError(bytes32,bool)"))
+    bytes4 internal constant POOL_EXISTENCE_ERROR_SELECTOR =
+        0x9ae94f01;
 
     // bytes4(keccak256("EthVaultNotSetError()"))
     bytes4 internal constant ETH_VAULT_NOT_SET_ERROR_SELECTOR =
@@ -367,16 +367,18 @@ library LibStakingRichErrors {
         );
     }
 
-    function PoolAlreadyExistsError(
-        bytes32 poolId
+    function PoolExistenceError(
+        bytes32 poolId,
+        bool alreadyExists
     )
         internal
         pure
         returns (bytes memory)
     {
         return abi.encodeWithSelector(
-            POOL_ALREADY_EXISTS_ERROR_SELECTOR,
-            poolId
+            POOL_EXISTENCE_ERROR_SELECTOR,
+            poolId,
+            alreadyExists
         );
     }
 

contracts/staking/contracts/src/stake/MixinStake.sol

@@ -181,6 +181,16 @@ contract MixinStake is
     )
         private
     {
+        // revert if pool with given poolId doesn't exist
+        if (rewardVault.operatorOf(poolId) == NIL_ADDRESS) {
+            LibRichErrors.rrevert(
+                LibStakingRichErrors.PoolExistenceError(
+                    poolId,
+                    false
+                )
+            );
+        }
+
         // cache amount delegated to pool by owner
         IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner = _loadUnsyncedBalance(delegatedStakeToPoolByOwner[owner][poolId]);
 
@@ -196,7 +206,7 @@ contract MixinStake is
     }
 
     /// @dev Un-Delegates a owners stake from a staking pool.
-    /// @param poolId Id of pool to un-delegate to.
+    /// @param poolId Id of pool to un-delegate from.
     /// @param owner who wants to un-delegate.
     /// @param amount of stake to un-delegate.
     function _undelegateStake(
@@ -206,6 +216,16 @@ contract MixinStake is
     )
         private
     {
+        // revert if pool with given poolId doesn't exist
+        if (rewardVault.operatorOf(poolId) == NIL_ADDRESS) {
+            LibRichErrors.rrevert(
+                LibStakingRichErrors.PoolExistenceError(
+                    poolId,
+                    false
+                )
+            );
+        }
+
         // cache amount delegated to pool by owner
         IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner = _loadUnsyncedBalance(delegatedStakeToPoolByOwner[owner][poolId]);
 

contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol

@@ -193,16 +193,16 @@ contract StakingPoolRewardVault is
             ));
         }
 
-        // pool must not exist
+        // pool must not already exist
         Pool storage pool = poolById[poolId];
-        if (pool.initialized) {
-            LibRichErrors.rrevert(LibStakingRichErrors.PoolAlreadyExistsError(
-                poolId
+        if (pool.operatorAddress != NIL_ADDRESS) {
+            LibRichErrors.rrevert(LibStakingRichErrors.PoolExistenceError(
+                poolId,
+                true
             ));
         }
 
         // initialize pool
-        pool.initialized = true;
         pool.operatorAddress = operatorAddress;
         pool.operatorShare = operatorShare;
 

contracts/staking/test/stake_test.ts

@@ -25,6 +25,7 @@ blockchainTests.resets('Stake Statuses', env => {
     // stake actor
     let staker: StakerActor;
     let poolIds: string[];
+    let unusedPoolId: string;
     let poolOperator: string;
     // tests
     before(async () => {
@@ -45,6 +46,7 @@ blockchainTests.resets('Stake Statuses', env => {
             await stakingApiWrapper.utils.createStakingPoolAsync(poolOperator, 4, false),
             await stakingApiWrapper.utils.createStakingPoolAsync(poolOperator, 5, false),
         ]);
+        unusedPoolId = await stakingApiWrapper.stakingContract.getNextStakingPoolId.callAsync();
     });
     describe('Stake', () => {
         it('should successfully stake zero ZRX', async () => {
@@ -288,6 +290,52 @@ blockchainTests.resets('Stake Statuses', env => {
                 new StakeInfo(StakeStatus.Delegated, poolIds[1]),
             );
         });
+        it('active -> delegated (non-existent pool)', async () => {
+            const amount = toBaseUnitAmount(10);
+            await staker.stakeAsync(amount);
+            await staker.moveStakeAsync(
+                new StakeInfo(StakeStatus.Active),
+                new StakeInfo(StakeStatus.Delegated, unusedPoolId),
+                amount,
+                new StakingRevertErrors.PoolExistenceError(unusedPoolId, false),
+            );
+        });
+        it('inactive -> delegated (non-existent pool)', async () => {
+            const amount = toBaseUnitAmount(10);
+            await staker.stakeAsync(amount);
+            await staker.moveStakeAsync(new StakeInfo(StakeStatus.Active), new StakeInfo(StakeStatus.Inactive), amount);
+            await staker.moveStakeAsync(
+                new StakeInfo(StakeStatus.Inactive),
+                new StakeInfo(StakeStatus.Delegated, unusedPoolId),
+                amount,
+                new StakingRevertErrors.PoolExistenceError(unusedPoolId, false),
+            );
+        });
+        it('delegated -> delegated (non-existent pool)', async () => {
+            const amount = toBaseUnitAmount(10);
+            await staker.stakeAsync(amount);
+            await staker.moveStakeAsync(
+                new StakeInfo(StakeStatus.Active),
+                new StakeInfo(StakeStatus.Delegated, poolIds[0]),
+                amount,
+            );
+            await staker.moveStakeAsync(
+                new StakeInfo(StakeStatus.Delegated, poolIds[0]),
+                new StakeInfo(StakeStatus.Delegated, unusedPoolId),
+                amount,
+                new StakingRevertErrors.PoolExistenceError(unusedPoolId, false),
+            );
+        });
+        it('delegated (non-existent pool) -> active', async () => {
+            const amount = toBaseUnitAmount(10);
+            await staker.stakeAsync(amount);
+            await staker.moveStakeAsync(
+                new StakeInfo(StakeStatus.Delegated, unusedPoolId),
+                new StakeInfo(StakeStatus.Active),
+                amount,
+                new StakingRevertErrors.PoolExistenceError(unusedPoolId, false),
+            );
+        });
     });
     describe('Unstake', () => {
         it('should successfully unstake zero ZRX', async () => {

contracts/staking/test/vaults_test.ts

@@ -34,7 +34,7 @@ blockchainTests('Staking Vaults', env => {
             const poolId = await stakingApiWrapper.utils.createStakingPoolAsync(poolOperator, operatorShare, true);
             const notStakingContractAddress = poolOperator;
             // should fail to create pool if it already exists
-            let revertError = new StakingRevertErrors.PoolAlreadyExistsError(poolId);
+            let revertError = new StakingRevertErrors.PoolExistenceError(poolId, true);
             let tx = stakingApiWrapper.rewardVaultContract.registerStakingPool.awaitTransactionSuccessAsync(
                 poolId,
                 poolOperator,