diff --git a/contracts/integrations/test/deployment_manager_test.ts b/contracts/integrations/test/deployment_manager_test.ts index d3cc022c84..26591a3b5c 100644 --- a/contracts/integrations/test/deployment_manager_test.ts +++ b/contracts/integrations/test/deployment_manager_test.ts @@ -171,7 +171,6 @@ blockchainTests('Deployment Manager', env => { stakingConstants.DEFAULT_PARAMS.epochDurationInSeconds, stakingConstants.DEFAULT_PARAMS.rewardDelegatedStakeWeight, stakingConstants.DEFAULT_PARAMS.minimumPoolStake, - stakingConstants.DEFAULT_PARAMS.maximumMakersInPool, stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaNumerator, stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaDenominator, ]); diff --git a/contracts/integrations/test/deployment_test.ts b/contracts/integrations/test/deployment_test.ts index 895a09e6f9..fd88c20cc5 100644 --- a/contracts/integrations/test/deployment_test.ts +++ b/contracts/integrations/test/deployment_test.ts @@ -292,7 +292,6 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { stakingConstants.DEFAULT_PARAMS.epochDurationInSeconds, stakingConstants.DEFAULT_PARAMS.rewardDelegatedStakeWeight, stakingConstants.DEFAULT_PARAMS.minimumPoolStake, - stakingConstants.DEFAULT_PARAMS.maximumMakersInPool, stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaNumerator, stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaDenominator, ]); diff --git a/contracts/staking/CHANGELOG.json b/contracts/staking/CHANGELOG.json index 8c747dbbee..6ffc8f5be0 100644 --- a/contracts/staking/CHANGELOG.json +++ b/contracts/staking/CHANGELOG.json @@ -72,5 +72,26 @@ } ], "timestamp": 1570135330 + }, + { + "version": "1.1.0-beta.1", + "changes": [ + { + "note": "Removed handshake when adding maker to pool.", + "pr": 2250 + }, + { + "note": "Removed upper limit on number of makers in a pool.", + "pr": 2250 + }, + { + "note": "Removed operator permissions from makers.", + "pr": 2250 + }, + { + "note": "Pool Id starts at 1 and increases by 1.", + "pr": 2250 + } + ] } ] diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 194dc70ef9..e2135f7b87 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -173,14 +173,6 @@ contract StakingProxy is )); } - // Pools must allow at least one maker - if (maximumMakersInPool == 0) { - LibRichErrors.rrevert( - LibStakingRichErrors.InvalidParamValueError( - LibStakingRichErrors.InvalidParamValueErrorCodes.InvalidMaximumMakersInPool - )); - } - // Minimum stake must be > 1 if (minimumPoolStake < 2) { LibRichErrors.rrevert( diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 37dc7159f6..239145da75 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -67,7 +67,8 @@ contract MixinExchangeFees is } // Get the pool id of the maker address. - bytes32 poolId = getStakingPoolIdOfMaker(makerAddress); + bytes32 poolId = poolIdByMaker[makerAddress]; + // Only attribute the protocol fee payment to a pool if the maker is // registered to a pool. if (poolId == NIL_POOL_ID) { diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 147831b8c8..f109726529 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -69,12 +69,10 @@ contract MixinStorage is mapping (address => uint256) internal _withdrawableStakeByOwner; // tracking Pool Id - bytes32 public nextPoolId = INITIAL_POOL_ID; + bytes32 public nextPoolId = bytes32(uint256(1)); - // mapping from Maker Address to a struct representing the pool the maker has joined and - // whether the operator of that pool has subsequently added the maker. - // (access externally using `getStakingPoolIdOfMaker`) - mapping (address => IStructs.MakerPoolJoinStatus) internal _poolJoinedByMakerAddress; + // mapping from Maker Address to Pool Id of maker + mapping (address => bytes32) public poolIdByMaker; // mapping from Pool Id to Pool mapping (bytes32 => IStructs.Pool) internal _poolById; @@ -83,7 +81,7 @@ contract MixinStorage is mapping (bytes32 => uint256) public rewardsByPoolId; // current epoch - uint256 public currentEpoch = INITIAL_EPOCH; + uint256 public currentEpoch; // current epoch start time uint256 public currentEpochStartTimeInSeconds; @@ -108,9 +106,6 @@ contract MixinStorage is // Minimum amount of stake required in a pool to collect rewards. uint256 public minimumPoolStake; - // Maximum number of maker addresses allowed to be registered to a pool. - uint256 public maximumMakersInPool; - // Numerator for cobb douglas alpha factor. uint32 public cobbDouglasAlphaNumerator; diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index 530e37d46b..353f5dda4d 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -92,14 +92,12 @@ interface IStakingEvents { /// @param epochDurationInSeconds Minimum seconds between epochs. /// @param rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. /// @param minimumPoolStake Minimum amount of stake required in a pool to collect rewards. - /// @param maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @param cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. event ParamsSet( uint256 epochDurationInSeconds, uint32 rewardDelegatedStakeWeight, uint256 minimumPoolStake, - uint256 maximumMakersInPool, uint256 cobbDouglasAlphaNumerator, uint256 cobbDouglasAlphaDenominator ); @@ -114,28 +112,12 @@ interface IStakingEvents { uint32 operatorShare ); - /// @dev Emitted by MixinStakingPool when a new maker requests to join a pool. - /// @param poolId Unique id of pool. - /// @param makerAddress Adress of maker joining the pool. - event PendingAddMakerToPool( - bytes32 indexed poolId, - address makerAddress - ); - - /// @dev Emitted by MixinStakingPool when a new maker is added to a pool. - /// @param poolId Unique id of pool. + /// @dev Emitted by MixinStakingPool when a maker sets their pool. /// @param makerAddress Adress of maker added to pool. - event MakerAddedToStakingPool( - bytes32 indexed poolId, - address makerAddress - ); - - /// @dev Emitted by MixinStakingPool when a maker is removed from a pool. /// @param poolId Unique id of pool. - /// @param makerAddress Adress of maker added to pool. - event MakerRemovedFromStakingPool( - bytes32 indexed poolId, - address makerAddress + event MakerStakingPoolSet( + address indexed makerAddress, + bytes32 indexed poolId ); /// @dev Emitted when a staking pool's operator share is decreased. diff --git a/contracts/staking/contracts/src/interfaces/IStorage.sol b/contracts/staking/contracts/src/interfaces/IStorage.sol index 300a8bfb18..1a9a6fd478 100644 --- a/contracts/staking/contracts/src/interfaces/IStorage.sol +++ b/contracts/staking/contracts/src/interfaces/IStorage.sol @@ -90,11 +90,6 @@ interface IStorage { view returns (uint256); - function maximumMakersInPool() - external - view - returns(uint256); - function cobbDouglasAlphaNumerator() external view diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index e91c68d4d3..61ebae8ada 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -95,24 +95,13 @@ interface IStructs { uint256 denominator; } - /// @dev State for keeping track of which pool a maker has joined, and if the operator has - /// added them (see MixinStakingPool). - /// @param poolId Unique Id of staking pool. - /// @param confirmed Whether the operator has added the maker to the pool. - struct MakerPoolJoinStatus { - bytes32 poolId; - bool confirmed; - } - /// @dev Holds the metadata for a staking pool. /// @param initialized True iff the balance struct is initialized. /// @param operator of the pool. /// @param operatorShare Fraction of the total balance owned by the operator, in ppm. - /// @param numberOfMakers Number of makers in the pool. struct Pool { bool initialized; address payable operator; uint32 operatorShare; - uint32 numberOfMakers; } } diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 6f9cea14d6..4051b44bda 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -47,13 +47,6 @@ library LibStakingRichErrors { InvalidEpochDuration } - enum MakerPoolAssignmentErrorCodes { - MakerAddressAlreadyRegistered, - MakerAddressNotRegistered, - MakerAddressNotPendingAdd, - PoolIsFull - } - enum ExchangeManagerErrorCodes { ExchangeAlreadyRegistered, ExchangeNotRegistered @@ -71,13 +64,9 @@ library LibStakingRichErrors { bytes4 internal constant INSUFFICIENT_BALANCE_ERROR_SELECTOR = 0x84c8b7c9; - // bytes4(keccak256("OnlyCallableByPoolOperatorOrMakerError(address,bytes32)")) - bytes4 internal constant ONLY_CALLABLE_BY_POOL_OPERATOR_OR_MAKER_ERROR_SELECTOR = - 0x7677eb13; - - // bytes4(keccak256("MakerPoolAssignmentError(uint8,address,bytes32)")) - bytes4 internal constant MAKER_POOL_ASSIGNMENT_ERROR_SELECTOR = - 0x69945e3f; + // bytes4(keccak256("OnlyCallableByPoolOperatorError(address,bytes32)")) + bytes4 internal constant ONLY_CALLABLE_BY_POOL_OPERATOR_ERROR_SELECTOR = + 0x82ded785; // bytes4(keccak256("BlockTimestampTooLowError(uint256,uint256)")) bytes4 internal constant BLOCK_TIMESTAMP_TOO_LOW_ERROR_SELECTOR = @@ -171,7 +160,7 @@ library LibStakingRichErrors { ); } - function OnlyCallableByPoolOperatorOrMakerError( + function OnlyCallableByPoolOperatorError( address senderAddress, bytes32 poolId ) @@ -180,29 +169,12 @@ library LibStakingRichErrors { returns (bytes memory) { return abi.encodeWithSelector( - ONLY_CALLABLE_BY_POOL_OPERATOR_OR_MAKER_ERROR_SELECTOR, + ONLY_CALLABLE_BY_POOL_OPERATOR_ERROR_SELECTOR, senderAddress, poolId ); } - function MakerPoolAssignmentError( - MakerPoolAssignmentErrorCodes errorCodes, - address makerAddress, - bytes32 poolId - ) - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - MAKER_POOL_ASSIGNMENT_ERROR_SELECTOR, - errorCodes, - makerAddress, - poolId - ); - } - function BlockTimestampTooLowError( uint256 epochEndTime, uint256 currentBlockTimestamp diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index e9b76656d2..f787dc09a9 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -34,10 +34,10 @@ contract MixinStakingPool is using LibSafeMath for uint256; using LibSafeDowncast for uint256; - /// @dev Asserts that the sender is the operator of the input pool or the input maker. + /// @dev Asserts that the sender is the operator of the input pool. /// @param poolId Pool sender must be operator of. - modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId) { - _assertSenderIsPoolOperatorOrMaker(poolId); + modifier onlyStakingPoolOperator(bytes32 poolId) { + _assertSenderIsPoolOperator(poolId); _; } @@ -68,8 +68,7 @@ contract MixinStakingPool is IStructs.Pool memory pool = IStructs.Pool({ initialized: true, operator: operator, - operatorShare: operatorShare, - numberOfMakers: 0 + operatorShare: operatorShare }); _poolById[poolId] = pool; @@ -81,7 +80,7 @@ contract MixinStakingPool is emit StakingPoolCreated(poolId, operator, operatorShare); if (addOperatorAsMaker) { - _addMakerToStakingPool(poolId, operator); + setMakerStakingPool(poolId); } return poolId; @@ -92,7 +91,7 @@ contract MixinStakingPool is /// @param newOperatorShare The newly decreased percentage of any rewards owned by the operator. function decreaseStakingPoolOperatorShare(bytes32 poolId, uint32 newOperatorShare) external - onlyStakingPoolOperatorOrMaker(poolId) + onlyStakingPoolOperator(poolId) { // load pool and assert that we can decrease uint32 currentOperatorShare = _poolById[poolId].operatorShare; @@ -111,92 +110,17 @@ contract MixinStakingPool is ); } - /// @dev Allows caller to join a staking pool if already assigned. + /// @dev Allows caller to join a staking pool as a maker. /// @param poolId Unique id of pool. - function joinStakingPoolAsMaker(bytes32 poolId) - external - { - // Is the maker already in a pool? - address makerAddress = msg.sender; - IStructs.MakerPoolJoinStatus memory poolJoinStatus = _poolJoinedByMakerAddress[makerAddress]; - if (poolJoinStatus.confirmed) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - poolJoinStatus.poolId - )); - } - - poolJoinStatus.poolId = poolId; - _poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - - // Maker has joined to the pool, awaiting operator confirmation - emit PendingAddMakerToPool( - poolId, - makerAddress - ); - } - - /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. - /// Note also that the maker must have previously called joinStakingPoolAsMaker. - /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function addMakerToStakingPool( - bytes32 poolId, - address makerAddress - ) - external - onlyStakingPoolOperatorOrMaker(poolId) - { - _addMakerToStakingPool(poolId, makerAddress); - } - - /// @dev Removes a maker from a staking pool. Note that this is only callable by the pool operator or maker. - /// Note also that the maker does not have to *agree* to leave the pool; this action is - /// at the sole discretion of the pool operator. - /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function removeMakerFromStakingPool( - bytes32 poolId, - address makerAddress - ) - external - onlyStakingPoolOperatorOrMaker(poolId) - { - bytes32 makerPoolId = getStakingPoolIdOfMaker(makerAddress); - if (makerPoolId != poolId) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotRegistered, - makerAddress, - makerPoolId - )); - } - - // remove the pool and confirmation from the maker status - delete _poolJoinedByMakerAddress[makerAddress]; - _poolById[poolId].numberOfMakers = uint256(_poolById[poolId].numberOfMakers).safeSub(1).downcastToUint32(); - - // Maker has been removed from the pool` - emit MakerRemovedFromStakingPool( - poolId, - makerAddress - ); - } - - /// @dev Returns the pool id of the input maker. - /// @param makerAddress Address of maker - /// @return Pool id, nil if maker is not yet assigned to a pool. - function getStakingPoolIdOfMaker(address makerAddress) + function setMakerStakingPool(bytes32 poolId) public - view - returns (bytes32) { - IStructs.MakerPoolJoinStatus memory poolJoinStatus = _poolJoinedByMakerAddress[makerAddress]; - if (poolJoinStatus.confirmed) { - return poolJoinStatus.poolId; - } else { - return NIL_POOL_ID; - } + address maker = msg.sender; + poolIdByMaker[maker] = poolId; + emit MakerStakingPoolSet( + maker, + poolId + ); } /// @dev Returns a staking pool @@ -209,65 +133,6 @@ contract MixinStakingPool is return _poolById[poolId]; } - /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. - /// Note also that the maker must have previously called joinStakingPoolAsMaker. - /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function _addMakerToStakingPool( - bytes32 poolId, - address makerAddress - ) - internal - { - // cache pool and join status for use throughout this function - IStructs.Pool memory pool = _poolById[poolId]; - IStructs.MakerPoolJoinStatus memory poolJoinStatus = _poolJoinedByMakerAddress[makerAddress]; - - // Is the maker already in a pool? - if (poolJoinStatus.confirmed) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - poolJoinStatus.poolId - )); - } - - // Is the maker trying to join this pool; or are they the operator? - bytes32 makerPendingPoolId = poolJoinStatus.poolId; - if (makerPendingPoolId != poolId && makerAddress != pool.operator) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - makerPendingPoolId - )); - } - - // Is the pool already full? - // NOTE: If maximumMakersInPool is decreased below the number of makers currently in a pool, - // the pool will no longer be able to add more makers. - if (pool.numberOfMakers >= maximumMakersInPool) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, - makerAddress, - poolId - )); - } - - // Add maker to pool - poolJoinStatus = IStructs.MakerPoolJoinStatus({ - poolId: poolId, - confirmed: true - }); - _poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - _poolById[poolId].numberOfMakers = uint256(pool.numberOfMakers).safeAdd(1).downcastToUint32(); - - // Maker has been added to the pool - emit MakerAddedToStakingPool( - poolId, - makerAddress - ); - } - /// @dev Computes the unique id that comes after the input pool id. /// @param poolId Unique id of pool. /// @return Next pool id after input pool. @@ -276,7 +141,7 @@ contract MixinStakingPool is pure returns (bytes32) { - return bytes32(uint256(poolId).safeAdd(POOL_ID_INCREMENT_AMOUNT)); + return bytes32(uint256(poolId).safeAdd(1)); } /// @dev Reverts iff a staking pool does not exist. @@ -327,19 +192,16 @@ contract MixinStakingPool is } } - /// @dev Asserts that the sender is the operator of the input pool or the input maker. + /// @dev Asserts that the sender is the operator of the input pool. /// @param poolId Pool sender must be operator of. - function _assertSenderIsPoolOperatorOrMaker(bytes32 poolId) + function _assertSenderIsPoolOperator(bytes32 poolId) private view { address operator = _poolById[poolId].operator; - if ( - msg.sender != operator && - getStakingPoolIdOfMaker(msg.sender) != poolId - ) { + if (msg.sender != operator) { LibRichErrors.rrevert( - LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( + LibStakingRichErrors.OnlyCallableByPoolOperatorError( msg.sender, poolId ) diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index b4384feeb7..a242e8b35d 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -32,14 +32,12 @@ contract MixinParams is /// @param _epochDurationInSeconds Minimum seconds between epochs. /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. /// @param _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. - /// @param _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @param _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. function setParams( uint256 _epochDurationInSeconds, uint32 _rewardDelegatedStakeWeight, uint256 _minimumPoolStake, - uint256 _maximumMakersInPool, uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator ) @@ -50,7 +48,6 @@ contract MixinParams is _epochDurationInSeconds, _rewardDelegatedStakeWeight, _minimumPoolStake, - _maximumMakersInPool, _cobbDouglasAlphaNumerator, _cobbDouglasAlphaDenominator ); @@ -60,7 +57,6 @@ contract MixinParams is /// @return _epochDurationInSeconds Minimum seconds between epochs. /// @return _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. /// @return _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. - /// @return _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @return _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @return _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. function getParams() @@ -70,7 +66,6 @@ contract MixinParams is uint256 _epochDurationInSeconds, uint32 _rewardDelegatedStakeWeight, uint256 _minimumPoolStake, - uint256 _maximumMakersInPool, uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator ) @@ -78,7 +73,6 @@ contract MixinParams is _epochDurationInSeconds = epochDurationInSeconds; _rewardDelegatedStakeWeight = rewardDelegatedStakeWeight; _minimumPoolStake = minimumPoolStake; - _maximumMakersInPool = maximumMakersInPool; _cobbDouglasAlphaNumerator = cobbDouglasAlphaNumerator; _cobbDouglasAlphaDenominator = cobbDouglasAlphaDenominator; } @@ -96,7 +90,6 @@ contract MixinParams is 10 days, // epochDurationInSeconds (90 * PPM_DENOMINATOR) / 100, // rewardDelegatedStakeWeight 100 * MIN_TOKEN_VALUE, // minimumPoolStake - 10, // maximumMakersInPool 1, // cobbDouglasAlphaNumerator 2 // cobbDouglasAlphaDenominator ); @@ -110,7 +103,6 @@ contract MixinParams is if (epochDurationInSeconds != 0 && rewardDelegatedStakeWeight != 0 && minimumPoolStake != 0 && - maximumMakersInPool != 0 && cobbDouglasAlphaNumerator != 0 && cobbDouglasAlphaDenominator != 0 ) { @@ -126,14 +118,12 @@ contract MixinParams is /// @param _epochDurationInSeconds Minimum seconds between epochs. /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. /// @param _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. - /// @param _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @param _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. function _setParams( uint256 _epochDurationInSeconds, uint32 _rewardDelegatedStakeWeight, uint256 _minimumPoolStake, - uint256 _maximumMakersInPool, uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator ) @@ -142,7 +132,6 @@ contract MixinParams is epochDurationInSeconds = _epochDurationInSeconds; rewardDelegatedStakeWeight = _rewardDelegatedStakeWeight; minimumPoolStake = _minimumPoolStake; - maximumMakersInPool = _maximumMakersInPool; cobbDouglasAlphaNumerator = _cobbDouglasAlphaNumerator; cobbDouglasAlphaDenominator = _cobbDouglasAlphaDenominator; @@ -150,7 +139,6 @@ contract MixinParams is _epochDurationInSeconds, _rewardDelegatedStakeWeight, _minimumPoolStake, - _maximumMakersInPool, _cobbDouglasAlphaNumerator, _cobbDouglasAlphaDenominator ); diff --git a/contracts/staking/contracts/test/TestAssertStorageParams.sol b/contracts/staking/contracts/test/TestAssertStorageParams.sol index 5fb94f0763..aa5c5c62b0 100644 --- a/contracts/staking/contracts/test/TestAssertStorageParams.sol +++ b/contracts/staking/contracts/test/TestAssertStorageParams.sol @@ -29,7 +29,6 @@ contract TestAssertStorageParams is uint256 epochDurationInSeconds; uint32 rewardDelegatedStakeWeight; uint256 minimumPoolStake; - uint256 maximumMakersInPool; uint32 cobbDouglasAlphaNumerator; uint32 cobbDouglasAlphaDenominator; } @@ -48,7 +47,6 @@ contract TestAssertStorageParams is epochDurationInSeconds = params.epochDurationInSeconds; rewardDelegatedStakeWeight = params.rewardDelegatedStakeWeight; minimumPoolStake = params.minimumPoolStake; - maximumMakersInPool = params.maximumMakersInPool; cobbDouglasAlphaNumerator = params.cobbDouglasAlphaNumerator; cobbDouglasAlphaDenominator = params.cobbDouglasAlphaDenominator; _assertValidStorageParams(); diff --git a/contracts/staking/contracts/test/TestProtocolFees.sol b/contracts/staking/contracts/test/TestProtocolFees.sol index 8194e4f3eb..383eeef0cc 100644 --- a/contracts/staking/contracts/test/TestProtocolFees.sol +++ b/contracts/staking/contracts/test/TestProtocolFees.sol @@ -48,13 +48,6 @@ contract TestProtocolFees is _removeAuthorizedAddressAtIndex(msg.sender, 0); } - function addMakerToPool(bytes32 poolId, address makerAddress) - external - { - _poolJoinedByMakerAddress[makerAddress].poolId = poolId; - _poolJoinedByMakerAddress[makerAddress].confirmed = true; - } - function advanceEpoch() external { @@ -76,6 +69,7 @@ contract TestProtocolFees is for (uint256 i = 0; i < makerAddresses.length; ++i) { pool.isMaker[makerAddresses[i]] = true; _makersToTestPoolIds[makerAddresses[i]] = poolId; + poolIdByMaker[makerAddresses[i]] = poolId; } } diff --git a/contracts/staking/contracts/test/TestStorageLayoutAndConstants.sol b/contracts/staking/contracts/test/TestStorageLayoutAndConstants.sol index 21a577bc7d..67bf4088a5 100644 --- a/contracts/staking/contracts/test/TestStorageLayoutAndConstants.sol +++ b/contracts/staking/contracts/test/TestStorageLayoutAndConstants.sol @@ -183,8 +183,8 @@ contract TestStorageLayoutAndConstants is slot := add(slot, 0x1) assertSlotAndOffset( - _poolJoinedByMakerAddress_slot, - _poolJoinedByMakerAddress_offset, + poolIdByMaker_slot, + poolIdByMaker_offset, slot, offset ) @@ -270,14 +270,6 @@ contract TestStorageLayoutAndConstants is ) slot := add(slot, 0x1) - assertSlotAndOffset( - maximumMakersInPool_slot, - maximumMakersInPool_offset, - slot, - offset - ) - slot := add(slot, 0x1) - assertSlotAndOffset( cobbDouglasAlphaNumerator_slot, cobbDouglasAlphaNumerator_offset, diff --git a/contracts/staking/test/actors/maker_actor.ts b/contracts/staking/test/actors/maker_actor.ts index ecca34c9d1..da076b63be 100644 --- a/contracts/staking/test/actors/maker_actor.ts +++ b/contracts/staking/test/actors/maker_actor.ts @@ -2,28 +2,22 @@ import { expect } from '@0x/contracts-test-utils'; import { RevertError } from '@0x/utils'; import * as _ from 'lodash'; -import { constants as stakingConstants } from '../utils/constants'; - import { PoolOperatorActor } from './pool_operator_actor'; export class MakerActor extends PoolOperatorActor { - public async joinStakingPoolAsMakerAsync(poolId: string, revertError?: RevertError): Promise { - // Join pool - const txReceiptPromise = this._stakingApiWrapper.stakingContract.joinStakingPoolAsMaker.awaitTransactionSuccessAsync( + public async setMakerStakingPoolAsync(poolId: string, revertError?: RevertError): Promise { + // add maker + const txReceiptPromise = this._stakingApiWrapper.stakingContract.setMakerStakingPool.awaitTransactionSuccessAsync( poolId, - { from: this._owner }, + { from: this.getOwner() }, ); - if (revertError !== undefined) { await expect(txReceiptPromise).to.revertWith(revertError); return; } await txReceiptPromise; - - // Pool id of the maker should be nil (join would've thrown otherwise) - const poolIdOfMaker = await this._stakingApiWrapper.stakingContract.getStakingPoolIdOfMaker.callAsync( - this._owner, - ); - expect(poolIdOfMaker, 'pool id of maker').to.be.equal(stakingConstants.NIL_POOL_ID); + // check the pool id of the maker + const poolIdOfMaker = await this._stakingApiWrapper.stakingContract.poolIdByMaker.callAsync(this.getOwner()); + expect(poolIdOfMaker, 'pool id of maker').to.be.equal(poolId); } } diff --git a/contracts/staking/test/actors/pool_operator_actor.ts b/contracts/staking/test/actors/pool_operator_actor.ts index bd74e5bb17..5116ea25b1 100644 --- a/contracts/staking/test/actors/pool_operator_actor.ts +++ b/contracts/staking/test/actors/pool_operator_actor.ts @@ -2,8 +2,6 @@ import { expect } from '@0x/contracts-test-utils'; import { RevertError } from '@0x/utils'; import * as _ from 'lodash'; -import { constants as stakingConstants } from '../utils/constants'; - import { BaseActor } from './base_actor'; export class PoolOperatorActor extends BaseActor { @@ -30,60 +28,11 @@ export class PoolOperatorActor extends BaseActor { if (addOperatorAsMaker) { // check the pool id of the operator - const poolIdOfMaker = await this._stakingApiWrapper.stakingContract.getStakingPoolIdOfMaker.callAsync( - this._owner, - ); + const poolIdOfMaker = await this._stakingApiWrapper.stakingContract.poolIdByMaker.callAsync(this._owner); expect(poolIdOfMaker, 'pool id of maker').to.be.equal(poolId); - // check the number of makers in the pool - const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); - expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal(1); } return poolId; } - public async addMakerToStakingPoolAsync( - poolId: string, - makerAddress: string, - revertError?: RevertError, - ): Promise { - // add maker - const txReceiptPromise = this._stakingApiWrapper.stakingContract.addMakerToStakingPool.awaitTransactionSuccessAsync( - poolId, - makerAddress, - { from: this._owner }, - ); - if (revertError !== undefined) { - await expect(txReceiptPromise).to.revertWith(revertError); - return; - } - await txReceiptPromise; - // check the pool id of the maker - const poolIdOfMaker = await this._stakingApiWrapper.stakingContract.getStakingPoolIdOfMaker.callAsync( - makerAddress, - ); - expect(poolIdOfMaker, 'pool id of maker').to.be.equal(poolId); - } - public async removeMakerFromStakingPoolAsync( - poolId: string, - makerAddress: string, - revertError?: RevertError, - ): Promise { - // remove maker - const txReceiptPromise = this._stakingApiWrapper.stakingContract.removeMakerFromStakingPool.awaitTransactionSuccessAsync( - poolId, - makerAddress, - { from: this._owner }, - ); - if (revertError !== undefined) { - await expect(txReceiptPromise).to.revertWith(revertError); - return; - } - await txReceiptPromise; - // check the pool id of the maker - const poolIdOfMakerAfterRemoving = await this._stakingApiWrapper.stakingContract.getStakingPoolIdOfMaker.callAsync( - makerAddress, - ); - expect(poolIdOfMakerAfterRemoving, 'pool id of maker').to.be.equal(stakingConstants.NIL_POOL_ID); - } public async decreaseStakingPoolOperatorShareAsync( poolId: string, newOperatorShare: number, diff --git a/contracts/staking/test/migration_test.ts b/contracts/staking/test/migration_test.ts index e20569f8ea..7438bdeb7c 100644 --- a/contracts/staking/test/migration_test.ts +++ b/contracts/staking/test/migration_test.ts @@ -120,9 +120,8 @@ blockchainTests('Migration tests', env => { expect(params[0]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.epochDurationInSeconds); expect(params[1]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.rewardDelegatedStakeWeight); expect(params[2]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.minimumPoolStake); - expect(params[3]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.maximumMakersInPool); - expect(params[4]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaNumerator); - expect(params[5]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaDenominator); + expect(params[3]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaNumerator); + expect(params[4]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaDenominator); }); }); @@ -309,16 +308,6 @@ blockchainTests('Migration tests', env => { }); expect(tx).to.be.fulfilled(''); }); - it('reverts if max makers in pool is 0', async () => { - const tx = proxyContract.setAndAssertParams.awaitTransactionSuccessAsync({ - ...stakingConstants.DEFAULT_PARAMS, - maximumMakersInPool: constants.ZERO_AMOUNT, - }); - const expectedError = new StakingRevertErrors.InvalidParamValueError( - StakingRevertErrors.InvalidParamValueErrorCodes.InvalidMaximumMakersInPool, - ); - expect(tx).to.revertWith(expectedError); - }); }); }); // tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/staking/test/pools_test.ts b/contracts/staking/test/pools_test.ts index 7aeb808faf..eddab9095e 100644 --- a/contracts/staking/test/pools_test.ts +++ b/contracts/staking/test/pools_test.ts @@ -54,24 +54,6 @@ blockchainTests('Staking Pool Management', env => { const poolId2 = await poolOperator.createStakingPoolAsync(operatorShare, false); expect(poolId2).to.be.equal(stakingConstants.SECOND_POOL_ID); }); - it('Should fail to create several staking pools with the operator as a maker in each', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - // create pool - const poolId1 = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId1).to.be.equal(stakingConstants.INITIAL_POOL_ID); - await poolOperator.createStakingPoolAsync( - operatorShare, - true, - new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - poolOperator.getOwner(), - poolId1, - ), - ); - }); it('Should fail to create a pool with operator share > 100', async () => { // test parameters const operatorAddress = users[0]; @@ -86,48 +68,6 @@ blockchainTests('Staking Pool Management', env => { // create pool await poolOperator.createStakingPoolAsync(operatorShare, false, revertError); }); - it('should fail to add a maker to a pool if maker has not called `joinStakingPoolAsMaker` yet', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - await poolOperator.addMakerToStakingPoolAsync( - poolId, - makerAddress, - new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - constants.NULL_BYTES32, - ), - ); - }); - it('should fail to add a maker to a pool if maker has called `joinStakingPoolAsMaker` on a different pool', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const otherPoolOperator = new PoolOperatorActor(users[1], stakingApiWrapper); - const makerAddress = users[2]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - const otherPoolId = await otherPoolOperator.createStakingPoolAsync(operatorShare, true); - // create pool - await maker.joinStakingPoolAsMakerAsync(poolId); - await otherPoolOperator.addMakerToStakingPoolAsync( - otherPoolId, - makerAddress, - new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - poolId, - ), - ); - }); it('Should successfully create a pool and add owner as a maker', async () => { // test parameters const operatorAddress = users[0]; @@ -156,7 +96,7 @@ blockchainTests('Staking Pool Management', env => { ); return expect(tx).to.revertWith(expectedError); }); - it('Should successfully add/remove a maker to a pool', async () => { + it('Should successfully add a maker to a pool', async () => { // test parameters const operatorAddress = users[0]; const operatorShare = (39 / 100) * PPM_DENOMINATOR; @@ -167,70 +107,7 @@ blockchainTests('Staking Pool Management', env => { const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); // maker joins pool - await maker.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); - // operator removes maker from pool - await poolOperator.removeMakerFromStakingPoolAsync(poolId, makerAddress); - }); - it('Should successfully add/remove a maker to a pool if approved by maker', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const maker1Address = users[1]; - const maker1 = new MakerActor(maker1Address, stakingApiWrapper); - const maker2Address = users[2]; - const maker2 = new MakerActor(maker2Address, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker1.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, maker1Address); - // maker joins pool - await maker2.joinStakingPoolAsMakerAsync(poolId); - // approved maker adds new maker to pool - await maker1.addMakerToStakingPoolAsync(poolId, maker2Address); - }); - it('should fail to add a maker to a pool if called by pending maker', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker.joinStakingPoolAsMakerAsync(poolId); - await maker.addMakerToStakingPoolAsync( - poolId, - makerAddress, - new StakingRevertErrors.OnlyCallableByPoolOperatorOrMakerError(makerAddress, poolId), - ); - }); - it('should fail to add a maker to a pool if not called by operator/registered maker', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const maker1Address = users[1]; - const maker1 = new MakerActor(maker1Address, stakingApiWrapper); - const maker2Address = users[2]; - const maker2 = new MakerActor(maker2Address, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker1.joinStakingPoolAsMakerAsync(poolId); - await maker2.addMakerToStakingPoolAsync( - poolId, - maker1Address, - new StakingRevertErrors.OnlyCallableByPoolOperatorOrMakerError(maker2Address, poolId), - ); + await maker.setMakerStakingPoolAsync(poolId); }); it('Maker should successfully remove themselves from a pool', async () => { // test parameters @@ -243,28 +120,9 @@ blockchainTests('Staking Pool Management', env => { const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); // maker joins pool - await maker.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); + await maker.setMakerStakingPoolAsync(poolId); // maker removes themselves from pool - await maker.removeMakerFromStakingPoolAsync(poolId, makerAddress); - }); - it('operator can remove a maker from their pool', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); - // operator removes maker from pool - await poolOperator.removeMakerFromStakingPoolAsync(poolId, makerAddress); + await maker.setMakerStakingPoolAsync(stakingConstants.NIL_POOL_ID); }); it('Should successfully add/remove multiple makers to the same pool', async () => { // test parameters @@ -273,227 +131,13 @@ blockchainTests('Staking Pool Management', env => { const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); const makerAddresses = users.slice(1, 4); const makers = makerAddresses.map(makerAddress => new MakerActor(makerAddress, stakingApiWrapper)); - // create pool const poolId = await poolOperator.createStakingPoolAsync(operatorShare, false); expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // add makers to pool - await Promise.all(makers.map(async maker => maker.joinStakingPoolAsMakerAsync(poolId))); - await Promise.all( - makerAddresses.map(async makerAddress => poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress)), - ); - - // check the number of makers in the pool - let pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); - expect(pool.numberOfMakers, 'number of makers in pool after adding').to.be.bignumber.equal(3); - - // remove maker from pool - await Promise.all( - makerAddresses.map(async makerAddress => - poolOperator.removeMakerFromStakingPoolAsync(poolId, makerAddress), - ), - ); - - // check the number of makers in the pool - pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); - expect(pool.numberOfMakers, 'number of makers in pool after removing').to.be.bignumber.equal(0); - }); - it('should fail to remove a maker from a pool if not called by operator/registered maker', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const maker1Address = users[1]; - const maker1 = new MakerActor(maker1Address, stakingApiWrapper); - const maker2Address = users[2]; - const maker2 = new MakerActor(maker2Address, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker1.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, maker1Address); - await maker2.removeMakerFromStakingPoolAsync( - poolId, - maker1Address, - new StakingRevertErrors.OnlyCallableByPoolOperatorOrMakerError(maker2Address, poolId), - ); - }); - it('Should fail if maker already assigned to another pool tries to join', async () => { - // test parameters - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const assignedPoolOperator = new PoolOperatorActor(users[0], stakingApiWrapper); - const otherPoolOperator = new PoolOperatorActor(users[1], stakingApiWrapper); - - const makerAddress = users[2]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - - // create pools - const assignedPoolId = await assignedPoolOperator.createStakingPoolAsync(operatorShare, true); - const otherPoolId = await otherPoolOperator.createStakingPoolAsync(operatorShare, true); - expect(assignedPoolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - expect(otherPoolId).to.be.equal(stakingConstants.SECOND_POOL_ID); - - // maker joins first pool - await maker.joinStakingPoolAsMakerAsync(assignedPoolId); - // first pool operator adds maker - await assignedPoolOperator.addMakerToStakingPoolAsync(assignedPoolId, makerAddress); - - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - assignedPoolId, - ); - // second pool operator now tries to add maker - await otherPoolOperator.addMakerToStakingPoolAsync(otherPoolId, makerAddress, revertError); - }); - it('Should fail to add maker to pool if the maker has not joined any pools', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - - const makerAddress = users[1]; - - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - stakingConstants.NIL_POOL_ID, - ); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress, revertError); - }); - it('Should fail to add maker to pool if the maker joined a different pool', async () => { - // test parameters - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const assignedPoolOperator = new PoolOperatorActor(users[0], stakingApiWrapper); - const otherPoolOperator = new PoolOperatorActor(users[1], stakingApiWrapper); - - const makerAddress = users[2]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - - // create pools - const joinedPoolId = await assignedPoolOperator.createStakingPoolAsync(operatorShare, true); - const otherPoolId = await otherPoolOperator.createStakingPoolAsync(operatorShare, true); - expect(joinedPoolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - expect(otherPoolId).to.be.equal(stakingConstants.SECOND_POOL_ID); - - // maker joins first pool - await maker.joinStakingPoolAsMakerAsync(joinedPoolId); - - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - joinedPoolId, - ); - // second pool operator now tries to add maker - await otherPoolOperator.addMakerToStakingPoolAsync(otherPoolId, makerAddress, revertError); - }); - it('Should fail to add the same maker twice', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // add maker to pool - await maker.joinStakingPoolAsMakerAsync(poolId); - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - poolId, - ); - // add same maker to pool again - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress, revertError); - }); - it('Should fail to remove a maker that does not exist', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotRegistered, - makerAddress, - stakingConstants.NIL_POOL_ID, - ); - // remove non-existent maker from pool - await poolOperator.removeMakerFromStakingPoolAsync(poolId, makerAddress, revertError); - }); - it('Should fail to remove a maker when called by someone other than the pool operator or maker', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - const neitherOperatorNorMakerAddress = users[2]; - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // add maker to pool - await maker.joinStakingPoolAsMakerAsync(poolId); - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); - // try to remove the maker address from an address other than the operator - const revertError = new StakingRevertErrors.OnlyCallableByPoolOperatorOrMakerError( - neitherOperatorNorMakerAddress, - poolId, - ); - const tx = stakingApiWrapper.stakingContract.removeMakerFromStakingPool.awaitTransactionSuccessAsync( - poolId, - makerAddress, - { from: neitherOperatorNorMakerAddress }, - ); - await expect(tx).to.revertWith(revertError); - }); - it('Should fail to add a maker if the pool is full', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - - const makerAddresses = users.slice(1, stakingConstants.DEFAULT_PARAMS.maximumMakersInPool.toNumber() + 2); - const makers = makerAddresses.map(makerAddress => new MakerActor(makerAddress, stakingApiWrapper)); - - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, false); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - - // add makers to pool - await Promise.all(makers.map(async maker => maker.joinStakingPoolAsMakerAsync(poolId))); - await Promise.all( - _.initial(makerAddresses).map(async makerAddress => - poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress), - ), - ); - - // check the number of makers in the pool - const pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); - expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal( - stakingConstants.DEFAULT_PARAMS.maximumMakersInPool, - ); - - const lastMakerAddress = _.last(makerAddresses) as string; - // Try to add last maker to the pool - const revertError = new StakingRevertErrors.MakerPoolAssignmentError( - StakingRevertErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, - lastMakerAddress, - poolId, - ); - await poolOperator.addMakerToStakingPoolAsync(poolId, lastMakerAddress, revertError); + await Promise.all(makers.map(async maker => maker.setMakerStakingPoolAsync(poolId))); + // remove makers to pool + await Promise.all(makers.map(async maker => maker.setMakerStakingPoolAsync(stakingConstants.NIL_POOL_ID))); }); it('Operator should successfully decrease their share of rewards', async () => { // test parameters @@ -508,24 +152,6 @@ blockchainTests('Staking Pool Management', env => { // decrease operator share await poolOperator.decreaseStakingPoolOperatorShareAsync(poolId, operatorShare - 1); }); - it('Maker should successfuly decrease their share of rewards', async () => { - // test parameters - const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_DENOMINATOR; - const poolOperator = new PoolOperatorActor(operatorAddress, stakingApiWrapper); - const makerAddress = users[1]; - const maker = new MakerActor(makerAddress, stakingApiWrapper); - // create pool - const poolId = await poolOperator.createStakingPoolAsync(operatorShare, true); - expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID); - // maker joins pool - await maker.joinStakingPoolAsMakerAsync(poolId); - // operator adds maker to pool - await poolOperator.addMakerToStakingPoolAsync(poolId, makerAddress); - - // decrease operator share - await maker.decreaseStakingPoolOperatorShareAsync(poolId, operatorShare - 1); - }); it('Should fail if operator tries to increase their share of rewards', async () => { // test parameters const operatorAddress = users[0]; @@ -563,7 +189,7 @@ blockchainTests('Staking Pool Management', env => { // decrease operator share await poolOperator.decreaseStakingPoolOperatorShareAsync(poolId, operatorShare, revertError); }); - it('should fail to decrease operator share if not called by operator/registered maker', async () => { + it('should fail to decrease operator share if not called by operator', async () => { // test parameters const operatorAddress = users[0]; const operatorShare = (39 / 100) * PPM_DENOMINATOR; @@ -576,7 +202,7 @@ blockchainTests('Staking Pool Management', env => { await maker.decreaseStakingPoolOperatorShareAsync( poolId, operatorShare - 1, - new StakingRevertErrors.OnlyCallableByPoolOperatorOrMakerError(makerAddress, poolId), + new StakingRevertErrors.OnlyCallableByPoolOperatorError(makerAddress, poolId), ); }); }); diff --git a/contracts/staking/test/unit_tests/params_test.ts b/contracts/staking/test/unit_tests/params_test.ts index 5a0a45fe43..f24c6ec983 100644 --- a/contracts/staking/test/unit_tests/params_test.ts +++ b/contracts/staking/test/unit_tests/params_test.ts @@ -37,7 +37,6 @@ blockchainTests('Configurable Parameters unit tests', env => { new BigNumber(_params.epochDurationInSeconds), new BigNumber(_params.rewardDelegatedStakeWeight), new BigNumber(_params.minimumPoolStake), - new BigNumber(_params.maximumMakersInPool), new BigNumber(_params.cobbDouglasAlphaNumerator), new BigNumber(_params.cobbDouglasAlphaDenominator), { from }, @@ -49,7 +48,6 @@ blockchainTests('Configurable Parameters unit tests', env => { expect(event.epochDurationInSeconds).to.bignumber.eq(_params.epochDurationInSeconds); expect(event.rewardDelegatedStakeWeight).to.bignumber.eq(_params.rewardDelegatedStakeWeight); expect(event.minimumPoolStake).to.bignumber.eq(_params.minimumPoolStake); - expect(event.maximumMakersInPool).to.bignumber.eq(_params.maximumMakersInPool); expect(event.cobbDouglasAlphaNumerator).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); expect(event.cobbDouglasAlphaDenominator).to.bignumber.eq(_params.cobbDouglasAlphaDenominator); // Assert `getParams()`. @@ -57,9 +55,8 @@ blockchainTests('Configurable Parameters unit tests', env => { expect(actual[0]).to.bignumber.eq(_params.epochDurationInSeconds); expect(actual[1]).to.bignumber.eq(_params.rewardDelegatedStakeWeight); expect(actual[2]).to.bignumber.eq(_params.minimumPoolStake); - expect(actual[3]).to.bignumber.eq(_params.maximumMakersInPool); - expect(actual[4]).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); - expect(actual[5]).to.bignumber.eq(_params.cobbDouglasAlphaDenominator); + expect(actual[3]).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); + expect(actual[4]).to.bignumber.eq(_params.cobbDouglasAlphaDenominator); return receipt; } diff --git a/contracts/staking/test/unit_tests/protocol_fees_test.ts b/contracts/staking/test/unit_tests/protocol_fees_test.ts index de2f8bf29b..4da63f00a1 100644 --- a/contracts/staking/test/unit_tests/protocol_fees_test.ts +++ b/contracts/staking/test/unit_tests/protocol_fees_test.ts @@ -183,6 +183,7 @@ blockchainTests('Protocol Fees unit tests', env => { DEFAULT_PROTOCOL_FEE_PAID, { from: exchangeAddress, value: DEFAULT_PROTOCOL_FEE_PAID }, ); + assertNoWETHTransferLogs(receipt.logs); const poolFees = await getProtocolFeesAsync(poolId); expect(poolFees).to.bignumber.eq(DEFAULT_PROTOCOL_FEE_PAID); diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index c40ef7fbaa..c53f46d36c 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -114,7 +114,6 @@ export class StakingApiWrapper { new BigNumber(_params.epochDurationInSeconds), new BigNumber(_params.rewardDelegatedStakeWeight), new BigNumber(_params.minimumPoolStake), - new BigNumber(_params.maximumMakersInPool), new BigNumber(_params.cobbDouglasAlphaNumerator), new BigNumber(_params.cobbDouglasAlphaDenominator), ); @@ -135,7 +134,6 @@ export class StakingApiWrapper { 'epochDurationInSeconds', 'rewardDelegatedStakeWeight', 'minimumPoolStake', - 'maximumMakersInPool', 'cobbDouglasAlphaNumerator', 'cobbDouglasAlphaDenominator', 'wethProxyAddress', diff --git a/contracts/staking/test/utils/constants.ts b/contracts/staking/test/utils/constants.ts index 5572234a20..de50d7cb21 100644 --- a/contracts/staking/test/utils/constants.ts +++ b/contracts/staking/test/utils/constants.ts @@ -5,8 +5,8 @@ const TEN_DAYS = 10 * 24 * 60 * 60; const PPM = 10 ** 6; export const constants = { TOKEN_MULTIPLIER: testConstants.DUMMY_TOKEN_DECIMALS, - INITIAL_POOL_ID: '0x0000000000000000000000000000000100000000000000000000000000000000', - SECOND_POOL_ID: '0x0000000000000000000000000000000200000000000000000000000000000000', + INITIAL_POOL_ID: '0x0000000000000000000000000000000000000000000000000000000000000001', + SECOND_POOL_ID: '0x0000000000000000000000000000000000000000000000000000000000000002', NIL_POOL_ID: '0x0000000000000000000000000000000000000000000000000000000000000000', NIL_ADDRESS: '0x0000000000000000000000000000000000000000', INITIAL_EPOCH: new BigNumber(0), @@ -14,7 +14,6 @@ export const constants = { epochDurationInSeconds: new BigNumber(TEN_DAYS), rewardDelegatedStakeWeight: new BigNumber(PPM * 0.9), minimumPoolStake: new BigNumber(10).pow(testConstants.DUMMY_TOKEN_DECIMALS).times(100), - maximumMakersInPool: new BigNumber(10), cobbDouglasAlphaNumerator: new BigNumber(1), cobbDouglasAlphaDenominator: new BigNumber(2), }, diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index d50d62549e..6134b5072f 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -8,7 +8,6 @@ export interface StakingParams { epochDurationInSeconds: Numberish; rewardDelegatedStakeWeight: Numberish; minimumPoolStake: Numberish; - maximumMakersInPool: Numberish; cobbDouglasAlphaNumerator: Numberish; cobbDouglasAlphaDenominator: Numberish; } diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index 294c26d183..aaa6645a01 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -109,6 +109,10 @@ { "note": "Add `InvalidMinimumPoolStake` to `StakingRevertErrors.InvalidParamValueErrorCode`.", "pr": 2155 + }, + { + "note": "Renamed `OnlyCallableByPoolOperatorOrMakerError` to `OnlyCallableByPoolOperatorError`.", + "pr": 2250 } ], "timestamp": 1570135330 diff --git a/packages/order-utils/src/staking_revert_errors.ts b/packages/order-utils/src/staking_revert_errors.ts index 0ffc0ec001..5b30bd5bb5 100644 --- a/packages/order-utils/src/staking_revert_errors.ts +++ b/packages/order-utils/src/staking_revert_errors.ts @@ -61,11 +61,11 @@ export class InsufficientBalanceError extends RevertError { } } -export class OnlyCallableByPoolOperatorOrMakerError extends RevertError { +export class OnlyCallableByPoolOperatorError extends RevertError { constructor(senderAddress?: string, poolId?: string) { super( - 'OnlyCallableByPoolOperatorOrMakerError', - 'OnlyCallableByPoolOperatorOrMakerError(address senderAddress, bytes32 poolId)', + 'OnlyCallableByPoolOperatorError', + 'OnlyCallableByPoolOperatorError(address senderAddress, bytes32 poolId)', { senderAddress, poolId }, ); } @@ -194,7 +194,7 @@ const types = [ InvalidParamValueError, MakerPoolAssignmentError, OnlyCallableByExchangeError, - OnlyCallableByPoolOperatorOrMakerError, + OnlyCallableByPoolOperatorError, OnlyCallableByStakingContractError, OnlyCallableIfInCatastrophicFailureError, OnlyCallableIfNotInCatastrophicFailureError,