Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

abci: fill block with app defined txs #201

Closed
tac0turtle opened this issue Oct 26, 2020 · 2 comments
Closed

abci: fill block with app defined txs #201

tac0turtle opened this issue Oct 26, 2020 · 2 comments
Labels
C:abci Component: ABCI S:proposal Status: Proposal

Comments

@tac0turtle
Copy link
Contributor

tac0turtle commented Oct 26, 2020

Protocol Change Proposal

Summary

The mempool decides how to fill blocks, in the Tendermint-Go implementation this is based on a FIFO list. There has been discussion to add a priority field to CheckTX and handle sorting on the Tendermint side (#162).

Problem Definition

With Tendermint handling sorting the Application and Tendermint boundary will begin to blur. This will lead to greater complexity.

Proposal

If tx_id were to be added to transactions and CheckTx (tendermint/tendermint#7917) a new method for the mempool ABCI connection could be created that requests a list of tx_ids from the application and tendermint picks these txs from the mempool, if the app does not care which transactions to include in a block, Tendermint would fallback to the FIFO method.

message RequestTxs {
	height = 1;
}

message ResponseTxs {
	repeated tx_keys = 1;
}

This would negate the need for sorting of the mempool in Tendermint.

Concern

This could lead to duplication of data across the application and Tendermint. A simple way to negate this concern is that the app only keeps knowledge of the key (id) and not the value (data)

@tac0turtle tac0turtle added C:abci Component: ABCI S:proposal Status: Proposal labels Oct 26, 2020
@tac0turtle
Copy link
Contributor Author

tac0turtle commented Nov 16, 2020

A concern brought up by @erikgrinaker is how to handle mempool eviction on a full mempool if the proposal were to moved forward. If the transactions are stored by tx_id Tendermint will not know when it's time to evict transactions. With a priority field in ResponseCheckTx Tendermint can make the assumption that the txs with the lowest priority can be evicted.

This is an implementation detail but should be taken into account when making a decision.

@cmwaters
Copy link
Contributor

I think the PrepareProposal method gives the application sufficient opportunity to add their own transactions. There have also been discussions here around transaction tracing and mempool garbage collection. I'm going to close this issue but feel free to reopen if you think there are extra details of importance here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C:abci Component: ABCI S:proposal Status: Proposal
Projects
None yet
Development

No branches or pull requests

2 participants