Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mempool evicition #81

Closed
4 tasks
liamsi opened this issue Oct 21, 2020 · 4 comments
Closed
4 tasks

Mempool evicition #81

liamsi opened this issue Oct 21, 2020 · 4 comments
Labels
C:abci The connection between ll-core and the (abci) app C:tendermint-modifications Required change to core tendermint components beyond types

Comments

@liamsi
Copy link
Member

liamsi commented Oct 21, 2020

Summary

To be able to evict the mempool depending on the data that was written into the block.

Problem Definition

Currently, tendermint evicts the mempool using the Mempool.Update method:
https://github.com/lazyledger/lazyledger-core/blob/6d99bdda1b59d1b1e5b6c83635ad879bfdffb19e/mempool/mempool.go#L38-L47

This works because in tendermnt the Tx that enter the mempool are the same as the data that gets written into the block.
In LL, this is likely going to be different: the tx that enter the mempool aren't those that end up in the block. Hence identifying Tx by their hash to see if they were included in the block wouldn't work.

Note that this is a general problem that needs to be solved, if tendermint allows for pre-poroccessing / mutating Tx before proposing a block. See: https://github.com/tendermint/tendermint/issues/2639#issuecomment-713026731

Proposal

Let the abci-app return IDs for Tx potenially on CheckTx and/or on deliverTx. These IDs can then be used to evict Tx in the mem-pool.
TODO: add in more details.

ref: tendermint/spec#154


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@liamsi liamsi added C:abci The connection between ll-core and the (abci) app C:tendermint-modifications Required change to core tendermint components beyond types labels Oct 21, 2020
@adlerjohn
Copy link
Member

adlerjohn commented Oct 21, 2020

Here's how I think we can do this.

  1. A blob is accepted into the mempool. Tendermint doesn't interpret the blob. (no change)
  2. CheckTx takes in a blob and returns a list of transactions IDs. Each transaction ID is the hash of the non-witness data. Since each blob in the mempool could correspond to multiple transactions, we need to return a list here. (change)
  3. Blobs in the mempool are links to two IDs:
    1. Hash of the blob in its entirety. This is used at the p2p layer for unconfirmed txs, i.e. before accepting a new tx, check against the blob hash, (unchanged)
    2. List of transactions IDs. This is used for evicting confirmed txs. When receiving a new block, evict any txs from the mempool that are included in the block based on transactions ID.(change)
  4. In order to evict txs that are seen in a block, DeliverTx should return a since transaction ID.

@liamsi
Copy link
Member Author

liamsi commented Oct 26, 2020

related: tendermint/spec#201
and tendermint/tendermint#7917

@liamsi
Copy link
Member Author

liamsi commented Dec 14, 2020

A first, non-invasive approach (at least for the tendermint side) would be to use "re-check" for evicting the mempool:

https://github.com/tendermint/tendermint/blob/f9c54d2710d7a83b2d40fd96f973a0ffba75e740/mempool/clist_mempool.go#L639-L656
and
https://github.com/tendermint/tendermint/blob/f9c54d2710d7a83b2d40fd96f973a0ffba75e740/mempool/clist_mempool.go#L464-L485

This means that the app needs to track the (mempool) Tx, that it processed and "accepted" in the preprocess step.

@liamsi
Copy link
Member Author

liamsi commented Feb 2, 2021

As per the above this isn't something urgent and IMO we can close this for now.

@liamsi liamsi closed this as completed Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:abci The connection between ll-core and the (abci) app C:tendermint-modifications Required change to core tendermint components beyond types
Projects
None yet
Development

No branches or pull requests

2 participants