Feat: Add Superfluid Oracle token adapter #381
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
This comment was marked as resolved.
This comment was marked as resolved.
|
Hi @Uxio0, this is a continuation of this: safe-global/safe-transaction-service#772 (comment). I'd appreciate your feedback on this and direction for next steps. 🙂 |
gnosis/eth/oracles/oracles.py
Outdated
| if not (Web3.isChecksumAddress(token_address)): | ||
| token_address = Web3.toChecksumAddress(token_address.lower()) |
There was a problem hiding this comment.
Was getting errors in the testing, but I'll remove if the token_address here has already been checked when Oracle in cuse. For reference:
web3.exceptions.InvalidAddress: ('Web3.py only accepts checksum addresses. The software that gave you this non-checksum address should be considered unsafe, please file it as a bug on their platform. Try using an ENS name instead. Or, if you must accept lower safety, use Web3.toChecksumAddress(lower_case_address).', '0x1dbc1809486460dcd189b8a15990bca3272ee04e')
There was a problem hiding this comment.
That's because you are using not checksummed addresses in the tests
gnosis/eth/tests/test_oracles.py
Outdated
| @@ -245,6 +246,38 @@ def test_get_price(self): | |||
| cream_oracle.get_price(Account.create().address) | |||
|
|
|||
|
|
|||
| class TestSuperfluidOracle(EthereumTestCaseMixin, TestCase): | |||
| def test_get_price(self): | |||
| ethereum_client_polygon = EthereumClient("https://polygon-rpc.com") | |||
There was a problem hiding this comment.
I don't like hardcoding nodes here as it will add more dependencies and point of failures during testing, is there any chance that you could use mainnet as the other end to end tests?
There was a problem hiding this comment.
Tokens not on mainnet, query will fail.
There was a problem hiding this comment.
Ok, then please implement something like this for Polygon using an environment variable `ETHEREUM_POLYGON_NODE: https://github.com/safe-global/safe-transaction-service/blob/master/safe_transaction_service/history/tests/utils.py#L7
Leave out Arbitrum as it's too many dependencies and if it works for Polygon it should work for Arbitrum
gnosis/eth/oracles/oracles.py
Outdated
| @@ -544,6 +546,48 @@ def get_price(self, token_address: str) -> float: | |||
| ) | |||
|
|
|||
|
|
|||
| class SuperfluidOracle(PriceOracle): | |||
There was a problem hiding this comment.
Please create the oracle in gnosis/eth/oracles/superfluid.py , we are doing that for the new oracles and will be migrating the old ones, that way it's easier to maintain
brymut
force-pushed
the
master
branch
2 times, most recently
from
49921f5
to
5e4bdc5
Compare
|
Applied changes from feedback @Uxio0 |
gnosis/eth/tests/utils.py
Outdated
| @@ -40,6 +40,36 @@ def just_test_if_mainnet_node() -> str: | |||
| return mainnet_node_url | |||
|
|
|||
|
|
|||
| def just_test_if_polygon_node() -> str: | |||
| polygon_node_url = os.environ.get("POLYGON_NODE") | |||
There was a problem hiding this comment.
Nice, but please use ETHEREUM_POLYGON_NODE (as we use ETHEREUM as the namespace for RPC related stuff)
Pull Request Test Coverage Report for Build 3313527665
💛 - Coveralls |
Thanks for the support too! |
As a continuation from here: safe-global/safe-transaction-service#772 (comment)
This work contributes towards adding a price data for Superfluid Super Tokens with the price data of their underlying tokens.
This PR adds the Superfluid Oracle token adapter and related test.
Related Issue: safe-global/safe-transaction-service#772
Original issue: superfluid-finance/protocol-monorepo#732