-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add Cowswap oracle #353
Add Cowswap oracle #353
Conversation
Uxio0
commented
Sep 9, 2022
- Related to Use CowSwap as a price oracle on Mainnet/Gnosis Chain safe-transaction-service#645
Pull Request Test Coverage Report for Build 3022306946
💛 - Coveralls |
if "amount" in result: | ||
# Decimals needs to be adjusted | ||
token_2_decimals = get_decimals(token_address_2, self.ethereum_client) | ||
return float(result["amount"]) / 10**token_2_decimals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use fractions
here since we are dealing with financial decimal values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, but I wouldn't do it as it would break oracle interface for all the implemented oracles. At some point we can refactor all the oracles to return Fraction
|
||
class TestCowswapOracle(EthereumTestCaseMixin, TestCase): | ||
def test_get_price(self): | ||
mainnet_node = just_test_if_mainnet_node() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC: is this testing the real node (E2E)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, we are doing E2E for all oracle providers