Mempool.MaxMsgBytes is in wrong position in config/toml.go

[bluele]

Previously, I fixed a related part at #3826, but during the release process of v 0.32.2, it seems that the degradation occurred due to the wrong commit merge.

tendermint/config/toml.go

Lines 280 to 306 in 4d7cd80

	[mempool]
	recheck = {{ .Mempool.Recheck }}
	broadcast = {{ .Mempool.Broadcast }}
	wal_dir = "{{ js .Mempool.WalPath }}"
	# Maximum number of transactions in the mempool
	size = {{ .Mempool.Size }}
	# 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 = {{ .Mempool.MaxTxsBytes }}
	# Size of the cache (used to filter transactions we saw earlier) in transactions
	cache_size = {{ .Mempool.CacheSize }}
	##### fast sync configuration options #####
	[fastsync]
	# Fast Sync version to use:
	# 1) "v0" (default) - the legacy fast sync implementation
	# 2) "v1" - refactor of v0 version for better testability
	version = "{{ .FastSync.Version }}"
	# Limit the size of TxMessage
	max_msg_bytes = {{ .Mempool.MaxMsgBytes }}

Related commits are here: 4d7cd80#diff-092cdc48047eeb4c0bca311a2e1b8ae6

Tendermint version

v0.32.2

[melekes]

Oops.. thank you for spotting this.

Speaking of MaxMsgBytes, don't you think we should switch to MaxTxBytes? By specifiyng max_msg_bytes in config we are forcing a user to account for something he/she does not know (amino overhead). I believe we should calculate it ourselves and only require users to specify desired max_tx_bytes.

[bluele]

Sure, it might be better.
However, the size limit of the data that mempool.Reactor can handle is max_tx_bytes plus amino overhead implicitly. Don't you mind that? (https://github.com/tendermint/tendermint/blob/master/mempool/reactor.go#L266)
If there is no problem here, I think it is a good decision.

[melekes]

Don't you mind that? (https://github.com/tendermint/tendermint/blob/master/mempool/reactor.go#L266)

Can't we add aminoOverhead (MaxTxBytes + aminoOverhead)?

[bluele]

From my understanding, I think we can :)
Would you like me to create a new PR for that fix?

[melekes]

Go ahead!