Bug Report: BinanceClientException when calling NewOrder with (SELL, MARKET, quantity)

[pzarzycki]

Reproduction steps

decimal quantity = 0.00128503M;
_spotAccountTrade.NewOrder(symbol:"BTCUSDT", side:SELL, type:MARKET, quantity:quantity);

quantity is a decimal variable with exactly value 0.00128503. quantity.ToString() returns as expected "0.00128503". Digital representation matches regex mentioned in the exception. Yet, API throws error as bellow.

Expected behavior

No exception thrown. Result returned.

Actual Behavior

Exception thrown:

Binance.Common.BinanceClientException HResult=0x80131500 Message=Illegal characters found in parameter 'quantity'; legal range is '^([0-9]{1,20})(.[0-9]{1,20})?$'. Source=Common

.NET version

.NET Framework 6.0.x

Operating system

Windows

Environment

Set Culture to some that uses "," (comma) for decimal separator.

dotnet --info

No response

[pzarzycki]

This will happen when default Culture is other than expected, e.g. "EN-GB", which uses "," instead of "." for decimal separator. BuildQueryString() in

.Append(HttpUtility.UrlEncode(queryParameter.Value.ToString()));

will then try to encode decimal as "x,xxx" instead of "x.xxx".

Interesting thing is that in the callee modules (.NET Core) decimal.ToString() was returning "x.xxxx" with dot, not comma.

Looks like BuildQueryString() needs explicit culture formatting management, e.g.:

using System.Globalization;

NumberFormatInfo nfi = new NumberFormatInfo();
nfi.NumberDecimalSeparator = ".";

value.ToString(nfi);

[tantialex]

The issue is caused due to the default culture of the thread executing the query parameter cast to string. This could be solved by the client by changing the thread's culture.

Thread.CurrentThread.CurrentCulture = CultureInfo.CreateSpecificCulture("de-DE");

However this would affect all string casting executed by the thread which may not be ideal. A better solution is to define the specific culture when casting.

Convert.ToString(value, CultureInfo.InvariantCulture);

This has been resolved with #14, and available in 1.3.1

[pzarzycki]

I found similar issue in this area. Binace API is accepting up to 20 decimal digits for quantities, but might happen that C# decimal will be longer, even just full of zeros, e.g.:

decimal qty = 0.1000000000000000000000000M

This will lead to a format exception from the API.

Of course user can take care of this before calling the API, but would be nice if client library does it automatically, since it's a rudimentary task and it does not require much logic. Max 20 decimal digits requirement is far below all of the min_step LOT constraints.

[tantialex]

The intention of the connector is to be as lightweight as possible, reducing the latency between client request and server response.

Adding a routine to validate parameters before sending them to the API will provide extra latency to all requests, even valid ones. The precision of parameters is the responsibility of the client, not the connector.

[ahmetkorkmaz27]

I am getting the following errors please help me binance-connector-dotnet/Src/Common/BinanceService.cs line 105 private async Task SendAsync(string requestUri, HttpMethod httpMethod, object content = null) error: not all code paths return a value line 146 throw clientException; error: the type caught or thrown must be derived from system.Exception line 184 throw httpException; error: the type caught or thrown must be derived from system.Exception

[tantialex]

@ahmetkorkmaz27 Please open a separate issue.