Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor and test: service staking optimization #135

Merged
merged 12 commits into from
Oct 25, 2023
2 changes: 1 addition & 1 deletion .github/workflows/workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
run: ./node_modules/.bin/eslint . --ext .js,.jsx,.ts,.tsx
- name: Run solhint
run: ./node_modules/.bin/solhint contracts/interfaces/*.sol contracts/*.sol contracts/test/*.sol \
contracts/multisigs/*.sol contracts/utils/*.sol
contracts/multisigs/*.sol contracts/utils/*.sol contracts/staking/*.sol

# Compile the code and run tests and deploy script(s)
- name: Compile the code
Expand Down
3 changes: 2 additions & 1 deletion .solcover.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module.exports = {
skipFiles: ["test/ComponentRegistryTest.sol",
skipFiles: [
"test/ComponentRegistryTest.sol",
"test/ERC20Token.sol",
"test/GnosisSafeABICreator.sol",
"test/MockAgentMech.sol",
Expand Down
16 changes: 11 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ In order to generalize `components` / `agents` / `services`, they are referred s

A graphical overview of the whole on-chain architecture is available here:

![architecture](https://github.com/valory-xyz/autonolas-registries/blob/main/docs/On-chain_architecture_v5.png?raw=true)
![architecture](https://github.com/valory-xyz/autonolas-registries/blob/main/docs/On-chain_architecture_v6.png?raw=true)

An overview of the design, details on how securing services with ETH or a custom ERC20 token, how service owners can opt for a set of authorized operators,
as well as how DAOs can manage their autonomous services are provided [here](https://github.com/valory-xyz/autonolas-registries/blob/main/docs/AgentServicesFunctionality.pdf?raw=true).
Expand All @@ -30,12 +30,11 @@ An overview of the state machine governing service management and usage is provi
A more detailed set of registries definitions are provided [here](https://github.com/valory-xyz/autonolas-registries/blob/main/docs/definitions.md).




- Abstract contracts:
- [GenericRegistry](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/GenericRegistry.sol)
- [UnitRegistry](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/UnitRegistry.sol)
- [GenericManager](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/GenericManager.sol)
- [ServiceStakingBase.sol](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/staking/ServiceStakingBase.sol)
- Core contracts:
- [AgentRegistry](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/AgentRegistry.sol)
- [ComponentRegistry](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/ComponentRegistry.sol)
Expand All @@ -49,6 +48,9 @@ A more detailed set of registries definitions are provided [here](https://github
- Utility contracts:
- [OperatorSignedHashes](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/utils/OperatorSignedHashes.sol)
- [OperatorWhitelist](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/utils/OperatorWhitelist.sol)
- Staking contracts:
- [ServiceStakingNativeToken.sol](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/staking/ServiceStakingNativeToken.sol)
- [ServiceStakingToken.sol](https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/staking/ServiceStakingToken.sol)

In order to deploy a service, its registered agent instances form a consensus mechanism via the means of multisigs using the generic multisig interface.
One of the most well-known multisigs is Gnosis Safe. The Gnosis interface implementation of a generic multisig interface is provided here:
Expand All @@ -71,8 +73,8 @@ As more multisigs come into play, their underlying implementation of the generic

### Prerequisites
- This repository follows the standard [`Hardhat`](https://hardhat.org/tutorial/) development process.
- The code is written on Solidity `0.8.15` and `0.8.19`.
- The standard versions of Node.js along with Yarn are required to proceed further (confirmed to work with Yarn `1.22.10` and npx/npm `6.14.11` and node `v12.22.0`).
- The code is written on Solidity `0.8.15`, `0.8.19`, `0.8.21`.
- The standard versions of Node.js along with Yarn are required to proceed further (confirmed to work with Yarn `1.22.19` and npx/npm `10.1.0` and node `v18.17.0`).

### Install the dependencies
The project has submodules to get the dependencies. Make sure you run `git clone --recursive` or init the submodules yourself.
Expand All @@ -99,6 +101,10 @@ Run the tests:
```
npx hardhat test
```
Run tests with forge:
```
forge test --hh -vvv
```

### Test with instrumented code
[Scribble](https://docs.scribble.codes/) annotated contracts are located in https://github.com/valory-xyz/autonolas-registries/blob/main/contracts/scribble.
Expand Down
32 changes: 29 additions & 3 deletions abis/0.8.21/ServiceStakingNativeToken.json

Large diffs are not rendered by default.

32 changes: 29 additions & 3 deletions abis/0.8.21/ServiceStakingToken.json

Large diffs are not rendered by default.

149 changes: 82 additions & 67 deletions contracts/staking/ServiceStakingBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -352,60 +352,52 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries {
uint256[][] memory serviceNonces
)
{
// Get the service Ids set length
uint256 size = setServiceIds.length;
serviceIds = new uint256[](size);

// Record service Ids
for (uint256 i = 0; i < size; ++i) {
// Get current service Id
serviceIds[i] = setServiceIds[i];
}

Comment on lines -355 to -364
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't allocate serviceIds if the rewards calculation is not going to be successful.

// Check the last checkpoint timestamp and the liveness period
// Check the last checkpoint timestamp and the liveness period, also check for available rewards to be not zero
uint256 tsCheckpointLast = tsCheckpoint;
if (block.timestamp - tsCheckpointLast >= livenessPeriod) {
// Get available rewards and last checkpoint timestamp
lastAvailableRewards = availableRewards;

// If available rewards are not zero, proceed with staking calculation
if (lastAvailableRewards > 0) {
// Get necessary arrays
eligibleServiceIds = new uint256[](size);
eligibleServiceRewards = new uint256[](size);
serviceNonces = new uint256[][](size);

// Calculate each staked service reward eligibility
for (uint256 i = 0; i < size; ++i) {
// Get the service info
ServiceInfo storage sInfo = mapServiceInfo[serviceIds[i]];

// Get current service multisig nonce
serviceNonces[i] = _getMultisigNonces(sInfo.multisig);

// Calculate the liveness nonce ratio
// Get the last service checkpoint: staking start time or the global checkpoint timestamp
uint256 serviceCheckpoint = tsCheckpointLast;
uint256 ts = sInfo.tsStart;
// Adjust the service checkpoint time if the service was staking less than the current staking period
if (ts > serviceCheckpoint) {
serviceCheckpoint = ts;
}

// Calculate the liveness ratio in 1e18 value
// This subtraction is always positive or zero, as the last checkpoint can be at most block.timestamp
ts = block.timestamp - serviceCheckpoint;
bool ratioPass = _isRatioPass(serviceNonces[i], sInfo.nonces, ts);

// Record the reward for the service if it has provided enough transactions
if (ratioPass) {
// Calculate the reward up until now and record its value for the corresponding service
uint256 reward = rewardsPerSecond * ts;
totalRewards += reward;
eligibleServiceRewards[numServices] = reward;
eligibleServiceIds[numServices] = serviceIds[i];
++numServices;
}
lastAvailableRewards = availableRewards;
if (block.timestamp - tsCheckpointLast >= livenessPeriod && lastAvailableRewards > 0) {
Comment on lines +357 to +358
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged that check in one: for the liveness period and available rewards.

// Get the service Ids set length
uint256 size = setServiceIds.length;

// Get necessary arrays
serviceIds = new uint256[](size);
eligibleServiceIds = new uint256[](size);
eligibleServiceRewards = new uint256[](size);
serviceNonces = new uint256[][](size);

// Calculate each staked service reward eligibility
for (uint256 i = 0; i < size; ++i) {
// Get current service Id
serviceIds[i] = setServiceIds[i];

// Get the service info
ServiceInfo storage sInfo = mapServiceInfo[serviceIds[i]];

// Get current service multisig nonce
serviceNonces[i] = _getMultisigNonces(sInfo.multisig);

// Calculate the liveness nonce ratio
// Get the last service checkpoint: staking start time or the global checkpoint timestamp
uint256 serviceCheckpoint = tsCheckpointLast;
uint256 ts = sInfo.tsStart;
// Adjust the service checkpoint time if the service was staking less than the current staking period
if (ts > serviceCheckpoint) {
serviceCheckpoint = ts;
}

// Calculate the liveness ratio in 1e18 value
// This subtraction is always positive or zero, as the last checkpoint can be at most block.timestamp
ts = block.timestamp - serviceCheckpoint;
bool ratioPass = _isRatioPass(serviceNonces[i], sInfo.nonces, ts);

// Record the reward for the service if it has provided enough transactions
if (ratioPass) {
// Calculate the reward up until now and record its value for the corresponding service
uint256 reward = rewardsPerSecond * ts;
totalRewards += reward;
eligibleServiceRewards[numServices] = reward;
eligibleServiceIds[numServices] = serviceIds[i];
++numServices;
}
}
}
Expand Down Expand Up @@ -508,7 +500,12 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries {
}

// Call the checkpoint
(uint256[] memory serviceIds, , , , , ) = checkpoint();
(uint256[] memory serviceIds, , , , , bool success) = checkpoint();

// If the checkpoint was not successful, the serviceIds set is not returned and needs to be allocated
if (!success) {
serviceIds = getServiceIds();
}
Comment on lines -511 to +508
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkpoint now might not return serviceIds in memory if it was not successful. So in that case it must be obtained separately.


// 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
Expand Down Expand Up @@ -562,26 +559,44 @@ abstract contract ServiceStakingBase is ERC721TokenReceiver, IErrorsRegistries {
uint256[] memory eligibleServiceRewards, , ) = _calculateStakingRewards();

// If there are eligible services, proceed with staking calculation and update rewards for the service Id
if (numServices > 0) {
for (uint256 i = 0; i < numServices; ++i) {
Comment on lines -565 to +562
Copy link
Collaborator Author

@kupermind kupermind Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check for the numServices, just loop with them. Also, the eligibleServiceIds.length is not correct as the length is equal to the full length of serviceIds, and it must have the maximum of numServices elements.

// Get the service index in the eligible service set and calculate its latest reward
for (uint256 i = 0; i < eligibleServiceIds.length; ++i) {
if (eligibleServiceIds[i] == serviceId) {
// If total allocated rewards are not enough, adjust the reward value
if (totalRewards > lastAvailableRewards) {
reward += (eligibleServiceRewards[i] * lastAvailableRewards) / totalRewards;
} else {
reward += eligibleServiceRewards[i];
}
break;
if (eligibleServiceIds[i] == serviceId) {
// If total allocated rewards are not enough, adjust the reward value
if (totalRewards > lastAvailableRewards) {
reward += (eligibleServiceRewards[i] * lastAvailableRewards) / totalRewards;
} else {
reward += eligibleServiceRewards[i];
}
break;
}
}
}

/// @dev Gets staked service Ids.
/// @return serviceIds Staked service Ids.
function getServiceIds() public view returns (uint256[] memory serviceIds) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the function that returns current staking serviceIds. The default ABI getter would not be enough as it does not return all of the Ids and the length, but just a single element with the provided index.

// Get the number of service Ids
uint256 size = setServiceIds.length;
serviceIds = new uint256[](size);

// Record service Ids
for (uint256 i = 0; i < size; ++i) {
serviceIds[i] = setServiceIds[i];
}
}

/// @dev Checks if the service is staked.
/// @param serviceId.
/// @return True, if the service is staked.
function isServiceStaked(uint256 serviceId) external view returns (bool) {
return mapServiceInfo[serviceId].tsStart > 0;
/// @return isStaked True, if the service is staked.
function isServiceStaked(uint256 serviceId) external view returns (bool isStaked) {
isStaked = (mapServiceInfo[serviceId].tsStart > 0);
}

/// @dev Gets the next reward checkpoint timestamp.
/// @return tsNext Next reward checkpoint timestamp.
function getNextRewardCheckpointTimestamp() external view returns (uint256 tsNext) {
// Last checkpoint timestamp plus the liveness period
tsNext = tsCheckpoint + livenessPeriod;
Comment on lines +596 to +600
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getter of the next reward checkpoint timestamp - the agent can always compare with the block.timestamp and decide whether to call a checkpoint() or skip for now.

}
}
2 changes: 1 addition & 1 deletion contracts/test/ERC20Token.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import "../../lib/solmate/src/tokens/ERC20.sol";
import {ERC20} from "../../lib/solmate/src/tokens/ERC20.sol";

/// @title ERC20Token - Smart contract for mocking the minimum OLAS token functionality
contract ERC20Token is ERC20 {
Expand Down
Binary file removed docs/On-chain_architecture_v5.png
Binary file not shown.
Binary file added docs/On-chain_architecture_v6.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 18 additions & 18 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,32 @@
"devDependencies": {
"@gnosis.pm/safe-contracts": "^1.3.0",
"@nomicfoundation/hardhat-chai-matchers": "^1.0.6",
"@nomicfoundation/hardhat-network-helpers": "^1.0.8",
"@nomicfoundation/hardhat-network-helpers": "^1.0.9",
"@nomicfoundation/hardhat-toolbox": "^2.0.2",
"@nomiclabs/hardhat-ethers": "^2.2.3",
"@nomiclabs/hardhat-etherscan": "^3.1.7",
"@typechain/ethers-v5": "^10.2.0",
"@typechain/hardhat": "^6.1.5",
"@types/mocha": "^10.0.1",
"chai": "^4.3.7",
"eslint": "^8.39.0",
"@typechain/ethers-v5": "^11.1.2",
"@typechain/hardhat": "^9.1.0",
"@types/mocha": "^10.0.3",
"chai": "^4.3.10",
"eslint": "^8.52.0",
"ethers": "^5.7.2",
"hardhat": "^2.14.0",
"hardhat-contract-sizer": "^2.8.0",
"hardhat-deploy": "^0.11.26",
"hardhat": "^2.18.2",
"hardhat-contract-sizer": "^2.10.0",
"hardhat-deploy": "^0.11.43",
"hardhat-deploy-ethers": "^0.3.0-beta.13",
"hardhat-gas-reporter": "^1.0.9",
"hardhat-tracer": "^2.2.2",
"solidity-coverage": "^0.8.2"
"hardhat-tracer": "^2.6.0",
"solidity-coverage": "^0.8.5"
},
"dependencies": {
"@anders-t/ethers-ledger": "^1.0.4",
"@ethersproject/contracts": "^5.6.2",
"@ethersproject/providers": "^5.6.8",
"@ethersproject/solidity": "^5.6.1",
"@ethersproject/wallet": "^5.6.2",
"eth-permit": "^0.2.1",
"ethereum-sources-downloader": "^0.1.19",
"solhint": "^3.4.0"
"@ethersproject/contracts": "^5.7.0",
"@ethersproject/providers": "^5.7.2",
"@ethersproject/solidity": "^5.7.0",
"@ethersproject/wallet": "^5.7.0",
"eth-permit": "^0.2.3",
"ethereum-sources-downloader": "^0.1.21",
"solhint": "^3.6.2"
}
}
Loading
Loading