Trade supports multiple routes #55
Conversation
|
I've changed the approach here, decided to remove all percentages and just use the actual amounts for each split The routing api will handle potential loss of precision from doing splits and return amounts per route s.t. they add up correctly |
| exactOut | ||
| .maximumAmountIn(new Percent(JSBI.BigInt(200), JSBI.BigInt(100))) | ||
| .equalTo(CurrencyAmount.fromRawAmount(token0, 46464)) | ||
| ).toBeTruthy() |
There was a problem hiding this comment.
could maybe also just define a matcher, not that it matters
| let spotOutputAmount = CurrencyAmount.fromRawAmount(this.outputAmount.currency, 0) | ||
| for (const { route, inputAmount } of this.routes) { | ||
| const midPrice = route.midPrice | ||
| spotOutputAmount = spotOutputAmount.add(midPrice.quote(inputAmount)) |
There was a problem hiding this comment.
mmmm is this correct... i don't really have an intuition about the meaning of price impact across routes
There was a problem hiding this comment.
I think so, and I think @NoahZinsmeister agrees?
There was a problem hiding this comment.
this is what i suggested, it seems consistent with how we were calculating it in the single-route case, and works out to a weighted average price impact across routes, where the weights are the quote amounts and the values are the individual price impact
Trade updated to support multiple routes. Change is back in a backward compatible way to avoid a major version bump: you will hit an error if you try and access the 'route' property when your Trade consists of multiple routes
I previously made a change to the SwapRouter to support multiple Trade objects being provided to
swapCallParameters. With this change that is no longer necessary, but I left it in anyway.