Eudico and Mir mempools pipeline #253
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## eudico #253 +/- ##
==========================================
- Coverage 23.94% 23.92% -0.03%
==========================================
Files 608 607 -1
Lines 65726 65701 -25
==========================================
- Hits 15738 15718 -20
+ Misses 47197 47195 -2
+ Partials 2791 2788 -3
|
dnkolegov
changed the title
| } | ||
|
|
||
| receivedRequests := <-submitChan | ||
| mpdsl.RequestTransactionIDs(m, mc.Self, receivedRequests, &requestTxIDsContext{receivedRequests}) |
There was a problem hiding this comment.
Ok I was probably wrong about only having to change this function. I didn't realize that the transaction IDs still must be computed. This implementation only gets the transactions from outside and has their IDs computed, but does not include them in the batch that is emitted as an event. In a strange way it probably works, just delayed, but this is not what I had in mind. What I meant is to directly emit a batch with the transactions received through the channel, without any buffering. For that, there needs to be more change to the module implementation.
There was a problem hiding this comment.
It seems to me this is a feature.
I can delete calling of mpdsl.RequestTransactionIDs and implement hashing without sending an event but it will be also called later in batch reconstruction that is a not part of mempool.
Let's discuss in sync
| tx := state.TxByID[txID] | ||
| for i := range txIDs { | ||
| tx := context.txs[i] | ||
| commonState.TxByID[txIDs[i]] = tx |
There was a problem hiding this comment.
Is there a reason why we still save the transaction in the commonState buffer? It would be nice if this buffer didn't exist at all and the transactions were directly added in the new batch and immediately forgotten.
| tx := state.TxByID[txID] | ||
| for i := range txIDs { | ||
| tx := context.txs[i] | ||
| commonState.TxByID[txIDs[i]] = tx | ||
|
|
||
| // TODO: add other limitations (if any) here. | ||
| if txCount == params.MaxTransactionsInBatch { |
There was a problem hiding this comment.
This check should probably not be enforced here any more. The handler should just take all transactions it receives and put them in a batch. The limit should be enforced when the transactions are fetched from the Eudico mempool.
This PR integrates Eudico and Mir mempools with each other.