increase precision by doing wmul first #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wei3erHase
reviewed
Apr 21, 2024
Comment on lines
67
to
72
| _priceSourceValue = WAD.wdiv(_priceSourceValue); | ||
| _result = WAD.wmul(_denominationPriceSourceValue).wdiv(_priceSourceValue); | ||
| } else { | ||
| _result = _priceSourceValue.wmul(_denominationPriceSourceValue); | ||
| } | ||
|
|
||
| _result = _priceSourceValue.wmul(_denominationPriceSourceValue); |
There was a problem hiding this comment.
this is changing the overall behaviour... _result is being then redeclared..
There was a problem hiding this comment.
ahh true thats actually a mistake. The final _result declaration should have been removed.
|
Sorry, missed deleting that line. Its now logically the same as before, but since wmul happens before wdiv the precision will be better. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
potential loss of precision
For example, for an ETH/HAI price of approximately 3000*1e18, the ideal calculation for the inverted price should preserve as much precision as possible:
10^18 x 10^18/ (3000 x 10^18) = 333 333 333 333 333.333333...
However, the current implementation first performs the division, which truncates the result and leads to a loss of decimal precision.
Therefore it will it will use this 333 333 333 333 333 instead.
By changing the order of operations theres more precision in the calculation