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

BL3P: added buy/sell/cancel #475

Merged
merged 14 commits into from
Nov 5, 2019
Merged

BL3P: added buy/sell/cancel #475

merged 14 commits into from
Nov 5, 2019

Conversation

johnnyasantoss
Copy link
Collaborator

Also, adds some new shiny console options for those three options.

Help for the "buy" command

ExchangeSharpConsole 0.6.2
Copyright (C) 2019 ExchangeSharpConsole

  --debug            Waits for a debugger to be attached.

  -e, --exchanges    Required. Regex of exchanges to test, null/empty for all.

  --dry-run          (Default: false) Prevents any requests from being
                     performed.

  -k, --key          (Default: keys.bin) Path to key file (generated with the
                     key utility).

  -i, --interval     (Default: 5000) Interval to fetch data in milliseconds.

  -w, --wait         (Default: false) Waits interactively.

  -r, --price        Required. The price to buy or sell at.

  -a, --amount       Required. Amount to buy or sell.

  --stop-price       The price to trigger a stop.
                     This has to be implemented and supported by the chosen
                     exchange.

  --order-type       Required. (Default: Limit) The type of order.
                     Possible values: 'limit', 'market' and 'stop'

  --margin           (Default: false) Whether the order is a margin order. Not
                     all exchanges support margin orders, so this parameter may
                     be ignored.
                     You should verify that your exchange supports margin orders
                     before passing this field as true and expecting it to be a
                     margin order.
                     The best way to determine this in code is to call one of
                     the margin account balance methods and see if it fails.

  --round            (Default: true) Whether the amount should be rounded.
                     Set to false if you know the exact amount, otherwise leave
                     as true so that the exchange does not reject the order due
                     to too many decimal places.

  -s, --symbol       Required. Symbol (currency pair) to be fetched from the
                     exchange.

  --help             Display this help screen.

  --version          Display version information.


private string GetSignKey(IHttpWebRequest request, string formData)
{
//TODO: Use csharp8 ranges
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To use System.Range we would have to drop net472 as a a target framework

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's not an option unfortunately right now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We already support netstandard2.0 which is compatible ¹ ² with net461. We are not using any APIs that are net472 specific (AFAIK) 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't that mean that we would have to drop support for netstandard 2.0 as well in order to use System.Range?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is true. One thing that I said that was wrong is that we need to drop net472 when actually what I should have said is that there's no need to support net472 when we already support netstandard20. With that clarified, I noticed that those types (Range and Index) are not available in all runtimes (only netstandard2.1 and netcoreapp30) and in that case we could use a package that ports those specific APIs to netstandard20 like this one https://github.com/bgrainger/IndexRange

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we could create an issue out of this conversation 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

we could use a package that ports those specific APIs to netstandard20 like this one https://github.com/bgrainger/IndexRange

Yes that's a good way to keep netstandard20 and start using the new APIs

Maybe we could create an issue out of this conversation 🤔

Yes feel free

namespace ExchangeSharp.API.Exchanges.BL3P.Enums
{
public enum BL3PCurrencyFee : byte
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be internal

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can't be made internal, because, it's one of the settings of ExchangeBL3PAPI. See prop ExchangeBL3PAPI.DefaultFeeCurrency.

@jjxtra
Copy link
Collaborator

jjxtra commented Nov 4, 2019

Looks like merge conflict, other than that I'm good to merge in

@johnnyasantoss
Copy link
Collaborator Author

@jjxtra @vslee Could you check the PR again? :)
Thanks!

@vslee vslee merged commit 208a487 into DigitalRuby:master Nov 5, 2019
@johnnyasantoss johnnyasantoss deleted the addorder branch November 6, 2019 14:34
@vslee vslee changed the title Add buy/sell/cancel to BL3P BL3P: added buy/sell/cancel Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants