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

Arbitrage #41

Closed
wants to merge 13 commits into from
Closed

Arbitrage #41

wants to merge 13 commits into from

Conversation

zyzek
Copy link
Contributor

@zyzek zyzek commented Oct 22, 2017

  • Completed the arbitrageur bot so that it can participate in all six available triangular arbitrage cycles;
  • Added new logic for _sell_quoted_ function;
  • Gave MarketPlayer a slightly briefer means of accessing its markets;
  • Agents now have a record of what trades they have made;
  • Additional fee functions available to order books;
  • More complete docstrings and types in various places;
  • Improved ask/bid logic;
  • Additional utility functionality in the orderbook (e.g. quantity functions);
  • Some members renamed for clarity and sense;
  • Rounding fixes;

Closes #38.
Closes #3.

@zyzek zyzek requested a review from 0xdomrom October 22, 2017 08:52
Copy link
Contributor

@0xdomrom 0xdomrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be an issue with making certain currencies unavailable. Probably stemming from updating values/fees, even though the logic seems sound. I'm looking into the issue, which probably existed beforehand, and do not want to merge until some headway is made into solving this.

…rdebook update, made arbitrage trade on a fixed value of profit, not %
@zyzek
Copy link
Contributor Author

zyzek commented Oct 23, 2017

In principle I agree that this shouldn't be merged until those issues are dealt with, but I wonder if we could put that into a separate PR?

Happy to keep it here if they were problems introduced by this patch, but if they were pre-existing then it would be good to get these arbitrageurs and other improvements into the master version.

@0xdomrom
Copy link
Contributor

I think some of the issues did come with the changes here, so for now, I'll close this.

@0xdomrom 0xdomrom closed this Oct 23, 2017
@0xdomrom 0xdomrom mentioned this pull request Oct 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Marketplayer Sell Function is Incorrect Arbitrage Bots
2 participants