Skip to content

Commit

Permalink
fix: handle edge cases for thawing pools
Browse files Browse the repository at this point in the history 
Signed-off-by: Tomás Migone <[email protected]>
  • Loading branch information
@tmigone
tmigone committed Sep 23, 2024
1 parent b506e4c commit 1111037
Show file tree
Hide file tree
Showing 8 changed files with 266 additions and 66 deletions.
42 changes: 29 additions & 13 deletions packages/horizon/contracts/staking/HorizonStaking.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {

// Delegation pool must exist before adding tokens
DelegationPoolInternal storage pool = _getDelegationPool(serviceProvider, verifier);
require(pool.shares > 0, HorizonStakingInvalidDelegationPool(serviceProvider, verifier));
require(
pool.shares > 0 || pool.sharesThawing > 0,
HorizonStakingInvalidDelegationPool(serviceProvider, verifier)
);

pool.tokens = pool.tokens + tokens;
_graphToken().pullTokens(msg.sender, tokens);
Expand Down Expand Up @@ -402,8 +405,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
(FIXED_POINT_PRECISION);
prov.tokens = prov.tokens - providerTokensSlashed;

// Reset provision's thawing pool if all thawing tokens were slashed
if (prov.tokensThawing == 0) {
// Reset provision's thawing pool if provision was fully slashed
if (prov.tokens == 0 && prov.tokensThawing == 0) {
prov.sharesThawing = 0;
prov.thawingNonce++;
}
Expand Down Expand Up @@ -435,8 +438,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
(pool.tokensThawing * (FIXED_POINT_PRECISION - delegationFractionSlashed)) /
FIXED_POINT_PRECISION;

// Reset delegation pool's thawing pool if all thawing tokens were slashed
if (pool.tokensThawing == 0) {
// Reset delegation pool's thawing pool if pool was fully slashed
if (pool.tokens == 0 && pool.tokensThawing == 0) {
pool.sharesThawing = 0;
pool.thawingNonce++;
}
Expand Down Expand Up @@ -690,12 +693,15 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {

Provision storage prov = _provisions[_serviceProvider][_verifier];

// Calculate shares to issue
// Thawing pool is reset/initialized in any of the following cases:
// - prov.tokensThawing == 0, pool is empty
bool initializeThawingPool = prov.tokensThawing == 0;
uint256 thawingShares = initializeThawingPool ? _tokens : ((prov.sharesThawing * _tokens) / prov.tokensThawing);
uint64 thawingUntil = uint64(block.timestamp + uint256(prov.thawingPeriod));

// Safety check to ensure pool has no shares when being initialized
// This should never happen if the pool is correctly reset or it's the first initialization
// This should never happen if the pool was reset due to slashing or it's the first initialization
if (initializeThawingPool) {
prov.sharesThawing = 0;
}
Expand Down Expand Up @@ -758,15 +764,19 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);
DelegationInternal storage delegation = pool.delegators[msg.sender];

// An invalid delegation pool has shares or thawing shares but no tokens
require(
pool.tokens != 0 || (pool.shares == 0 && pool.sharesThawing == 0),
HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier)
);

// Calculate shares to issue
uint256 shares = (pool.tokens == 0 || pool.tokens == pool.tokensThawing)
? _tokens
: ((_tokens * pool.shares) / (pool.tokens - pool.tokensThawing));
// Delegation pool is reset/initialized in any of the following cases:
// - pool.tokens == 0 and pool.shares == 0, pool is completely empty. Note that we don't test shares == 0 because
// the invalid delegation pool check already ensures shares are 0 if tokens are 0
// - pool.tokens == pool.tokensThawing, the entire pool is thawing
bool initializePool = pool.tokens == 0 || pool.tokens == pool.tokensThawing;
uint256 shares = initializePool ? _tokens : ((_tokens * pool.shares) / (pool.tokens - pool.tokensThawing));
require(shares != 0 && shares >= _minSharesOut, HorizonStakingSlippageProtection(shares, _minSharesOut));

pool.tokens = pool.tokens + _tokens;
Expand All @@ -784,26 +794,28 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
* @dev Note that due to slashing the delegation pool can enter an invalid state if all it's tokens are slashed.
* An invalid pool can only be recovered by adding back tokens into the pool with {IHorizonStakingMain-addToDelegationPool}.
* Any time the delegation pool is invalidated, the thawing pool is also reset and any pending undelegate requests get
* invalidated.
* invalidated. Delegation that is caught thawing when the pool is invalidated will be completely lost.
*/
function _undelegate(address _serviceProvider, address _verifier, uint256 _shares) private returns (bytes32) {
require(_shares > 0, HorizonStakingInvalidZeroShares());
DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);
DelegationInternal storage delegation = pool.delegators[msg.sender];
require(delegation.shares >= _shares, HorizonStakingInsufficientShares(delegation.shares, _shares));

// An invalid delegation pool has shares but no tokens
// An invalid delegation pool has shares or thawing shares but no tokens
require(pool.tokens != 0, HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier));

// Convert delegation pool shares to thawing pool shares
// Calculate thawing shares to issue - convert delegation pool shares to thawing pool shares
// delegation pool shares -> delegation pool tokens -> thawing pool shares
// Thawing pool is reset/initialized in any of the following cases:
// - prov.tokensThawing == 0, pool is empty
uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares;
bool initializeThawingPool = pool.tokensThawing == 0;
uint256 thawingShares = initializeThawingPool ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing);
uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod));

// Safety check to ensure thawing pool has no shares when being initialized
// This should never happen if the pool is correctly reset or it's the first initialization
// This should never happen if the pool was reset due to slashing or it's the first initialization
if (initializeThawingPool) {
pool.sharesThawing = 0;
}
Expand Down Expand Up @@ -847,6 +859,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
uint256 _nThawRequests
) private {
DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);

// An invalid delegation pool has shares or thawing shares but no tokens
// Note we don't test for shares, the existence of thaw requests means thawing shares also exist
// even if they are fulfilled to 0 tokens
require(pool.tokens != 0, HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier));

uint256 tokensThawed = 0;
Expand Down
46 changes: 35 additions & 11 deletions packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, uint64(block.timestamp));
assertEq(afterProvision.maxVerifierCutPending, maxVerifierCut);
assertEq(afterProvision.thawingPeriodPending, thawingPeriod);
assertEq(afterProvision.thawingNonce, 0);
assertEq(afterServiceProvider.tokensStaked, beforeServiceProvider.tokensStaked);
assertEq(afterServiceProvider.tokensProvisioned, tokens + beforeServiceProvider.tokensProvisioned);
assertEq(afterServiceProvider.__DEPRECATED_tokensAllocated, beforeServiceProvider.__DEPRECATED_tokensAllocated);
Expand Down Expand Up @@ -385,6 +386,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
assertEq(afterProvision.maxVerifierCutPending, beforeProvision.maxVerifierCutPending);
assertEq(afterProvision.thawingPeriodPending, beforeProvision.thawingPeriodPending);
assertEq(afterProvision.thawingNonce, beforeProvision.thawingNonce);
assertEq(afterServiceProvider.tokensStaked, beforeServiceProvider.tokensStaked);
assertEq(afterServiceProvider.tokensProvisioned, beforeServiceProvider.tokensProvisioned + tokens);
assertEq(afterServiceProvider.__DEPRECATED_tokensAllocated, beforeServiceProvider.__DEPRECATED_tokensAllocated);
Expand Down Expand Up @@ -447,6 +449,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
assertEq(afterProvision.maxVerifierCutPending, beforeProvision.maxVerifierCutPending);
assertEq(afterProvision.thawingPeriodPending, beforeProvision.thawingPeriodPending);
assertEq(afterProvision.thawingNonce, beforeProvision.thawingNonce);
assertEq(thawRequestId, expectedThawRequestId);
assertEq(afterThawRequest.shares, thawingShares);
assertEq(afterThawRequest.thawingUntil, block.timestamp + beforeProvision.thawingPeriod);
Expand Down Expand Up @@ -525,6 +528,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
assertEq(afterProvision.maxVerifierCutPending, beforeProvision.maxVerifierCutPending);
assertEq(afterProvision.thawingPeriodPending, beforeProvision.thawingPeriodPending);
assertEq(afterProvision.thawingNonce, beforeProvision.thawingNonce);
assertEq(afterServiceProvider.tokensStaked, beforeServiceProvider.tokensStaked);
assertEq(
afterServiceProvider.tokensProvisioned,
Expand Down Expand Up @@ -638,6 +642,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, beforeValues.provision.createdAt);
assertEq(afterProvision.maxVerifierCutPending, beforeValues.provision.maxVerifierCutPending);
assertEq(afterProvision.thawingPeriodPending, beforeValues.provision.thawingPeriodPending);
assertEq(afterProvision.thawingNonce, beforeValues.provision.thawingNonce);

// assert: provision new verifier
assertEq(afterProvisionNewVerifier.tokens, beforeValues.provisionNewVerifier.tokens + tokens);
Expand All @@ -654,6 +659,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
afterProvisionNewVerifier.thawingPeriodPending,
beforeValues.provisionNewVerifier.thawingPeriodPending
);
assertEq(afterProvisionNewVerifier.thawingNonce, beforeValues.provisionNewVerifier.thawingNonce);

// assert: service provider
assertEq(afterServiceProvider.tokensStaked, beforeValues.serviceProvider.tokensStaked);
Expand Down Expand Up @@ -737,6 +743,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
assertEq(afterProvision.maxVerifierCutPending, maxVerifierCut);
assertEq(afterProvision.thawingPeriodPending, thawingPeriod);
assertEq(afterProvision.thawingNonce, beforeProvision.thawingNonce);
}

function _acceptProvisionParameters(address serviceProvider) internal {
Expand Down Expand Up @@ -772,6 +779,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.thawingPeriod, beforeProvision.thawingPeriodPending);
assertEq(afterProvision.thawingPeriod, afterProvision.thawingPeriodPending);
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
assertEq(afterProvision.thawingNonce, beforeProvision.thawingNonce);
}

function _setOperator(address operator, address verifier, bool allow) internal {
Expand Down Expand Up @@ -880,6 +888,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(beforePool.shares + calcShares, afterPool.shares);
assertEq(beforePool.tokensThawing, afterPool.tokensThawing);
assertEq(beforePool.sharesThawing, afterPool.sharesThawing);
assertEq(beforePool.thawingNonce, afterPool.thawingNonce);
assertEq(beforeDelegation.shares + calcShares, afterDelegation.shares);
assertEq(beforeDelegation.__DEPRECATED_tokensLocked, afterDelegation.__DEPRECATED_tokensLocked);
assertEq(beforeDelegation.__DEPRECATED_tokensLockedUntil, afterDelegation.__DEPRECATED_tokensLockedUntil);
Expand Down Expand Up @@ -979,6 +988,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
: beforeValues.pool.sharesThawing + calcValues.thawingShares,
afterPool.sharesThawing
);
assertEq(beforeValues.pool.thawingNonce, afterPool.thawingNonce);
assertEq(beforeValues.delegation.shares - shares, afterDelegation.shares);
assertEq(afterThawRequest.shares, calcValues.thawingShares);
assertEq(afterThawRequest.thawingUntil, calcValues.thawingUntil);
Expand Down Expand Up @@ -1118,6 +1128,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterValues.pool.shares, beforeValues.pool.shares);
assertEq(afterValues.pool.tokensThawing, calcValues.tokensThawing);
assertEq(afterValues.pool.sharesThawing, calcValues.sharesThawing);
assertEq(afterValues.pool.thawingNonce, beforeValues.pool.thawingNonce);

for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) {
ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]);
Expand Down Expand Up @@ -1217,6 +1228,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(beforePool.shares, afterPool.shares);
assertEq(beforePool.tokensThawing, afterPool.tokensThawing);
assertEq(beforePool.sharesThawing, afterPool.sharesThawing);
assertEq(beforePool.thawingNonce, afterPool.thawingNonce);
}

function _setDelegationFeeCut(
Expand Down Expand Up @@ -1380,6 +1392,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(beforeValues.pool.shares, afterPool.shares);
assertEq(beforeValues.pool.tokensThawing, afterPool.tokensThawing);
assertEq(beforeValues.pool.sharesThawing, afterPool.sharesThawing);
assertEq(beforeValues.pool.thawingNonce, afterPool.thawingNonce);
assertEq(0, afterDelegation.shares - beforeValues.delegation.shares);
assertEq(beforeValues.delegatedTokens, afterDelegatedTokens);
assertEq(beforeValues.delegatorBalance + tokens, afterDelegatorBalance);
Expand All @@ -1389,6 +1402,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(beforeValues.pool.shares + calcShares, afterPool.shares);
assertEq(beforeValues.pool.tokensThawing, afterPool.tokensThawing);
assertEq(beforeValues.pool.sharesThawing, afterPool.sharesThawing);
assertEq(beforeValues.pool.thawingNonce, afterPool.thawingNonce);
assertEq(calcShares, afterDelegation.shares - beforeValues.delegation.shares);
assertEq(beforeValues.delegatedTokens + tokens, afterDelegatedTokens);
assertEq(beforeValues.delegatorBalance, afterDelegatorBalance);
Expand Down Expand Up @@ -1494,14 +1508,17 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterProvision.maxVerifierCutPending, before.provision.maxVerifierCutPending);
assertEq(afterProvision.thawingPeriod, before.provision.thawingPeriod);
assertEq(afterProvision.thawingPeriodPending, before.provision.thawingPeriodPending);

assertEq(
afterProvision.thawingNonce,
afterProvision.tokens == 0 ? before.provision.thawingNonce + 1 : before.provision.thawingNonce);
if (isDelegationSlashingEnabled) {
uint256 poolThawingTokens = (before.pool.tokensThawing *
(1e18 - ((calcValues.delegationTokensSlashed * 1e18) / before.pool.tokens))) / (1e18);
assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens);
assertEq(afterPool.shares, before.pool.shares);
assertEq(afterPool.tokensThawing, poolThawingTokens);
assertEq(afterPool.sharesThawing, afterPool.tokensThawing == 0 ? 0 : before.pool.sharesThawing);
assertEq(afterPool.thawingNonce, afterPool.tokens == 0 ? before.pool.thawingNonce + 1 : before.pool.thawingNonce);
}

assertEq(before.stakingBalance - tokensSlashed, afterStakingBalance);
Expand Down Expand Up @@ -1819,6 +1836,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
assertEq(afterValues.pool.shares, beforeValues.pool.shares);
assertEq(afterValues.pool.tokensThawing, beforeValues.pool.tokensThawing);
assertEq(afterValues.pool.sharesThawing, beforeValues.pool.sharesThawing);
assertEq(afterValues.pool.thawingNonce, beforeValues.pool.thawingNonce);

assertEq(afterValues.serviceProvider.tokensProvisioned, beforeValues.serviceProvider.tokensProvisioned);
if (rewardsDestination != address(0)) {
Expand Down Expand Up @@ -2295,14 +2313,17 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
bytes32 thawRequestId = thawRequestList.head;
while (thawRequestId != bytes32(0) && (iterations == 0 || thawRequestsFulfilled < iterations)) {
ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId);
bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce);
if (thawRequest.thawingUntil <= block.timestamp) {
thawRequestsFulfilled++;
uint256 tokens = delegation
? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing
: (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing;
tokensThawed += tokens;
tokensThawing -= tokens;
sharesThawing -= thawRequest.shares;
if (isThawRequestValid) {
uint256 tokens = delegation
? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing
: (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing;
tokensThawed += tokens;
tokensThawing -= tokens;
sharesThawing -= thawRequest.shares;
}
} else {
break;
}
Expand All @@ -2317,11 +2338,14 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
thawRequestId = thawRequestList.head;
while (thawRequestId != bytes32(0) && (iterations == 0 || i < iterations)) {
ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId);
bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce);

if (thawRequest.thawingUntil <= block.timestamp) {
uint256 tokens = delegation
? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing
: (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing;
thawRequestsFulfilledListTokens[i] = tokens;
if (isThawRequestValid) {
thawRequestsFulfilledListTokens[i] = delegation
? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing
: (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing;
}
thawRequestsFulfilledListIds[i] = thawRequestId;
thawRequestsFulfilledList[i] = staking.getThawRequest(thawRequestId);
thawRequestId = thawRequestsFulfilledList[i].next;
Expand Down
Loading

0 comments on commit 1111037

Please sign in to comment.