feat: parallelise intra-block processing #137
Conversation
|
|
||
| export const pollingByOwnerDurationSeconds = new client.Histogram({ | ||
| name: "watch_tower_polling_by_owner_duration_seconds", | ||
| help: "Duration of polling run", |
There was a problem hiding this comment.
same tests as the one above
| const log = getLogger( | ||
| "checkForAndPlaceOrder:checkForAndPlaceOrder", | ||
| chainId.toString(), | ||
| blockNumber.toString(), | ||
| ownerRef | ||
| (ownerCounter++).toString() |
There was a problem hiding this comment.
it won't give us the same order between blocks now (unless we use value from the map argument)
you can use the index of map, and rename the var to ownerCounter
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map
|
|
||
| return { | ||
| order, | ||
| _delete: true, |
There was a problem hiding this comment.
this is not pretty. Do you see a chance of breaking down the logic a bit:
const orderPromises = Array.from(conditionalOrders.values()).map(breakItIntoFunctions)
This way you can also define better types for what these functions should return.
This type looks not too nice:
Also, could you give a name like the others props for _delete. Think what you model here, in this case is that we should not keep polling this order for future blocks, then you can pick a name that reflects that.
doNotCheckAgain, dropOrderForever, getTheFu**OutOfHere
| continue; | ||
| } | ||
| // Get all the results and filter out the undefined ones | ||
| const results = (await Promise.all(orderPromises)).filter((r) => !!r); |
| "Skipping conditional order. Reason: AcceptPolicy: SKIP" | ||
| ); | ||
|
|
||
| return; |
There was a problem hiding this comment.
It might not be obvious why you return undefined
You could model it in the data this return, or if you really want to return undefined, to make it more readable you can do:
type SkipOrder = undefined;
const SKIP_ORDER: SkipOrder = undefined;
case FilterAction.SKIP:
log.debug(
"Skipping conditional order. Reason: AcceptPolicy: SKIP"
);
return SKIP_ORDER;| // Process all the orders | ||
| for (const result of results) { | ||
| if (!result) { | ||
| throw new Error("Unexpected error: orderResult is undefined"); |
There was a problem hiding this comment.
you filtered before, afait you just have a type issue.
what you might be looking for is https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards
| const id = ConditionalOrderSDK.leafToId(order.params); | ||
|
|
||
| // Search for the order in the registry and update / delete it | ||
| for (const o of Array.from(conditionalOrders.values())) { |
There was a problem hiding this comment.
can we not use names like o
| const id = ConditionalOrderSDK.leafToId(order.params); | ||
|
|
||
| // Search for the order in the registry and update / delete it | ||
| for (const o of Array.from(conditionalOrders.values())) { |
There was a problem hiding this comment.
you made this Array.from(conditionalOrders.values()) a few times in the code
| if (conditionalOrders === undefined && results.length > 0) { | ||
| throw new Error( | ||
| "Unexpected error: conditionalOrders is undefined but results is not empty" | ||
| ); |
There was a problem hiding this comment.
Same comment as for line 295
|
|
||
| // Search for the order in the registry and update / delete it | ||
| for (const o of Array.from(conditionalOrders.values())) { | ||
| if (ConditionalOrderSDK.leafToId(o.params) === id) { |
There was a problem hiding this comment.
Arent you calculating the leafToId for all conditionalOrders.values() orders for each result you have?
it looks like you repeat the work over and over.
Isn't it better to build a Map "ID --> Order"?
Description
In order to increase performance of the watch-tower, multiple conditional orders can be polled simultaneously in order to reduce per-block processing times.
Changes
How to test
mainnet,xdai, orgoerli)/api/dump/<chainId>endpointRelated Issues
Fixes #128