From 27d9f47e5f6d3efbf5c8492b2774ea2199288e65 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Mon, 20 Jul 2020 18:23:00 -0700 Subject: [PATCH 01/15] WIP + FUnctionality --- .../contracts/ServiceProviderFactory.sol | 161 +++++++++++++++--- eth-contracts/test/delegateManager.test.js | 28 ++- eth-contracts/test/serviceProvider.test.js | 42 ++++- 3 files changed, 196 insertions(+), 35 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 8f240a907bc..8c56e9ce87b 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -24,6 +24,7 @@ contract ServiceProviderFactory is InitializableV2 { address private serviceTypeManagerAddress; address private claimsManagerAddress; uint256 private decreaseStakeLockupDuration; + uint256 private deployerCutLockupDuration; /// @dev - Stores following entities /// 1) Directly staked amount by SP, not including delegators @@ -47,8 +48,11 @@ contract ServiceProviderFactory is InitializableV2 { uint256 lockupExpiryBlock; } - /// @dev - Mapping of service provider address to details - mapping(address => ServiceProviderDetails) private spDetails; + /// @dev - Data structure for time delay during deployer cut update + struct UpdateDeployerCutRequest { + uint256 newDeployerCut; + uint256 lockupExpiryBlock; + } /// @dev - Struct maintaining information about sp /// @dev - blocknumber is block.number when endpoint registered @@ -59,6 +63,9 @@ contract ServiceProviderFactory is InitializableV2 { address delegateOwnerWallet; } + /// @dev - Mapping of service provider address to details + mapping(address => ServiceProviderDetails) private spDetails; + /// @dev - Uniquely assigned serviceProvider ID, incremented for each service type /// @notice - Keeps track of the total number of services registered regardless of /// whether some have been deregistered since @@ -82,6 +89,9 @@ contract ServiceProviderFactory is InitializableV2 { /// @dev - Mapping of service provider -> decrease stake request mapping(address => DecreaseStakeRequest) private decreaseStakeRequests; + /// @dev - Mapping of service provider -> update deployer cut requests + mapping(address => UpdateDeployerCutRequest) private updateDeployerCutRequests; + event RegisteredServiceProvider( uint256 indexed _spID, bytes32 indexed _serviceType, @@ -124,6 +134,7 @@ contract ServiceProviderFactory is InitializableV2 { ); event DecreaseStakeLockupDurationUpdated(uint256 indexed _lockupDuration); + event UpdateDeployerCutLockupDurationUpdated(uint256 indexed _lockupDuration); event GovernanceAddressUpdated(address indexed _newGovernanceAddress); event StakingAddressUpdated(address indexed _newStakingAddress); event ClaimsManagerAddressUpdated(address indexed _newClaimsManagerAddress); @@ -146,6 +157,8 @@ contract ServiceProviderFactory is InitializableV2 { decreaseStakeLockupDuration = _decreaseStakeLockupDuration; _updateGovernanceAddress(_governanceAddress); InitializableV2.initialize(); + // TODO: Make constructor or w/e + deployerCutLockupDuration = 100; } /** @@ -575,6 +588,84 @@ contract ServiceProviderFactory is InitializableV2 { return spId; } + function requestUpdateDeployerCut(address _serviceProvider, uint256 _cut) external + { + _requireIsInitialized(); + + // TODO: Or governance + require( + msg.sender == _serviceProvider || msg.sender == governanceAddress, + "ServiceProviderFactory: Service Provider deployer cut update restricted to deployer" + ); + + require( + (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock == 0) && + (updateDeployerCutRequests[_serviceProvider].newDeployerCut == 0), + "ServiceProviderFactory: Update deployer cut operation pending" + ); + + require( + _cut <= DEPLOYER_CUT_BASE, + "ServiceProviderFactory: Service Provider cut cannot exceed base value" + ); + + updateDeployerCutRequests[_serviceProvider] = UpdateDeployerCutRequest({ + lockupExpiryBlock: block.number + deployerCutLockupDuration, + newDeployerCut: _cut + }); + } + + function cancelUpdateDeployerCut(address _serviceProvider) external + { + _requireIsInitialized(); + require( + (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && + (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), + "ServiceProviderFactory: No update deployer cut operation pending" + ); + + require( + msg.sender == _serviceProvider || msg.sender == governanceAddress, + "ServiceProviderFactory: Service Provider cut update operation restricted to deployer" + ); + + // Zero out request information + delete updateDeployerCutRequests[_serviceProvider]; + } + + /** + * @notice Evalue request to update service provider cut of claims + * @notice Update service provider cut as % of delegate claim, divided by the deployerCutBase. + * @dev SPs will interact with this value as a percent, value translation done client side + @dev A value of 5 dictates a 5% cut, with ( 5 / 100 ) * delegateReward going to an SP from each delegator each round. + */ + function updateDeployerCut(address _serviceProvider) external + { + _requireIsInitialized(); + require( + (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && + (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), + "ServiceProviderFactory: No update deployer cut operation pending" + ); + + require( + msg.sender == _serviceProvider, + "ServiceProviderFactory: Service Provider cut update operation restricted to deployer" + ); + + require( + updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock <= block.number, + "ServiceProviderFactory: Lockup must be expired" + ); + + spDetails[_serviceProvider].deployerCut = updateDeployerCutRequests[_serviceProvider].newDeployerCut; + + // Zero out request information + delete updateDeployerCutRequests[_serviceProvider]; + + emit ServiceProviderCutUpdated(_serviceProvider, spDetails[_serviceProvider].deployerCut); + } + /** * @notice Update service provider balance * @dev Called by DelegateManager by functions modifying entire stake like claim and slash @@ -599,34 +690,21 @@ contract ServiceProviderFactory is InitializableV2 { _updateServiceProviderBoundStatus(_serviceProvider); } - /** - * @notice Update service provider cut of claims - * @notice Update service provider cut as % of delegate claim, divided by the deployerCutBase. - * @dev SPs will interact with this value as a percent, value translation done client side - @dev A value of 5 dictates a 5% cut, with ( 5 / 100 ) * delegateReward going to an SP from each delegator each round. - * @param _serviceProvider - address of service provider - * @param _cut - new deployer cut value - */ - function updateServiceProviderCut( - address _serviceProvider, - uint256 _cut - ) external - { + /// @notice Update service provider lockup duration + function updateDecreaseStakeLockupDuration(uint256 _duration) external { _requireIsInitialized(); require( - msg.sender == _serviceProvider, - "ServiceProviderFactory: Service Provider cut update operation restricted to deployer"); + msg.sender == governanceAddress, + ERROR_ONLY_GOVERNANCE + ); - require( - _cut <= DEPLOYER_CUT_BASE, - "ServiceProviderFactory: Service Provider cut cannot exceed base value"); - spDetails[_serviceProvider].deployerCut = _cut; - emit ServiceProviderCutUpdated(_serviceProvider, _cut); + decreaseStakeLockupDuration = _duration; + emit DecreaseStakeLockupDurationUpdated(_duration); } /// @notice Update service provider lockup duration - function updateDecreaseStakeLockupDuration(uint256 _duration) external { + function updateDeployerCutLockupDuration(uint256 _duration) external { _requireIsInitialized(); require( @@ -634,8 +712,8 @@ contract ServiceProviderFactory is InitializableV2 { ERROR_ONLY_GOVERNANCE ); - decreaseStakeLockupDuration = _duration; - emit DecreaseStakeLockupDurationUpdated(_duration); + _updateDeployerCutLockupDuration(_duration); + emit UpdateDeployerCutLockupDurationUpdated(_duration); } /// @notice Get denominator for deployer cut calculations @@ -647,6 +725,15 @@ contract ServiceProviderFactory is InitializableV2 { return DEPLOYER_CUT_BASE; } + /// @notice Get current deployer cut update lockup duration + function getDeployerCutLockupDuration() + external view returns (uint256) + { + _requireIsInitialized(); + + return deployerCutLockupDuration; + } + /// @notice Get total number of service providers for a given serviceType function getTotalServiceTypeProviders(bytes32 _serviceType) external view returns (uint256) @@ -735,6 +822,20 @@ contract ServiceProviderFactory is InitializableV2 { decreaseStakeRequests[_serviceProvider].lockupExpiryBlock ); } + /** + * @notice Get information about pending decrease stake requests for service provider + * @param _serviceProvider - address of service provider + */ + function getPendingUpdateDeployerCutRequest(address _serviceProvider) + external view returns (uint256 newDeployerCut, uint256 lockupExpiryBlock) + { + _requireIsInitialized(); + + return ( + updateDeployerCutRequests[_serviceProvider].newDeployerCut, + updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock + ); + } /// @notice Get current unstake lockup duration function getDecreaseStakeLockupDuration() @@ -892,6 +993,16 @@ contract ServiceProviderFactory is InitializableV2 { governanceAddress = _governanceAddress; } + function _updateDeployerCutLockupDuration(uint256 _newDuration) internal + { + // TODO: Extend this to accept other setters + require( + ClaimsManager(claimsManagerAddress).getFundingRoundBlockDiff() < _newDuration, + "ServiceProviderFactory: _governanceAddress is not a valid governance contract" + ); + deployerCutLockupDuration = _newDuration; + } + /** * @notice Compare a given amount input against valid min and max bounds for service provider * @param _serviceProvider - address of service provider diff --git a/eth-contracts/test/delegateManager.test.js b/eth-contracts/test/delegateManager.test.js index 8b11fb84054..d99caf8c06a 100644 --- a/eth-contracts/test/delegateManager.test.js +++ b/eth-contracts/test/delegateManager.test.js @@ -413,9 +413,31 @@ contract('DelegateManager', async (accounts) => { let finalBal = await token.balanceOf(stakerAccount) assert.isTrue(initialBal.eq(finalBal.add(DEFAULT_AMOUNT)), 'Expect funds to be transferred') - // Update SP Deployer Cut to 10% - await serviceProviderFactory.updateServiceProviderCut(stakerAccount, 10, { from: stakerAccount }) - await serviceProviderFactory.updateServiceProviderCut(stakerAccount2, 10, { from: stakerAccount2 }) + let updatedCut = 10 + + // Request Update SP Deployer Cut to 10% + await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCut, { from: stakerAccount }) + await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount2, updatedCut, { from: stakerAccount2 }) + + // Advance to 2nd update block number + let pending2ndUpdate = await serviceProviderFactory.getPendingUpdateDeployerCutRequest(stakerAccount2) + await time.advanceBlockTo(pending2ndUpdate.lockupExpiryBlock) + + // Evaluate both updates + await serviceProviderFactory.updateDeployerCut( + stakerAccount2, + { from: stakerAccount2 } + ) + await serviceProviderFactory.updateDeployerCut( + stakerAccount, + { from: stakerAccount } + ) + + // Confirm updates + let info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount) + assert.isTrue((info.deployerCut).eq(_lib.toBN(updatedCut)), 'Expect updated cut') + info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount2) + assert.isTrue((info.deployerCut).eq(_lib.toBN(updatedCut)), 'Expect updated cut') }) it('Initial state + claim', async () => { diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index d22d098b48f..b9518507719 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -492,16 +492,44 @@ contract('ServiceProvider test', async (accounts) => { it('Update service provider cut', async () => { let updatedCutValue = 10 + // Permission of request to input account await _lib.assertRevert( - serviceProviderFactory.updateServiceProviderCut( + serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: accounts[4] }), + 'deployer cut update restricted to deployer' + ) + // Eval fails if no pending operation + await _lib.assertRevert( + serviceProviderFactory.updateDeployerCut( stakerAccount, - updatedCutValue, { from: accounts[4] }), - 'Service Provider cut update operation restricted to deployer') - let updateTx = await serviceProviderFactory.updateServiceProviderCut( + 'No update deployer cut operation pending' + ) + + let deployerCutUpdateDuration = await serviceProviderFactory.getDeployerCutLockupDuration() + let requestTx = await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: stakerAccount }) + let requestBlock = _lib.toBN(requestTx.receipt.blockNumber) + + // Retrieve pending info + let pendingOp = await serviceProviderFactory.getPendingUpdateDeployerCutRequest(stakerAccount) + assert.isTrue( + (requestBlock.add(deployerCutUpdateDuration)).eq(pendingOp.lockupExpiryBlock), + 'Unexpected expiry block' + ) + + await _lib.assertRevert( + serviceProviderFactory.updateDeployerCut( + stakerAccount, + { from: stakerAccount } + ), + 'Lockup must be expired' + ) + + await time.advanceBlockTo(pendingOp.lockupExpiryBlock) + + let updateTx = await serviceProviderFactory.updateDeployerCut( stakerAccount, - updatedCutValue, - { from: stakerAccount }) + { from: stakerAccount } + ) await expectEvent.inTransaction( updateTx.tx, @@ -518,7 +546,7 @@ contract('ServiceProvider test', async (accounts) => { let base = await serviceProviderFactory.getServiceProviderDeployerCutBase() assert.isTrue(_lib.toBN(newCut).gt(base), 'Expect invalid newCut') await _lib.assertRevert( - serviceProviderFactory.updateServiceProviderCut(stakerAccount, newCut, { from: stakerAccount }), + serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, newCut, { from: stakerAccount }), 'Service Provider cut cannot exceed base value') }) From 1c21a35752dd8c82db12df8bac034dda79eb860b Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Mon, 20 Jul 2020 18:33:04 -0700 Subject: [PATCH 02/15] lintylint --- eth-contracts/contracts/ServiceProviderFactory.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 8c56e9ce87b..99cfa89d782 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -658,7 +658,9 @@ contract ServiceProviderFactory is InitializableV2 { "ServiceProviderFactory: Lockup must be expired" ); - spDetails[_serviceProvider].deployerCut = updateDeployerCutRequests[_serviceProvider].newDeployerCut; + spDetails[_serviceProvider].deployerCut = ( + updateDeployerCutRequests[_serviceProvider].newDeployerCut + ); // Zero out request information delete updateDeployerCutRequests[_serviceProvider]; @@ -822,6 +824,7 @@ contract ServiceProviderFactory is InitializableV2 { decreaseStakeRequests[_serviceProvider].lockupExpiryBlock ); } + /** * @notice Get information about pending decrease stake requests for service provider * @param _serviceProvider - address of service provider From 17357a2cea24243ee118b4d48688546921880a7c Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Mon, 20 Jul 2020 20:21:20 -0700 Subject: [PATCH 03/15] NATSPEC + 1 more test validation --- eth-contracts/contracts/ServiceProviderFactory.sol | 10 +++++++++- eth-contracts/test/serviceProvider.test.js | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 99cfa89d782..4e88b8c0fa5 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -588,11 +588,15 @@ contract ServiceProviderFactory is InitializableV2 { return spId; } + /** + * @notice Update the deployer cut for a given service provider + * @param _serviceProvider - address of service provider + * @param _cut - new value for deployer cut + */ function requestUpdateDeployerCut(address _serviceProvider, uint256 _cut) external { _requireIsInitialized(); - // TODO: Or governance require( msg.sender == _serviceProvider || msg.sender == governanceAddress, "ServiceProviderFactory: Service Provider deployer cut update restricted to deployer" @@ -615,6 +619,10 @@ contract ServiceProviderFactory is InitializableV2 { }); } + /** + * @notice Cancel a pending request to update deployer cut + * @param _serviceProvider - address of service provider + */ function cancelUpdateDeployerCut(address _serviceProvider) external { _requireIsInitialized(); diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index b9518507719..c1e47b0e6e2 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -505,6 +505,11 @@ contract('ServiceProvider test', async (accounts) => { 'No update deployer cut operation pending' ) + await _lib.assertRevert( + serviceProviderFactory.cancelUpdateDeployerCut(stakerAccount), + 'No update deployer cut operation pending' + ) + let deployerCutUpdateDuration = await serviceProviderFactory.getDeployerCutLockupDuration() let requestTx = await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: stakerAccount }) let requestBlock = _lib.toBN(requestTx.receipt.blockNumber) From b867cf38ad71f7888ae200a3565eba6fcb4cc39d Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 10:28:02 -0700 Subject: [PATCH 04/15] Constructor modification --- .../contracts/ServiceProviderFactory.sol | 17 +++++++++++------ .../7_versioning_service_migration.js | 13 +++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 4e88b8c0fa5..ae2dab0eb0f 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -151,14 +151,16 @@ contract ServiceProviderFactory is InitializableV2 { */ function initialize ( address _governanceAddress, - uint256 _decreaseStakeLockupDuration + address _claimsManagerAddress, + uint256 _decreaseStakeLockupDuration, + uint256 _deployerCutLockupDuration ) public initializer { - decreaseStakeLockupDuration = _decreaseStakeLockupDuration; _updateGovernanceAddress(_governanceAddress); + claimsManagerAddress = _claimsManagerAddress; + decreaseStakeLockupDuration = _decreaseStakeLockupDuration; + _updateDeployerCutLockupDuration(_deployerCutLockupDuration); InitializableV2.initialize(); - // TODO: Make constructor or w/e - deployerCutLockupDuration = 100; } /** @@ -1004,12 +1006,15 @@ contract ServiceProviderFactory is InitializableV2 { governanceAddress = _governanceAddress; } + /** + * @notice Set the deployer cut lockup duration + * @param _newDuration - incoming duration + */ function _updateDeployerCutLockupDuration(uint256 _newDuration) internal { - // TODO: Extend this to accept other setters require( ClaimsManager(claimsManagerAddress).getFundingRoundBlockDiff() < _newDuration, - "ServiceProviderFactory: _governanceAddress is not a valid governance contract" + "ServiceProviderFactory: Incoming duration must be greater than funding round block diff" ); deployerCutLockupDuration = _newDuration; } diff --git a/eth-contracts/migrations/7_versioning_service_migration.js b/eth-contracts/migrations/7_versioning_service_migration.js index 2b9f04d7944..ab433d3c7e6 100644 --- a/eth-contracts/migrations/7_versioning_service_migration.js +++ b/eth-contracts/migrations/7_versioning_service_migration.js @@ -35,6 +35,10 @@ const dpTypeMax = _lib.audToWei(2000000) // - 1/13 block/s * 604800 s/wk ~= 46523 block/wk const decreaseStakeLockupDuration = 46523 +// modifying deployer cut = 8 days in blocks +// - 1/13 block/s * 691200 s/8 days ~= 53169 block/wk +const deployerCutLockupDuration = 53169 + module.exports = (deployer, network, accounts) => { deployer.then(async () => { const config = contractConfig[network] @@ -106,8 +110,13 @@ module.exports = (deployer, network, accounts) => { const serviceProviderFactory0 = await deployer.deploy(ServiceProviderFactory, { from: proxyDeployerAddress }) const serviceProviderFactoryCalldata = _lib.encodeCall( 'initialize', - ['address', 'uint256'], - [process.env.governanceAddress, decreaseStakeLockupDuration] + ['address', 'address', 'uint256', 'uint256'], + [ + process.env.governanceAddress, + process.env.claimsManagerAddress, + decreaseStakeLockupDuration, + deployerCutLockupDuration + ] ) const serviceProviderFactoryProxy = await deployer.deploy( AudiusAdminUpgradeabilityProxy, From f5f5a74bcf4f5e2388834323222858246517b56f Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 13:32:10 -0700 Subject: [PATCH 05/15] Test propagation of constructor change --- eth-contracts/test/delegateManager.test.js | 51 ++++++++++------------ eth-contracts/test/governance.test.js | 46 ++++++++++--------- eth-contracts/test/serviceProvider.test.js | 43 +++++++++--------- eth-contracts/utils/lib.js | 41 +++++++++++++++++ 4 files changed, 109 insertions(+), 72 deletions(-) diff --git a/eth-contracts/test/delegateManager.test.js b/eth-contracts/test/delegateManager.test.js index d99caf8c06a..ecdf5cc0ecd 100644 --- a/eth-contracts/test/delegateManager.test.js +++ b/eth-contracts/test/delegateManager.test.js @@ -30,12 +30,13 @@ const VOTING_PERIOD = 10 const EXECUTION_DELAY = VOTING_PERIOD const VOTING_QUORUM_PERCENT = 10 const DECREASE_STAKE_LOCKUP_DURATION = 10 +const DEPLOYER_CUT_LOCKUP_DURATION = 11 const callValue0 = _lib.toBN(0) contract('DelegateManager', async (accounts) => { - let staking, stakingAddress, token, registry, governance, claimsManager0, claimsManagerProxy + let staking, stakingAddress, token, registry, governance let serviceProviderFactory, serviceTypeManager, claimsManager, delegateManager // intentionally not using acct0 to make sure no TX accidentally succeeds without specifying sender @@ -111,12 +112,28 @@ contract('DelegateManager', async (accounts) => { // Register discprov serviceType await _lib.addServiceType(testDiscProvType, serviceTypeMinStake, serviceTypeMaxStake, governance, guardianAddress, serviceTypeManagerProxyKey) + claimsManager = await _lib.deployClaimsManager( + artifacts, + registry, + governance, + proxyDeployerAddress, + guardianAddress, + token.address, + 10, + claimsManagerProxyKey + ) + // Deploy ServiceProviderFactory let serviceProviderFactory0 = await ServiceProviderFactory.new({ from: proxyDeployerAddress }) const serviceProviderFactoryCalldata = _lib.encodeCall( 'initialize', - ['address', 'uint256'], - [governance.address, DECREASE_STAKE_LOCKUP_DURATION] + ['address', 'address', 'uint256', 'uint256'], + [ + governance.address, + claimsManager.address, + DECREASE_STAKE_LOCKUP_DURATION, + DEPLOYER_CUT_LOCKUP_DURATION + ] ) let serviceProviderFactoryProxy = await AudiusAdminUpgradeabilityProxy.new( serviceProviderFactory0.address, @@ -127,29 +144,7 @@ contract('DelegateManager', async (accounts) => { serviceProviderFactory = await ServiceProviderFactory.at(serviceProviderFactoryProxy.address) await registry.addContract(serviceProviderFactoryKey, serviceProviderFactoryProxy.address, { from: proxyDeployerAddress }) - // Deploy new claimsManager proxy - claimsManager0 = await ClaimsManager.new({ from: proxyDeployerAddress }) - const claimsInitializeCallData = _lib.encodeCall( - 'initialize', - ['address', 'address'], - [token.address, governance.address] - ) - claimsManagerProxy = await AudiusAdminUpgradeabilityProxy.new( - claimsManager0.address, - governance.address, - claimsInitializeCallData, - { from: proxyDeployerAddress } - ) - claimsManager = await ClaimsManager.at(claimsManagerProxy.address) - - // Register claimsManagerProxy - await registry.addContract( - claimsManagerProxyKey, - claimsManagerProxy.address, - { from: proxyDeployerAddress } - ) - - // Register new contract as a minter, from the same address that deployed the contract + // Register new ClaimsManager contract as a minter, from the same address that deployed the contract await governance.guardianExecuteTransaction( tokenRegKey, callValue0, @@ -196,7 +191,7 @@ contract('DelegateManager', async (accounts) => { stakingProxyKey, staking, serviceProviderFactoryProxy.address, - claimsManagerProxy.address, + claimsManager.address, delegateManagerProxy.address ) @@ -256,7 +251,7 @@ contract('DelegateManager', async (accounts) => { serviceProviderFactory, staking.address, serviceTypeManagerProxy.address, - claimsManagerProxy.address, + claimsManager.address, delegateManagerProxy.address ) }) diff --git a/eth-contracts/test/governance.test.js b/eth-contracts/test/governance.test.js index c5908dbe101..b8f2a25db3c 100644 --- a/eth-contracts/test/governance.test.js +++ b/eth-contracts/test/governance.test.js @@ -53,6 +53,7 @@ contract('Governance.sol', async (accounts) => { const maxInProgressProposals = 20 const maxDescriptionLength = 250 const executionDelay = votingPeriod + const deployerCutLockupDuration = 11 // intentionally not using acct0 to make sure no TX accidentally succeeds without specifying sender const [, proxyAdminAddress, proxyDeployerAddress, newUpdateAddress] = accounts @@ -156,12 +157,29 @@ contract('Governance.sol', async (accounts) => { // Register discprov serviceType await _lib.addServiceType(testDiscProvType, spMinStake, spMaxStake, governance, guardianAddress, serviceTypeManagerProxyKey) + // Deploy + register claimsManagerProxy + claimsManager = await _lib.deployClaimsManager( + artifacts, + registry, + governance, + proxyDeployerAddress, + guardianAddress, + token.address, + 10, + claimsManagerProxyKey + ) + // Deploy + Register ServiceProviderFactory contract const serviceProviderFactory0 = await ServiceProviderFactory.new({ from: proxyDeployerAddress }) const serviceProviderFactoryCalldata = _lib.encodeCall( 'initialize', - ['address', 'uint256'], - [governance.address, decreaseStakeLockupDuration] + ['address', 'address', 'uint256', 'uint256'], + [ + governance.address, + claimsManager.address, + decreaseStakeLockupDuration, + deployerCutLockupDuration + ] ) const serviceProviderFactoryProxy = await AudiusAdminUpgradeabilityProxy.new( serviceProviderFactory0.address, @@ -172,26 +190,6 @@ contract('Governance.sol', async (accounts) => { serviceProviderFactory = await ServiceProviderFactory.at(serviceProviderFactoryProxy.address) await registry.addContract(serviceProviderFactoryKey, serviceProviderFactoryProxy.address, { from: proxyDeployerAddress }) - // Deploy + register claimsManagerProxy - const claimsManager0 = await ClaimsManager.new({ from: proxyDeployerAddress }) - const claimsInitializeCallData = _lib.encodeCall( - 'initialize', - ['address', 'address'], - [token.address, governance.address] - ) - const claimsManagerProxy = await AudiusAdminUpgradeabilityProxy.new( - claimsManager0.address, - governance.address, - claimsInitializeCallData, - { from: proxyDeployerAddress } - ) - claimsManager = await ClaimsManager.at(claimsManagerProxy.address) - await registry.addContract( - claimsManagerProxyKey, - claimsManagerProxy.address, - { from: proxyDeployerAddress } - ) - // Register new contract as a minter, from the same address that deployed the contract await governance.guardianExecuteTransaction( tokenRegKey, @@ -231,7 +229,7 @@ contract('Governance.sol', async (accounts) => { stakingProxyKey, staking, serviceProviderFactoryProxy.address, - claimsManagerProxy.address, + claimsManager.address, delegateManagerProxy.address ) // ---- Set up claims manager contract permissions @@ -264,7 +262,7 @@ contract('Governance.sol', async (accounts) => { serviceProviderFactory, staking.address, serviceTypeManagerProxy.address, - claimsManagerProxy.address, + claimsManager.address, delegateManager.address ) }) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index c1e47b0e6e2..03df71c5e70 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -29,13 +29,14 @@ const VOTING_PERIOD = 10 const EXECUTION_DELAY = VOTING_PERIOD const VOTING_QUORUM_PERCENT = 10 const DECREASE_STAKE_LOCKUP_DURATION = 10 +const DEPLOYER_CUT_LOCKUP_DURATION = 11 const INITIAL_BAL = _lib.audToWeiBN(1000) const DEFAULT_AMOUNT = _lib.audToWeiBN(120) contract('ServiceProvider test', async (accounts) => { - let token, registry, staking0, stakingInitializeData, proxy, claimsManager0, claimsManagerProxy, claimsManager, governance + let token, registry, staking0, stakingInitializeData, proxy, claimsManager, governance let staking, serviceProviderFactory, serviceTypeManager, mockDelegateManager // intentionally not using acct0 to make sure no TX accidentally succeeds without specifying sender @@ -169,24 +170,21 @@ contract('ServiceProvider test', async (accounts) => { await registry.addContract(serviceTypeManagerProxyKey, serviceTypeManager.address, { from: proxyDeployerAddress }) // Deploy + register claimsManagerProxy - claimsManager0 = await ClaimsManager.new({ from: proxyDeployerAddress }) - const claimsInitializeCallData = _lib.encodeCall( - 'initialize', - ['address', 'address'], - [token.address, governance.address] - ) - claimsManagerProxy = await AudiusAdminUpgradeabilityProxy.new( - claimsManager0.address, - governance.address, - claimsInitializeCallData, - { from: proxyDeployerAddress } + claimsManager = await _lib.deployClaimsManager( + artifacts, + registry, + governance, + proxyDeployerAddress, + guardianAddress, + token.address, + 10, + claimsManagerProxyKey ) - claimsManager = await ClaimsManager.at(claimsManagerProxy.address) - await registry.addContract(claimsManagerProxyKey, claimsManagerProxy.address, { from: proxyDeployerAddress }) + // End claims manager setup // Deploy mock delegate manager with only function to forward processClaim call mockDelegateManager = await MockDelegateManager.new() - await mockDelegateManager.initialize(claimsManagerProxy.address) + await mockDelegateManager.initialize(claimsManager.address) await registry.addContract(delegateManagerKey, mockDelegateManager.address, { from: proxyDeployerAddress }) /** addServiceTypes creatornode and discprov via Governance */ @@ -219,8 +217,13 @@ contract('ServiceProvider test', async (accounts) => { let serviceProviderFactory0 = await ServiceProviderFactory.new({ from: proxyDeployerAddress }) const serviceProviderFactoryCalldata = _lib.encodeCall( 'initialize', - ['address', 'uint256'], - [governance.address, DECREASE_STAKE_LOCKUP_DURATION] + ['address', 'address', 'uint256', 'uint256'], + [ + governance.address, + claimsManager.address, + DECREASE_STAKE_LOCKUP_DURATION, + DEPLOYER_CUT_LOCKUP_DURATION + ] ) let serviceProviderFactoryProxy = await AudiusAdminUpgradeabilityProxy.new( serviceProviderFactory0.address, @@ -248,7 +251,7 @@ contract('ServiceProvider test', async (accounts) => { stakingProxyKey, staking, serviceProviderFactoryProxy.address, - claimsManagerProxy.address, + claimsManager.address, _lib.addressZero ) // ---- Set up claims manager contract permissions @@ -289,7 +292,7 @@ contract('ServiceProvider test', async (accounts) => { serviceProviderFactory, staking.address, serviceTypeManagerProxy.address, - claimsManagerProxy.address, + claimsManager.address, mockDelegateManager.address ) @@ -315,7 +318,7 @@ contract('ServiceProvider test', async (accounts) => { initTxs.claimsManagerTx.tx, ServiceProviderFactory, 'ClaimsManagerAddressUpdated', - { _newClaimsManagerAddress: claimsManagerProxy.address } + { _newClaimsManagerAddress: claimsManager.address } ) }) diff --git a/eth-contracts/utils/lib.js b/eth-contracts/utils/lib.js index 4241a1c0468..667fa4ceb5e 100644 --- a/eth-contracts/utils/lib.js +++ b/eth-contracts/utils/lib.js @@ -250,6 +250,45 @@ export const deployGovernance = async ( return governance } +export const deployClaimsManager = async ( + artifacts, + registry, + governance, + proxyDeployerAddress, + guardianAddress, + tokenAddress, + fundingDiff, + registryKey +) => { + const governanceAddress = governance.address + const ClaimsManager = artifacts.require('ClaimsManager') + const AudiusAdminUpgradeabilityProxy = artifacts.require('AudiusAdminUpgradeabilityProxy') + const claimsManager0 = await ClaimsManager.new({ from: proxyDeployerAddress }) + const claimsInitializeCallData = encodeCall( + 'initialize', + ['address', 'address'], + [tokenAddress, governanceAddress] + ) + let claimsManagerProxy = await AudiusAdminUpgradeabilityProxy.new( + claimsManager0.address, + governanceAddress, + claimsInitializeCallData, + { from: proxyDeployerAddress } + ) + let claimsManager = await ClaimsManager.at(claimsManagerProxy.address) + await registry.addContract(registryKey, claimsManagerProxy.address, { from: proxyDeployerAddress }) + + // Update funding found block diff + await governance.guardianExecuteTransaction( + registryKey, + toBN(0), + 'updateFundingRoundBlockDiff(uint256)', + abiEncode(['uint256'], [fundingDiff]), + { from: guardianAddress } + ) + return claimsManager +} + export const addServiceType = async (serviceType, typeMin, typeMax, governance, guardianAddress, serviceTypeManagerRegKey) => { const addServiceTypeSignature = 'addServiceType(bytes32,uint256,uint256)' const callValue0 = toBN(0) @@ -538,10 +577,12 @@ export const configureServiceProviderFactoryAddresses = async ( { from: guardianAddress }) assert.equal(serviceTypeManagerAddress, await spFactory.getServiceTypeManagerAddress(), 'Unexpected service type manager address') + /* await assertRevert( spFactory.increaseStake(100), "claimsManagerAddress is not set" ) + */ let claimsManagerTx = await governance.guardianExecuteTransaction( key, toBN(0), From 6e08557e64d20f788dcd69abeec3c4e2b7d9e416 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 15:11:23 -0700 Subject: [PATCH 06/15] NIT + fix lint --- .../contracts/ServiceProviderFactory.sol | 6 +++--- eth-contracts/utils/lib.js | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index ae2dab0eb0f..779763d6011 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -157,7 +157,7 @@ contract ServiceProviderFactory is InitializableV2 { ) public initializer { _updateGovernanceAddress(_governanceAddress); - claimsManagerAddress = _claimsManagerAddress; + claimsManagerAddress = _claimsManagerAddress; decreaseStakeLockupDuration = _decreaseStakeLockupDuration; _updateDeployerCutLockupDuration(_deployerCutLockupDuration); InitializableV2.initialize(); @@ -592,7 +592,7 @@ contract ServiceProviderFactory is InitializableV2 { /** * @notice Update the deployer cut for a given service provider - * @param _serviceProvider - address of service provider + * @param _serviceProvider - address of service provider * @param _cut - new value for deployer cut */ function requestUpdateDeployerCut(address _serviceProvider, uint256 _cut) external @@ -623,7 +623,7 @@ contract ServiceProviderFactory is InitializableV2 { /** * @notice Cancel a pending request to update deployer cut - * @param _serviceProvider - address of service provider + * @param _serviceProvider - address of service provider */ function cancelUpdateDeployerCut(address _serviceProvider) external { diff --git a/eth-contracts/utils/lib.js b/eth-contracts/utils/lib.js index 667fa4ceb5e..358b2c326da 100644 --- a/eth-contracts/utils/lib.js +++ b/eth-contracts/utils/lib.js @@ -123,7 +123,15 @@ export const encodeCall = (name, args, values) => { return '0x' + methodId + params } -export const registerServiceProvider = async (token, staking, serviceProviderFactory, type, endpoint, amount, account) => { +export const registerServiceProvider = async ( + token, + staking, + serviceProviderFactory, + type, + endpoint, + amount, + account +) => { // Approve staking transfer await token.approve(staking.address, amount, { from: account }) // register service provider @@ -577,12 +585,6 @@ export const configureServiceProviderFactoryAddresses = async ( { from: guardianAddress }) assert.equal(serviceTypeManagerAddress, await spFactory.getServiceTypeManagerAddress(), 'Unexpected service type manager address') - /* - await assertRevert( - spFactory.increaseStake(100), - "claimsManagerAddress is not set" - ) - */ let claimsManagerTx = await governance.guardianExecuteTransaction( key, toBN(0), From c6073fe84ed1cfcb6ef635f9b0d3253dbb79d598 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 16:51:41 -0700 Subject: [PATCH 07/15] Expose governance for final cut op --- .../contracts/ServiceProviderFactory.sol | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 779763d6011..961b4218e65 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -17,6 +17,9 @@ contract ServiceProviderFactory is InitializableV2 { string private constant ERROR_ONLY_GOVERNANCE = ( "ServiceProviderFactory: Only callable by Governance contract" ); + string private constant ERROR_ONLY_SP_GOVERNANCE = ( + "ServiceProviderFactory: Only callable by Service Provider or Governance" + ); address private stakingAddress; address private delegateManagerAddress; @@ -601,7 +604,7 @@ contract ServiceProviderFactory is InitializableV2 { require( msg.sender == _serviceProvider || msg.sender == governanceAddress, - "ServiceProviderFactory: Service Provider deployer cut update restricted to deployer" + ERROR_ONLY_SP_GOVERNANCE ); require( @@ -628,15 +631,11 @@ contract ServiceProviderFactory is InitializableV2 { function cancelUpdateDeployerCut(address _serviceProvider) external { _requireIsInitialized(); - require( - (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && - (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), - "ServiceProviderFactory: No update deployer cut operation pending" - ); + _requirePendingDeployerCutOperation(); require( msg.sender == _serviceProvider || msg.sender == governanceAddress, - "ServiceProviderFactory: Service Provider cut update operation restricted to deployer" + ERROR_ONLY_SP_GOVERNANCE ); // Zero out request information @@ -652,15 +651,11 @@ contract ServiceProviderFactory is InitializableV2 { function updateDeployerCut(address _serviceProvider) external { _requireIsInitialized(); - require( - (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && - (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), - "ServiceProviderFactory: No update deployer cut operation pending" - ); + _requirePendingDeployerCutOperation(); require( - msg.sender == _serviceProvider, - "ServiceProviderFactory: Service Provider cut update operation restricted to deployer" + msg.sender == _serviceProvider || msg.sender == governanceAddress, + ERROR_ONLY_SP_GOVERNANCE ); require( @@ -1063,6 +1058,13 @@ contract ServiceProviderFactory is InitializableV2 { } // ========================================= Private Functions ========================================= + function _requirePendingDeployerCutOperation (address _serviceProvider) private view { + require( + (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && + (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), + "ServiceProviderFactory: No update deployer cut operation pending" + ); + } function _requireStakingAddressIsSet() private view { require( From 6d7287910202d83ce4d063a10ab57f4cb7501d07 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 16:57:39 -0700 Subject: [PATCH 08/15] Compile error --- eth-contracts/contracts/ServiceProviderFactory.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index 961b4218e65..d406e1ba8ea 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -631,7 +631,7 @@ contract ServiceProviderFactory is InitializableV2 { function cancelUpdateDeployerCut(address _serviceProvider) external { _requireIsInitialized(); - _requirePendingDeployerCutOperation(); + _requirePendingDeployerCutOperation(_serviceProvider); require( msg.sender == _serviceProvider || msg.sender == governanceAddress, @@ -651,7 +651,7 @@ contract ServiceProviderFactory is InitializableV2 { function updateDeployerCut(address _serviceProvider) external { _requireIsInitialized(); - _requirePendingDeployerCutOperation(); + _requirePendingDeployerCutOperation(_serviceProvider); require( msg.sender == _serviceProvider || msg.sender == governanceAddress, From 97f750faf898bd547f8f8ae1076c16c33ccc75a4 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 17:12:32 -0700 Subject: [PATCH 09/15] one more test case --- eth-contracts/test/serviceProvider.test.js | 24 +++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index 03df71c5e70..9ad86253361 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -498,7 +498,7 @@ contract('ServiceProvider test', async (accounts) => { // Permission of request to input account await _lib.assertRevert( serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: accounts[4] }), - 'deployer cut update restricted to deployer' + 'Only callable by Service Provider or Governance' ) // Eval fails if no pending operation await _lib.assertRevert( @@ -556,6 +556,28 @@ contract('ServiceProvider test', async (accounts) => { await _lib.assertRevert( serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, newCut, { from: stakerAccount }), 'Service Provider cut cannot exceed base value') + + // Set an invalid value for lockup + await _lib.assertRevert( + governance.guardianExecuteTransaction( + serviceProviderFactoryKey, + callValue, + 'updateDeployerCutLockupDuration(address)', + _lib.abiEncode(['uint256'], [1]), + { from: guardianAddress } + ) + ) + + let validUpdatedDuration = DECREASE_STAKE_LOCKUP_DURATION + 1 + governance.guardianExecuteTransaction( + serviceProviderFactoryKey, + callValue, + 'updateDeployerCutLockupDuration(address)', + _lib.abiEncode(['uint256'], [validUpdatedDuration]), + { from: guardianAddress } + ) + let fromChainDuration = await serviceProviderFactory.getDeployerCutLockupDuration() + assert.isTrue(fromChainDuration.eq(_lib.toBN(validUpdatedDuration)), 'Expected update') }) it('Fails to register duplicate endpoint w/same account', async () => { From ba45ac96a619621cd6df1bfaf47dd5ee45499304 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 17:48:07 -0700 Subject: [PATCH 10/15] Test fix --- eth-contracts/test/serviceProvider.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index 9ad86253361..ef973cb93ca 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -35,7 +35,7 @@ const INITIAL_BAL = _lib.audToWeiBN(1000) const DEFAULT_AMOUNT = _lib.audToWeiBN(120) -contract('ServiceProvider test', async (accounts) => { +contract.only('ServiceProvider test', async (accounts) => { let token, registry, staking0, stakingInitializeData, proxy, claimsManager, governance let staking, serviceProviderFactory, serviceTypeManager, mockDelegateManager @@ -562,17 +562,17 @@ contract('ServiceProvider test', async (accounts) => { governance.guardianExecuteTransaction( serviceProviderFactoryKey, callValue, - 'updateDeployerCutLockupDuration(address)', + 'updateDeployerCutLockupDuration(uint256)', _lib.abiEncode(['uint256'], [1]), { from: guardianAddress } ) ) - let validUpdatedDuration = DECREASE_STAKE_LOCKUP_DURATION + 1 - governance.guardianExecuteTransaction( + let validUpdatedDuration = DEPLOYER_CUT_LOCKUP_DURATION + 1 + await governance.guardianExecuteTransaction( serviceProviderFactoryKey, callValue, - 'updateDeployerCutLockupDuration(address)', + 'updateDeployerCutLockupDuration(uint256)', _lib.abiEncode(['uint256'], [validUpdatedDuration]), { from: guardianAddress } ) From 7c7e0c93d6fad3a4a31151604dc40b3a83fd0814 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 18:40:54 -0700 Subject: [PATCH 11/15] Enforce decreaseStake lockup duration relationship --- .../contracts/ServiceProviderFactory.sol | 26 ++++++++++++++----- eth-contracts/test/delegateManager.test.js | 4 +-- eth-contracts/test/governance.test.js | 4 +-- eth-contracts/test/serviceProvider.test.js | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index d406e1ba8ea..a2a8ea86cf1 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -161,7 +161,7 @@ contract ServiceProviderFactory is InitializableV2 { { _updateGovernanceAddress(_governanceAddress); claimsManagerAddress = _claimsManagerAddress; - decreaseStakeLockupDuration = _decreaseStakeLockupDuration; + _updateDecreaseStakeLockupDuration(_decreaseStakeLockupDuration); _updateDeployerCutLockupDuration(_deployerCutLockupDuration); InitializableV2.initialize(); } @@ -706,7 +706,7 @@ contract ServiceProviderFactory is InitializableV2 { ERROR_ONLY_GOVERNANCE ); - decreaseStakeLockupDuration = _duration; + _updateDecreaseStakeLockupDuration(_duration); emit DecreaseStakeLockupDurationUpdated(_duration); } @@ -1003,15 +1003,29 @@ contract ServiceProviderFactory is InitializableV2 { /** * @notice Set the deployer cut lockup duration - * @param _newDuration - incoming duration + * @param _duration - incoming duration */ - function _updateDeployerCutLockupDuration(uint256 _newDuration) internal + function _updateDeployerCutLockupDuration(uint256 _duration) internal { require( - ClaimsManager(claimsManagerAddress).getFundingRoundBlockDiff() < _newDuration, + ClaimsManager(claimsManagerAddress).getFundingRoundBlockDiff() < _duration, "ServiceProviderFactory: Incoming duration must be greater than funding round block diff" ); - deployerCutLockupDuration = _newDuration; + deployerCutLockupDuration = _duration; + } + + /** + * @notice Set the decrease stake lockup duration + * @param _duration - incoming duration + */ + function _updateDecreaseStakeLockupDuration(uint256 _duration) internal + { + Governance governance = Governance(governanceAddress); + require( + _duration > governance.getVotingPeriod() + governance.getExecutionDelay(), + "ServiceProviderFactory: decreaseStakeLockupDuration duration must be greater than governance votingPeriod + executionDelay" + ); + decreaseStakeLockupDuration = _duration; } /** diff --git a/eth-contracts/test/delegateManager.test.js b/eth-contracts/test/delegateManager.test.js index b98a9754fbc..e4b7db996b7 100644 --- a/eth-contracts/test/delegateManager.test.js +++ b/eth-contracts/test/delegateManager.test.js @@ -29,9 +29,9 @@ const DEFAULT_AMOUNT = _lib.toBN(DEFAULT_AMOUNT_VAL) const VOTING_PERIOD = 10 const EXECUTION_DELAY = VOTING_PERIOD const VOTING_QUORUM_PERCENT = 10 -const DECREASE_STAKE_LOCKUP_DURATION = 10 const DEPLOYER_CUT_LOCKUP_DURATION = 11 -const UNDELEGATE_LOCKUP_DURATION = 21 +const UNDELEGATE_LOCKUP_DURATION = VOTING_PERIOD + EXECUTION_DELAY + 1 +const DECREASE_STAKE_LOCKUP_DURATION = UNDELEGATE_LOCKUP_DURATION const callValue0 = _lib.toBN(0) diff --git a/eth-contracts/test/governance.test.js b/eth-contracts/test/governance.test.js index d1925be8651..b51ebe8e31e 100644 --- a/eth-contracts/test/governance.test.js +++ b/eth-contracts/test/governance.test.js @@ -49,12 +49,12 @@ contract('Governance.sol', async (accounts) => { const votingPeriod = 10 const votingQuorumPercent = 10 - const decreaseStakeLockupDuration = 10 const maxInProgressProposals = 20 const maxDescriptionLength = 250 const executionDelay = votingPeriod const deployerCutLockupDuration = 11 - const undelegateLockupDuration = 21 + const undelegateLockupDuration = votingPeriod + executionDelay + 1 + const decreaseStakeLockupDuration = undelegateLockupDuration // intentionally not using acct0 to make sure no TX accidentally succeeds without specifying sender const [, proxyAdminAddress, proxyDeployerAddress, newUpdateAddress] = accounts diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index ef973cb93ca..b4df69bd5d9 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -28,7 +28,7 @@ const MIN_STAKE_AMOUNT = 10 const VOTING_PERIOD = 10 const EXECUTION_DELAY = VOTING_PERIOD const VOTING_QUORUM_PERCENT = 10 -const DECREASE_STAKE_LOCKUP_DURATION = 10 +const DECREASE_STAKE_LOCKUP_DURATION = VOTING_PERIOD + EXECUTION_DELAY + 1 const DEPLOYER_CUT_LOCKUP_DURATION = 11 const INITIAL_BAL = _lib.audToWeiBN(1000) From ac0c88e6038641b2667a525b8384fd10227a767e Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 18:49:15 -0700 Subject: [PATCH 12/15] RM ONLY --- eth-contracts/test/serviceProvider.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index b4df69bd5d9..645d27e4f03 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -35,7 +35,7 @@ const INITIAL_BAL = _lib.audToWeiBN(1000) const DEFAULT_AMOUNT = _lib.audToWeiBN(120) -contract.only('ServiceProvider test', async (accounts) => { +contract('ServiceProvider test', async (accounts) => { let token, registry, staking0, stakingInitializeData, proxy, claimsManager, governance let staking, serviceProviderFactory, serviceTypeManager, mockDelegateManager From 4c28192b574b367d9214bd26a4e2d0e0ce59c056 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 19:16:33 -0700 Subject: [PATCH 13/15] Test fix and reset to 0 case addressed --- .../contracts/ServiceProviderFactory.sol | 3 +-- eth-contracts/test/serviceProvider.test.js | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/eth-contracts/contracts/ServiceProviderFactory.sol b/eth-contracts/contracts/ServiceProviderFactory.sol index a2a8ea86cf1..52521efead7 100644 --- a/eth-contracts/contracts/ServiceProviderFactory.sol +++ b/eth-contracts/contracts/ServiceProviderFactory.sol @@ -1074,8 +1074,7 @@ contract ServiceProviderFactory is InitializableV2 { // ========================================= Private Functions ========================================= function _requirePendingDeployerCutOperation (address _serviceProvider) private view { require( - (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0) && - (updateDeployerCutRequests[_serviceProvider].newDeployerCut != 0), + (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0), "ServiceProviderFactory: No update deployer cut operation pending" ); } diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index 645d27e4f03..02b0cb00cd9 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -493,7 +493,7 @@ contract('ServiceProvider test', async (accounts) => { ) }) - it('Update service provider cut', async () => { + it.only('Update service provider cut', async () => { let updatedCutValue = 10 // Permission of request to input account await _lib.assertRevert( @@ -547,14 +547,23 @@ contract('ServiceProvider test', async (accounts) => { _updatedCut: `${updatedCutValue}` } ) - let info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount) assert.isTrue((info.deployerCut).eq(_lib.toBN(updatedCutValue)), 'Expect updated cut') - let newCut = 110 + + // Reset the value for updated cut to 0 + updatedCutValue = 0 + requestTx = await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: stakerAccount }) + pendingOp = await serviceProviderFactory.getPendingUpdateDeployerCutRequest(stakerAccount) + await time.advanceBlockTo(pendingOp.lockupExpiryBlock) + updateTx = await serviceProviderFactory.updateDeployerCut(stakerAccount, { from: stakerAccount }) + info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount) + assert.isTrue((info.deployerCut).eq(_lib.toBN(updatedCutValue)), 'Expect updated cut') + + let invalidCut = 110 let base = await serviceProviderFactory.getServiceProviderDeployerCutBase() - assert.isTrue(_lib.toBN(newCut).gt(base), 'Expect invalid newCut') + assert.isTrue(_lib.toBN(invalidCut).gt(base), 'Expect invalid newCut') await _lib.assertRevert( - serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, newCut, { from: stakerAccount }), + serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, invalidCut, { from: stakerAccount }), 'Service Provider cut cannot exceed base value') // Set an invalid value for lockup From c4803f8d597a393474ef4fe41ba211ebd298cbe7 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Tue, 21 Jul 2020 21:09:04 -0700 Subject: [PATCH 14/15] RM only --- eth-contracts/test/serviceProvider.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index 02b0cb00cd9..bf12becbc4f 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -493,7 +493,7 @@ contract('ServiceProvider test', async (accounts) => { ) }) - it.only('Update service provider cut', async () => { + it('Update service provider cut', async () => { let updatedCutValue = 10 // Permission of request to input account await _lib.assertRevert( From 8af02d4abb551bb8a0c92d22a8e81a4761723276 Mon Sep 17 00:00:00 2001 From: Hareesh Nagaraj Date: Wed, 22 Jul 2020 11:35:24 -0700 Subject: [PATCH 15/15] Add cancel test verification --- eth-contracts/test/serviceProvider.test.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/eth-contracts/test/serviceProvider.test.js b/eth-contracts/test/serviceProvider.test.js index bf12becbc4f..7f317bf5e16 100644 --- a/eth-contracts/test/serviceProvider.test.js +++ b/eth-contracts/test/serviceProvider.test.js @@ -1,4 +1,5 @@ import * as _lib from '../utils/lib.js' +import { update } from 'lodash' const { time, expectEvent } = require('@openzeppelin/test-helpers') const Staking = artifacts.require('Staking') @@ -559,6 +560,25 @@ contract('ServiceProvider test', async (accounts) => { info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount) assert.isTrue((info.deployerCut).eq(_lib.toBN(updatedCutValue)), 'Expect updated cut') + // Confirm cancellation works + let preUpdatecut = updatedCutValue + updatedCutValue = 10 + // Submit request + requestTx = await serviceProviderFactory.requestUpdateDeployerCut(stakerAccount, updatedCutValue, { from: stakerAccount }) + // Confirm request status + pendingOp = await serviceProviderFactory.getPendingUpdateDeployerCutRequest(stakerAccount) + assert.isTrue(pendingOp.newDeployerCut.eq(_lib.toBN(updatedCutValue)), 'Expect in flight request') + assert.isTrue(!pendingOp.lockupExpiryBlock.eq(_lib.toBN(0)), 'Expect in flight request') + // Submit cancellation + await serviceProviderFactory.cancelUpdateDeployerCut(stakerAccount, { from: stakerAccount }) + // Confirm request status + pendingOp = await serviceProviderFactory.getPendingUpdateDeployerCutRequest(stakerAccount) + assert.isTrue(pendingOp.newDeployerCut.eq(_lib.toBN(0)), 'Expect cancelled request') + assert.isTrue(pendingOp.lockupExpiryBlock.eq(_lib.toBN(0)), 'Expect cancelled request') + // Confirm no change in deployer cut + info = await serviceProviderFactory.getServiceProviderDetails(stakerAccount) + assert.isTrue((info.deployerCut).eq(_lib.toBN(preUpdatecut)), 'Expect updated cut') + let invalidCut = 110 let base = await serviceProviderFactory.getServiceProviderDeployerCutBase() assert.isTrue(_lib.toBN(invalidCut).gt(base), 'Expect invalid newCut')