Provide interface to send multiple Msg per transaction [do not merge] #134
Conversation
johnqh
requested review from
rosepuppy,
aforaleka,
jaredvu,
moo-onthelawn,
ruixhuang,
mike-dydx and
prashanDYDX
…ction # Conflicts: # v4-client-js/__native__/__ios__/v4-native-client.js # v4-client-js/package.json # v4-client-js/src/clients/modules/post.ts
v4-client-js/src/clients/types.ts
Outdated
There was a problem hiding this comment.
It is more accurate to keep the enum values in constants instead of types.ts.
There was a problem hiding this comment.
There were some circular reference error.
| await this.validateGoodTilBlock(goodTilBlock); | ||
|
|
||
| const marketsResponse = await this.indexerClient.markets.getPerpetualMarkets(marketId); |
There was a problem hiding this comment.
Can we add another parameter to placeShortTermOrderMsgs so that these Indexer GET requests can be optional?
| private defaultMsgBroadcastMode(msg: EncodeObject): BroadcastMode { | ||
| if (this.isShortTermOrderMsg(msg)) { | ||
| return Method.BroadcastTxSync; | ||
| } else { | ||
| return Method.BroadcastTxCommit; |
There was a problem hiding this comment.
can shorten to return this.isShortTermOrderMsg(msg) ? Method.BroadcastTxSync : Method.BroadcastTxCommit;
There was a problem hiding this comment.
similar to what you have in defaultBroadcastMode
| if (msg.typeUrl === '/dydxprotocol.clob.MsgPlaceOrder' || | ||
| msg.typeUrl === '/dydxprotocol.clob.MsgCancelOrder') { | ||
| const orderFlags = msg.typeUrl === '/dydxprotocol.clob.MsgPlaceOrder' | ||
| ? (msg.value as MsgPlaceOrder).order?.orderId?.orderFlags | ||
| : (msg.value as MsgCancelOrder).orderId?.orderFlags; | ||
|
|
||
| return orderFlags === OrderFlags.SHORT_TERM; | ||
| } else { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @description Whether the msg is a short term placeOrder or cancelOrder msg. | ||
| */ | ||
| private isOrderMsg(msg: EncodeObject): boolean { | ||
| return (msg.typeUrl === '/dydxprotocol.clob.MsgPlaceOrder' || | ||
| msg.typeUrl === '/dydxprotocol.clob.MsgCancelOrder'); | ||
| } |
There was a problem hiding this comment.
All dYdX specific typeUrls are included in src/clients/constants.ts. Should use those constants instead of hard coded strings here.
There was a problem hiding this comment.
Didn't know that. Changed
| }); | ||
| } | ||
|
|
||
| async msgs( |
johnqh
changed the title
|
why was this never merged? should we bother trying to merge this or can we close this? @jaredvu |
|
Based on the above thread, I think the only case this could help today is with Transfer+LongTermOrder. Timestamp nonces should get us most of the way there and is a more general solution. Think we can just close this for now. |
After #132
Examples of multiple msgs:
Close all positions: [placeOrder, placeOrder...]
Cancel all orders: [cancelOrder, cancelOrder...]
Isolated margin order: [transfer, placeOrder] (maybe [transfer, withdraw, placeOrder] to make sure wallet balance is above certain level
SL/TP [placeOrder, placeOrder]
Changes:
For native, it should take a list of transactions from Abacus (Jared is working on the interface), and encode it to a json to send to transaction() function. It should have a payload like:
This PR has a rough implementation of transaction(...) function for native app integration. The payload may still change depending on the changes Jared is making with DYDXChainTransactionsProtocol
More importantly, this PR should not break existing code. I will need everyone's help to review/test and make sure it works with existing web and native apps