FRP: Detect is a protocol is missing

[DarkGhost7]

What is your proposal 🚀

This is a request to detect if a protocol is missing that is linked in strategies. For example if it shows up here in the strategy description https://github.com/yearn/ydaemon/blob/main/data/meta/strategies/1/AaveBorrow.json#L13 (any strategy) but doesn't have a corresponding protocol file, then it should show as a ko error on the protocol tab saying file doesn't exist.

What does this solve ? 🛠

Makes sure we have all the data correctly input Should show this error for missing protocols on the protocol tab

[karelianpie]

@DarkGhost7 I have started looking into this, however the mapping is not 1:1 with the protocol names in the strategies and the protocols themselves.

These are the unique protocol names in the strategies:

['ConvexFinance', 'CurveFinance', 'IdleFinance', 'Aave', 'CreamFinance', 'Fiat Dao', 'Balancer', 'MushroomFinance', 'MakerDAO', 'Blockchain Adventurers Guild', 'Uniswap v3', 'Notional Finance', 'LidoFinance', 'Yearn', 'Hegic', 'Sushi', 'CompoundFinance', 'KeeperDAO', 'dydx', 'Tokemak', 'Universe', 'StargateFinance', 'AngleProtocol', 'GoldfinchFinance', '88Mph', 'PoolTogether', 'VesperFinance', 'League Dao', 'InverseFinance', 'IronBank', 'AlphaHomora', 'AAVE', 'Uniswap', 'Liquity', 'ArrakisFinance', '1inch', 'Synthetix']

and these are the protocol names in the protocols themselves:

['1inch', '88 MPH', 'Aave', 'Alpha Homora', 'Angle', 'Arrakis Finance', 'Balancer', 'C.R.E.A.M. Finance', 'Compound Finance', 'Convex Finance', 'Curve Finance', 'Fiat DAO', 'Goldfinch Finance', 'Hegic', 'Idle Finance', 'Inverse Finance', 'Iron Bank', 'KeeperDAO', 'League DAO', 'Liquity', 'MakerDAO', 'Mushroom Finance', 'Notional Finance', 'Pool Together', 'Stargate Finance', 'Sushi', 'Synthetix', 'Tokemak', 'Uniswap v3', 'Universe', 'Vesper Finance', 'dYdX']

Which means that we cannot programatically show the missing files in the protocols tab. We could however create a manual mapping for the above if the file name follows the protocol name in the strategies. Or we could display the missing protocol files in a new strategy tab.

[DarkGhost7]

I can fix it so they match my new question would be should we do aave v3, aave v2 or how should this be handled? https://github.com/yearn/ydaemon/pull/59/files#diff-eb6b1858f7ff807516cf7dec5df22a8051397680c435e7619f52aa834faa08d8R3 I added aave v3 on optimism since that is all they will use but made the protocol name the same. Most in the strategies just need a space. Will add to my to do list. @karelianpie

[karelianpie]

I can fix it so they match my new question would be should we do aave v3, aave v2 or how should this be handled? https://github.com/yearn/ydaemon/pull/59/files#diff-eb6b1858f7ff807516cf7dec5df22a8051397680c435e7619f52aa834faa08d8R3 I added aave v3 on optimism since that is all they will use but made the protocol name the same. Most in the strategies just need a space. Will add to my to do list. @karelianpie

In that case the name should match the name in the strategy that uses that protocol and be in a file named Aave2.json or Aave3.json. What we need is to get the protocol names in the strategy's JSON, then with that name find if there's a file with that name in /data/meta/protocols/${chainId} and if not, match that protocol name with the correct protocol in the protocol tab. So for example, in the strategies JSONs the protocol name that is there is 88Mph and then we have a matching 88Mph.json but when we parse that file the name in these is "name": "88 MPH" so it won't match. The protocol names in the strategies's protocols prop should match the name of the protocol in the protocol's JSON name prop.

I've just noticed that even in the strategies there are mismatches, for example 'Aave' and 'AAVE'. Ideally we would have a single keyword in all these 3 places; 1) the name of the protocol file; 2) the name of the protocol in that file; and 3) the name of the protocol in the strategies' protocols.

Does the above make sense @DarkGhost7? Please let me know if I'm misunderstanding something :)

[DarkGhost7]

So the way I see it is just check - Is there a mapping from protocol "name" field that matches the strategy's "protocols" field, else if any of the terms don't match throw an error. Protocols needs to be the source for correct names and I will fix everything else to match.

[DarkGhost7]

refactoring names here https://github.com/yearn/ydaemon/pull/63

[karelianpie]

So the way I see it is just check - Is there a mapping from protocol "name" field that matches the strategy's "protocols" field, else if any of the terms don't match throw an error. Protocols needs to be the source for correct names and I will fix everything else to match.

The initial concern was where to throw the error, if a strategy has a non-existent protocol name then this error should be shown in a potential strategies tab. The reason being that it might just be a typo and we cannot be certain that that protocol should even exist. A worse alternative would be to create that non-existent protocol in the current protocol tab with the strategy that has it in its "protocols" field. My reasoning for this is the separation of concerns between strategies and protocols, the strategies know about protocols (they have a "protocols" field) but the protocols should not be aware of what strategies are using them.

So I propose we create a new strategies tab where initially we can have the non-existent protocols missing for each strategy. Example:

{
  "$schema": "strategy",
  "name": "88 MPH Reinvest",
  "description": "Supplies {{token}} to [88 MPH](https://88mph.app/earn) to earn a fixed-rate yield and MPH tokens. Earned tokens are harvested, sold for more {{token}} which is deposited back into the strategy.",
  "addresses": [],
  "protocols": ["88 MPH", "NEP"],
  "localization": {}
}

The strategy above would have an error for the NEP in the strategy tab because that protocol does not exist.

In any case, standardising all names is definitely needed.

Wdyt @DarkGhost7?

[DarkGhost7]

Sounds reasonable to me. Wdyt @Majorfi

[Majorfi]

@karelianpie sounds good!

[DarkGhost7]

refactoring names here yearn/ydaemon#63

@karelianpie we have added a new field

[karelianpie]

@karelianpie we have added a new field

@DarkGhost7 could you expand on the reasoning for this new field?

Also, I've just pushed the changes discussed here to https://github.com/yearn/ySync/pull/71

In there, the "Protocols" list of names comes directly from the name field in the protocol files themselves, then I compare to the protocols field in the strategies and if there's no match then I'm considering them invalid. I thought of adding that section of protocol names so that it's easier to debug why a protocol is valid or invalid.

/cc @Majorfi

[DarkGhost7]

The reason for a protocolVersion is because for example aave on optimism is v3 where on eth it is v2. This gives more granularity to the risk team if we can add to our metadata which specific protocol the strategy is using

[karelianpie]

The reason for a protocolVersion is because for example aave on optimism is v3 where on eth it is v2. This gives more granularity to the risk team if we can add to our metadata which specific protocol the strategy is using

I see, so I'm assuming it won't impact the work done for this issue? Or should we already make use of it now?

[DarkGhost7]

Let me check the pr for yDaemon has been merged now, and I have fixed alot of protocol names

[DarkGhost7]

The reason for a protocolVersion is because for example aave on optimism is v3 where on eth it is v2. This gives more granularity to the risk team if we can add to our metadata which specific protocol the strategy is using

I see, so I'm assuming it won't impact the work done for this issue? Or should we already make use of it now?

For now it should be fine