test: add BancorExchangeProvider pricing tests #532
Merged
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
philbow61
changed the base branch from
feat/GDollar-of-latest-develop
to
main
philbow61
changed the base branch from
main
to
feat/GDollar-of-latest-develop
This reverts commit d45a22e.
RyRy79261
reviewed
Oct 18, 2024
| (result, precision) = power(_reserveBalance, baseD, _reserveWeight, MAX_WEIGHT); | ||
| uint256 temp1 = _supply * result; | ||
| uint256 temp2 = _supply << precision; | ||
| return (temp1 - temp2 - 1) / result + 1; |
There was a problem hiding this comment.
I'm not entrenched in context enough to assess the potential implications for this implementation.
But at the minimum, the setting of baseD, temp1 & temp2 as variables in this function can be removed to improve gas efficiency.
(result, precision) = power(_reserveBalance, (_reserveBalance - _amount), _reserveWeight, MAX_WEIGHT);
return ((_supply * result) - (_supply << precision) - 1) / result + 1;
```
Co-authored-by: baroooo <baranseltekin@gmail.com>
RyRy79261
approved these changes
Oct 22, 2024
bowd
pushed a commit
that referenced
this pull request
Oct 24, 2024
7704a9b chore: reserve storage gap in new contracts 040b7fe GoodDollar Fork Tests (#530) 2ee14b4 Feat(454): Exchange provider tests (#529) 578747c test: add BancorExchangeProvider pricing tests (#532) fcd3d02 feat: improve tests around the expansion controller (#533) d3a31fa Streamline comments and errors (#528) ec64f0a fix: ☦ pray to the lords of echidna 13d3a98 fix: add back forge install to the CI step c236009 fix: echidna tests c50a198 feat: add Broker liquidity check (#523) 914ba7d Address Slither Issues in GoodDollar/Bancor Contracts (#526) 8bcf3fd Feat/change gdollar modifiers (#524) 379a511 feat: make validate() internal bcbd9aa chore: added "yarn todo" task to log out open todos in the code 0631c60 refactor: renamed SafeERC20 to SafeERC20MintableBurnable 2800297 feat: simplified SafeERC20 e858391 feat: extend open zeppelin's IERC20 instead of duplicating it 196f6be chore: fixed linter and compiler warnings 096efe2 test: fix fork integration test e5308d9 feat: add GoodDollar contracts + tests 3135626 feat: update Broker + TradingLimits to 0.8 and make G$ changes
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.
Description
This Pr adds tests for the pricing logic of the bancor exchange provider.
During the testing, we realized that the BancorFormula.sol contract is missing one function we need.
This function is getAmountIn for the tokenIn being the BancorToken. The new function is called
saleCost()The math formula for this function is derived from the
saleTargetAmount()function already implemented. It's important that the base of power operation is larger one otherwise the power function in that contract reverts.These are the current formulas used for the different pricing functions:
This is how we derived the formula for
saleCost()Related issues
How to review