types: Cap evidence in block validation#2560
Merged
Conversation
2359e85 to
dc0c77b
Compare
Codecov Report
@@ Coverage Diff @@
## develop #2560 +/- ##
==========================================
+ Coverage 61.23% 61.4% +0.16%
==========================================
Files 202 202
Lines 16733 16747 +14
==========================================
+ Hits 10246 10283 +37
+ Misses 5617 5593 -24
- Partials 870 871 +1
|
Contributor
@ebuchman Does this mean this PR is WIP? |
ValarDragon
reviewed
Oct 8, 2018
Contributor
ValarDragon
left a comment
There was a problem hiding this comment.
LGTM, just needs a test case!
This was referenced Oct 9, 2018
Contributor
Author
|
Updated the block validation tests - made them table-driven, opened an issue to write more of them (#2589), and added a test for the evidence cap :) |
melekes
reviewed
Oct 9, 2018
state/validation_test.go
Outdated
| require.Error(t, err) | ||
| {"EvidenceHash", func(block *types.Block) { block.EvidenceHash = wrongHash }}, // wrong evidence hash | ||
| {"Proposer wrong", func(block *types.Block) { block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address() }}, // wrong proposer | ||
| {"Proposer invalid", func(block *types.Block) { block.ProposerAddress = []byte("wrong size") }}, // invalid proposer |
Contributor
There was a problem hiding this comment.
no need in // invalid proposer comment if we already have "Proposer invalid" as a test name
state/validation_test.go
Outdated
| // Manipulation of any header field causes failure. | ||
| testCases := []struct { | ||
| name string | ||
| f func(block *types.Block) |
Contributor
There was a problem hiding this comment.
tendermint/types/block_test.go
Line 58 in 3fcb62b
This was referenced Oct 9, 2018
Closed
melekes
added a commit
that referenced
this pull request
Oct 10, 2018
Refs #2587: ``` We only start validating block.Time when Height > 1, because there is no commit to compute the median timestamp from for the first block. This means a faulty proposer could make the first block with whatever time they want. Instead, we should require the timestamp of block 1 to match the genesis time. I discovered this while refactoring the ValidateBlock tests to be table-driven while working on tests for #2560. ```
ebuchman
pushed a commit
that referenced
this pull request
Oct 12, 2018
* require block.Time of the fist block to be genesis time Refs #2587: ``` We only start validating block.Time when Height > 1, because there is no commit to compute the median timestamp from for the first block. This means a faulty proposer could make the first block with whatever time they want. Instead, we should require the timestamp of block 1 to match the genesis time. I discovered this while refactoring the ValidateBlock tests to be table-driven while working on tests for #2560. ``` * do not accept blocks with negative height * update changelog and spec * nanos precision for test genesis time * Fix failing test (#2607)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1637
Still need to write tests
EDIT: wrote tests!