config/mempool: define default parameters

[Bidon15]

Introduction ✌🏻

Here is the current params we have for mempool part

# Mempool version to use:
#   1) "v0" - (default) FIFO mempool.
#   2) "v1" - prioritized mempool.
version = "v0"

recheck = true
broadcast = true
wal_dir = ""

# Maximum number of transactions in the mempool
size = 5000

# Limit the total size of all txs in the mempool.
# This only accounts for raw transactions (e.g. given 1MB transactions and
# max_txs_bytes=5MB, mempool will only accept 5 transactions).
max_txs_bytes = 1073741824

# Size of the cache (used to filter transactions we saw earlier) in transactions
cache_size = 10000

# Do not remove invalid transactions from the cache (default: false)
# Set to true if it's not possible for any invalid transaction to become valid
# again in the future.
keep-invalid-txs-in-cache = true

# Maximum size of a single transaction.
# NOTE: the max size of a tx transmitted over the network is {max_tx_bytes}.
max_tx_bytes = 1048576

# Maximum size of a batch of transactions to send to a peer
# Including space needed by encoding (one varint per transaction).
# XXX: Unused due to https://github.com/tendermint/tendermint/issues/5796
max_batch_bytes = 0

# ttl-duration, if non-zero, defines the maximum amount of time a transaction
# can exist for in the mempool.
#
# Note, if ttl-num-blocks is also defined, a transaction will be removed if it
# has existed in the mempool at least ttl-num-blocks number of blocks or if it's
# insertion time into the mempool is beyond ttl-duration.
ttl-duration = "0s"

# ttl-num-blocks, if non-zero, defines the maximum number of blocks a transaction
# can exist for in the mempool.
#
# Note, if ttl-duration is also defined, a transaction will be removed if it
# has existed in the mempool at least ttl-num-blocks number of blocks or if
# it's insertion time into the mempool is beyond ttl-duration.
ttl-num-blocks = 15

New Definition 📜

• Are we using as default prioritized mempool or FIFO?
  version = "v0"
• Here I don't understand why we have 1024 MB for max_txs_bytes ?
  • Why do we need such figure considering that we won't have 1GB blocks out of the gate?
• Are we fine with ttl-* figures?

Notes 📝

Ref: #585

[rootulp]

Questions

Are we using as default prioritized mempool or FIFO?

ADR-067 introduced a priority mempool but there was a memory issue that was addressed in tendermint/tendermint#8944 but hasn't been backported to v0.34.x so I don't think we should enable it while on Tendermint v0.34. When we upgrade to Tendermint v0.36 I think we can enable it.

Here I don't understand why we have 1024 MB for max_txs_bytes ?

• Why do we need such figure considering that we won't have 1GB blocks out of the gate?

These are the default values. Are these values causing issues @Bidon15 ?

Are we fine with ttl-* figures?

ttl-duration = "0s"
ttl-num-blocks = 15

• ttl-duration: matches the default 0.
• ttl-num-blocks: was set to 15 in this commit. Given the default is 0 I'm curious what the motivation was behind this value? Tagging @sweexordious if he has any insight here.

Links

• Tendermint MempoolConfig

Parameters

Parameter	Description	Default	Arabica	Mainnet Proposed
version	Mempool version to use. 1) "v0" - (default) FIFO mempool. 2) "v1" - prioritized mempool.	"v0"	"v0"	"v1"
home	RootDir is the root directory for all data. This should be configured via the $TMHOME env variable or --home cmd flag rather than overriding this struct field.
recheck	Recheck (default: true) defines whether Tendermint should recheck the validity for all remaining transaction in the mempool after a block. Since a block affects the application state, some transactions in the mempool may become invalid. If this does not apply to your application, you can disable rechecking	true	true	true
broadcast	Broadcast (default: true) defines whether the mempool should relay transactions to other peers. Setting this to false will stop the mempool from relaying transactions to other peers until they are included in a block. In other words, if Broadcast is disabled, only the peer you send the tx to will see it until it is included in a block.	true	true	true
wal_dir	WalPath (default: "") configures the location of the Write Ahead Log (WAL) for the mempool. The WAL is disabled by default. To enable, set WalPath to where you want the WAL to be written (e.g. "data/mempool.wal").	""	""	""
size	Maximum number of transactions in the mempool	5000	5000	5000
max_txs_bytes	Limit the total size of all txs in the mempool. This only accounts for raw transactions	1024 * 1024 * 1024 // 1GB	1024 * 1024 * 1024 // 1GB	1024 * 1024 * 1024 // 1GB
cache_size	Size of the cache (used to filter transactions we saw earlier) in transactions	10000	10000	10000
keep-invalid-txs-in-cache	Do not remove invalid transactions from the cache	false	true	false
max_tx_bytes	Maximum size of a single transaction	1024 * 1024 // 1MB	1024 * 1024 // 1MB	2 * 1024 * 1024 // 2 MiB
max_batch_bytes	Unused due to tendermint/tendermint#5796		0	0
ttl-duration	TTLDuration, if non-zero, defines the maximum amount of time a transaction can exist for in the mempool	0 * time.Second	"0s"	0
ttl-num-blocks	TTLNumBlocks, if non-zero, defines the maximum number of blocks a transaction can exist for in the mempool	0	15	10

[evan-forbes]

ttl-num-blocks: was set to 15 in this commit.

it was in celestiaorg/cosmos-sdk@b57bc39, when mamaki was released, the default was not to sign a pfd with the correct square sizes. This could result in a pfd getting stuck in the mempool. If a tx gets stuck in the mempool, then its difficult for a user to submit another tx, as the checktx state will tell them that the nonce they're using is incorrect.

imo, we can revert this back to the default.

[evan-forbes]

ADR-067 introduced a priority mempool but there was a tendermint/tendermint#8775 that was addressed in tendermint/tendermint#8944 but hasn't been backported to v0.34.x so I don't think we should enable it while on Tendermint v0.34. When we upgrade to Tendermint v0.36 I think we can enable it.

I'm assuming tendermint v0.37, as v0.36 was deprecated?

I thought the memory leak was fixed here?
tendermint/tendermint#8962 (comment)

[rootulp]

imo, we can revert this back to the default.

Sounds good, will do!

I'm assuming tendermint v0.37, as v0.36 was deprecated?

Agreed based on https://github.com/tendermint/tendermint/releases

I thought the memory leak was fixed here?

🤦 I'm not sure how I missed that when scanning the v0.34.20 changelog. I was searching for the PR #8944 and somehow failed to see the bug fix for priority mempool.

[Bidon15]

These are the default values. Are these values causing issues @Bidon15 ?

For testing purposes, to get 4MB blocks, you need 4 1mb txs to get it. I wonder why we don't allow 4MB from the get-go? Considering that we have 1Gb total

[rootulp]

For testing purposes, to get 4MB blocks, you need 4 1mb txs to get it. I wonder why we don't allow 4MB from the get-go?

If I understand correctly, you're proposing increasing max_tx_bytes from the default 1 MiB to 4 MiB so that a block can be filled with just one transaction. Given the share size was recently increased to 512 bytes, the maximum block size is now 8 MiB. If the desire still exists, we should consider increasing max_tx_bytes to 8 MiB. I'll investigate if there are any risks associated with this increase.

[evan-forbes]

relevant celestiaorg/celestia-core#867

[rootulp]

With respect to the priority mempool, we should monitor tendermint/tendermint#9388. We likely don't want to enable it considering it seems on a path to deprecation.

[Bidon15]

If I understand correctly, you're proposing increasing max_tx_bytes from the default 1 MiB to 4 MiB so that a block can be filled with just one transaction.

Yes

Given the share size was recently increased to 512 bytes, the maximum block size is now 8 MiB. If the desire still exists, we should consider increasing max_tx_bytes to 8 MiB. I'll investigate if there are any risks associated with this increase.

This is not a burning question, so 4Mb(or 8Mb) in 1 tx is more of a feature request from testing 👍

[rootulp]

I'm a little stuck on gathering data for the mempool's max_tx_bytes value. I reproed a tx too large error for the integration test in this commit and was able to mitigate by increasing the max_tx_bytes in the integration test's network (i.e. this commit) but this doesn't give us signal on what's safe to use for max_tx_bytes by default.

I think we would need to run a mock network with multiple validators peering to each other and see what the impact is to validator performance if we start spamming transactions that are 8 MiB. @Bidon15 are you able to help me set something like this up?

[evan-forbes]

yes, agreed, having more confidence certainly requires more information and analysis. Any value we pick is essentially going blind, as no one else using this mempool has txs this large afaik.

We do not know the actual effect on bandwidth usage for individual nodes or the network as a whole. We should prioritize obtaining that info, and in the mean time, increase the default limit if users require it. We should be able to get good starting data on this by monitoring the bandwidth used by each node after submitting transactions.

I don't think we should automatically increase it to the entire square. Probably just 2MB or something until we have more confidence by performing more testing

[Bidon15]

@Bidon15 are you able to help me set something like this up?

Sure. Let's discuss how we can make it as the foundational work is mostly done in tg already, where we spam like total 4MB blocks

[rootulp]

Update from synchronous params discussion during onsite: we want to use the prioritized mempool version: "v1" so that we have a mechanism to drop the lowest fee transactions from the mempool. Separately, celestia-app should leverage ABCI++ to drop the lowest fee transactions that can't be included in a block

[evan-forbes]

We should also set some default value for ttl-num-blocks as per celestiaorg/celestia-core#812

[rootulp]

I updated the table above.