Price is not calculated correctly according to the documentation in the contract
Summary
In BoundedStepwiseExponentialPriceAdapter.sol contract the returned value of the getPrice function does not match the formula in the documentation, which is price = initialPrice +/- scalingFactor * e ^ (timeCoefficient * timeBucket).
Vulnerability Detail
Below are 2 examples of the calculation issue.
Example when time elapsed >= bucket size:
Initial price = 100
Scaling factor = 1
Time coefficient = 1
Is price decreasing = true
Bucket size = 1 hour (3600)
Time elapsed = 1 hour (3600)
According to the contract formula, this should be the calculation:
But the getPrice result with the same parameters is 98.281718171540955000 (98281718171540955000 in wei)
Test snippet of the example
### Test snippet of `getPrice` with the parameters of the example
Add this to `boundedStepwiseExponentialPriceAdapter.spec.ts` test and run `npx hardhat test`
This test should pass, but it fails instead.
```js
it.only("Should return the correct price", async () => {
subjectInitialPrice = ether(100);
subjectCoefficient = 1;
subjectExponent = ether(1);
subjectBucketSize = ONE_HOUR_IN_SECONDS;
subjectIsDecreasing = true;
subjectMaxPrice = ether(100);
subjectMinPrice = ether(90);
subjectIncreaseTime = ONE_HOUR_IN_SECONDS;
const returnedPrice = await subject();
const expectedPrice = ether("97.281718171540954765"); // 100 - 1 * e^(1 * 1)
const tolerance = 100;
expect(returnedPrice).to.be.closeTo(expectedPrice, tolerance); // Returned price is 98.281718171540955000
});
```
As a result, the calculation in the contract is incorrect by 1 (1e18 wei), which could be impactful for tokens, that have a price closer to 0.
Example when scaling factor is > 1 and the time elapsed < bucket size:
Initial price = 100
Scaling factor = 2
Time coefficient = 1
Is price decreasing = true
Bucket size = 1 hour (3600)
Time elapsed = 0.5 hours (1800)
According to the formula:
timeBucket = 1800 / 3600 = 0 (Actually 0.5, but because of rounding down in Solidity it is 0)
initialPrice - scalingFactor * e ^ (timeCoefficient * timeBucket) =
= 100 - 2 * e^(1 * 0) =
= 100 - 2 * e^0 =
= 100 - 2 * 1 =
= 100 - 2 =
= 98
But the getPrice result with the same parameters is 99 (99000000000000000000 in wei)
Test snippet of the example
### Test snippet of `getPrice` with the parameters of the example
Add this to `boundedStepwiseExponentialPriceAdapter.spec.ts` test and run `npx hardhat test`
Note: in this test, price is expected to match the formula calculation.
```js
it.only("Should return the correct price", async () => {
subjectInitialPrice = ether(100);
subjectCoefficient = 2;
subjectExponent = ether(1);
subjectBucketSize = ONE_HOUR_IN_SECONDS;
subjectIsDecreasing = true;
subjectMaxPrice = ether(100);
subjectMinPrice = ether(90);
subjectIncreaseTime = ONE_HOUR_IN_SECONDS.div(2);
const returnedPrice = await subject();
const expectedPrice = ether("98"); // 100 - 2 * e^(1 * 0)
const tolerance = 100;
expect(returnedPrice).to.be.closeTo(expectedPrice, tolerance); // Returned price is 99
});
```
As a result, there is one of 2 issues (depending on what Index Protocol expects):
If scaling factor > 1 = initial step/bucket will not be the initial price.
If that is the case, then the calculation is off by 1 (1e18 wei)
Calculation should start from the initial price.
If that is the case, then the calculations will always be off by scaling factory * 1e18 - 1e18 (1e18 wei in this case)
Impact
Because of the incorrect calculation, there are 2 issues:
With the current calculation the price will be +1 (1e18) every time. This means if the price of the token is closer to 0, the incorrect calculation in getPrice will have a bigger impact since the calculation will always add 1 to the price.
If the scalingFactor > 1, then the initial step/bucket will not be the initial price.
Explanation:
If expExpression is equal to WAD (1e18) - it means that the time elapsed is less than the bucket size and the price should not change.
Else the price should change by scalingFactor * expExpression
If the calculation should follow the formula, that is in the contract documentation, then the priceChange should be changed to:
Brenzee
high
Price is not calculated correctly according to the documentation in the contract
Summary
In
BoundedStepwiseExponentialPriceAdapter.solcontract the returned value of thegetPricefunction does not match the formula in the documentation, which isprice = initialPrice +/- scalingFactor * e ^ (timeCoefficient * timeBucket).Vulnerability Detail
Below are 2 examples of the calculation issue.
Example when time elapsed >= bucket size:
According to the contract formula, this should be the calculation:
Calculation in Wolfram Alpha
But the
getPriceresult with the same parameters is 98.281718171540955000 (98281718171540955000in wei)Test snippet of the example
### Test snippet of `getPrice` with the parameters of the example Add this to `boundedStepwiseExponentialPriceAdapter.spec.ts` test and run `npx hardhat test` This test should pass, but it fails instead. ```js it.only("Should return the correct price", async () => { subjectInitialPrice = ether(100); subjectCoefficient = 1; subjectExponent = ether(1); subjectBucketSize = ONE_HOUR_IN_SECONDS; subjectIsDecreasing = true; subjectMaxPrice = ether(100); subjectMinPrice = ether(90); subjectIncreaseTime = ONE_HOUR_IN_SECONDS; const returnedPrice = await subject(); const expectedPrice = ether("97.281718171540954765"); // 100 - 1 * e^(1 * 1) const tolerance = 100; expect(returnedPrice).to.be.closeTo(expectedPrice, tolerance); // Returned price is 98.281718171540955000 }); ```As a result, the calculation in the contract is incorrect by 1 (1e18 wei), which could be impactful for tokens, that have a price closer to 0.
Example when scaling factor is > 1 and the time elapsed < bucket size:
According to the formula:
Calculation in Wolfram Alpha
But the
getPriceresult with the same parameters is 99 (99000000000000000000in wei)Test snippet of the example
### Test snippet of `getPrice` with the parameters of the example Add this to `boundedStepwiseExponentialPriceAdapter.spec.ts` test and run `npx hardhat test` Note: in this test, price is expected to match the formula calculation. ```js it.only("Should return the correct price", async () => { subjectInitialPrice = ether(100); subjectCoefficient = 2; subjectExponent = ether(1); subjectBucketSize = ONE_HOUR_IN_SECONDS; subjectIsDecreasing = true; subjectMaxPrice = ether(100); subjectMinPrice = ether(90); subjectIncreaseTime = ONE_HOUR_IN_SECONDS.div(2); const returnedPrice = await subject(); const expectedPrice = ether("98"); // 100 - 2 * e^(1 * 0) const tolerance = 100; expect(returnedPrice).to.be.closeTo(expectedPrice, tolerance); // Returned price is 99 }); ```As a result, there is one of 2 issues (depending on what Index Protocol expects):
scaling factory * 1e18 - 1e18(1e18 wei in this case)Impact
Because of the incorrect calculation, there are 2 issues:
getPricewill have a bigger impact since the calculation will always add 1 to the price.scalingFactor> 1, then the initial step/bucket will not be the initial price.Code Snippet
BoundedStepwiseExponentialPriceAdapter.sol L:73Tool used
Manual Review
Recommendation
My recommendation depends on what Index Protocol wants.
If the initial step/bucket should be the initial price, then the
priceChangeshould be changed to:Explanation: If
expExpressionis equal toWAD(1e18) - it means that the time elapsed is less than the bucket size and the price should not change. Else the price should change byscalingFactor * expExpressionIf the calculation should follow the formula, that is in the contract documentation, then the
priceChangeshould be changed to:Duplicate of #39