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

is okx thread safe? #4779

Open
douggie opened this issue Oct 15, 2023 · 2 comments
Open

is okx thread safe? #4779

douggie opened this issue Oct 15, 2023 · 2 comments

Comments

@douggie
Copy link
Collaborator

douggie commented Oct 15, 2023

I noticed a lot of okx requests get rejected with invaild sign when submitting multiple requests at same time.

{"instId":"BTC-USDT","ordId":"633717666398052352","clOrderId":null}
2023-10-15 09:27:46.33 [pool-18-thread-7] TRACE si.mazi.rescu.HttpTemplate - Request headers = {Accept=application/json, OK-ACCESS-KEY=abc key, OK-ACCESS-TIMESTAMP=2023-10-15T09:27:46.332Z, OK-ACCESS-SIGN=qr7/wIi21sfltMSzysANBSwoaqBOCM/XFrWhR8kk+Wc=, OK-ACCESS-PASSPHRASE=abc phrase, Content-Type=application/json}```

```2023-10-15 09:27:46.33 [pool-18-thread-14] DEBUG si.mazi.rescu.HttpTemplate - Executing POST request at https://www.okx.com/api/v5/trade/cancel-order  body
{"instId":"BTC-USDT","ordId":"633717666570018817","clOrderId":null}
2023-10-15 09:27:46.33 [pool-18-thread-14] TRACE si.mazi.rescu.HttpTemplate - Request headers = {Accept=application/json, OK-ACCESS-KEY=abc key, OK-ACCESS-TIMESTAMP=2023-10-15T09:27:46.332Z, OK-ACCESS-SIGN=LTCY6+x9eKDUlajK6evemhNCl62YbxG/q6zy1ml2q6w=, OK-ACCESS-PASSPHRASE=abc phrase, Content-Type=application/json}

Such issues don't happen on binace xchange implementation, they both seem to use the same decorator pattern, so wondering if there is something else here is not thread safe. Pointers welcomed on where the threading issue might be! Making the public String digestParams(RestInvocation restInvocation) synchronised seems a bit overkill public synchronized String digestParams(RestInvocation restInvocation)

Binance

      Instrument pair, Long orderId, String origClientOrderId, String newClientOrderId,  Boolean isMarginOrder)
      throws IOException, BinanceException {
    return decorateApiCall(
            () ->
                    (pair instanceof FuturesContract)
            ? binanceFutures.cancelFutureOrder(
                            BinanceAdapters.toSymbol(pair),
                            orderId,
                            origClientOrderId,
                            getRecvWindow(),
                            getTimestampFactory(),
                            super.apiKey,
                            super.signatureCreator
                    ) :
                        isMarginOrder ?
                            binance.cancelMarginOrder(
                                BinanceAdapters.toSymbol(pair),
                                Boolean.FALSE,
                                orderId,
                                origClientOrderId,
                                newClientOrderId,
                                getRecvWindow(),
                                getTimestampFactory(),
                                super.apiKey,
                                super.signatureCreator)
            :   binance.cancelOrder(
                    BinanceAdapters.toSymbol(pair),
                    orderId,
                    origClientOrderId,
                    newClientOrderId,
                    getRecvWindow(),
                    getTimestampFactory(),
                    super.apiKey,
                    super.signatureCreator))
        .withRetry(retry("cancelOrder"))
        .withRateLimiter(rateLimiter(REQUEST_WEIGHT_RATE_LIMITER))
        .call();
  }
public OkexResponse<List<OkexOrderResponse>> cancelOkexOrder(OkexCancelOrderRequest order)
      throws IOException {
    try {
      return decorateApiCall(
              () ->
                  okexAuthenticated.cancelOrder(
                      exchange.getExchangeSpecification().getApiKey(),
                      signatureCreator,
                      DateUtils.toUTCISODateString(new Date()),
                      (String)
                          exchange
                              .getExchangeSpecification()
                              .getExchangeSpecificParametersItem(PARAM_PASSPHRASE),
                      (String)
                          exchange
                              .getExchangeSpecification()
                              .getExchangeSpecificParametersItem(PARAM_SIMULATED),
                      order))
          .withRateLimiter(rateLimiter(OkexAuthenticated.cancelOrderPath))
          .call();
    } catch (OkexException e) {
      throw handleError(e);
    }
  }
@douggie
Copy link
Collaborator Author

douggie commented Oct 15, 2023

I wonder if it is to do with the enconder methods not being thread safe. According to java docs they are https://docs.oracle.com/javase/8/docs/api/java/util/Base64.Encoder.html

Binance

Mac mac = getMac();
    mac.update(input.getBytes(StandardCharsets.UTF_8));
    String printBase64Binary = bytesToHex(mac.doFinal());

okx

    Mac mac = getMac();
    mac.update(sb.toString().getBytes());
    return Base64.getEncoder().encodeToString(mac.doFinal());

@douggie
Copy link
Collaborator Author

douggie commented Oct 15, 2023

Think the issue might be with the mac object, looking over javax.crypto.Mac I am not sure it is thread safe, so wonder if we need a new mac each time akin to this in org.knowm.xchange.service.BaseParamsDigest

  public Mac getMac() {
    return mac;
  }

to

  public Mac getMac() {
    try {
      return (Mac) mac.clone();
    } catch (Error | Exception e) {
      throw new IllegalStateException(e);
    }

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

No branches or pull requests

1 participant