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.

Sign up for GitHub

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

Jump to bottom

Added single-step-skipping JSON tests #143

Merged
merged 6 commits into from
Feb 6, 2020
Merged

Added single-step-skipping JSON tests #143

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f9d872a
Added single-step-skipping JSON tests
Shivani912 Feb 3, 2020
0a30022
Add a check for the parts.hash in commit.validate impl
liamsi Feb 3, 2020
bbb07cf
All tests passing
Shivani912 Feb 5, 2020
413b780
cargo fmt and delete comment
liamsi Feb 5, 2020
46a3c9d
clean up
Shivani912 Feb 5, 2020
d009ab3
Merge branch 'master' of github.com:Shivani912/tendermint-rs
Shivani912 Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions tendermint/src/lite_impl/signed_header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ impl lite::Commit for block::signed_header::SignedHeader {
if self.commit.precommits.len() != vals.validators().len() {
return Err(lite::Error::InvalidCommitSignatures);
}
if let Some(parts) = &self.commit.block_id.parts {
if parts.hash != self.header_hash() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liamsi parts.hash is the merkleRoot(partsHeader) which is different from the header_hash. That's why this check is making many other tests to fail too. We want to check that the commit.BlockID is equal to each of the precommit.BlockID. In reality, there can be precommits for other blocks and that's totally fine. But then, those precommits should not be considered for this block. This means that when we calculate the total voting power i.e. signed_power, we do not count voting power of validators who signed these precommits. Maybe this check should go under the voting_power_in method(?)

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for the clarification! To move forward, I'd suggest to merge this, and open an issue referencing this comment and the failing test.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, is this about the bug I raised here: #145 (comment) ? We should be testing (either here or in voting_power_in) that the votes being tallied are for the right block ID

// TODO(ismail): we need a better error here!
return Err(lite::Error::InvalidCommit);
}
}
Ok(())
}
}
Expand Down
24 changes: 18 additions & 6 deletions tendermint/tests/lite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ struct Duration(u64);

#[derive(Deserialize, Clone, Debug)]
struct TestCases {
batch_name: String,
test_cases: Vec<TestCase>,
}

#[derive(Deserialize, Clone, Debug)]
struct TestCase {
test_name: String,
description: String,
initial: Initial,
input: Vec<LiteBlock>,
Expand Down Expand Up @@ -44,19 +44,31 @@ fn read_json_fixture(name: &str) -> String {

#[test]
fn val_set_tests_verify() {
let cases: TestCases = serde_json::from_str(&read_json_fixture("val_set_tests")).unwrap();
let cases: TestCases = serde_json::from_str(&read_json_fixture("single_step_sequential/val_set_tests")).unwrap();
run_test_cases(cases);
}

#[test]
fn commit_tests_verify() {
let cases: TestCases = serde_json::from_str(&read_json_fixture("commit_tests")).unwrap();
let cases: TestCases = serde_json::from_str(&read_json_fixture("single_step_sequential/commit_tests")).unwrap();
run_test_cases(cases);
}

#[test]
fn header_tests_verify() {
let cases: TestCases = serde_json::from_str(&read_json_fixture("header_tests")).unwrap();
let cases: TestCases = serde_json::from_str(&read_json_fixture("single_step_sequential/header_tests")).unwrap();
run_test_cases(cases);
}

#[test]
fn single_skip_val_set_tests_verify() {
let cases: TestCases = serde_json::from_str(&read_json_fixture("single_step_skipping/val_set_tests")).unwrap();
run_test_cases(cases);
}

#[test]
fn single_skip_commit_tests_verify() {
let cases: TestCases = serde_json::from_str(&read_json_fixture("single_step_skipping/commit_tests")).unwrap();
run_test_cases(cases);
}

Expand All @@ -76,8 +88,8 @@ fn run_test_cases(cases: TestCases) {
let tm_now = tc.initial.now;
let now = tm_now.to_system_time().unwrap();

for (_, input) in tc.input.iter().enumerate() {
println!("{}", tc.description);
for (i, input) in tc.input.iter().enumerate() {
println!("i: {}, {}", i, tc.description);
let untrusted_signed_header = &input.signed_header;
let untrusted_vals = &input.validator_set;
let untrusted_next_vals = &input.next_validator_set;
Expand Down
Loading