1030/reverted trades #1087
1030/reverted trades #1087
Conversation
alfetopito
force-pushed
the
1030/reverted-trades
branch
from
879fe3f to
f2f557d
Compare
|
Travis automatic deployment: |
alfetopito
force-pushed
the
1030/reverted-trades
branch
from
32d8854 to
1bc9014
Compare
There was a problem hiding this comment.
Just the concern about how the trades are matched that we discussed in the batch, so you make sure for same batch/user/orderId you check the block number and log index to decide wich trade was reverted
Other than that, just minor things addressed in the comments.
alfetopito
force-pushed
the
1030/reverted-trades
branch
from
4f0bfd0 to
758adb2
Compare
alfetopito
force-pushed
the
1030/reverted-trades
branch
from
fad42a0 to
1205bdc
Compare
(there might be trade reverts without trade events on a block)
And accounting for events happening on the same block
matching reverts only to them + new trades
|
|
||
| // Filter out trades in that range (curr ... curr -2). | ||
| // The `revertKey` is composed by batchId|orderId, so this regex looks for the batchIds in the keys | ||
| const batchesRegex = new RegExp(`^(${currentBatchId}|${currentBatchId - 1}|${currentBatchId - 2})\|`) |
There was a problem hiding this comment.
feels a bit strange this way of filtering
also, why you want pending trades from last 2 batches?
There was a problem hiding this comment.
feels a bit strange this way of filtering
I could instead go over the list of trades and look for the batches instead but...
...there is already a list of batchId|orderId.
Happy to change if you have a better suggestion.
also, why you want pending trades from last 2 batches?
To be "safe(r)". Would you like to live dangerously and reduce it to 1?
Closes #1030
Update Jun 08
Removing reverted trades.
Todo:
getTradesservice return