added rough support for mirror.finance liquidity pools on terra money #25
Conversation
blockapi/api/__init__.py
Outdated
| @@ -30,3 +30,5 @@ | |||
| from blockapi.api.tzstats import * | |||
| from blockapi.api.zchain import * | |||
| from blockapi.api.zensystem import * | |||
|
|
|||
| from blockapi.api.mirror import * | |||
There was a problem hiding this comment.
All the previous imports are sorted alphabetically, could you please do the same?
There was a problem hiding this comment.
Yep, no problem. The whole code is just a prototype.
blockapi/api/mirror.py
Outdated
| from blockapi.services import BlockchainAPI | ||
|
|
||
|
|
||
| class MirrorApi(BlockchainAPI): |
There was a problem hiding this comment.
You are extending the functionality of TerraMoneyApi and copying some of its methods and attributes verbatim.
It is understandable you do not want to modify that class on the first try, but you could subclass it.
There was a problem hiding this comment.
I can extend TerraMoneyApi, subclass it I guess. I wasn't sure if it wont brake the testing mechanism(need to have a look) and if to have one base_url and testnet_url in the singleton api class is generally a good idea. Just not sure here. Point is I need two api servers (lcd.terra.dev a and fcd.terra.dev) - suggestions? Adding it to the class would not work with url_build mechanism. Its just a prototype.
blockapi/api/mirror.py
Outdated
| class MirrorApi(BlockchainAPI): | ||
|
|
||
|
|
||
| symbol = 'LP' |
There was a problem hiding this comment.
Not sure either. LUNA makes sense for the getbalance from terraMoneyApi. the amount that fetch_pool_balances return is in LP(it is not a coin though)...
There was a problem hiding this comment.
well, Mirror service is not blockchain, so in this case symbol doesn't make sense
but if there have to be one, LUNA is closer because LP is not token, it's just prefix for LP tokens
btw base currency in Mirror is UST (USD stable coin)
There was a problem hiding this comment.
Ok, lets put there LUNA it is, settled! :) On the other hand - mmm you call get_balance you get a number without ticker in reality denominated in LP an you think that if you get a number you get the ticker to it by reading the attribute in the api (I would).
blockapi/api/mirror.py
Outdated
|
|
||
|
|
||
| symbol = 'LP' | ||
| base_url = 'https://tequila-lcd.terra.dev' |
There was a problem hiding this comment.
base_url is not correct either - this is for testnet.
blockapi/api/mirror.py
Outdated
| 'umnt': 'MNT' | ||
| } | ||
|
|
||
| liqudity_pool_suffix = "LP" |
There was a problem hiding this comment.
You use this variable only once, is it really necessary to define it?
There was a problem hiding this comment.
I can put it just into the evaluation + "LP". I just expected this one to change, so others don't have to search it for too long.
blockapi/api/mirror.py
Outdated
| contracts = self.get_contracts() | ||
| symbol = contract_addr | ||
|
|
||
| for contract in contracts: |
There was a problem hiding this comment.
Can be rewritten more succintly as
for contract, info in contracts.items():
if info['token'] == contract_addr:
symbol = info['symbol']
| symbol = contracts[contract]['symbol'] | ||
|
|
||
| if (symbol == contract_addr): | ||
| ex = Exception('Could not get symbol based on token address! Check the local copy of whitelisted contracts if it is up to date!') |
There was a problem hiding this comment.
Assignment ex = Exception(...) is not neccessary, just raise Exception(...). Also, would it make sense to make a specific exception for this case? Subclassing Exception?
There was a problem hiding this comment.
Definitely. About the exceptions - I have seen this somewhere else in blockapi making instead of exception of the unknown ticker a string "unknown" in the api output. What about contract address from API instead of the ticker? Will there be ETL of that data? What is better? Inconsistency in archived data (with further possibility to reconstruct what ticker it really was from the contract address) or no data because it would crash? I would also crossvalidate the input data - that the token balance I requested is the one I get in the response from lcd.terra.dev - if that wont fit? Exception?
blockapi/api/mirror.py
Outdated
|
|
||
| # loop through the pool contracts and construct the response | ||
| # that consist of pool balances of individual pools | ||
| for contract in contracts: |
There was a problem hiding this comment.
Again, this iteration can be made more readable by for contract, info in contracts.items():
blockapi/api/mirror.py
Outdated
| return response | ||
|
|
||
| @classmethod | ||
| def _get_symbol(cls, denom): |
There was a problem hiding this comment.
As discussed at the beginning, this is a verbatim copy of TerraMoneyApi.
There was a problem hiding this comment.
It s a proto. Having code duplicities is baad :)
There was a problem hiding this comment.
I would also encapsulate in the class some of the methods i wrote that don't need to be public.
| max_items_per_page = 100 | ||
| page_offset_step = 1 | ||
|
|
||
| supported_requests = { |
There was a problem hiding this comment.
To also support the requests of TerraMoneyApi, you can do
supported_requests = {
**TerraMoneyApi.supported_requests,
**{
'get_mirror_pool_balance': '/wasm/contracts/{lp_token_contract}/store?query_msg={{ "balance" : {{ "address" : "{address}" }} }}',
'get_mirror_pool': '/wasm/contracts/{pair_contract}/store?query_msg={{ "pool": {{ }} }}'
}
}
with proper formatting of course :)
There was a problem hiding this comment.
I am not sure what you mean. You mean to put the endpoint and the get parameter after "?" in one string? okok :)
No description provided.