burakoner / OKX.Api

Up-to-date, most-complete, well-organized, well-documented, easy-to-use, multi-task and multi-thread compatible OKX Cryptocurrency Exchange Rest and Websocket Api Wrapper
MIT License
39 stars 15 forks source link

GetTransactionHistoryAsync Bug #21

Closed forReason closed 1 year ago

forReason commented 1 year ago

GetTransactionHistoryAsync only returns the trades from the last trading day, if supplied no filter/pagination values.

Task<RestCallResult<IEnumerable<OkxTransaction>>> tradeResult = API.RestClient.Trade.GetTransactionHistoryAsync();

"https://www.okx.com/api/v5/trade/fills?limit=100" returns the last trading day

However, when supplying values, the request is malformed. This is my request:

// Get the current time minus 3 months
DateTime monthsAgo = DateTime.UtcNow.AddMonths(-2);

// Convert to Unix timestamp
long unixTimeMonthsAgo = ((DateTimeOffset)monthsAgo).ToUnixTimeMilliseconds();
Task<RestCallResult<IEnumerable<OkxTransaction>>> tradeResult = API.RestClient.Trade.GetTransactionHistoryAsync(begin: unixTimeMonthsAgo,before: 603645086244941837);
await tradeResult;

"https://www.okx.com/api/v5/trade/fills?before=603645086244941837&begin=603645086244941837&end=603645086244941837&limit=100" malformed request

forReason commented 1 year ago

upon further investigation: begin is not added to the request at all:

var tradeResult = GetTransactionHistoryAsync(begin: 1685111064573);

"https://www.okx.com/api/v5/trade/fills?limit=100"

At the same time, before is inserted multiple times:

var tradeResult = GetTransactionHistoryAsync(before: 603645086244941837);

https://www.okx.com/api/v5/trade/fills?before=603645086244941837&begin=603645086244941837&end=603645086244941837&limit=100

forReason commented 1 year ago

unchanged code where the suspected issue is.

/// <summary>
    /// Retrieve recently-filled transaction details in the last 3 day.
    /// </summary>
    /// <param name="instrumentType">Instrument Type</param>
    /// <param name="instrumentId">Instrument ID</param>
    /// <param name="instFamily">Instrument family</param>
    /// <param name="underlying">Underlying</param>
    /// <param name="orderId">Order ID</param>
    /// <param name="after">Pagination of data to return records earlier than the requested ts, Unix timestamp format in milliseconds, e.g. 1597026383085</param>
    /// <param name="before">Pagination of data to return records newer than the requested ts, Unix timestamp format in milliseconds, e.g. 1597026383085</param>
    /// <param name="begin">Filter with a begin timestamp. Unix timestamp format in milliseconds, e.g. 1597026383085</param>
    /// <param name="end">Filter with a begin timestamp. Unix timestamp format in milliseconds, e.g. 1597026383085</param>
    /// <param name="limit">Number of results per request. The maximum is 100; the default is 100.</param>
    /// <param name="ct">Cancellation Token</param>
    /// <returns></returns>
    public virtual async Task<RestCallResult<IEnumerable<OkxTransaction>>> GetTransactionHistoryAsync(
        OkxInstrumentType? instrumentType = null,
        string instrumentId = null,
        string instFamily = null,
        string underlying = null,
        long? orderId = null,
        long? after = null,
        long? before = null,
        long? begin = null,
        long? end = null,
        int limit = 100,
        CancellationToken ct = default)
    {
        limit.ValidateIntBetween(nameof(limit), 1, 100);
        var parameters = new Dictionary<string, object>();
        parameters.AddOptionalParameter("instType", JsonConvert.SerializeObject(instrumentType, new InstrumentTypeConverter(false)));
        parameters.AddOptionalParameter("uly", underlying);
        parameters.AddOptionalParameter("instFamily", instFamily);
        parameters.AddOptionalParameter("instId", instrumentId);
        parameters.AddOptionalParameter("ordId", orderId?.ToOkxString());
        parameters.AddOptionalParameter("after", after?.ToOkxString());
        parameters.AddOptionalParameter("before", before?.ToOkxString());
        parameters.AddOptionalParameter("begin", before?.ToOkxString());
        parameters.AddOptionalParameter("end", before?.ToOkxString());
        parameters.AddOptionalParameter("limit", limit.ToOkxString());

        return await SendOKXRequest<IEnumerable<OkxTransaction>>(GetUri(v5TradeFills), HttpMethod.Get, ct, signed: true, queryParameters: parameters).ConfigureAwait(false);
    }
forReason commented 1 year ago

This would be the corrected code (untested):

public virtual async Task<RestCallResult<IEnumerable<OkxTransaction>>> GetTransactionHistoryAsync(
        OkxInstrumentType? instrumentType = null,
        string instrumentId = null,
        string instFamily = null,
        string underlying = null,
        long? orderId = null,
        long? after = null,
        long? before = null,
        long? begin = null,
        long? end = null,
        int limit = 100,
        CancellationToken ct = default)
    {
        limit.ValidateIntBetween(nameof(limit), 1, 100);
        var parameters = new Dictionary<string, object>();
        parameters.AddOptionalParameter("instType", JsonConvert.SerializeObject(instrumentType, new InstrumentTypeConverter(false)));
        parameters.AddOptionalParameter("uly", underlying);
        parameters.AddOptionalParameter("instFamily", instFamily);
        parameters.AddOptionalParameter("instId", instrumentId);
        parameters.AddOptionalParameter("ordId", orderId?.ToOkxString());
        parameters.AddOptionalParameter("after", after?.ToOkxString());
        parameters.AddOptionalParameter("before", before?.ToOkxString());
        parameters.AddOptionalParameter("begin", begin?.ToOkxString()); // Use 'begin' variable for the 'begin' parameter
        parameters.AddOptionalParameter("end", end?.ToOkxString()); // Use 'end' variable for the 'end' parameter
        parameters.AddOptionalParameter("limit", limit.ToOkxString());

        return await SendOKXRequest<IEnumerable<OkxTransaction>>(GetUri(v5TradeFills), HttpMethod.Get, ct, signed: true, queryParameters: parameters).ConfigureAwait(false);
    }
forReason commented 1 year ago

created corresponding pull ruquest: Update OKXTradeRestApiClient.cs #23

burakoner commented 1 year ago

Thank you for your feedback and commit.

burakoner commented 1 year ago

Version 1.2.0 released