From 07d15992a6d597eb5f49fc6f32ce88c9444c8a73 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Tue, 21 Nov 2023 20:50:11 +0000 Subject: [PATCH 01/16] refactor: evict inactive services in the checkpoint --- contracts/staking/ServiceStakingBase.sol | 182 +++++++++++++------ contracts/test/ReentrancyStakingAttacker.sol | 17 +- test/ServiceStaking.js | 9 +- 3 files changed, 133 insertions(+), 75 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index f6f0af44..beb384c1 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -132,10 +132,10 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } event ServiceStaked(uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256[] nonces); - event Checkpoint(uint256 availableRewards, uint256 numServices); + event Checkpoint(uint256 indexed epoch, uint256 availableRewards, uint256[] serviceIds, uint256[] rewards); event ServiceUnstaked(uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256[] nonces, uint256 reward, uint256 tsStart); - event ServiceEvicted(uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256 inactivity); + event ServicesEvicted(uint256[] serviceIds, address[] owners, address[] multisigs, uint256[] serviceInactivity); event Deposit(address indexed sender, uint256 amount, uint256 balance, uint256 availableRewards); event Withdraw(address indexed to, uint256 amount); @@ -167,6 +167,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Max allowed inactivity uint256 public immutable maxAllowedInactivity; + // Epoch counter + uint256 public epochCounter; // Token / ETH balance uint256 public balance; // Token / ETH available rewards @@ -435,20 +437,80 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } } + /// @dev Evicts the service due to its extended inactivity. + function _evict(uint256[] memory evictServiceIds) internal { + uint256 totalNumServices = evictServiceIds.length; + uint256 numEvictServices; + + // Get the number of evicted services + for (uint256 i = 0; i < totalNumServices; ++i) { + if (evictServiceIds[i] > 0) { + numEvictServices++; + } + } + + if (numEvictServices > 0) { + // Get arrays of exact sizes + uint256[] memory serviceIds = new uint256[](numEvictServices); + address[] memory owners = new address[](numEvictServices); + address[] memory multisigs = new address[](numEvictServices); + uint256[] memory serviceInactivity = new uint256[](numEvictServices); + uint256[] memory serviceIndexes = new uint256[](numEvictServices); + + // Fill in arrays + uint256 sCounter; + uint256 serviceId; + for (uint256 i = 0; i < totalNumServices; ++i) { + if (evictServiceIds[i] > 0) { + serviceId = evictServiceIds[i]; + serviceIds[sCounter] = serviceId; + + ServiceInfo storage sInfo = mapServiceInfo[serviceId]; + owners[sCounter] = sInfo.owner; + multisigs[sCounter] = sInfo.multisig; + serviceInactivity[sCounter] = sInfo.inactivity; + serviceIndexes[sCounter] = i; + sCounter++; + } + } + + // Evict services from the global set of staked services + uint256 idx; + for (uint256 i = numEvictServices - 1; i > 0; --i) { + // Decrease the number of services + totalNumServices--; + // Get the evicted service index + idx = serviceIndexes[i]; + // Assign last service Id to the index that points to the evicted service Id + setServiceIds[idx] = setServiceIds[totalNumServices]; + // Pop the last element + setServiceIds.pop(); + } + + // Deal with the very first element + // Get the evicted service index + idx = serviceIndexes[0]; + // Assign last service Id to the index that points to the evicted service Id + setServiceIds[idx] = setServiceIds[totalNumServices - 1]; + // Pop the last element + setServiceIds.pop(); + + emit ServicesEvicted(serviceIds, owners, multisigs, serviceInactivity); + } + } + /// @dev Checkpoint to allocate rewards up until a current time. - /// @return All staking service Ids. - /// @return All staking updated nonces. - /// @return Number of reward-eligible staking services during current checkpoint period. - /// @return Eligible service Ids. - /// @return Eligible service rewards. - /// @return success True, if the checkpoint was successful. + /// @return All staking service Ids (including evicted ones during within a current epoch). + /// @return All staking updated nonces (including evicted ones during within a current epoch). + /// @return Set of eligible service Ids. + /// @return Corresponding set of eligible service rewards. + /// @return evictServiceIds Evicted service Ids. function checkpoint() public returns ( uint256[] memory, uint256[][] memory, - uint256, uint256[] memory, uint256[] memory, - bool success + uint256[] memory evictServiceIds ) { // Calculate staking rewards @@ -457,9 +519,16 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { uint256[] memory serviceIds, uint256[][] memory serviceNonces, uint256[] memory serviceInactivity) = _calculateStakingRewards(); + // Get arrays for eligible service Ids and rewards of exact size + uint256[] memory finalEligibleServiceIds; + uint256[] memory finalEligibleServiceRewards; + evictServiceIds = new uint256[](serviceIds.length); + uint256 curServiceId; + // If there are eligible services, proceed with staking calculation and update rewards if (numServices > 0) { - uint256 curServiceId; + finalEligibleServiceIds = new uint256[](numServices); + finalEligibleServiceRewards = new uint256[](numServices); // If total allocated rewards are not enough, adjust the reward value if (totalRewards > lastAvailableRewards) { // Traverse all the eligible services and adjust their rewards proportional to leftovers @@ -472,6 +541,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { updatedTotalRewards += updatedReward; // Add reward to the overall service reward curServiceId = eligibleServiceIds[i]; + finalEligibleServiceIds[i] = eligibleServiceIds[i]; + finalEligibleServiceRewards[i] = updatedReward; mapServiceInfo[curServiceId].reward += updatedReward; } @@ -479,10 +550,12 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { updatedReward = (eligibleServiceRewards[0] * lastAvailableRewards) / totalRewards; updatedTotalRewards += updatedReward; curServiceId = eligibleServiceIds[0]; + finalEligibleServiceIds[0] = eligibleServiceIds[0]; // If the reward adjustment happened to have small leftovers, add it to the first service if (lastAvailableRewards > updatedTotalRewards) { updatedReward += lastAvailableRewards - updatedTotalRewards; } + finalEligibleServiceRewards[0] = updatedReward; // Add reward to the overall service reward mapServiceInfo[curServiceId].reward += updatedReward; // Set available rewards to zero @@ -492,6 +565,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { for (uint256 i = 0; i < numServices; ++i) { // Add reward to the service overall reward curServiceId = eligibleServiceIds[i]; + finalEligibleServiceIds[i] = eligibleServiceIds[i]; + finalEligibleServiceRewards[i] = eligibleServiceRewards[i]; mapServiceInfo[curServiceId].reward += eligibleServiceRewards[i]; } @@ -505,30 +580,43 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // If service Ids are returned, then the checkpoint takes place if (serviceIds.length > 0) { - // Updated current service nonces + // Record service inactivities and updated current service nonces for (uint256 i = 0; i < serviceIds.length; ++i) { // Get the current service Id - uint256 curServiceId = serviceIds[i]; + curServiceId = serviceIds[i]; + // Record service nonces mapServiceInfo[curServiceId].nonces = serviceNonces[i]; // Increase service inactivity if it is greater than zero if (serviceInactivity[i] > 0) { - mapServiceInfo[curServiceId].inactivity += serviceInactivity[i]; + // Get the overall continuous service inactivity + uint256 inactivity = mapServiceInfo[curServiceId].inactivity + serviceInactivity[i]; + mapServiceInfo[curServiceId].inactivity = inactivity; + // Check for the maximum allowed inactivity time + if (inactivity > maxAllowedInactivity) { + // Evict a service if it has been inactive for more than a maximum allowed inactivity time + evictServiceIds[i] = curServiceId; + } } else { // Otherwise, set it back to zero mapServiceInfo[curServiceId].inactivity = 0; } } + // Evict inactive services + _evict(evictServiceIds); + // Record the current timestamp such that next calculations start from this point of time tsCheckpoint = block.timestamp; - success = true; + // Increase the epoch counter + uint256 eCounter = epochCounter; + epochCounter = eCounter + 1; - emit Checkpoint(lastAvailableRewards, numServices); + emit Checkpoint(eCounter, lastAvailableRewards, finalEligibleServiceIds, finalEligibleServiceRewards); } - return (serviceIds, serviceNonces, numServices, eligibleServiceIds, eligibleServiceRewards, success); + return (serviceIds, serviceNonces, finalEligibleServiceIds, finalEligibleServiceRewards, evictServiceIds); } /// @dev Unstakes the service. @@ -540,26 +628,37 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { revert OwnerOnly(msg.sender, sInfo.owner); } - // Check that the service has staked long enough, or if there are no rewards left uint256 tsStart = sInfo.tsStart; + // Check if the service is staked + if (tsStart == 0) { + revert ServiceNotStaked(serviceId); + } + + // Check that the service has staked long enough, or if there are no rewards left uint256 ts = block.timestamp - tsStart; if (ts <= maxAllowedInactivity && availableRewards > 0) { revert NotEnoughTimeStaked(serviceId, ts, maxAllowedInactivity); } // Call the checkpoint - (uint256[] memory serviceIds, , , , , bool success) = checkpoint(); + (uint256[] memory serviceIds, , , , uint256[] memory evictServiceIds) = checkpoint(); // If the checkpoint was not successful, the serviceIds set is not returned and needs to be allocated - if (!success) { + if (serviceIds.length == 0) { serviceIds = getServiceIds(); + evictServiceIds = new uint256[](serviceIds.length); } // Get the service index in the set of services // The index must always exist as the service is currently staked, otherwise it has no record in the map uint256 idx; + bool inSet; for (; idx < serviceIds.length; ++idx) { - if (serviceIds[idx] == serviceId) { + // Service is still in a global staking set if its index is found and the service was not evicted + if (serviceIds[idx] == serviceId && evictServiceIds[idx] != serviceId) { + inSet = true; + break; + } else if (evictServiceIds[idx] == serviceId) { break; } } @@ -575,7 +674,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Update the set of staked service Ids // If the index was not found, the service was evicted and is not part of staked services set - if (idx < serviceIds.length) { + if (inSet) { setServiceIds[idx] = setServiceIds[setServiceIds.length - 1]; setServiceIds.pop(); } @@ -591,39 +690,6 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { emit ServiceUnstaked(serviceId, msg.sender, multisig, nonces, reward, tsStart); } - /// @dev Evicts the service due to its extended inactivity. - function evict(uint256 serviceId) external { - ServiceInfo storage sInfo = mapServiceInfo[serviceId]; - // Get the service inactivity time - uint256 inactivity = sInfo.inactivity; - - // Evict the service if it is inactive more than the max number of inactivity periods - if (inactivity > maxAllowedInactivity) { - // Get service Ids to find the service - uint256[] memory serviceIds = getServiceIds(); - - // Get the service index in the set of services - // The index must always exist if the service is currently staked, otherwise it never existed or was evicted - uint256 idx; - for (; idx < serviceIds.length; ++idx) { - if (serviceIds[idx] == serviceId) { - break; - } - } - - // The service was already evicted - if (idx == serviceIds.length) { - revert ServiceNotFound(serviceId); - } - - // Evict the service from the set of staked services - setServiceIds[idx] = setServiceIds[setServiceIds.length - 1]; - setServiceIds.pop(); - - emit ServiceEvicted(serviceId, sInfo.owner, sInfo.multisig, inactivity); - } - } - /// @dev Calculates service staking reward during the last checkpoint period. /// @param serviceId Service Id. /// @return reward Service reward. @@ -655,11 +721,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { ServiceInfo memory sInfo = mapServiceInfo[serviceId]; reward = sInfo.reward; - // Check if the service is staked - if (sInfo.tsStart == 0) { - revert ServiceNotStaked(serviceId); - } - + // Add pending reward reward += calculateServiceStakingLastReward(serviceId); } diff --git a/contracts/test/ReentrancyStakingAttacker.sol b/contracts/test/ReentrancyStakingAttacker.sol index 2cbbc942..b24177bc 100644 --- a/contracts/test/ReentrancyStakingAttacker.sol +++ b/contracts/test/ReentrancyStakingAttacker.sol @@ -12,20 +12,17 @@ interface IServiceStaking { /// @param serviceId Service Id. function unstake(uint256 serviceId) external; - /// @dev Checkpoint to allocate rewards up until a current time. - /// @return All staking service Ids. - /// @return All staking updated nonces. - /// @return Number of reward-eligible staking services during current checkpoint period. - /// @return Eligible service Ids. - /// @return Eligible service rewards. - /// @return success True, if the checkpoint was successful. + /// @return All staking service Ids (including evicted ones during within a current epoch). + /// @return All staking updated nonces (including evicted ones during within a current epoch). + /// @return Set of eligible service Ids. + /// @return Corresponding set of eligible service rewards. + /// @return evictServiceIds Evicted service Ids. function checkpoint() external returns ( uint256[] memory, + uint256[][] memory, uint256[] memory, - uint256, uint256[] memory, - uint256[] memory, - bool + uint256[] memory evictServiceIds ); } diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index a857ffcc..87a9f776 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -4,7 +4,7 @@ const { ethers } = require("hardhat"); const helpers = require("@nomicfoundation/hardhat-network-helpers"); const safeContracts = require("@gnosis.pm/safe-contracts"); -describe("ServiceStaking", function () { +describe.only("ServiceStaking", function () { let componentRegistry; let agentRegistry; let serviceRegistry; @@ -536,10 +536,9 @@ describe("ServiceStaking", function () { await serviceStakingToken.stake(sId); }); - it("Should fail when calculating staking rewards for the not staked service", async function () { - await expect( - serviceStaking.calculateServiceStakingReward(serviceId) - ).to.be.revertedWithCustomError(serviceStaking, "ServiceNotStaked"); + it("Returns zero rewards for the not staked service", async function () { + const reward = await serviceStaking.calculateServiceStakingReward(serviceId); + expect(reward).to.equal(0); }); }); From e0d6e0eefdb6a168de1d5a761bd0aff3778c49d2 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Tue, 21 Nov 2023 20:52:06 +0000 Subject: [PATCH 02/16] chore: adding natspec --- contracts/staking/ServiceStakingBase.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index beb384c1..a249b566 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -437,7 +437,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } } - /// @dev Evicts the service due to its extended inactivity. + /// @dev Evicts services due to their extended inactivity. + /// @param evictServiceIds Service Ids to be evicted. function _evict(uint256[] memory evictServiceIds) internal { uint256 totalNumServices = evictServiceIds.length; uint256 numEvictServices; From 4afe6850d89033b85f2cfc7d0472b5ea1d9251e6 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 22 Nov 2023 17:00:35 +0000 Subject: [PATCH 03/16] test: adding to the full coverage --- contracts/staking/ServiceStakingBase.sol | 30 ++-- test/ServiceStaking.js | 182 ++++++++++++++++++++++- 2 files changed, 192 insertions(+), 20 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index a249b566..72286233 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -70,10 +70,6 @@ error LowerThan(uint256 provided, uint256 expected); /// @param serviceId Service Id. error WrongServiceConfiguration(uint256 serviceId); -/// @dev Service is not staked. -/// @param serviceId Service Id. -error ServiceNotStaked(uint256 serviceId); - /// @dev Service is not unstaked. /// @param serviceId Service Id. error ServiceNotUnstaked(uint256 serviceId); @@ -109,6 +105,12 @@ struct ServiceInfo { /// @author Andrey Lebedev - /// @author Mariapia Moscatiello - abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { + enum ServiceStakingState { + Unstaked, + Staked, + Evicted + } + // Input staking parameters struct StakingParams { // Maximum number of staking services @@ -373,6 +375,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { /// @param eligibleServiceRewards Corresponding rewards for eligible service Ids. /// @param serviceIds All the staking service Ids. /// @param serviceNonces Current service nonces. + /// @param serviceInactivity Service inactivity records. function _calculateStakingRewards() internal view returns ( uint256 lastAvailableRewards, uint256 numServices, @@ -629,11 +632,9 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { revert OwnerOnly(msg.sender, sInfo.owner); } + // Get the staking start time + // Note that if the service info exists, the service is staked or evicted, and thus start time is always valid uint256 tsStart = sInfo.tsStart; - // Check if the service is staked - if (tsStart == 0) { - revert ServiceNotStaked(serviceId); - } // Check that the service has staked long enough, or if there are no rewards left uint256 ts = block.timestamp - tsStart; @@ -726,11 +727,16 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { reward += calculateServiceStakingLastReward(serviceId); } - /// @dev Checks if the service is staked. + /// @dev Gets the service staking state. /// @param serviceId. - /// @return isStaked True, if the service is staked. - function isServiceStaked(uint256 serviceId) external view returns (bool isStaked) { - isStaked = (mapServiceInfo[serviceId].tsStart > 0); + /// @return stakingState Staking state of the service. + function getServiceStakingState(uint256 serviceId) external view returns (ServiceStakingState stakingState) { + ServiceInfo memory sInfo = mapServiceInfo[serviceId]; + if (sInfo.inactivity > maxAllowedInactivity) { + stakingState = ServiceStakingState.Evicted; + } else if (sInfo.tsStart > 0) { + stakingState = ServiceStakingState.Staked; + } } /// @dev Gets the next reward checkpoint timestamp. diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index 87a9f776..b6f14f36 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -4,7 +4,7 @@ const { ethers } = require("hardhat"); const helpers = require("@nomicfoundation/hardhat-network-helpers"); const safeContracts = require("@gnosis.pm/safe-contracts"); -describe.only("ServiceStaking", function () { +describe("ServiceStaking", function () { let componentRegistry; let agentRegistry; let serviceRegistry; @@ -233,6 +233,7 @@ describe.only("ServiceStaking", function () { await expect(ServiceStakingToken.deploy(testServiceParams, AddressZero, AddressZero, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroAddress"); await expect(ServiceStakingToken.deploy(testServiceParams, serviceRegistry.address, AddressZero, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroValue"); + await expect(ServiceStakingToken.deploy(testServiceParams, serviceRegistry.address, AddressZero, token.address, bytecodeHash)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroAddress"); await expect(ServiceStakingToken.deploy(testServiceParams, serviceRegistry.address, serviceRegistryTokenUtility.address, AddressZero, bytecodeHash)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroAddress"); }); }); @@ -543,6 +544,28 @@ describe.only("ServiceStaking", function () { }); context("Staking and unstaking", function () { + it("Should fail when trying to unstake without staking for a minimum required time", async function () { + // Take a snapshot of the current state of the blockchain + const snapshot = await helpers.takeSnapshot(); + + // Deposit to the contract + await deployer.sendTransaction({to: serviceStaking.address, value: ethers.utils.parseEther("1")}); + + // Approve services + await serviceRegistry.approve(serviceStaking.address, serviceId); + + // Stake the service + await serviceStaking.stake(serviceId); + + // Trying to unstake right away + await expect( + serviceStaking.unstake(serviceId) + ).to.be.revertedWithCustomError(serviceStaking, "NotEnoughTimeStaked"); + + // Restore a previous state of blockchain + snapshot.restore(); + }); + it("Stake and unstake without any service activity", async function () { // Take a snapshot of the current state of the blockchain const snapshot = await helpers.takeSnapshot(); @@ -557,8 +580,8 @@ describe.only("ServiceStaking", function () { await serviceStaking.stake(serviceId); // Check that the service is staked - const isStaked = await serviceStaking.isServiceStaked(serviceId); - expect(isStaked).to.equal(true); + let stakingState = await serviceStaking.getServiceStakingState(serviceId); + expect(stakingState).to.equal(1); // Get the service multisig contract const service = await serviceRegistry.getService(serviceId); @@ -571,18 +594,35 @@ describe.only("ServiceStaking", function () { const reward = await serviceStaking.calculateServiceStakingReward(serviceId); expect(reward).to.equal(0); + // Call the checkpoint + await serviceStaking.checkpoint(); + + // By this time, services are evicted + // Check that the service is evicted + stakingState = await serviceStaking.getServiceStakingState(serviceId); + expect(stakingState).to.equal(2); + + // After the eviction, check the final serviceIds set to be empty + const serviceIds = await serviceStaking.getServiceIds(); + expect(serviceIds.length).to.equal(0); + + // Try to stake evicted service + await expect( + serviceStaking.stake(serviceId) + ).to.be.revertedWithCustomError(serviceStaking, "ServiceNotUnstaked"); + // Unstake the service const balanceBefore = await ethers.provider.getBalance(multisig.address); await serviceStaking.unstake(serviceId); const balanceAfter = await ethers.provider.getBalance(multisig.address); + // Check that the service is unstaked + stakingState = await serviceStaking.getServiceStakingState(serviceId); + expect(stakingState).to.equal(0); + // The multisig balance before and after unstake must be the same (zero reward) expect(balanceBefore).to.equal(balanceAfter); - // Check the final serviceIds set to be empty - const serviceIds = await serviceStaking.getServiceIds(); - expect(serviceIds.length).to.equal(0); - // Restore a previous state of blockchain snapshot.restore(); }); @@ -963,9 +1003,135 @@ describe.only("ServiceStaking", function () { await safeContracts.executeTx(multisig, txHashData, [signMessageData], 0); } + // Check staking status to be staked + for (let i = 0; i < 3; i++) { + const stakingState = await serviceStaking.getServiceStakingState(serviceId + i); + expect(stakingState).to.equal(1); + } // Increase the time for the liveness period - await helpers.time.increase(maxInactivity); + await helpers.time.increase(livenessPeriod); + + // Call the checkpoint + await serviceStaking.checkpoint(); + + // Check staking status to be staked + for (let i = 0; i < 3; i++) { + const stakingState = await serviceStaking.getServiceStakingState(serviceId + i); + expect(stakingState).to.equal(1); + } + + for (let i = 0; i < 3; i++) { + // Calculate service staking reward that must be greater than zero except for the serviceId == 3 + const reward = await serviceStaking.calculateServiceStakingReward(serviceId + i); + if (i < 2) { + expect(reward).to.greaterThan(0); + } else { + expect(reward).to.equal(0); + } + + // Get the service multisig contract + const service = await serviceRegistry.getService(serviceId + i); + const multisig = await ethers.getContractAt("GnosisSafe", service.multisig); + + // Unstake services + const balanceBefore = ethers.BigNumber.from(await ethers.provider.getBalance(multisig.address)); + await serviceStaking.unstake(serviceId + i); + const balanceAfter = ethers.BigNumber.from(await ethers.provider.getBalance(multisig.address)); + + // The balance before and after the unstake call must be different except for the serviceId == 3 + if (i < 2) { + expect(balanceAfter).to.gt(balanceBefore); + } else { + expect(reward).to.equal(0); + } + } + + // Check the final serviceIds set to be empty + const serviceIds = await serviceStaking.getServiceIds(); + expect(serviceIds.length).to.equal(0); + + // Restore a previous state of blockchain + snapshot.restore(); + }); + + it("Stake and unstake with one service being inactive", async function () { + // Take a snapshot of the current state of the blockchain + const snapshot = await helpers.takeSnapshot(); + + // Deposit to the contract + await deployer.sendTransaction({to: serviceStaking.address, value: ethers.utils.parseEther("1")}); + + // Create and deploy one more service (serviceId == 3) + await serviceRegistry.create(deployer.address, defaultHash, agentIds, agentParams, threshold); + await serviceRegistry.activateRegistration(deployer.address, serviceId + 2, {value: regDeposit}); + await serviceRegistry.registerAgents(operator.address, serviceId + 2, [agentInstances[2].address], agentIds, {value: regBond}); + await serviceRegistry.deploy(deployer.address, serviceId + 2, gnosisSafeMultisig.address, payload); + + for (let i = 0; i < 3; i++) { + // Approve services + await serviceRegistry.approve(serviceStaking.address, serviceId + i); + + // Stake the first service + await serviceStaking.stake(serviceId + i); + + // Get the service multisig contract + const service = await serviceRegistry.getService(serviceId + i); + const multisig = await ethers.getContractAt("GnosisSafe", service.multisig); + + // Make transactions by the service multisig, except for the service Id == 3 + if (i < 2) { + const nonce = await multisig.nonce(); + const txHashData = await safeContracts.buildContractCall(multisig, "getThreshold", [], nonce, 0, 0); + const signMessageData = await safeContracts.safeSignMessage(agentInstances[i], multisig, txHashData, 0); + await safeContracts.executeTx(multisig, txHashData, [signMessageData], 0); + } + } + + // Increase the time for the liveness period + await helpers.time.increase(livenessPeriod); + + // Calculate service staking reward that must be greater than zero except for the serviceId == 3 + for (let i = 0; i < 2; i++) { + const reward = await serviceStaking.calculateServiceStakingReward(serviceId + i); + expect(reward).to.greaterThan(0); + } + + // Call the checkpoint at this time + await serviceStaking.checkpoint(); + + // Execute one more multisig tx for services except for the service Id == 3 + for (let i = 0; i < 2; i++) { + // Get the service multisig contract + const service = await serviceRegistry.getService(serviceId + i); + const multisig = await ethers.getContractAt("GnosisSafe", service.multisig); + + const nonce = await multisig.nonce(); + const txHashData = await safeContracts.buildContractCall(multisig, "getThreshold", [], nonce, 0, 0); + const signMessageData = await safeContracts.safeSignMessage(agentInstances[i], multisig, txHashData, 0); + await safeContracts.executeTx(multisig, txHashData, [signMessageData], 0); + } + + // Check staking status to be staked + for (let i = 0; i < 3; i++) { + const stakingState = await serviceStaking.getServiceStakingState(serviceId + i); + expect(stakingState).to.equal(1); + } + + // Increase the time for the twice the liveness period + await helpers.time.increase(2 * livenessPeriod); + + // Call the checkpoint + await serviceStaking.checkpoint(); + + // Check staking status to be staked for the first two services + for (let i = 0; i < 2; i++) { + const stakingState = await serviceStaking.getServiceStakingState(serviceId + i); + expect(stakingState).to.equal(1); + } + // The last service must be evicted by that time + const stakingState = await serviceStaking.getServiceStakingState(serviceId + 2); + expect(stakingState).to.equal(2); for (let i = 0; i < 3; i++) { // Calculate service staking reward that must be greater than zero except for the serviceId == 3 From 2a99ef52320aac734b1cfecd78b6b3a82514f924 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 22 Nov 2023 17:59:15 +0000 Subject: [PATCH 04/16] test: adjust forge tests --- test/ServiceStaking.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/ServiceStaking.t.sol b/test/ServiceStaking.t.sol index 6ccd2763..7c2feec1 100644 --- a/test/ServiceStaking.t.sol +++ b/test/ServiceStaking.t.sol @@ -233,7 +233,7 @@ contract ServiceStaking is BaseSetup { for (uint256 j = 0; j < numServices; ++j) { uint256 serviceId = j + 1; // Unstake if the service is not yet unstaked, otherwise ignore - if (!serviceStakingNativeToken.isServiceStaked(serviceId)) { + if (uint8(serviceStakingNativeToken.getServiceStakingState(serviceId)) > 0) { vm.startPrank(deployer); serviceStakingNativeToken.unstake(serviceId); vm.stopPrank(); @@ -306,7 +306,7 @@ contract ServiceStaking is BaseSetup { for (uint256 j = 0; j < numServices; ++j) { uint256 serviceId = j + 1; // Unstake if the service is not yet unstaked, otherwise ignore - if (!serviceStakingNativeToken.isServiceStaked(serviceId)) { + if (uint8(serviceStakingNativeToken.getServiceStakingState(serviceId)) > 0) { vm.startPrank(deployer); serviceStakingNativeToken.unstake(serviceId); vm.stopPrank(); @@ -377,7 +377,7 @@ contract ServiceStaking is BaseSetup { for (uint256 j = 0; j < numServices; ++j) { uint256 serviceId = j + 1; // Unstake if the service is not yet unstaked, otherwise ignore - if (!serviceStakingNativeToken.isServiceStaked(serviceId)) { + if (uint8(serviceStakingNativeToken.getServiceStakingState(serviceId)) > 0) { vm.startPrank(deployer); serviceStakingNativeToken.unstake(serviceId); vm.stopPrank(); @@ -449,7 +449,7 @@ contract ServiceStaking is BaseSetup { for (uint256 j = 0; j < numServices; ++j) { uint256 serviceId = j + numServices + 1; // Unstake if the service is not yet unstaked, otherwise ignore - if (!serviceStakingToken.isServiceStaked(serviceId)) { + if (uint8(serviceStakingToken.getServiceStakingState(serviceId)) > 0) { vm.startPrank(deployer); serviceStakingToken.unstake(serviceId); vm.stopPrank(); From f0376bed74fa410c432b66f1eacf16a02cffc864 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 22 Nov 2023 18:44:40 +0000 Subject: [PATCH 05/16] chore: adding epoch counters into emits --- contracts/staking/ServiceStakingBase.sol | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 72286233..03e77e52 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -133,11 +133,13 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { bytes32 configHash; } - event ServiceStaked(uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256[] nonces); + event ServiceStaked(uint256 indexed epoch, uint256 indexed serviceId, address indexed multisig, address owner, + uint256[] nonces); event Checkpoint(uint256 indexed epoch, uint256 availableRewards, uint256[] serviceIds, uint256[] rewards); - event ServiceUnstaked(uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256[] nonces, - uint256 reward, uint256 tsStart); - event ServicesEvicted(uint256[] serviceIds, address[] owners, address[] multisigs, uint256[] serviceInactivity); + event ServiceUnstaked(uint256 indexed epoch, uint256 indexed serviceId, address indexed multisig, address owner, + uint256[] nonces, uint256 reward, uint256 tsStart); + event ServicesEvicted(uint256 indexed epoch, uint256[] serviceIds, address[] owners, address[] multisigs, + uint256[] serviceInactivity); event Deposit(address indexed sender, uint256 amount, uint256 balance, uint256 availableRewards); event Withdraw(address indexed to, uint256 amount); @@ -333,7 +335,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Transfer the service for staking IService(serviceRegistry).safeTransferFrom(msg.sender, address(this), serviceId); - emit ServiceStaked(serviceId, msg.sender, service.multisig, nonces); + emit ServiceStaked(epochCounter, serviceId, service.multisig, msg.sender, nonces); } /// @dev Gets service multisig nonces. @@ -499,7 +501,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Pop the last element setServiceIds.pop(); - emit ServicesEvicted(serviceIds, owners, multisigs, serviceInactivity); + emit ServicesEvicted(epochCounter, serviceIds, owners, multisigs, serviceInactivity); } } @@ -689,7 +691,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { _withdraw(multisig, reward); } - emit ServiceUnstaked(serviceId, msg.sender, multisig, nonces, reward, tsStart); + emit ServiceUnstaked(epochCounter, serviceId, multisig, msg.sender, nonces, reward, tsStart); } /// @dev Calculates service staking reward during the last checkpoint period. From 7077fbdcb3f2647cc1ca53ec9422ea62e5696eaf Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 09:50:03 +0000 Subject: [PATCH 06/16] refactor: small optimizing refactor --- contracts/staking/ServiceStakingBase.sol | 26 +++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 03e77e52..4cbde93c 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -444,7 +444,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { /// @dev Evicts services due to their extended inactivity. /// @param evictServiceIds Service Ids to be evicted. - function _evict(uint256[] memory evictServiceIds) internal { + /// @param serviceInactivity Corresponding service inactivity records. + function _evict(uint256[] memory evictServiceIds, uint256[] memory serviceInactivity) internal { uint256 totalNumServices = evictServiceIds.length; uint256 numEvictServices; @@ -460,7 +461,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { uint256[] memory serviceIds = new uint256[](numEvictServices); address[] memory owners = new address[](numEvictServices); address[] memory multisigs = new address[](numEvictServices); - uint256[] memory serviceInactivity = new uint256[](numEvictServices); + uint256[] memory inactivity = new uint256[](numEvictServices); uint256[] memory serviceIndexes = new uint256[](numEvictServices); // Fill in arrays @@ -474,7 +475,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { ServiceInfo storage sInfo = mapServiceInfo[serviceId]; owners[sCounter] = sInfo.owner; multisigs[sCounter] = sInfo.multisig; - serviceInactivity[sCounter] = sInfo.inactivity; + inactivity[sCounter] = serviceInactivity[i]; serviceIndexes[sCounter] = i; sCounter++; } @@ -501,7 +502,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Pop the last element setServiceIds.pop(); - emit ServicesEvicted(epochCounter, serviceIds, owners, multisigs, serviceInactivity); + emit ServicesEvicted(epochCounter, serviceIds, owners, multisigs, inactivity); } } @@ -596,10 +597,10 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Increase service inactivity if it is greater than zero if (serviceInactivity[i] > 0) { // Get the overall continuous service inactivity - uint256 inactivity = mapServiceInfo[curServiceId].inactivity + serviceInactivity[i]; - mapServiceInfo[curServiceId].inactivity = inactivity; + serviceInactivity[i] = mapServiceInfo[curServiceId].inactivity + serviceInactivity[i]; + mapServiceInfo[curServiceId].inactivity = serviceInactivity[i]; // Check for the maximum allowed inactivity time - if (inactivity > maxAllowedInactivity) { + if (serviceInactivity[i] > maxAllowedInactivity) { // Evict a service if it has been inactive for more than a maximum allowed inactivity time evictServiceIds[i] = curServiceId; } @@ -610,7 +611,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } // Evict inactive services - _evict(evictServiceIds); + _evict(evictServiceIds, serviceInactivity); // Record the current timestamp such that next calculations start from this point of time tsCheckpoint = block.timestamp; @@ -658,11 +659,12 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { uint256 idx; bool inSet; for (; idx < serviceIds.length; ++idx) { - // Service is still in a global staking set if its index is found and the service was not evicted - if (serviceIds[idx] == serviceId && evictServiceIds[idx] != serviceId) { - inSet = true; + // Service is still in a global staking set if it is found in the services set, + // and is not present in the evicted set + if (evictServiceIds[idx] == serviceId) { break; - } else if (evictServiceIds[idx] == serviceId) { + } else if (serviceIds[idx] == serviceId) { + inSet = true; break; } } From c1329977f22b48c69fb5ee8f05f1a33b7645b962 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 11:28:40 +0000 Subject: [PATCH 07/16] refactor: small optimizing refactor --- contracts/staking/ServiceStakingBase.sol | 97 ++++++++++++------------ 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 4cbde93c..31212147 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -445,65 +445,61 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { /// @dev Evicts services due to their extended inactivity. /// @param evictServiceIds Service Ids to be evicted. /// @param serviceInactivity Corresponding service inactivity records. - function _evict(uint256[] memory evictServiceIds, uint256[] memory serviceInactivity) internal { + /// @param numEvictServices Number of services to evict. + function _evict( + uint256[] memory evictServiceIds, + uint256[] memory serviceInactivity, + uint256 numEvictServices + ) internal + { uint256 totalNumServices = evictServiceIds.length; - uint256 numEvictServices; - // Get the number of evicted services + // Get arrays of exact sizes + uint256[] memory serviceIds = new uint256[](numEvictServices); + address[] memory owners = new address[](numEvictServices); + address[] memory multisigs = new address[](numEvictServices); + uint256[] memory inactivity = new uint256[](numEvictServices); + uint256[] memory serviceIndexes = new uint256[](numEvictServices); + + // Fill in arrays + uint256 sCounter; + uint256 serviceId; for (uint256 i = 0; i < totalNumServices; ++i) { if (evictServiceIds[i] > 0) { - numEvictServices++; + serviceId = evictServiceIds[i]; + serviceIds[sCounter] = serviceId; + + ServiceInfo storage sInfo = mapServiceInfo[serviceId]; + owners[sCounter] = sInfo.owner; + multisigs[sCounter] = sInfo.multisig; + inactivity[sCounter] = serviceInactivity[i]; + serviceIndexes[sCounter] = i; + sCounter++; } } - if (numEvictServices > 0) { - // Get arrays of exact sizes - uint256[] memory serviceIds = new uint256[](numEvictServices); - address[] memory owners = new address[](numEvictServices); - address[] memory multisigs = new address[](numEvictServices); - uint256[] memory inactivity = new uint256[](numEvictServices); - uint256[] memory serviceIndexes = new uint256[](numEvictServices); - - // Fill in arrays - uint256 sCounter; - uint256 serviceId; - for (uint256 i = 0; i < totalNumServices; ++i) { - if (evictServiceIds[i] > 0) { - serviceId = evictServiceIds[i]; - serviceIds[sCounter] = serviceId; - - ServiceInfo storage sInfo = mapServiceInfo[serviceId]; - owners[sCounter] = sInfo.owner; - multisigs[sCounter] = sInfo.multisig; - inactivity[sCounter] = serviceInactivity[i]; - serviceIndexes[sCounter] = i; - sCounter++; - } - } - - // Evict services from the global set of staked services - uint256 idx; - for (uint256 i = numEvictServices - 1; i > 0; --i) { - // Decrease the number of services - totalNumServices--; - // Get the evicted service index - idx = serviceIndexes[i]; - // Assign last service Id to the index that points to the evicted service Id - setServiceIds[idx] = setServiceIds[totalNumServices]; - // Pop the last element - setServiceIds.pop(); - } - - // Deal with the very first element + // Evict services from the global set of staked services + uint256 idx; + for (uint256 i = numEvictServices - 1; i > 0; --i) { + // Decrease the number of services + totalNumServices--; // Get the evicted service index - idx = serviceIndexes[0]; + idx = serviceIndexes[i]; // Assign last service Id to the index that points to the evicted service Id - setServiceIds[idx] = setServiceIds[totalNumServices - 1]; + setServiceIds[idx] = setServiceIds[totalNumServices]; // Pop the last element setServiceIds.pop(); - - emit ServicesEvicted(epochCounter, serviceIds, owners, multisigs, inactivity); } + + // Deal with the very first element + // Get the evicted service index + idx = serviceIndexes[0]; + // Assign last service Id to the index that points to the evicted service Id + setServiceIds[idx] = setServiceIds[totalNumServices - 1]; + // Pop the last element + setServiceIds.pop(); + + emit ServicesEvicted(epochCounter, serviceIds, owners, multisigs, inactivity); } /// @dev Checkpoint to allocate rewards up until a current time. @@ -587,6 +583,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // If service Ids are returned, then the checkpoint takes place if (serviceIds.length > 0) { + numServices = 0; // Record service inactivities and updated current service nonces for (uint256 i = 0; i < serviceIds.length; ++i) { // Get the current service Id @@ -603,6 +600,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { if (serviceInactivity[i] > maxAllowedInactivity) { // Evict a service if it has been inactive for more than a maximum allowed inactivity time evictServiceIds[i] = curServiceId; + // Increase number of evicted services + numServices++; } } else { // Otherwise, set it back to zero @@ -611,7 +610,9 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } // Evict inactive services - _evict(evictServiceIds, serviceInactivity); + if (numServices > 0) { + _evict(evictServiceIds, serviceInactivity, numServices); + } // Record the current timestamp such that next calculations start from this point of time tsCheckpoint = block.timestamp; From 639044c83a5e386af5489e5cd1999a75d8a6cc2a Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 11:30:18 +0000 Subject: [PATCH 08/16] chore: adding comments --- contracts/staking/ServiceStakingBase.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 31212147..162ef72e 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -452,6 +452,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { uint256 numEvictServices ) internal { + // Get the total number of staked services + // All the passed arrays have the length of the number of staked services uint256 totalNumServices = evictServiceIds.length; // Get arrays of exact sizes From 4c403f0482a4e12b92cebb107afad0b79c07e8b2 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 13:10:59 +0000 Subject: [PATCH 09/16] test: adding tests --- scripts/audit_chains/audit_contracts_setup.js | 6 +- test/ServiceStaking.js | 75 ++++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/scripts/audit_chains/audit_contracts_setup.js b/scripts/audit_chains/audit_contracts_setup.js index 4e852f7b..90dc26b9 100644 --- a/scripts/audit_chains/audit_contracts_setup.js +++ b/scripts/audit_chains/audit_contracts_setup.js @@ -351,7 +351,11 @@ async function main() { for (let j = 0; j < contracts.length; j++) { console.log("Checking " + contracts[j]["name"]); const execSync = require("child_process").execSync; - execSync("scripts/audit_chains/audit_repo_contract.sh " + network + " " + contracts[j]["name"] + " " + contracts[j]["address"]); + try { + execSync("scripts/audit_chains/audit_repo_contract.sh " + network + " " + contracts[j]["name"] + " " + contracts[j]["address"]); + } catch (err) { + err.stderr.toString(); + } } } // ################################# /VERIFY CONTRACTS WITH REPO ################################# diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index b6f14f36..bf251d4d 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -38,7 +38,7 @@ describe("ServiceStaking", function () { const initSupply = "5" + "0".repeat(26); const payload = "0x"; const serviceParams = { - maxNumServices: 10, + maxNumServices: 3, rewardsPerSecond: "1" + "0".repeat(15), minStakingDeposit: 10, livenessPeriod: livenessPeriod, // Ten seconds @@ -318,6 +318,24 @@ describe("ServiceStaking", function () { ).to.be.revertedWithCustomError(sStaking, "WrongServiceConfiguration"); }); + it("Should fail when the multisig hash is incorrect", async function () { + // Deploy a contract with a different service config specification + const ServiceStakingNativeToken = await ethers.getContractFactory("ServiceStakingNativeToken"); + const testBytecodeHash = "0x" + "0".repeat(63) + "1"; + const sStaking = await ServiceStakingNativeToken.deploy(serviceParams, serviceRegistry.address, testBytecodeHash); + await sStaking.deployed(); + + // Deposit to the contract + await deployer.sendTransaction({to: sStaking.address, value: ethers.utils.parseEther("1")}); + + // Approve services + await serviceRegistry.approve(sStaking.address, serviceId); + + await expect( + sStaking.stake(serviceId) + ).to.be.revertedWithCustomError(sStaking, "UnauthorizedMultisig"); + }); + it("Should fail when the specified threshold of the service does not match", async function () { // Deploy a contract with a different service config specification const ServiceStakingNativeToken = await ethers.getContractFactory("ServiceStakingNativeToken"); @@ -1167,6 +1185,61 @@ describe("ServiceStaking", function () { snapshot.restore(); }); + it("Stake the full allowed number of services, evict all of them and stake again", async function () { + // Take a snapshot of the current state of the blockchain + const snapshot = await helpers.takeSnapshot(); + + // Deposit to the contract + await deployer.sendTransaction({to: serviceStaking.address, value: ethers.utils.parseEther("1")}); + + // Create and deploy one more service (serviceId == 3) + await serviceRegistry.create(deployer.address, defaultHash, agentIds, agentParams, threshold); + await serviceRegistry.activateRegistration(deployer.address, serviceId + 2, {value: regDeposit}); + await serviceRegistry.registerAgents(operator.address, serviceId + 2, [agentInstances[2].address], agentIds, {value: regBond}); + await serviceRegistry.deploy(deployer.address, serviceId + 2, gnosisSafeMultisig.address, payload); + + // Stake 3 initial services + for (let i = 0; i < 3; i++) { + // Approve services + await serviceRegistry.approve(serviceStaking.address, serviceId + i); + + // Stake the first service + await serviceStaking.stake(serviceId + i); + } + + // Increase the time for the liveness period + await helpers.time.increase(maxInactivity); + + // Call the checkpoint at this time (all services will be evicted) + await serviceStaking.checkpoint(); + + // Check the staking state of all the services + for (let i = 0; i < 3; i++) { + const stakingState = await serviceStaking.getServiceStakingState(serviceId + i); + expect(stakingState).to.equal(2); + } + + // Try to stake the same service again + await expect( + serviceStaking.stake(serviceId) + ).to.be.revertedWithCustomError(serviceStaking, "ServiceNotUnstaked"); + + // Create and deploy yet one more service (serviceId == 4) + await serviceRegistry.create(deployer.address, defaultHash, [1], agentParams, threshold); + await serviceRegistry.activateRegistration(deployer.address, serviceId + 3, {value: regDeposit}); + await serviceRegistry.registerAgents(operator.address, serviceId + 3, [agentInstances[3].address], agentIds, {value: regBond}); + await serviceRegistry.deploy(deployer.address, serviceId + 3, gnosisSafeMultisig.address, payload); + + // Approve services + await serviceRegistry.approve(serviceStaking.address, serviceId + 3); + + // Stake the service + await serviceStaking.stake(serviceId + 3); + + // Restore a previous state of blockchain + snapshot.restore(); + }); + it("Stake and unstake with the service activity", async function () { // Take a snapshot of the current state of the blockchain const snapshot = await helpers.takeSnapshot(); From 84d79c5fe293ec7405f67273c06f98e1f65088bf Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 13:21:15 +0000 Subject: [PATCH 10/16] chore: gitleaksignore --- .gitleaksignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitleaksignore b/.gitleaksignore index 36d1df14..d7a911ab 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -79,4 +79,6 @@ f1e395c1b3dae68fb7a74f9e493d7115d5784bf3:scripts/deployment/l2/globals_gnosis_ma 6f313cbb0917d4d8f44295884dbf70aa58008a03:scripts/deployment/l2/globals_gnosis_chiado.json:generic-api-key:1 4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:1 20a061b4aa92431dae2000a29b0168daa5af471e:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 -4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 \ No newline at end of file +4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 +08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 +08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 \ No newline at end of file From 1a2b10550903a02a357727f8d9f2f2b02640f4fb Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 13:23:14 +0000 Subject: [PATCH 11/16] chore: gitleaksignore --- .gitleaksignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitleaksignore b/.gitleaksignore index d7a911ab..82af62f7 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -81,4 +81,6 @@ f1e395c1b3dae68fb7a74f9e493d7115d5784bf3:scripts/deployment/l2/globals_gnosis_ma 20a061b4aa92431dae2000a29b0168daa5af471e:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 -08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 \ No newline at end of file +08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 +4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:1 +4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 \ No newline at end of file From fba0b1a80a50c1cdc0b9e46acb616830cb4fe77b Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 23 Nov 2023 13:24:55 +0000 Subject: [PATCH 12/16] chore: gitleaksignore --- .gitleaksignore | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitleaksignore b/.gitleaksignore index 82af62f7..e3360bd5 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -82,5 +82,5 @@ f1e395c1b3dae68fb7a74f9e493d7115d5784bf3:scripts/deployment/l2/globals_gnosis_ma 4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 -4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:1 -4b2749c036e684682089b3b5ab57d75041fb7a03:scripts/deployment/l2/globals_polygon_mumbai.json:generic-api-key:2 \ No newline at end of file +f876f6e96465e05f30236366a6cac1ed7b2017d8:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 +f876f6e96465e05f30236366a6cac1ed7b2017d8:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 \ No newline at end of file From 29a37d14e1f8e7bb36f424314b109f0cdddce8da Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Fri, 24 Nov 2023 09:42:46 +0000 Subject: [PATCH 13/16] chore: comment updates --- test/ServiceStaking.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index bf251d4d..85756fa7 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -260,7 +260,7 @@ describe("ServiceStaking", function () { await serviceRegistry.approve(sStaking.address, serviceId); await serviceRegistry.approve(sStaking.address, serviceId + 1); - // Stake the first service + // Stake the service await sStaking.stake(serviceId); // Staking next service is going to fail @@ -519,7 +519,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Try to unstake not by the owner @@ -594,7 +594,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Check that the service is staked @@ -719,7 +719,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Get the service multisig contract @@ -804,7 +804,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStakingToken.address, sId); - // Stake the first service + // Stake the service await serviceStakingToken.stake(sId); // Get the service multisig contract @@ -862,7 +862,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Get the service multisig contract @@ -916,7 +916,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Get the service multisig contract @@ -981,7 +981,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId + i); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId + i); // Get the service multisig contract @@ -1090,7 +1090,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId + i); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId + i); // Get the service multisig contract @@ -1203,7 +1203,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId + i); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId + i); } @@ -1250,7 +1250,7 @@ describe("ServiceStaking", function () { // Approve services await serviceRegistry.approve(serviceStaking.address, serviceId); - // Stake the first service + // Stake the service await serviceStaking.stake(serviceId); // Take the staking timestamp From eee7f30c9db1ea16fd4300ff73d182ba639c978c Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Fri, 24 Nov 2023 17:41:24 +0000 Subject: [PATCH 14/16] refactor and chore: addressing comments --- contracts/staking/ServiceStakingBase.sol | 35 +++++++++++-------- .../deployment/l2/globals_gnosis_chiado.json | 2 +- .../deployment/l2/globals_gnosis_mainnet.json | 1 + test/ServiceStaking.js | 10 +++++- test/ServiceStaking.t.sol | 13 ++++--- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 162ef72e..5fb7476f 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -96,7 +96,7 @@ struct ServiceInfo { uint256 tsStart; // Accumulated service staking reward uint256 reward; - // Accumulated inactivity that will be used to decide whether the service must be evicted + // Accumulated inactivity that might lead to the service eviction uint256 inactivity; } @@ -119,6 +119,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { uint256 rewardsPerSecond; // Minimum service staking deposit value required for staking uint256 minStakingDeposit; + // Max number of accumulated inactivity periods after which the service is evicted + uint256 maxNumInactivityPeriods; // Liveness period uint256 livenessPeriod; // Liveness ratio in the format of 1e18 @@ -133,11 +135,11 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { bytes32 configHash; } - event ServiceStaked(uint256 indexed epoch, uint256 indexed serviceId, address indexed multisig, address owner, + event ServiceStaked(uint256 epoch, uint256 indexed serviceId, address indexed owner, address indexed multisig, uint256[] nonces); event Checkpoint(uint256 indexed epoch, uint256 availableRewards, uint256[] serviceIds, uint256[] rewards); - event ServiceUnstaked(uint256 indexed epoch, uint256 indexed serviceId, address indexed multisig, address owner, - uint256[] nonces, uint256 reward, uint256 tsStart); + event ServiceUnstaked(uint256 epoch, uint256 indexed serviceId, address indexed owner, address indexed multisig, + uint256[] nonces, uint256 reward); event ServicesEvicted(uint256 indexed epoch, uint256[] serviceIds, address[] owners, address[] multisigs, uint256[] serviceInactivity); event Deposit(address indexed sender, uint256 amount, uint256 balance, uint256 availableRewards); @@ -145,8 +147,6 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Contract version string public constant VERSION = "0.1.0"; - // Max number of accumulated inactivity periods after which the service can be evicted - uint256 public constant MAX_INACTIVITY_PERIODS = 3; // Maximum number of staking services uint256 public immutable maxNumServices; // Rewards per second @@ -154,6 +154,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Minimum service staking deposit value required for staking // The staking deposit must be always greater than 1 in order to distinguish between native and ERC20 tokens uint256 public immutable minStakingDeposit; + // Max number of accumulated inactivity periods after which the service is evicted + uint256 public immutable maxNumInactivityPeriods; // Liveness period uint256 public immutable livenessPeriod; // Liveness ratio in the format of 1e18 @@ -194,7 +196,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Initial checks if (_stakingParams.maxNumServices == 0 || _stakingParams.rewardsPerSecond == 0 || _stakingParams.livenessPeriod == 0 || _stakingParams.livenessRatio == 0 || - _stakingParams.numAgentInstances == 0) { + _stakingParams.numAgentInstances == 0 || _stakingParams.maxNumInactivityPeriods == 0) { revert ZeroValue(); } if (_stakingParams.minStakingDeposit < 2) { @@ -237,7 +239,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { proxyHash = _proxyHash; // Calculate max allowed inactivity - maxAllowedInactivity = MAX_INACTIVITY_PERIODS * livenessPeriod; + maxAllowedInactivity = maxNumInactivityPeriods * livenessPeriod; // Set the checkpoint timestamp to be the deployment one tsCheckpoint = block.timestamp; @@ -258,6 +260,8 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { function _withdraw(address to, uint256 amount) internal virtual; /// @dev Stakes the service. + /// @notice Each service must be staked for a minimum of maxAllowedInactivity time, or until the funds are not zero. + /// maxAllowedInactivity = maxNumInactivityPeriods * livenessPeriod /// @param serviceId Service Id. function stake(uint256 serviceId) external { // Check if there available rewards @@ -267,6 +271,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Check if the evicted service has not yet unstaked ServiceInfo storage sInfo = mapServiceInfo[serviceId]; + // tsStart being greater than zero means that the service was not yet unstaked: still staking or evicted if (sInfo.tsStart > 0) { revert ServiceNotUnstaked(serviceId); } @@ -335,7 +340,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { // Transfer the service for staking IService(serviceRegistry).safeTransferFrom(msg.sender, address(this), serviceId); - emit ServiceStaked(epochCounter, serviceId, service.multisig, msg.sender, nonces); + emit ServiceStaked(epochCounter, serviceId, msg.sender, service.multisig, nonces); } /// @dev Gets service multisig nonces. @@ -424,7 +429,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } // Calculate the liveness ratio in 1e18 value - // This subtraction is always positive or zero, as the last checkpoint can be at most block.timestamp + // This subtraction is always positive or zero, as the last checkpoint is at most block.timestamp ts = block.timestamp - serviceCheckpoint; bool ratioPass = _isRatioPass(serviceNonces[i], sInfo.nonces, ts); @@ -505,10 +510,10 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { } /// @dev Checkpoint to allocate rewards up until a current time. - /// @return All staking service Ids (including evicted ones during within a current epoch). - /// @return All staking updated nonces (including evicted ones during within a current epoch). - /// @return Set of eligible service Ids. - /// @return Corresponding set of eligible service rewards. + /// @return All staking service Ids (including evicted ones within a current epoch). + /// @return All staking updated nonces (including evicted ones within a current epoch). + /// @return Set of reward-eligible service Ids. + /// @return Corresponding set of reward-eligible service rewards. /// @return evictServiceIds Evicted service Ids. function checkpoint() public returns ( uint256[] memory, @@ -696,7 +701,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { _withdraw(multisig, reward); } - emit ServiceUnstaked(epochCounter, serviceId, multisig, msg.sender, nonces, reward, tsStart); + emit ServiceUnstaked(epochCounter, serviceId, msg.sender, multisig, nonces, reward); } /// @dev Calculates service staking reward during the last checkpoint period. diff --git a/scripts/deployment/l2/globals_gnosis_chiado.json b/scripts/deployment/l2/globals_gnosis_chiado.json index 72810555..d3756bb0 100644 --- a/scripts/deployment/l2/globals_gnosis_chiado.json +++ b/scripts/deployment/l2/globals_gnosis_chiado.json @@ -1 +1 @@ -{"contractVerification":true,"useLedger":false,"derivationPath":"m/44'/60'/2'/0/0","providerName":"chiado","gasPriceInGwei":"3","gnosisSafeAddress":"0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552","gnosisSafeProxyFactoryAddress":"0xa6B71E26C5e0845f74c812102Ca7114b6a896AB2","baseURI":"https://gateway.autonolas.tech/ipfs/","serviceRegistryName":"Service Registry","serviceRegistrySymbol":"AUTONOLAS-SERVICE-V1","AMBContractProxyForeignAddress":"0x87A19d769D875964E9Cd41dDBfc397B2543764E6","bridgeMediatorAddress":"0x670Ac235EE13C0B2a5065282bBB0c61cfB354592","bridgeMediatorMockTimelockAddress":"0x0a50009D55Ed5700ac8FF713709d5Ad5fa843896","serviceRegistryAddress":"0x31D3202d8744B16A120117A053459DDFAE93c855","serviceManagerAddress":"0x29086141ecdc310058fc23273F8ef7881d20C2f7","gnosisSafeMultisigImplementationAddress":"0xeB49bE5DF00F74bd240DE4535DDe6Bc89CEfb994","gnosisSafeSameAddressMultisigImplementationAddress":"0xE16adc7777B7C2a0d35033bd3504C028AB28EE8b","operatorWhitelistAddress":"0x6f7661F52fE1919996d0A4F68D09B344093a349d","serviceRegistryTokenUtilityAddress":"0xc2c7E40674f1C7Bb99eFe5680Efd79842502bED4","serviceManagerTokenAddress":"0xc965a32185590Eb5a5fffDba29E96126b7650eDe","olasAddress":"0xE40AE73aa0Ed3Ec35fdAF56e01FCd0D1Ff1d9AB6","multisigProxyHash130":"0xb89c1b3bdf2cf8827818646bce9a8f6e372885f8c55e5c07acbd307cb133b000","serviceStakingParams":{"maxNumServices":"100","rewardsPerSecond":"1000000000000000","minStakingDeposit":"50000000000000000000","livenessPeriod":"86400","livenessRatio":"700000000000000","numAgentInstances":"1","agentIds":["12"],"threshold":"0","configHash":"0x0000000000000000000000000000000000000000000000000000000000000000"},"serviceStakingTokenAddress":"0x0338893fB1A1D9Df03F72CC53D8f786487d3D03E","serviceStakingNativeTokenAddress":"0x6d9b08701Af43D68D991c074A27E4d90Af7f2276"} \ No newline at end of file +{"contractVerification":true,"useLedger":false,"derivationPath":"m/44'/60'/2'/0/0","providerName":"chiado","gasPriceInGwei":"3","gnosisSafeAddress":"0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552","gnosisSafeProxyFactoryAddress":"0xa6B71E26C5e0845f74c812102Ca7114b6a896AB2","baseURI":"https://gateway.autonolas.tech/ipfs/","serviceRegistryName":"Service Registry","serviceRegistrySymbol":"AUTONOLAS-SERVICE-V1","AMBContractProxyForeignAddress":"0x87A19d769D875964E9Cd41dDBfc397B2543764E6","bridgeMediatorAddress":"0x670Ac235EE13C0B2a5065282bBB0c61cfB354592","bridgeMediatorMockTimelockAddress":"0x0a50009D55Ed5700ac8FF713709d5Ad5fa843896","serviceRegistryAddress":"0x31D3202d8744B16A120117A053459DDFAE93c855","serviceManagerAddress":"0x29086141ecdc310058fc23273F8ef7881d20C2f7","gnosisSafeMultisigImplementationAddress":"0xeB49bE5DF00F74bd240DE4535DDe6Bc89CEfb994","gnosisSafeSameAddressMultisigImplementationAddress":"0xE16adc7777B7C2a0d35033bd3504C028AB28EE8b","operatorWhitelistAddress":"0x6f7661F52fE1919996d0A4F68D09B344093a349d","serviceRegistryTokenUtilityAddress":"0xc2c7E40674f1C7Bb99eFe5680Efd79842502bED4","serviceManagerTokenAddress":"0xc965a32185590Eb5a5fffDba29E96126b7650eDe","olasAddress":"0xE40AE73aa0Ed3Ec35fdAF56e01FCd0D1Ff1d9AB6","multisigProxyHash130":"0xb89c1b3bdf2cf8827818646bce9a8f6e372885f8c55e5c07acbd307cb133b000","serviceStakingParams":{"maxNumServices":"100","rewardsPerSecond":"1000000000000000","minStakingDeposit":"50000000000000000000","maxNumInactivityPeriods":"3","livenessPeriod":"86400","livenessRatio":"700000000000000","numAgentInstances":"1","agentIds":["12"],"threshold":"0","configHash":"0x0000000000000000000000000000000000000000000000000000000000000000"},"serviceStakingTokenAddress":"0x0338893fB1A1D9Df03F72CC53D8f786487d3D03E","serviceStakingNativeTokenAddress":"0x6d9b08701Af43D68D991c074A27E4d90Af7f2276"} \ No newline at end of file diff --git a/scripts/deployment/l2/globals_gnosis_mainnet.json b/scripts/deployment/l2/globals_gnosis_mainnet.json index 1d57b590..b7117361 100644 --- a/scripts/deployment/l2/globals_gnosis_mainnet.json +++ b/scripts/deployment/l2/globals_gnosis_mainnet.json @@ -25,6 +25,7 @@ "maxNumServices": "100", "rewardsPerSecond": "385802469136", "minStakingDeposit": "50000000000000000000", + "maxNumInactivityPeriods": "3", "livenessPeriod": "86400", "livenessRatio": "520833333333333", "numAgentInstances": "1", diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index 85756fa7..bd706987 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -41,6 +41,7 @@ describe("ServiceStaking", function () { maxNumServices: 3, rewardsPerSecond: "1" + "0".repeat(15), minStakingDeposit: 10, + maxNumInactivityPeriods: 3, livenessPeriod: livenessPeriod, // Ten seconds livenessRatio: "1" + "0".repeat(16), // 0.01 transaction per second (TPS) numAgentInstances: 1, @@ -105,7 +106,7 @@ describe("ServiceStaking", function () { serviceStaking = await ServiceStakingNativeToken.deploy(serviceParams, serviceRegistry.address, bytecodeHash); await serviceStaking.deployed(); - maxInactivity = Number(await serviceStaking.MAX_INACTIVITY_PERIODS()) * livenessPeriod + 1; + maxInactivity = Number(await serviceStaking.maxNumInactivityPeriods()) * livenessPeriod + 1; const ServiceStakingToken = await ethers.getContractFactory("ServiceStakingToken"); serviceStakingToken = await ServiceStakingToken.deploy(serviceParams, serviceRegistry.address, @@ -165,6 +166,7 @@ describe("ServiceStaking", function () { maxNumServices: 0, rewardsPerSecond: 0, minStakingDeposit: 0, + maxNumInactivityPeriods: 0, livenessPeriod: 0, livenessRatio: 0, numAgentInstances: 0, @@ -183,6 +185,9 @@ describe("ServiceStaking", function () { testServiceParams.rewardsPerSecond = 1; await expect(ServiceStakingNativeToken.deploy(testServiceParams, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingNativeToken, "ZeroValue"); + testServiceParams.maxNumInactivityPeriods = 1; + await expect(ServiceStakingNativeToken.deploy(testServiceParams, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingNativeToken, "ZeroValue"); + testServiceParams.livenessPeriod = 1; await expect(ServiceStakingNativeToken.deploy(testServiceParams, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingNativeToken, "ZeroValue"); @@ -220,6 +225,9 @@ describe("ServiceStaking", function () { testServiceParams.rewardsPerSecond = 1; await expect(ServiceStakingToken.deploy(testServiceParams, AddressZero, AddressZero, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroValue"); + testServiceParams.maxNumInactivityPeriods = 1; + await expect(ServiceStakingToken.deploy(testServiceParams, AddressZero, AddressZero, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroValue"); + testServiceParams.livenessPeriod = 1; await expect(ServiceStakingToken.deploy(testServiceParams, AddressZero, AddressZero, AddressZero, bytes32Zero)).to.be.revertedWithCustomError(ServiceStakingToken, "ZeroValue"); diff --git a/test/ServiceStaking.t.sol b/test/ServiceStaking.t.sol index 7c2feec1..6ff039fd 100644 --- a/test/ServiceStaking.t.sol +++ b/test/ServiceStaking.t.sol @@ -56,6 +56,8 @@ contract BaseSetup is Test { uint256 internal rewardsPerSecond = 0.0001 ether; // Minimum service staking deposit value required for staking uint256 internal minStakingDeposit = regDeposit; + // Max number of accumulated inactivity periods after which the service is evicted + uint256 internal maxNumInactivityPeriods = 3; // Liveness period uint256 internal livenessPeriod = 1 days; // Liveness ratio in the format of 1e18 @@ -104,7 +106,8 @@ contract BaseSetup is Test { // Deploy service staking native token and arbitrary ERC20 token ServiceStakingBase.StakingParams memory stakingParams = ServiceStakingBase.StakingParams(maxNumServices, - rewardsPerSecond, minStakingDeposit, livenessPeriod, livenessRatio, numAgentInstances, emptyArray, 0, bytes32(0)); + rewardsPerSecond, minStakingDeposit, maxNumInactivityPeriods, livenessPeriod, livenessRatio, + numAgentInstances, emptyArray, 0, bytes32(0)); serviceStakingNativeToken = new ServiceStakingNativeToken(stakingParams, address(serviceRegistry), multisigProxyHash); serviceStakingToken = new ServiceStakingToken(stakingParams, address(serviceRegistry), address(serviceRegistryTokenUtility), @@ -223,7 +226,7 @@ contract ServiceStaking is BaseSetup { } // Move several liveness checks ahead - vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.MAX_INACTIVITY_PERIODS() + 1); + vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.maxNumInactivityPeriods() + 1); // Call the checkpoint serviceStakingNativeToken.checkpoint(); @@ -296,7 +299,7 @@ contract ServiceStaking is BaseSetup { } // Move several liveness checks ahead - vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.MAX_INACTIVITY_PERIODS() + 1); + vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.maxNumInactivityPeriods() + 1); // Call the checkpoint serviceStakingNativeToken.checkpoint(); @@ -367,7 +370,7 @@ contract ServiceStaking is BaseSetup { } // Move several liveness checks ahead - vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.MAX_INACTIVITY_PERIODS() + 1); + vm.warp(block.timestamp + livenessPeriod * serviceStakingNativeToken.maxNumInactivityPeriods() + 1); // Call the checkpoint serviceStakingNativeToken.checkpoint(); @@ -439,7 +442,7 @@ contract ServiceStaking is BaseSetup { } // Move several liveness checks ahead - vm.warp(block.timestamp + livenessPeriod * serviceStakingToken.MAX_INACTIVITY_PERIODS() + 1); + vm.warp(block.timestamp + livenessPeriod * serviceStakingToken.maxNumInactivityPeriods() + 1); // Call the checkpoint serviceStakingToken.checkpoint(); From 89b1d5ee800ce5faddf8b475553e58260162dd42 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Fri, 24 Nov 2023 18:11:11 +0000 Subject: [PATCH 15/16] fix: correcting contract and tests --- contracts/staking/ServiceStakingBase.sol | 3 ++- test/ServiceStaking.js | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/staking/ServiceStakingBase.sol b/contracts/staking/ServiceStakingBase.sol index 5fb7476f..268877cb 100644 --- a/contracts/staking/ServiceStakingBase.sol +++ b/contracts/staking/ServiceStakingBase.sol @@ -210,6 +210,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { maxNumServices = _stakingParams.maxNumServices; rewardsPerSecond = _stakingParams.rewardsPerSecond; minStakingDeposit = _stakingParams.minStakingDeposit; + maxNumInactivityPeriods = _stakingParams.maxNumInactivityPeriods; livenessPeriod = _stakingParams.livenessPeriod; livenessRatio = _stakingParams.livenessRatio; numAgentInstances = _stakingParams.numAgentInstances; @@ -239,7 +240,7 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries { proxyHash = _proxyHash; // Calculate max allowed inactivity - maxAllowedInactivity = maxNumInactivityPeriods * livenessPeriod; + maxAllowedInactivity = _stakingParams.maxNumInactivityPeriods * livenessPeriod; // Set the checkpoint timestamp to be the deployment one tsCheckpoint = block.timestamp; diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index bd706987..5a7a5b3e 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -34,7 +34,6 @@ describe("ServiceStaking", function () { const agentParams = [[1, regBond]]; const threshold = 1; const livenessPeriod = 10; // Ten seconds - let maxInactivity; const initSupply = "5" + "0".repeat(26); const payload = "0x"; const serviceParams = { @@ -49,6 +48,7 @@ describe("ServiceStaking", function () { threshold: 0, configHash: bytes32Zero }; + const maxInactivity = serviceParams.maxNumInactivityPeriods * livenessPeriod + 1; beforeEach(async function () { signers = await ethers.getSigners(); @@ -106,8 +106,6 @@ describe("ServiceStaking", function () { serviceStaking = await ServiceStakingNativeToken.deploy(serviceParams, serviceRegistry.address, bytecodeHash); await serviceStaking.deployed(); - maxInactivity = Number(await serviceStaking.maxNumInactivityPeriods()) * livenessPeriod + 1; - const ServiceStakingToken = await ethers.getContractFactory("ServiceStakingToken"); serviceStakingToken = await ServiceStakingToken.deploy(serviceParams, serviceRegistry.address, serviceRegistryTokenUtility.address, token.address, bytecodeHash); From de4b5d97ae87bbd924a28eac6234b6a6ac237719 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Fri, 24 Nov 2023 18:12:20 +0000 Subject: [PATCH 16/16] chore: gitleaksignore --- .gitleaksignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitleaksignore b/.gitleaksignore index e3360bd5..db316595 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -83,4 +83,6 @@ f1e395c1b3dae68fb7a74f9e493d7115d5784bf3:scripts/deployment/l2/globals_gnosis_ma 08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 08e2ef7d21dc119fc4ae119771ac4f5888d3d3fc:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 f876f6e96465e05f30236366a6cac1ed7b2017d8:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:1 -f876f6e96465e05f30236366a6cac1ed7b2017d8:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 \ No newline at end of file +f876f6e96465e05f30236366a6cac1ed7b2017d8:scripts/deployment/l2/globals_gnosis_mainnet.json:generic-api-key:2 +eee7f30c9db1ea16fd4300ff73d182ba639c978c:scripts/deployment/l2/globals_gnosis_chiado.json:generic-api-key:1 +eee7f30c9db1ea16fd4300ff73d182ba639c978c:scripts/deployment/l2/globals_gnosis_chiado.json:generic-api-key:2 \ No newline at end of file