KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
99 stars 88 forks source link

assets fiat value evaluation in atomicdex-api #680

Open tonymorony opened 4 years ago

tonymorony commented 4 years ago

During the https://github.com/KomodoPlatform/atomicDEX-API/issues/659 feature discussion, we've realized with @artemii235 that it'll be hard to update all GUIs (who use USD price providers) and makers who using CLIs without USD API providers to some consistent way. Also anyway it's very hard to determine what is the minimum asset amount relies only on USD value. You have also taken on account the cost of 51% attack lets say so 1000$ tx in coin A is safer than 1000$ in coin B tx. Moreover, GUIs using only single-source USD price atm what adding failure points. Also, there is a problem with not liquid markets/assets without USD price in endpoints GUI use (for example SuperNET assets).

I'm proposing a solution to make significant_amount param in the config with such additional logic in API:

if entry in coins file has significant_amount param for this coin && amount of coin in a swap which needs to be sent in swap is > than significant_amount:
    then requires_notarization switching to false and not applying to this swap tx

We can review this param value for all coins trading in GUIs on a monthly basis for now.

It should add much practical sense: for example, we'll set for DEX significant_amount: 10 and all trades under 10 DEX will be fast but all trades over 10 DEX will be protected by dPoW and also much easier to maintain for now because both GUIs and CLI makers using coins file as a data source.

UPD: realised that it also needed limit for the amount which allowed to be sent in single block (or in given blocks window lets say) with "protection disabling" (actually not disabling but just not so strict) otherwise if I have 1000 DEX order another person can just make 200 parallel swaps for 5 DEX and for all 200 swaps will not be enabled enhanced security.

ca333 commented 4 years ago

We can review this param value for all coins trading in GUIs on a monthly basis for now. - it is maybe better if we do not set such values/tresholds and "leave it up to the user" - furthermore a factor like this one (using a coin amount) underlies the market fluctuations, high volatility et cetera and thus is out of our control to a certain degree. Accurate maintenance could be difficult. Also, IF we set it and the user is not able to change this value on his own we could have the next problem as would be taken as degradation in the user experience. And forcing the user to set such a value for each of the coins is likely overkill. The general goal is to make as little decisions for the user as possible.

However, i think the significant_amount param idea is a good addition if we slightly tweak its usage/description. When i proposed that we let the user define a FIAT value which we apply for all coins/trades i did with assumption that we will take this value just on UI level and transform it to a coin figure which we use for such a "significant_amount check". In this case user sets it just one time and based on this config the required_confs/require_nota are handled "in background" for the user. And if the user does not want to config such a min. value he can still handle it on a case by case in each buy/sell/setprice operation. I believe with this path we keep full control from enduser perspective but still have powerful options for enhanced security.

Another nut to crack and which fits this topic is if we could get very reliable FIAT price endpoints directly into mm2? - this would be truly a next level enhancement. Hope you and @artemii235 will find time for some brainstorming on this topic and also if it makes sense - i believe yes. Always have this concerns that we rely on external/centralized price APIs like CMC, gecko, et cetera. Getting our "own" (one day hopefully decentralized) source for this sort of data could solve a lot of the "problems" we discussed recently. And for the beginning its likely even enough to use our "go-price-server" so that mm2 just fetches from that endpoint - if this is an option for us we could add new mm2 start param priceAPI_urls or so which is array of such price-endpoints - and by using >1 endpoints makes it almost "decentralized" 😅

tonymorony commented 4 years ago

Thats a very valid points!

However there is a problem and paradox:

Two types of assets possible to protect by notarzation: ACs and KMD (techincally it's quite the same, so we can call its just as Komodo assets) and most of the Komodo assets which require notarization got not very liquid markets (I believe its temporary point tho :) ) and there might be a problem with USD value determining (it even might be vary dangerous for non-liquid and easy manipulatable markets) so USD endpoint will not help much for the swaps speedup.

However totally agree on that it might be super useful in future to implement USD prices fetching system (from multiple APIs/sources/oracles) right in API so GUI devs will have it standartized and well-tested so will not need to re-invent for each new interface. Plus it might give us a space for new ideas implementation for Atomicdex-API (such as native trading automation or automatical portfolio rebalancing for example).

So renaming that issue to discuss adding on roadmap with @artemii235 and @ca333 after current milestones reaching :)

ca333 commented 4 years ago

Thats a very valid points!

However there is a problem and paradox:

Two types of assets possible to protect by notarzation: ACs and KMD (techincally it's quite the same, so we can call its just as Komodo assets) and most of the Komodo assets which require notarization got not very liquid markets (I believe its temporary point tho :) ) and there might be a problem with USD value determining (it even might be vary dangerous for non-liquid and easy manipulatable markets) so USD endpoint will not help much for the swaps speedup.

However totally agree on that it might be super useful in future to implement USD prices fetching system (from multiple APIs/sources/oracles) right in API so GUI devs will have it standartized and well-tested so will not need to re-invent for each new interface. Plus it might give us a space for new ideas implementation for Atomicdex-API (such as native trading automation or automatical portfolio rebalancing for example).

So renaming that issue to discuss adding on roadmap with @artemii235 and @ca333 after current milestones reaching :)

good point - i would say for ACs that have no available price data on our priceAPI endpoint and also no "significant" DEX orderbook entries we should return a "N/A" instead of our current "0" - this would mean such assets (if no_price_data && no_significant_vol) get only option to enable or disable confs settings but not value based threshold config. I believe thats a good path for such "low/no liquidity" ACs.

Agree, USD evaluation check could be affected by price manipulation if the only source is a limited/small DEX orderbook, so the "no_significant_vol check" could hopefully prevent this problem to a certain degree.