[TS] [CLOB-895] short term place orders for TS #39
Conversation
| @@ -1,7 +1,9 @@ | |||
| import { Order_TimeInForce } from '@dydxprotocol/v4-proto/src/codegen/dydxprotocol/clob/order'; | |||
|
|
|||
There was a problem hiding this comment.
Instead of modifying composite_examples.ts, can you copy it to a separate composite_short_term_examples.ts with the new code? We still need this composite_examples.ts to test regular placeOrder for FE.
| @@ -23,25 +25,24 @@ async function test(): Promise<void> { | |||
| const subaccount = new Subaccount(wallet, 0); | |||
| for (const orderParams of ordersParams) { | |||
There was a problem hiding this comment.
ordersParams come from human_readable_orders.json, which is designed to include all combinations for our FE. For testing short_term orders, let's create a new short_term_orders.json file for all the test cases. I think you can eliminate a bunch of them and make the test quicker.
| @@ -1,7 +1,9 @@ | |||
| import { Order_TimeInForce } from '@dydxprotocol/v4-proto/src/codegen/dydxprotocol/clob/order'; | |||
There was a problem hiding this comment.
Is this the correct Order_TIF we want to be using as a a parameter? Note that I took out everything related to OrderExecution too
There was a problem hiding this comment.
Added a translation layer in example to translate orderExecution string to Order_TimeInForce
jonfung-dydx
changed the title
| subticks, | ||
| timeInForce, | ||
| reduceOnly, | ||
| 0, // Client metadata is 0 for short term orders. |
There was a problem hiding this comment.
Note: client metadata can be non-zero for short-term and stateful orders
| } | ||
| } | ||
|
|
||
| function orderExecutionToTimeInForce(orderExecution: string): Order_TimeInForce { |
There was a problem hiding this comment.
Suggestion: do you think this function should be defined within v4-client-js/src/clients/composite-client.ts and called within placeShortTermOrder?
There was a problem hiding this comment.
I guess I'm fine with you leaving it here if it's meant to translate from a JSON order
There was a problem hiding this comment.
this is only meant to translate from the JSON example file, so will keep it in here
Equivalent of #32 for TS. LMK If I missed anything.