Disable includes.json files affects for endpoint:crypto/ crypto_lwba aliases #368
Conversation
NPM Publishing labels 🏷️🟢 This PR has valid version labels and will cause a |
| @@ -152,7 +152,9 @@ export class PriceAdapter< | |||
| } | |||
|
|
|||
| for (const endpoint of priceEndpoints) { | |||
| endpoint.requestTransforms?.push(requestTransform) | |||
| if (!['crypto', 'crypto-lwba'].includes(endpoint.name)){ | |||
There was a problem hiding this comment.
Recommend we lowercase it to avoid issues with for example Crypto
And is this safe? I see that for cryptocompare we are overrding BTC and ETH
There was a problem hiding this comment.
+1 that I'm not sure this is safe - can we instead upgrade the includes to have a concept of which endpoints it applies to?
My main concern is that this will change all EA's, and there is no way to tell from the EA implementation level that these includes will be ignored for certain endpoints.
There was a problem hiding this comment.
agree here, i think hardcoding 'crypto', 'crypto-lwba' might also miss endpoint name aliases in EAs where the "standard" endpoint name isn't used (eg: price instead of crypto)
For a more complete solution, my initial thought is something like eg:
[
{
"from": "BTC",
"to": "ETH",
"includes": [
{
"from": "ETH",
"to": "BTC",
"inverse": true,
"endpoints": ["crypto", "crypto-lwba"]
}
]
}
]where endpoints defaults to all endpoints - current behaviour - for backward compatibility
| @@ -152,7 +152,9 @@ export class PriceAdapter< | |||
| } | |||
|
|
|||
| for (const endpoint of priceEndpoints) { | |||
| endpoint.requestTransforms?.push(requestTransform) | |||
| if (!['crypto', 'crypto-lwba'].includes(endpoint.name)){ | |||
There was a problem hiding this comment.
+1 that I'm not sure this is safe - can we instead upgrade the includes to have a concept of which endpoints it applies to?
My main concern is that this will change all EA's, and there is no way to tell from the EA implementation level that these includes will be ignored for certain endpoints.
DF 20953 - RWA updates- Disable includes.json files affects for endpoint:crypto/ crypto_lwba aliases
https://smartcontract-it.atlassian.net/browse/DF-20953