conesnsu: follow up to removing some consensus params (#2427)

* follow up to removing some consensus params Refs #2382
* change args type to int64 in state#makeParams
* make valsCount and evidenceCount ints again
* MaxEvidenceBytesPerBlock: include magic number in godoc
* [spec] creating a proposal
* test state#TxFilter
* panic if MaxDataBytes is less than 0
* fixes after review
* use amino#UvarintSize to calculate overhead
https://github.com/tendermint/go-amino/blob/0c74291f3bb61e24f8ea1d13148cece479f79d35/encoder.go#L85-L90
* avoid cyclic imports
* you can do better Go, come on
* remove testdouble package

types/block.go

@@ -15,7 +15,7 @@ import (
 
 const (
 	// MaxHeaderBytes is a maximum header size (including amino overhead).
-	MaxHeaderBytes = 511
+	MaxHeaderBytes int64 = 511
 
 	// MaxAminoOverheadForBlock - maximum amino overhead to encode a block (up to
 	// MaxBlockSizeBytes in size) not including it's parts except Data.
@@ -24,7 +24,7 @@ const (
 	// 2 fields (2 embedded):               2 bytes
 	// Uvarint length of Data.Txs:          4 bytes
 	// Data.Txs field:                      1 byte
-	MaxAminoOverheadForBlock = 11
+	MaxAminoOverheadForBlock int64 = 11
 )
 
 // Block defines the atomic unit of a Tendermint blockchain.
@@ -205,23 +205,48 @@ func (b *Block) StringShort() string {
 //-----------------------------------------------------------------------------
 
 // MaxDataBytes returns the maximum size of block's data.
-func MaxDataBytes(maxBytes, valsCount, evidenceCount int) int {
-	return maxBytes -
+//
+// XXX: Panics on negative result.
+func MaxDataBytes(maxBytes int64, valsCount, evidenceCount int) int64 {
+	maxDataBytes := maxBytes -
 		MaxAminoOverheadForBlock -
 		MaxHeaderBytes -
-		(valsCount * MaxVoteBytes) -
-		(evidenceCount * MaxEvidenceBytes)
+		int64(valsCount)*MaxVoteBytes -
+		int64(evidenceCount)*MaxEvidenceBytes
+
+	if maxDataBytes < 0 {
+		panic(fmt.Sprintf(
+			"Negative MaxDataBytes. BlockSize.MaxBytes=%d is too small to accommodate header&lastCommit&evidence=%d",
+			maxBytes,
+			-(maxDataBytes - maxBytes),
+		))
+	}
+
+	return maxDataBytes
+
 }
 
 // MaxDataBytesUnknownEvidence returns the maximum size of block's data when
 // evidence count is unknown. MaxEvidenceBytesPerBlock will be used as the size
 // of evidence.
-func MaxDataBytesUnknownEvidence(maxBytes, valsCount int) int {
-	return maxBytes -
+//
+// XXX: Panics on negative result.
+func MaxDataBytesUnknownEvidence(maxBytes int64, valsCount int) int64 {
+	maxDataBytes := maxBytes -
 		MaxAminoOverheadForBlock -
 		MaxHeaderBytes -
-		(valsCount * MaxVoteBytes) -
+		int64(valsCount)*MaxVoteBytes -
 		MaxEvidenceBytesPerBlock(maxBytes)
+
+	if maxDataBytes < 0 {
+		panic(fmt.Sprintf(
+			"Negative MaxDataBytesUnknownEvidence. BlockSize.MaxBytes=%d is too small to accommodate header&lastCommit&evidence=%d",
+			maxBytes,
+			-(maxDataBytes - maxBytes),
+		))
+	}
+
+	return maxDataBytes
 }
 
 //-----------------------------------------------------------------------------

types/block_test.go

@@ -266,7 +266,7 @@ func TestMaxHeaderBytes(t *testing.T) {
 	bz, err := cdc.MarshalBinary(h)
 	require.NoError(t, err)
 
-	assert.Equal(t, MaxHeaderBytes, len(bz))
+	assert.EqualValues(t, MaxHeaderBytes, len(bz))
 }
 
 func randCommit() *Commit {
@@ -279,3 +279,60 @@ func randCommit() *Commit {
 	}
 	return commit
 }
+
+func TestBlockMaxDataBytes(t *testing.T) {
+	testCases := []struct {
+		maxBytes      int64
+		valsCount     int
+		evidenceCount int
+		panics        bool
+		result        int64
+	}{
+		0: {-10, 1, 0, true, 0},
+		1: {10, 1, 0, true, 0},
+		2: {721, 1, 0, true, 0},
+		3: {722, 1, 0, false, 0},
+		4: {723, 1, 0, false, 1},
+	}
+
+	for i, tc := range testCases {
+		if tc.panics {
+			assert.Panics(t, func() {
+				MaxDataBytes(tc.maxBytes, tc.valsCount, tc.evidenceCount)
+			}, "#%v", i)
+		} else {
+			assert.Equal(t,
+				tc.result,
+				MaxDataBytes(tc.maxBytes, tc.valsCount, tc.evidenceCount),
+				"#%v", i)
+		}
+	}
+}
+
+func TestBlockMaxDataBytesUnknownEvidence(t *testing.T) {
+	testCases := []struct {
+		maxBytes  int64
+		valsCount int
+		panics    bool
+		result    int64
+	}{
+		0: {-10, 1, true, 0},
+		1: {10, 1, true, 0},
+		2: {801, 1, true, 0},
+		3: {802, 1, false, 0},
+		4: {803, 1, false, 1},
+	}
+
+	for i, tc := range testCases {
+		if tc.panics {
+			assert.Panics(t, func() {
+				MaxDataBytesUnknownEvidence(tc.maxBytes, tc.valsCount)
+			}, "#%v", i)
+		} else {
+			assert.Equal(t,
+				tc.result,
+				MaxDataBytesUnknownEvidence(tc.maxBytes, tc.valsCount),
+				"#%v", i)
+		}
+	}
+}

types/evidence.go

@@ -12,7 +12,7 @@ import (
 
 const (
 	// MaxEvidenceBytes is a maximum size of any evidence (including amino overhead).
-	MaxEvidenceBytes = 440
+	MaxEvidenceBytes int64 = 440
 )
 
 // ErrEvidenceInvalid wraps a piece of evidence and the error denoting how or why it is invalid.
@@ -52,14 +52,16 @@ func RegisterEvidences(cdc *amino.Codec) {
 	cdc.RegisterConcrete(MockBadEvidence{}, "tendermint/MockBadEvidence", nil)
 }
 
-// MaxEvidenceBytesPerBlock returns the maximum evidence size per block.
-func MaxEvidenceBytesPerBlock(blockMaxBytes int) int {
+// MaxEvidenceBytesPerBlock returns the maximum evidence size per block -
+// 1/10th of the maximum block size.
+func MaxEvidenceBytesPerBlock(blockMaxBytes int64) int64 {
 	return blockMaxBytes / 10
 }
 
 //-------------------------------------------
 
-// DuplicateVoteEvidence contains evidence a validator signed two conflicting votes.
+// DuplicateVoteEvidence contains evidence a validator signed two conflicting
+// votes.
 type DuplicateVoteEvidence struct {
 	PubKey crypto.PubKey
 	VoteA  *Vote

types/evidence_test.go

@@ -108,7 +108,7 @@ func TestMaxEvidenceBytes(t *testing.T) {
 	bz, err := cdc.MarshalBinary(ev)
 	require.NoError(t, err)
 
-	assert.Equal(t, MaxEvidenceBytes, len(bz))
+	assert.EqualValues(t, MaxEvidenceBytes, len(bz))
 }
 
 func randomDuplicatedVoteEvidence() *DuplicateVoteEvidence {

types/genesis_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	tmtime "github.com/tendermint/tendermint/types/time"
 )

types/params.go

@@ -23,7 +23,7 @@ type ConsensusParams struct {
 
 // BlockSize contain limits on the block size.
 type BlockSize struct {
-	MaxBytes int   `json:"max_txs_bytes"`
+	MaxBytes int64 `json:"max_bytes"`
 	MaxGas   int64 `json:"max_gas"`
 }
 
@@ -102,7 +102,7 @@ func (params ConsensusParams) Update(params2 *abci.ConsensusParams) ConsensusPar
 	// XXX: it's cast city over here. It's ok because we only do int32->int
 	// but still, watch it champ.
 	if params2.BlockSize != nil {
-		res.BlockSize.MaxBytes = int(params2.BlockSize.MaxBytes)
+		res.BlockSize.MaxBytes = params2.BlockSize.MaxBytes
 		res.BlockSize.MaxGas = params2.BlockSize.MaxGas
 	}
 	if params2.EvidenceParams != nil {

types/params_test.go

@@ -9,30 +9,23 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
-func newConsensusParams(txsBytes, evidenceAge int) ConsensusParams {
-	return ConsensusParams{
-		BlockSize:      BlockSize{MaxBytes: txsBytes},
-		EvidenceParams: EvidenceParams{MaxAge: int64(evidenceAge)},
-	}
-}
-
 func TestConsensusParamsValidation(t *testing.T) {
 	testCases := []struct {
 		params ConsensusParams
 		valid  bool
 	}{
 		// test block size
-		0: {newConsensusParams(1, 1), true},
-		1: {newConsensusParams(0, 1), false},
-		2: {newConsensusParams(47*1024*1024, 1), true},
-		3: {newConsensusParams(10, 1), true},
-		4: {newConsensusParams(100*1024*1024, 1), true},
-		5: {newConsensusParams(101*1024*1024, 1), false},
-		6: {newConsensusParams(1024*1024*1024, 1), false},
-		7: {newConsensusParams(1024*1024*1024, -1), false},
+		0: {makeParams(1, 0, 1), true},
+		1: {makeParams(0, 0, 1), false},
+		2: {makeParams(47*1024*1024, 0, 1), true},
+		3: {makeParams(10, 0, 1), true},
+		4: {makeParams(100*1024*1024, 0, 1), true},
+		5: {makeParams(101*1024*1024, 0, 1), false},
+		6: {makeParams(1024*1024*1024, 0, 1), false},
+		7: {makeParams(1024*1024*1024, 0, -1), false},
 		// test evidence age
-		8: {newConsensusParams(1, 0), false},
-		9: {newConsensusParams(1, -1), false},
+		8: {makeParams(1, 0, 0), false},
+		9: {makeParams(1, 0, -1), false},
 	}
 	for i, tc := range testCases {
 		if tc.valid {
@@ -43,14 +36,14 @@ func TestConsensusParamsValidation(t *testing.T) {
 	}
 }
 
-func makeParams(txsBytes, blockGas, evidenceAge int) ConsensusParams {
+func makeParams(blockBytes, blockGas, evidenceAge int64) ConsensusParams {
 	return ConsensusParams{
 		BlockSize: BlockSize{
-			MaxBytes: txsBytes,
-			MaxGas:   int64(blockGas),
+			MaxBytes: blockBytes,
+			MaxGas:   blockGas,
 		},
 		EvidenceParams: EvidenceParams{
-			MaxAge: int64(evidenceAge),
+			MaxAge: evidenceAge,
 		},
 	}
 }

types/protobuf.go

@@ -115,7 +115,7 @@ func (tm2pb) ValidatorUpdates(vals *ValidatorSet) []abci.ValidatorUpdate {
 func (tm2pb) ConsensusParams(params *ConsensusParams) *abci.ConsensusParams {
 	return &abci.ConsensusParams{
 		BlockSize: &abci.BlockSize{
-			MaxBytes: int32(params.BlockSize.MaxBytes),
+			MaxBytes: params.BlockSize.MaxBytes,
 			MaxGas:   params.BlockSize.MaxGas,
 		},
 		EvidenceParams: &abci.EvidenceParams{
@@ -211,11 +211,11 @@ func (pb2tm) ValidatorUpdates(vals []abci.ValidatorUpdate) ([]*Validator, error)
 func (pb2tm) ConsensusParams(csp *abci.ConsensusParams) ConsensusParams {
 	return ConsensusParams{
 		BlockSize: BlockSize{
-			MaxBytes: int(csp.BlockSize.MaxBytes), // XXX
+			MaxBytes: csp.BlockSize.MaxBytes,
 			MaxGas:   csp.BlockSize.MaxGas,
 		},
 		EvidenceParams: EvidenceParams{
-			MaxAge: csp.EvidenceParams.MaxAge, // XXX
+			MaxAge: csp.EvidenceParams.MaxAge,
 		},
 	}
 }

types/vote.go

@@ -12,7 +12,7 @@ import (
 
 const (
 	// MaxVoteBytes is a maximum vote size (including amino overhead).
-	MaxVoteBytes = 200
+	MaxVoteBytes int64 = 200
 )
 
 var (

types/vote_test.go

@@ -147,5 +147,5 @@ func TestMaxVoteBytes(t *testing.T) {
 	bz, err := cdc.MarshalBinary(vote)
 	require.NoError(t, err)
 
-	assert.Equal(t, MaxVoteBytes, len(bz))
+	assert.EqualValues(t, MaxVoteBytes, len(bz))
 }