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

_validPrice can include floor and ceiling values when checking the validity #3

Closed
cleanunicorn opened this issue Sep 12, 2022 · 0 comments

Comments

@cleanunicorn
Copy link
Member

Description

A price is considered valid if the internal method _validPrice returns true.

/// @notice helper function to determine if price is within a valid range
function _validPrice(Decimal.D256 memory price)
internal
view
returns (bool valid)
{
valid =
price.greaterThan(
Decimal.ratio(floor, Constants.BASIS_POINTS_GRANULARITY)
) &&
price.lessThan(
Decimal.ratio(ceiling, Constants.BASIS_POINTS_GRANULARITY)
);
}

This method checks if the provided price is strictly greater than the floor and strictly less than the floor.

price.greaterThan(
Decimal.ratio(floor, Constants.BASIS_POINTS_GRANULARITY)
) &&
price.lessThan(
Decimal.ratio(ceiling, Constants.BASIS_POINTS_GRANULARITY)
);

These methods are taken from the Decimal library.

function greaterThan(D256 memory self, D256 memory b)
internal
pure
returns (bool)
{
return compareTo(self, b) == 2;
}
function lessThan(D256 memory self, D256 memory b)
internal
pure
returns (bool)
{
return compareTo(self, b) == 0;
}

Using a strictly greater/less check will exclude the values of floor and ceiling as being valid. This forces the governance to add floor and ceiling values similar to floor = 0.7999999999999 and ceiling = 1.199999999999 to describe a valid interval from 0.8 to 1.2, included.

If the check would accept the limits as valid, the governance could set floor = 0.8 and ceiling = 1.2, which are a lot easier to reason about and verify.

Recommendation

Consider using the methods greaterThanOrEqualTo and lessThanOrEqualTo provided by Decimal to avoid using awkward values.

function greaterThanOrEqualTo(D256 memory self, D256 memory b)
internal
pure
returns (bool)
{
return compareTo(self, b) > 0;
}
function lessThanOrEqualTo(D256 memory self, D256 memory b)
internal
pure
returns (bool)
{
return compareTo(self, b) < 2;
}

[optional] References

@cleanunicorn cleanunicorn changed the title _validPrice should include floor and ceiling values when checking the validity _validPrice can include floor and ceiling values when checking the validity Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant