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.

Sign up for GitHub

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
Merged

refactor and test: service staking optimization #135

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
40ab5e7
refactor and test: service staking optimization
kupermind Oct 24, 2023
de4a4f3
refactor and test: service staking optimization
kupermind Oct 24, 2023
eeee254
refactor and test: service staking optimization
kupermind Oct 24, 2023
5233a15
test: adding checks for tests
kupermind Oct 24, 2023
83605bc
chore: updating staking contract abis
kupermind Oct 24, 2023
18bfc4d
chore: updating packages
kupermind Oct 24, 2023
8140d0f
doc: updating readme and the diagram
kupermind Oct 24, 2023
267e13d
chore: formatting
kupermind Oct 24, 2023
a424c5a
chore: erc20 test contract small change
kupermind Oct 24, 2023
7b4af73
test: correcting mech-related ones
kupermind Oct 24, 2023
3cd4974
chore: updating github workflow
kupermind Oct 24, 2023
34a81d8
doc: updating README
kupermind Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 82 additions & 67 deletions contracts/staking/ServiceStakingBase.sol
View file
Open in desktop
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 •
edited
Loading

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.

}
}
61 changes: 61 additions & 0 deletions test/ServiceStaking.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,67 @@ describe("ServiceStakingNativeToken", function () {
snapshot.restore();
});

it("Stake and unstake right away without any service activity for two services", 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);
await serviceRegistry.approve(serviceStaking.address, serviceId + 1);

// Stake services
await serviceStaking.stake(serviceId);
await serviceStaking.stake(serviceId + 1);

// Call the checkpoint to make sure the rewards logic is not hit
await serviceStaking.checkpoint();

// Get the next checkpoint timestamp and compare with the next reward timestamp
const tsNext = Number(await serviceStaking.getNextRewardCheckpointTimestamp());
const tsLast = Number(await serviceStaking.tsCheckpoint());
const livenessPeriod = Number(await serviceStaking.livenessPeriod());
expect(tsNext - tsLast).to.equal(livenessPeriod);

// Calculate service staking reward that must be zero
let reward = await serviceStaking.calculateServiceStakingReward(serviceId);
expect(reward).to.equal(0);
reward = await serviceStaking.calculateServiceStakingReward(serviceId + 1);
expect(reward).to.equal(0);

// Get the service multisig contract
let service = await serviceRegistry.getService(serviceId);
let multisig = await ethers.getContractAt("GnosisSafe", service.multisig);

// Unstake services
let balanceBefore = await ethers.provider.getBalance(multisig.address);
await serviceStaking.unstake(serviceId);
let balanceAfter = await ethers.provider.getBalance(multisig.address);

// The multisig balance before and after unstake must be the same (zero reward)
expect(balanceBefore).to.equal(balanceAfter);

// Get the service multisig contract
service = await serviceRegistry.getService(serviceId + 1);
multisig = await ethers.getContractAt("GnosisSafe", service.multisig);

balanceBefore = await ethers.provider.getBalance(multisig.address);
await serviceStaking.unstake(serviceId + 1);
balanceAfter = await ethers.provider.getBalance(multisig.address);

// The multisig balance before and after unstake must be the same (zero reward)
expect(balanceBefore).to.equal(balanceAfter);

// Check the final serviceIds
const serviceIds = await serviceStaking.getServiceIds();
expect(serviceIds.length).to.equal(0);

// 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();
Expand Down
Loading