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

Make order form functional #93

Merged
merged 1 commit into from
Mar 27, 2018
Merged

Make order form functional #93

merged 1 commit into from
Mar 27, 2018

Conversation

lukechilds
Copy link
Member

@lukechilds lukechilds commented Mar 27, 2018

This makes the buy and sell order forms functional.

Currently the error handling is pretty naive but I don't think that's important right now. We should probably grey out the order form if there is already a pending trade but we aren't storing the trade history so no way to do that yet.

Also, currently the trade is just logged to the console. It should be pushed into a local DB and kept in sync with mm based on the event stream. However this is gonna take a while to get working reliably so I'll add trade history in a follow up PR.

@sindresorhus
Copy link
Contributor

I tried buying without any funds and got this message:

cant find a deposit that is close enough in size. make another deposit that is just a bit larger than what you want to trade

Can we improve those messages directly in mm? https://github.com/jl777/SuperNET/blob/c564d5f61267d6d844864a8efc9cfa7d01a7a29f/iguana/exchanges/LP_ordermatch.c#L1507

@lukechilds
Copy link
Member Author

Can we improve those messages directly in mm?

We could, but bear in mind there are no error codes, only error messages. So changing any error message is potentially a breaking change. Any existing error checking code will break.

e.g our code:

if (result.error === 'only one pending request at a time')

will fail if that message changes. Obviously not ideal, but there's no other way we can target that specific error.

I'll create an issue asking for error codes.

@lukechilds
Copy link
Member Author

jl777/SuperNET#717

@sindresorhus sindresorhus merged commit b04abde into master Mar 27, 2018
@sindresorhus sindresorhus deleted the order branch March 27, 2018 14:37
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.

2 participants