fatxpool: retracted transaction shall be prioritorized over the mempool

[michalkucharczyk]

Currently the transactions from retracted block are submitted to the block, and later (if view is at its limits) could be dropped by the transaction from the mempool.

It should be guaranteed that retracted transactions are not dropped and are prioritized over the txs from the mempool.

Retracted transactions shall also be added to the mempool (or other buffer keeping retracted transactions). Adding to mempool may require adding new flag.

[michalkucharczyk]

There are some interesting questions to be answered before proceeding with implementation.

The most important is: should we prioritize txs from the retracted blocks over txs in the mempool if all mempool transactions have higher priority and we are at the limits? I think in this case we shall also respect priority - so higher prio should go into the block first.

If we want extra buffer for retracted transaction - how big it should be?

Is there any attack scenario that could be performed by bad validator producing forks in order to get its transactions first?

[bkchr]

I don't get this issue.

so higher prio should go into the block first.

This should always be the case. This is the reason we have priorities. We can not enforce these priorities, but reasonable implementations should follow them.

If we want extra buffer for retracted transaction - how big it should be?

If we have set reconciliation for transactions, likelihood of this should be small?

[michalkucharczyk]

I don't get this issue.

This is to increase reliability - to ensure that once transaction was included in the block on one fork, it will also be included on other forks.

If we have set reconciliation for transactions, likelihood of this should be small?

That is true. It could happen only if some transaction were included on one fork by some collator but for some reason did not land in other collators' pools.

When we are under the pool limits, everything should work as expected. Transactions will be included on all forks. This is already supported.

But when transaction pool is operating at pool limits, some transactions from retracted blocks may not get into the blocks on the other forks. The reason for this is because we do not send retracted transactions back into the mempool. We only submit them to the view. Later we "top up" view with mempool's txs. When internal mempool is full or view is as its limits, then mempool txs can overflow view, limits will be enforced on the view and retracted txs can get dropped if the priority is too low.

I think the best solution would be to push retracted transaction also to the mempool, and treat them as usual transactions - if their priority is too low, they should simply be dropped.

Any other mechanism (e.g. additional buffer) is - in my opinion - unnecessary complication of the internal pool flows.

[bkchr]

When internal mempool is full or view is as its limits, then mempool txs can overflow view, limits will be enforced on the view and retracted txs can get dropped if the priority is too low.

Ahh, yeah I was not aware that we don't forward them to the mempool. Kind of a weird behavior, because I would think that if something is in a view, it also exists in the mempool. As the views are just "views into the mempool".

[michalkucharczyk]

Ahh, yeah I was not aware that we don't forward them to the mempool. Kind of a weird behavior, because I would think that if something is in a view, it also exists in the mempool. As the views are just "views into the mempool".

Yes that makes sense - it shall be fixed in this issue.

To give some background: Initially I skipped that because some transactions in imported blocks could be marked as no-broadcast, and I did not want to add them to the pool. Now I understand the system better, and it makes 100% sense to add them to the pool.