feat(config): new default value for mempool.max_txs_bytes parameter
#2767
Conversation
Looks good. |
|
I'd say that 64 Gib is a low number but this is just a guess. If it happens that a few big txs are submitted at the same time, that would take up a big chunk of the mempool capacity and leave a reduced space for the regular-sized txs. Cosmwasm smart contracts could take a couple of Mbs (see #1518 (comment)). |
In this case, they have to change the default Notice that this is not a consensus parameter, is something pretty easy to tune. The real point here is whether it makes sense to allow 1GB transactions by default with block size of 4MB (default again). It would take 256 rounds to drain all transactions from the mempool. |
Good point, I will fix it. But should we maintain this document (as well)? Another source to manually update each time we change a config param... |
I would probably remove it, yeah. Now that we have |
|
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
…#2767) Closes #2756. The proposed value is 64 MiB = 16 x default `block.max_bytes` (consensus parameter). --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Andy Nogueira <me@andynogueira.dev> (cherry picked from commit 732ca9a)
… (backport #2767) (#2770) Closes #2756. The proposed value is 64 MiB = 16 x default `block.max_bytes` (consensus parameter). --- #### PR checklist - [ ] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request #2767 done by [Mergify](https://mergify.com). Co-authored-by: Daniel <daniel.cason@informal.systems>
Upstream changed the default to a less ludicrous number, 1GB was way too many bytes. cometbft/cometbft#2767
## Describe your changes Lower the default value for `max_txs_bytes` from 1GiB to 10MiB. Upstream also substantially lowered this default recently. cometbft/cometbft#2767 See also the [new `nop` mempool type](cometbft/cometbft#1643) they added to deal with the mempool being a potential DoS vector a la [p2p-storms](https://github.com/notional-labs/placid). ## Issue ticket number and link ## Checklist before requesting a review - [X] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: The mempool and its configuration are outside consensus. --------- Signed-off-by: Erwan Or <erwan.ounn.84@gmail.com> Co-authored-by: Erwan Or <erwan.ounn.84@gmail.com> Co-authored-by: Erwan Or <erwanor@penumbralabs.xyz>
The default value was reduced upstream recently: cometbft/cometbft#2767
Closes #2756.
The proposed value is 64 MiB = 16 x default
block.max_bytes(consensus parameter).PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments