Return OrderResponse struct from buy & sell limit

[rupurt]

Builds upon #13

[dvcrn]

@rupurt finally had some time to take a closer look. The problem is that you replaced the config for key/secret with System.get_env which will return nil if the env var is not present.

The library needs a string for creating the signature

    signature =
      :crypto.hmac(
        :sha256,
        Application.get_env(:binance, :secret_key),    # <------ here 
        argument_string
      )
      |> Base.encode16()

[rupurt]

Gotcha. How do you think I should go about testing that? Are you ok with this approach with the environment variables?

[dvcrn]

Remove the System.get_env parts for now. Having "" as default is sane and the user can always override it in his own config later on.

Also mix test runs on MIX_ENV=test. Since the exvcr stuff is only needed for tests, could you move that into a separate config_test.ex file and load that when in test?

[rupurt]

@dvcrn I believe the config/config.exs file is restricted to the project and won't get loaded. So projects that use the binance package won't get any exvcr config.

[dvcrn]

Ah yeah, good point! But yeah, in either way, the System.get_env stuff isn't needed inside this project I think. If a project wants to handle the credentials like this, they can just overwrite it in their config.

[rupurt]

@dvcrn updated with the empty strings. It looks like it's still failing with the same problem :{

[dvcrn]

Now it's failing at a different problem though

  6) test .order_limit_buy can create an order with am immediate or cancel duration (BinanceTest)
     test/binance_test.exs:130
     ** (ArgumentError) argument error
     code: Binance.order_limit_buy("LTCBTC", 0.1, 0.01, "IOC")
     stacktrace:
       (kernel) :os.system_time(:millisecond) # <--------
       (binance) lib/binance.ex:269: Binance.create_order/11
       (binance) lib/binance.ex:336: Binance.order_limit_buy/4
       test/binance_test.exs:133: (test)

Not quite sure why this happens. I'll take a closer look later!

[rupurt]

@dvcrn that's the same error I've been seeing on CI. Do you mind trying with an upgraded Erlang/Elixir?

[dvcrn]

I added OTP 20 to the travis build matrix. If you rebase master and push, it should matrix build it against 1.5, 1.6, OTP 19 and OTP 20.

[rupurt]

Thanks @dvcrn. Looks like that did the trick :)

[dvcrn]

Now the question is why it breaks on OTP 19, and why it didn't before this PR since this doesn't seem to touch any system / time stuff...

[dvcrn]

If you just want to parse a map into a struct, you can use exconstructor which is already a dep of this project

Add use ExConstructor to your struct (like here

Then use .New() to parse (like here)

[rupurt]

@dvcrn it was untested code. OTP 19 is now a couple of years old and OTP 21 is in rc2.

[rupurt]

@dvcrn cool pattern with ExConstructor. I've just pushed up the requested changes.

[dvcrn]

Looking good, thanks for the fast updates!
I'll change the CI target later so the tests pass and will merge after that.

[rupurt]

😎

Fwiw OTP 21 got released this week so I added it to the travis config #16