Skip to content

fix: use solidity division instead of fixed point scaling (OZ L-04) #1136

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

Open
wants to merge 1 commit into
base: horizon-oz2/l05-provision-params
Choose a base branch
from
Open

fix: use solidity division instead of fixed point scaling (OZ L-04) #1136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 4 additions & 15 deletions packages/horizon/contracts/staking/HorizonStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
using PPMMath for uint256;
using LinkedList for LinkedList.List;

/// @dev Fixed point precision
uint256 private constant FIXED_POINT_PRECISION = 1e18;

/// @dev Maximum number of simultaneous stake thaw requests (per provision) or undelegations (per delegation)
uint256 private constant MAX_THAW_REQUESTS = 1_000;

Expand Down Expand Up @@ -449,12 +446,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
// Burn remainder
_graphToken().burnTokens(providerTokensSlashed - tokensVerifier);

// Provision accounting
uint256 provisionFractionSlashed = (providerTokensSlashed * FIXED_POINT_PRECISION + prov.tokens - 1) /
prov.tokens;
prov.tokensThawing =
(prov.tokensThawing * (FIXED_POINT_PRECISION - provisionFractionSlashed)) /
(FIXED_POINT_PRECISION);
// Provision accounting - round down, 1 wei max precision loss
prov.tokensThawing = (prov.tokensThawing * (prov.tokens - providerTokensSlashed)) / prov.tokens;
prov.tokens = prov.tokens - providerTokensSlashed;

// If the slashing leaves the thawing shares with no thawing tokens, cancel pending thawings by:
Expand Down Expand Up @@ -485,13 +478,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
// Burn tokens
_graphToken().burnTokens(tokensToSlash);

// Delegation pool accounting
uint256 delegationFractionSlashed = (tokensToSlash * FIXED_POINT_PRECISION + pool.tokens - 1) /
pool.tokens;
// Delegation pool accounting - round down, 1 wei max precision loss
pool.tokensThawing = (pool.tokensThawing * (pool.tokens - tokensToSlash)) / pool.tokens;
pool.tokens = pool.tokens - tokensToSlash;
pool.tokensThawing =
(pool.tokensThawing * (FIXED_POINT_PRECISION - delegationFractionSlashed)) /
FIXED_POINT_PRECISION;

// If the slashing leaves the thawing shares with no thawing tokens, cancel pending thawings by:
// - deleting all thawing shares
Expand Down
9 changes: 3 additions & 6 deletions packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1581,9 +1581,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
uint256 tokensSlashed = calcValues.providerTokensSlashed +
(isDelegationSlashingEnabled ? calcValues.delegationTokensSlashed : 0);
uint256 provisionThawingTokens = (before.provision.tokensThawing *
(1e18 -
((calcValues.providerTokensSlashed * 1e18 + before.provision.tokens - 1) /
before.provision.tokens))) / (1e18);
(before.provision.tokens - calcValues.providerTokensSlashed)) / before.provision.tokens;

// assert
assertEq(afterProvision.tokens + calcValues.providerTokensSlashed, before.provision.tokens);
Expand All @@ -1604,9 +1602,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
);
if (isDelegationSlashingEnabled) {
uint256 poolThawingTokens = (before.pool.tokensThawing *
(1e18 -
((calcValues.delegationTokensSlashed * 1e18 + before.pool.tokens - 1) / before.pool.tokens))) /
(1e18);
(before.pool.tokens - calcValues.delegationTokensSlashed)) /
before.pool.tokens;
assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens);
assertEq(afterPool.shares, before.pool.shares);
assertEq(afterPool.tokensThawing, poolThawingTokens);
Expand Down
12 changes: 9 additions & 3 deletions packages/horizon/test/staking/provision/parameters.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,17 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest {
vm.assume(amount > 0);
vm.assume(amount <= MAX_STAKING_TOKENS);
vm.assume(maxVerifierCut <= MAX_PPM);

// create provision with initial parameters
uint32 initialMaxVerifierCut = 1000;
uint64 initialThawingPeriod = 14 days; // Max thawing period is 28 days
_createProvision(users.indexer, subgraphDataServiceAddress, amount, initialMaxVerifierCut, initialThawingPeriod);
uint64 initialThawingPeriod = 14 days; // Max thawing period is 28 days
_createProvision(
users.indexer,
subgraphDataServiceAddress,
amount,
initialMaxVerifierCut,
initialThawingPeriod
);

// change the max thawing period allowed so that the initial thawing period is not valid anymore
uint64 newMaxThawingPeriod = 7 days;
Expand Down
62 changes: 37 additions & 25 deletions packages/horizon/test/staking/slash/slash.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,45 +141,57 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
_slash(users.indexer, subgraphDataServiceAddress, tokens + delegationTokens, 0);
}

function testSlash_RoundDown_TokensThawing_Provision() public useIndexer {
uint256 tokens = 1 ether + 1;
function testSlash_RoundDown_TokensThawing_Provision(
uint256 tokens,
uint256 slashTokens,
uint256 tokensToThaw
) public useIndexer {
vm.assume(slashTokens <= tokens);
vm.assume(tokensToThaw <= tokens);
vm.assume(tokensToThaw > 0);

_useProvision(subgraphDataServiceAddress, tokens, MAX_PPM, MAX_THAWING_PERIOD);
_thaw(users.indexer, subgraphDataServiceAddress, tokensToThaw);

_thaw(users.indexer, subgraphDataServiceAddress, tokens);
Provision memory beforeProvision = staking.getProvision(users.indexer, subgraphDataServiceAddress);

resetPrank(subgraphDataServiceAddress);
_slash(users.indexer, subgraphDataServiceAddress, 1, 0);
_slash(users.indexer, subgraphDataServiceAddress, slashTokens, 0);

resetPrank(users.indexer);
Provision memory provision = staking.getProvision(users.indexer, subgraphDataServiceAddress);
assertEq(provision.tokens, tokens - 1);
// Tokens thawing should be rounded down
assertEq(provision.tokensThawing, tokens - 2);
Provision memory afterProvision = staking.getProvision(users.indexer, subgraphDataServiceAddress);
assertEq(afterProvision.tokens, beforeProvision.tokens - slashTokens);
assertEq(
afterProvision.tokensThawing,
(beforeProvision.tokensThawing * (beforeProvision.tokens - slashTokens)) / beforeProvision.tokens
);
}

function testSlash_RoundDown_TokensThawing_Delegation(
uint256 tokens
uint256 tokens,
uint256 delegationTokensToSlash,
uint256 delegationTokensToUndelegate
) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing {
uint256 delegationTokens = 10 ether;

vm.assume(delegationTokensToSlash <= delegationTokens);
vm.assume(delegationTokensToUndelegate <= delegationTokens);
vm.assume(delegationTokensToUndelegate > 0);

resetPrank(users.delegator);
uint256 delegationTokens = 1 ether + 1;
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);
_undelegate(users.indexer, subgraphDataServiceAddress, delegationTokensToUndelegate);

DelegationInternal memory delegation = _getStorage_Delegation(
users.indexer,
subgraphDataServiceAddress,
users.delegator,
false
);
_undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares);
DelegationPool memory beforePool = staking.getDelegationPool(users.indexer, subgraphDataServiceAddress);

// Slash
resetPrank(subgraphDataServiceAddress);
// Slash 1 token from delegation
_slash(users.indexer, subgraphDataServiceAddress, tokens + 1, 0);
_slash(users.indexer, subgraphDataServiceAddress, tokens + delegationTokensToSlash, 0);

resetPrank(users.delegator);
DelegationPool memory pool = staking.getDelegationPool(users.indexer, subgraphDataServiceAddress);
assertEq(pool.tokens, delegationTokens - 1);
// Tokens thawing should be rounded down
assertEq(pool.tokensThawing, delegationTokens - 2);
DelegationPool memory afterPool = staking.getDelegationPool(users.indexer, subgraphDataServiceAddress);
assertEq(afterPool.tokens, beforePool.tokens - delegationTokensToSlash);
assertEq(
afterPool.tokensThawing,
(beforePool.tokensThawing * (beforePool.tokens - delegationTokensToSlash)) / beforePool.tokens
);
}
}