Moved sorting of trades to eco router #221
Conversation
Mi-Lan
commented
Nov 28, 2022
•
Mi-Lan
commented
- Moved trades type sorting to the sdk so that we have it in one place
- Added new folder inside trades called ecoRouterTrades which handles trades fetching and trades ordering
Mi-Lan
changed the title
| import { EcoRouterResults } from './types' | ||
|
|
||
| export async function getTradesPromise( | ||
| parsedAmount: CurrencyAmount, |
There was a problem hiding this comment.
Can we call this currencyAmount? We don't know if it's parsed or not so this is an assumption.
There was a problem hiding this comment.
I think it will be more confusing to call it same as type. What do you mean by well not know if its parsed or not? It always accepts the same type currency with the amount inside that is requested
| recipient: receiver, | ||
| tradeType: TradeType.EXACT_INPUT, | ||
| }) | ||
| .then((res) => resolve(res ? res : undefined)) |
There was a problem hiding this comment.
Not sure if this logic makes sense. If it's undefined it will return undefined. Are we turning false, null values into undefined?
There was a problem hiding this comment.
So this is because of type because it should return either trade or undefined
| .catch((error) => { | ||
| console.error(error) | ||
| errors.push(error) | ||
| resolve(undefined) | ||
| }) |
There was a problem hiding this comment.
We are repeating this logic every .catch maybe we can have a function catch error that receives and error and returns undefined (or just return). Calling resolve is the same as calling return, so we can simply that too.
There was a problem hiding this comment.
Hmm in not sure about this actually because its a promise not a function...
berteotti
force-pushed
the
develop
branch
2 times, most recently
from
a841304 to
3472ae2
Compare
