Skip to content

mempool: make max_tx_bytes configurable instead of max_msg_bytes#3877

Merged
melekes merged 4 commits intotendermint:masterfrom
bluele:fix/mempool-config
Aug 5, 2019
Merged

mempool: make max_tx_bytes configurable instead of max_msg_bytes#3877
melekes merged 4 commits intotendermint:masterfrom
bluele:fix/mempool-config

Conversation

@bluele
Copy link
Contributor

@bluele bluele commented Aug 3, 2019

Fix #3868 (comment)

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@bluele bluele requested review from ebuchman, melekes and xla as code owners August 3, 2019 02:19
@codecov-io
Copy link

codecov-io commented Aug 3, 2019

Codecov Report

Merging #3877 into master will increase coverage by 0.08%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #3877      +/-   ##
==========================================
+ Coverage      66%   66.08%   +0.08%     
==========================================
  Files         234      234              
  Lines       20280    20240      -40     
==========================================
- Hits        13385    13375      -10     
+ Misses       5845     5824      -21     
+ Partials     1050     1041       -9
Impacted Files Coverage Δ
config/toml.go 65.95% <ø> (ø) ⬆️
mempool/clist_mempool.go 81.78% <100%> (ø) ⬆️
config/config.go 66.46% <33.33%> (ø) ⬆️
mempool/reactor.go 79.5% <60%> (+0.16%) ⬆️
crypto/random.go 50% <0%> (-30%) ⬇️
consensus/ticker.go 91.66% <0%> (-4.17%) ⬇️
privval/signer_remote.go 80% <0%> (-2%) ⬇️
blockchain/v0/reactor.go 71.22% <0%> (-1.89%) ⬇️
store/store.go 94.28% <0%> (-0.21%) ⬇️
blockchain/v0/pool.go 81.31% <0%> (+0.65%) ⬆️
... and 4 more

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 Cool, thanks

config/toml.go Outdated

# Limit the size of TxMessage
max_msg_bytes = {{ .Mempool.MaxMsgBytes }}
# Limit the size of Tx
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Limit the size of Tx
# Maximum size of a single transaction.

config/toml.go Outdated
# Limit the size of TxMessage
max_msg_bytes = {{ .Mempool.MaxMsgBytes }}
# Limit the size of Tx
# NOTE: the limit size of TxMessage is this value plus amino overhead(8byte)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# NOTE: the limit size of TxMessage is this value plus amino overhead(8byte)
# NOTE: the max size of a tx transmitted over the network is {max_tx_bytes} + {amino overhead}.

@melekes melekes added the T:breaking Type: Breaking Change label Aug 5, 2019
@bluele
Copy link
Contributor Author

bluele commented Aug 5, 2019

Thanks for your review.
I have pushed fixes.

@melekes melekes merged commit e179787 into tendermint:master Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:breaking Type: Breaking Change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants