Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

getTradeDetails: unexpected executionRate #1

Closed
samajammin opened this issue Jul 22, 2019 · 3 comments
Closed

getTradeDetails: unexpected executionRate #1

samajammin opened this issue Jul 22, 2019 · 3 comments

Comments

@samajammin
Copy link

Hey @NoahZinsmeister - first off, thank you for building this great library.

I'm just digging in & created a few calls based on your Data & Computation examples. I want to confirm I understand the meaning of these outputs.

Here's my script:

const tokenReserves: IUniswapTokenReservesNormalized = await getTokenReserves(
  address, // Dai: '0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359'
  chainIdOrProvider, // mainnet: 1
);
const marketDetails: IUniswapMarketDetails = getMarketDetails(
  undefined, // undefined === ETH
  tokenReserves,
);
const buyAmount: BigNumber = new BigNumber(amount); // amount: '1'
const tradeAmount: BigNumber = buyAmount.multipliedBy(decimals); // decimals: 18
const tradeDetails: IUniswapTradeDetails = getTradeDetails(
  TRADE_EXACT.OUTPUT,
  tradeAmount,
  marketDetails,
);

When I log out tradeDetails, I get this output (I converted it to JSON). For low buyAmount values (1 in my example), the tradeDetails.executionRate returns some wacky results: 18 in this example, vs. the pre-execution market rate and post-execution market rate of ~215.87. It's only when I increase the buyAmount, e.g. to 1000, does it return an executionRate similar to the market rates (here's my results for 1000).

First, what is the executionRate? I assume it is the actual rate for the trade after accounting for slippage, i.e. my realized rate from executing the given trade? Second, Any idea what's happening here? Perhaps some side effect of turning a small BigNumber into a string? Or is this an actual bug? Even with a small amount of DAI, I'd expect the executionRate to be about the same - even closer to the pre & post market rates.

@samajammin
Copy link
Author

Also, do you mind explaining what tradeDetails.outputAmount represents in this situation (and inputAmount)? Given that I'm passing in TRADE_EXACT.OUTPUT to getTradeDetails() in order to buy exactly the buyAmount of tokens, I expected tradeDetails.outputAmount.amount to be the same value as buyAmount, which it's not. Thanks in advance!

@samajammin
Copy link
Author

I believe I figured it out - the tradeAmount calculation should multiply by 10^decimals, not just multiply by decimals.

const tradeAmount: BigNumber = buyAmount.multipliedBy(decimals);

Should be:

const tradeAmount: BigNumber = buyAmount.multipliedBy(10 ** decimals);

That yields the expected results.

If my logic is correct, please feel free to close the issue 😄

@NoahZinsmeister
Copy link
Contributor

NoahZinsmeister commented Jul 23, 2019

hi @sbrichards, thanks for this. you're correct that you need to multiply buyAmount by 10**<decimals>, glad that seems to have fixed the issue! going to close this, feel free to reopen if you're seeing any other weird behavior.

(also, to answer one of your questions, executionRate is the actual rate that the trade will get, including price slippage)

jnaviask pushed a commit to hicommonwealth/uniswap-sdk that referenced this issue Aug 19, 2020
jnaviask pushed a commit to hicommonwealth/uniswap-sdk that referenced this issue Aug 19, 2020
Combine swap send into one component
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants