Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trade and fee resolving edge cases #135

Open
jhoogstraat opened this issue May 16, 2022 · 2 comments
Open

trade and fee resolving edge cases #135

jhoogstraat opened this issue May 16, 2022 · 2 comments
Labels

Comments

@jhoogstraat
Copy link
Contributor

jhoogstraat commented May 16, 2022

resolve_trades currently only works when there is exactly one Buy and one Sell op for any particular utc_time.
This leaves many cases unhandled, which could result in miscalculated gains.
Here are four such cases:

  1. Multiple buy/sell-pairs at a particular utc_time
  2. Orders not being fulfilled at once, leading to Buy/Sell ops later on.
  3. Commissions / Withdrawals / Deposits / Airdrop / CoinLendInterest increasing the op count (more than 2 ops in that case)
  4. It can happen that bnb small asset exchange happen at multiple utc_times (sell ops one second before buy ops)

The first case can be resolved for 3 coins if there is a "bridge coin" (sell btc -> usdt, buy eth -> usdt), because usdt is included in both orders.
The third case can be resolved by filtering the irrelevant ops.

Binance does provide an order history, which contains the required information to resolve trades correctly, even when multiple trades occur. Maybe that can help us.

pr: #136

@jhoogstraat jhoogstraat changed the title Resolving multiple trades Resolving trades edge cases May 16, 2022
@jhoogstraat jhoogstraat changed the title Resolving trades edge cases trade resolving edge cases May 16, 2022
@provinzio
Copy link
Owner

Looks like deposit of eur and direct conversion to BNB. Transaction related should be something like buy/sell

@jhoogstraat
Copy link
Contributor Author

Ok, you are right, it didn't work because of the Deposit, which was not excluded when resolving trades.

@jhoogstraat jhoogstraat changed the title trade resolving edge cases trade and fee resolving edge cases May 16, 2022
@provinzio provinzio added the Book label Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants