Skip to content

fix: ensure invalid thaw requests are ignored by getThawedTokens (OZ L-01) #1129

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

Open
wants to merge 1 commit into
base: horizon-oz2/m01-limit-thawing-period
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ interface IHorizonStakingMain {
* @param shares The amount of shares being thawed
* @param thawingUntil The timestamp until the stake is thawed
* @param thawRequestId The ID of the thaw request
* @param nonce The nonce of the thaw request
*/
event ThawRequestCreated(
IHorizonStakingTypes.ThawRequestType indexed requestType,
Expand All @@ -262,7 +263,8 @@ interface IHorizonStakingMain {
address owner,
uint256 shares,
uint64 thawingUntil,
bytes32 thawRequestId
bytes32 thawRequestId,
uint256 nonce
);

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/horizon/contracts/staking/HorizonStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
_owner,
_shares,
_thawingUntil,
thawRequestId
thawRequestId,
_thawingNonce
);
return thawRequestId;
}
Expand Down Expand Up @@ -1173,6 +1174,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
uint256 tokens = 0;
bool validThawRequest = thawRequest.thawingNonce == thawingNonce;
if (validThawRequest) {
// sharesThawing cannot be zero if there is a valid thaw request so the next division is safe
tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
tokensThawing = tokensThawing - tokens;
sharesThawing = sharesThawing - thawRequest.shares;
Expand Down
18 changes: 11 additions & 7 deletions packages/horizon/contracts/staking/HorizonStakingBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,18 @@ abstract contract HorizonStakingBase is
bytes32 thawRequestId = thawRequestList.head;
while (thawRequestId != bytes32(0)) {
ThawRequest storage thawRequest = _getThawRequest(requestType, thawRequestId);
if (thawRequest.thawingUntil <= block.timestamp) {
uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
tokensThawing = tokensThawing - tokens;
sharesThawing = sharesThawing - thawRequest.shares;
thawedTokens = thawedTokens + tokens;
} else {
break;
if (thawRequest.thawingNonce == prov.thawingNonce) {
if (thawRequest.thawingUntil <= block.timestamp) {
// sharesThawing cannot be zero if there is a valid thaw request so the next division is safe
uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
tokensThawing = tokensThawing - tokens;
sharesThawing = sharesThawing - thawRequest.shares;
thawedTokens = thawedTokens + tokens;
} else {
break;
}
}

thawRequestId = thawRequest.next;
}
return thawedTokens;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
serviceProvider,
thawingShares,
uint64(block.timestamp + beforeProvision.thawingPeriod),
expectedThawRequestId
expectedThawRequestId,
beforeProvision.thawingNonce
);
vm.expectEmit(address(staking));
emit IHorizonStakingMain.ProvisionThawed(serviceProvider, verifier, tokens);
Expand Down Expand Up @@ -1019,7 +1020,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
beneficiary,
calcValues.thawingShares,
calcValues.thawingUntil,
calcValues.thawRequestId
calcValues.thawRequestId,
beforeValues.pool.thawingNonce
);
vm.expectEmit();
emit IHorizonStakingMain.TokensUndelegated(serviceProvider, verifier, delegator, calcValues.tokens);
Expand Down
72 changes: 72 additions & 0 deletions packages/horizon/test/staking/provision/thaw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,76 @@ contract HorizonStakingThawTest is HorizonStakingTest {
);
vm.assertEq(thawedTokens, thawAmount * thawSteps);
}

function testThaw_GetThawedTokens_AfterProvisionFullySlashed(
uint256 amount,
uint64 thawingPeriod,
uint256 thawAmount
) public useIndexer useProvision(amount, 0, thawingPeriod) {
// thaw some funds so there are some shares thawing and tokens thawing
thawAmount = bound(thawAmount, 1, amount);
_thaw(users.indexer, subgraphDataServiceAddress, thawAmount);

// skip to after the thawing period has passed
skip(thawingPeriod + 1);

// slash all of it
resetPrank(subgraphDataServiceAddress);
_slash(users.indexer, subgraphDataServiceAddress, amount, 0);

// get the thawed tokens - should be zero now as the pool was reset
uint256 thawedTokens = staking.getThawedTokens(
ThawRequestType.Provision,
users.indexer,
subgraphDataServiceAddress,
users.indexer
);
vm.assertEq(thawedTokens, 0);
}

function testThaw_GetThawedTokens_AfterRecoveringProvision(
uint256 amount,
uint64 thawingPeriod,
uint256 thawAmount
) public useIndexer useProvision(amount, 0, thawingPeriod) {
// thaw some funds so there are some shares thawing and tokens thawing
thawAmount = bound(thawAmount, 1, amount);
_thaw(users.indexer, subgraphDataServiceAddress, thawAmount);

// skip to after the thawing period has passed
skip(thawingPeriod + 1);

// slash all of it
resetPrank(subgraphDataServiceAddress);
_slash(users.indexer, subgraphDataServiceAddress, amount, 0);

// get the thawed tokens - should be zero now as the pool was reset
uint256 thawedTokens = staking.getThawedTokens(
ThawRequestType.Provision,
users.indexer,
subgraphDataServiceAddress,
users.indexer
);
vm.assertEq(thawedTokens, 0);

// put some funds back in
resetPrank(users.indexer);
_stake(amount);
_addToProvision(users.indexer, subgraphDataServiceAddress, amount);

// thaw some more funds
_thaw(users.indexer, subgraphDataServiceAddress, thawAmount);

// skip to after the thawing period has passed
skip(block.timestamp + thawingPeriod + 1);

// get the thawed tokens - should be the amount we thawed
thawedTokens = staking.getThawedTokens(
ThawRequestType.Provision,
users.indexer,
subgraphDataServiceAddress,
users.indexer
);
vm.assertEq(thawedTokens, thawAmount);
}
}