PlaceOrderAsync fails with clientOrderId supplied

[code-hatchery]

Using PlaceOrderAsync I supply a clientOrderId parameter of 32 characters but response gives me an error for this parameter.

When I check the actual request I see you've changed my clientOrderId putting the value you generate for Tag parameter at the beginning so now this value is too long, so I get the error.

https://github.com/JKorf/OKX.Net/blob/54c04284d36992cacffdbe8f6fb65ec1870a358e/OKX.Net/Clients/UnifiedApi/OKXRestClientUnifiedApiTrading.cs#L73C83-L73C83

So the problems I can see are:

1. If I supply a clientOrderId it should not be changed for any reason.
2. It should be an optional parameter, not mandatory and not sent if I don't supply a value.
3. Tag should be an optional parameter that I can pass.
4. A few other places in OKXRestClientUnifiedApiTrading.cs you are doing the same issue.

[prodigy]

I just noticed the same thing and am wondering as to why this is done. Can you elaborate on the reasons? I want to send a Guid which happens to be exactly 32 characters long. I guess that is the reason why the field supports values of 32 characters length. Adding this _ref field, breaks the ability to send Guids.

[JKorf]

Yes, I can explain. This is done to pass in a reference ID. Depending on the exchange it's possible to receive some rebates on the trading fee paid by users of the library. This is to no detriment of the user, so not an issue imo.

However, in this case I can agree that it's an issue as it impacts the library usage, because OKX requires the reference id to be included in the clientOrderId and tag parameters in the order requests. I'll agree with you that the current implementation isn't optimal. So instead I'm thinking about not sending the reference id when the user specifies a client order id, and only send it when the user isn't using the functionality, so the user isn't impacted.

Let me know your thoughts and/or suggestions

[prodigy]

Thank you for explaining. After looking through the code I noticed setting BrokerId would result in that value being prefixed instead of the hardcoded _ref. It makes total sense for you to collect parts of the trading fees and I think it is a great way of making this library worth your while and I totally support it.

In the case of a clientOrderId being set, and it being longer than 32 characters combined with your ref, the clientOrderId should only be the value passed in.

For the cases where the Id would be shorter than 32 characters with your ref included, how about continuing to prefix it. But everywhere where you parse incoming order updates, the ref should be stripped. This way I can actually check on the value I set as clientOrderId and don't have to manually remove the ref sent? Any thoughts?

[JKorf]

I've adjusted the logic to only update the clientOrderId with the brokerId when the clientOrderId isn't passed in by the user, which is the most conservative option I think.