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

config: reduce default mempool size #2300

Merged
merged 2 commits into from
Aug 30, 2018
Merged

Conversation

ValarDragon
Copy link
Contributor

This reduces the mempool size from 100k to 5k. Note that each secp256k1 sig
takes .5ms to compute. Therefore an adversary could previously delay every
node on the network's computation time upon receiving a block by 50 seconds.

This now reduces that ability to being able to only delay each node by 2.5
seconds. This change should be reverted once ABCI recheck is implemented.

ref #2127

I think this should be included next release, to improve network resiliency.

  • Updated CHANGELOG_PENDING.md

This reduces the mempool size from 100k to 5k. Note that each secp256k1 sig
takes .5ms to compute. Therefore an adversary could previously delay every
node on the network's computation time upon receiving a block by 50 seconds.

This now reduces that ability to being able to only delay each node by 2.5
seconds. This change should be reverted once ABCI recheck is implemented.
// Each signature verification takes .5ms, size reduced until we implement
// ABCI Recheck
Size: 5000,
CacheSize: 10000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why reduce cache size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed kinda weird to have a 20x ratio between Size and CacheSize, though I guess there isn't a need to reduce it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about the ratio. It's about memory (total cache size) and being the first layer of protection from replay

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think more limited defaults is fine.

@ebuchman
Copy link
Contributor

Broke a mempol test though ;)

panic: Error after CheckTx: Mempool is full

goroutine 620 [running]:
github.com/tendermint/tendermint/consensus.deliverTxsRange(0xc4201cdb00, 0x0, 0x2710)
	/go/src/github.com/tendermint/tendermint/consensus/mempool_test.go:91 +0x310
created by github.com/tendermint/tendermint/consensus.TestMempoolTxConcurrentWithCommit
	/go/src/github.com/tendermint/tendermint/consensus/mempool_test.go:103 +0x291

@ebuchman ebuchman merged commit 61ab10d into develop Aug 30, 2018
@ebuchman ebuchman deleted the dev/reduce_mempool_size branch August 30, 2018 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants