follow up to removing some consensus params#2427
Conversation
|
|
||
| // TxFilter returns a function to filter transactions. The function limits the | ||
| // size of a transaction to the maximum block's data size. | ||
| func TxFilter(state State) func(tx types.Tx) bool { |
types/block.go
Outdated
|
|
||
| // MaxDataBytes returns the maximum size of block's data. | ||
| func MaxDataBytes(maxBytes, valsCount, evidenceCount int) int { | ||
| func MaxDataBytes(maxBytes, valsCount, evidenceCount int64) int64 { |
There was a problem hiding this comment.
valsCount and evidenceCount should probably remain ints
Codecov Report
@@ Coverage Diff @@
## develop #2427 +/- ##
==========================================
+ Coverage 60.99% 61% +0.01%
==========================================
Files 197 198 +1
Lines 16202 16200 -2
==========================================
+ Hits 9882 9883 +1
+ Misses 5482 5481 -1
+ Partials 838 836 -2
|
xla
left a comment
There was a problem hiding this comment.
👍 
🍡
Very clean addition for the tx filtering!
| } | ||
| } | ||
|
|
||
| func randomGenesisDoc() *types.GenesisDoc { |
There was a problem hiding this comment.
Can we indicate that this function is used for testing with a prefix maybe?
There was a problem hiding this comment.
Doesn't being in the _test.go suffice?
There was a problem hiding this comment.
They share the same namespace during testing, maybe I'm overly paranoid but usually like to flag it explicitely to not get tempted to use it in other contexts.
mempool/mempool.go
Outdated
| // amino.UvarintSize is not used here because it won't be possible to reuse buf | ||
| aminoOverhead := binary.PutUvarint(buf[:], uint64(len(memTx.tx))) | ||
| if maxBytes > -1 && totalBytes+len(memTx.tx)+aminoOverhead > maxBytes { | ||
| aminoOverhead := int64(binary.PutUvarint(buf[:], uint64(len(memTx.tx)))) |
There was a problem hiding this comment.
Is having to do this everywhere a code smell for our code, or for Go itself, or for neither?
| } | ||
| } | ||
|
|
||
| func randomGenesisDoc() *types.GenesisDoc { |
There was a problem hiding this comment.
Doesn't being in the _test.go suffice?
types/block.go
Outdated
| const ( | ||
| // MaxHeaderBytes is a maximum header size (including amino overhead). | ||
| MaxHeaderBytes = 511 | ||
| MaxHeaderBytes = int64(511) |
There was a problem hiding this comment.
Would MaxHeaderBytes int64 = 511 be better for these?
|
@melekes Looks like we have a cyclic dependency: https://circleci.com/gh/tendermint/tendermint/24992#tests/containers/1 |
|
Still a cyclic dep issue? |
types/testdouble/genesis.go
Outdated
|
|
||
| // RandomGenesisDoc returns a random genesis document with one validator. | ||
| // Genesis time is set to now(). | ||
| func RandomGenesisDoc() *types.GenesisDoc { |
There was a problem hiding this comment.
do folks think this is a good idea? maybe we should just export the same method from types package?
There was a problem hiding this comment.
I didn't totally understand why this happened.
There was a problem hiding this comment.
Wanted to reuse RandomGenesisDoc() function.
Refs #2382