Tendermint/v0.33 #196

greg-szabo · 2020-03-29T08:28:31Z

This is a minimum viable product for Tendermint v0.33 compatibility. Lots of improvements can be made (more tests, further optimizations) but most probably those should be added as new issues and PRs.

I'm writing an ADR that should address the decisions made in updating/implementing the RPC structs.

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

* initial new commit struct * update functions * fixed deserialization for new commit struct * updated links to generator + val_set_test file * Update tendermint/src/account.rs Co-Authored-By: Ismail Khoffi <[email protected]> * fmt * clippy * rpc tests: grab latest example fro commit: - use https://docs.tendermint.com/master/rpc/#/Info/commit - update chain_id: cosmoshub-1 to cosmoshub-2 everywhere else - manually kept `id` a string in JSONrpc responses * Actually let's go cosmoshub-3: - grabbed from: https://rpc.cosmos.network/commit?height=2 * Fix commit test - regenerated commit.json via `curl -X GET "http://localhost:26657/commit?height=10" -H "accept: application/json"` * Fix block test - "regenerated" block.json via `curl -X GET "http://localhost:26657/block?height=10" -H "accept: application/json"` * Fix first_block test - "regenerated" block.json via `curl -X GET "http://localhost:26657/block?height=1" -H "accept: application/json" ` Co-authored-by: Shivani Joshi <[email protected]> Co-authored-by: Shivani Joshi <[email protected]>

fix /abci_info & /genesis: - add app_version & use #[serde(default)] to deal with omitted fields in JSON - make app_state in /genesis reply optional - fix string in abci_info test (kvstore wont reply with "GaiaApp") verify tests pass via running `tendermint node --proxy_app=kvstore` and `cargo test -- --nocapture --ignored

…or CI

…int-rs into tendermint/v0.33

codecov-io · 2020-03-29T08:36:15Z

Codecov Report

Merging #196 into master will not change coverage by %.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #196   +/-   ##
======================================
  Coverage    39.1%   39.1%           
======================================
  Files          99      99           
  Lines        3610    3610           
  Branches      580     580           
======================================
  Hits         1413    1413           
  Misses       1800    1800           
  Partials      397     397

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2993d23...2993d23. Read the comment docs.

liamsi · 2020-04-08T12:26:34Z

Could #198 be fixed part of this?

#206

* manually update Proof and introduce ProofOp to match the actual JSON encoding * 0.33 branch seems out of date with master, committing to switch back * Fix test * Fix rpc endpoint test (how wasn't this detected before by the integration tests)?

greg-szabo · 2020-04-09T12:15:23Z

Removed WIP, let's see if everything is in place for merge. I'd consider @liamsi or @ebuchman as the best people to approve this.

liamsi · 2020-04-09T12:53:17Z

I'll go through the changes again and collect everything in a followup issue. I think we can merge this later today.

romac · 2020-04-10T10:07:25Z

FYI there are missing serializers for TrustThresholdFraction. I just opened a PR to fix those at #210.

Also, I just ran the relayer (which depends on this branch) against a Tendermint 0.33.3 node and got an RPC error saying missing field 'height' at line 59 column 7. I suspect it comes from either tendermint::rpc::Client::latest_commit or tendermint::rpc::Client::commit.

ancazamfir · 2020-04-10T10:52:27Z

I tried with gaia nodes running IBC and get Some("missing field num_txs at line 30 column 7"), this is with tendermint v0.33.2

* rename iter -> signed_votes and add validation checks - add basic validation to validate method (depending on `BlockIDFlag`) - move check for unknown validator to validate method (for CommitSig)

liamsi

This PR is becoming way too large... I put some of the outstanding issues in #211 and would recommend merging this instead of adding more to it. I'd also recommend another pair of eyes here though as I authored parts of this PR too.

melekes

tendermint/src/block/commit_sig.rs

melekes · 2020-04-16T11:44:18Z

tendermint/src/lite_impl/signed_header.rs

@@ -52,42 +45,109 @@ impl lite::Commit for block::signed_header::SignedHeader {
    }

    fn validate(&self, vals: &Self::ValidatorSet) -> Result<(), Error> {
-        if self.commit.precommits.len() != vals.validators().len() {
+        // TODO: self.commit.block_id cannot be zero in the same way as in go


only the very first block (height: 1) is allowed (actually required) to have 0 signatures

tendermint/src/lite_impl/signed_header.rs

melekes · 2020-04-16T12:04:29Z

tendermint/src/lite_impl/signed_header.rs

+                        self
+                    );
+                }
+                // TODO: this last check is only necessary if we do full verification (2/3) but the


seems to me this should not be the concern of Commit to validate if it's a part of some validator set, but it should be rather the job of external object, who's probably holding a validator set to make sure commit signatures match validator set. that's how it's done in Go. It's interesting that you've decided to revert this relationship.

tarcieri · 2020-04-16T21:52:21Z

Probably good to create a 0.32 based release from master first!

@liamsi I think it'd be good to cut a final tendermint-rs v0.12 release (the current release is v0.12.0-rc0) then merge this.

Happy to help with that.

liamsi · 2020-04-21T11:40:52Z

Merging this now. Whenever we want, we can now release this as 0.14.0 (a 0.12.0 and a 0.13.0 were released in the meantime). Will add the adding changelog entries to #211.

- #196 (comment)

* Drop redundant prefixes in BlockIDFlag enum variants - #196 (comment) * improve error message

Shivani912 and others added 28 commits March 17, 2020 17:52

initial new commit struct

bd3430e

update functions

d129ad2

fixed deserialization for new commit struct

b64840b

updated links to generator + val_set_test file

b893a25

fix error

525b419

more json bisection tests

ae3ee39

abci_info struct app_version implemented

0e6a073

Fixed fmt

1a9a96e

Updated integration tests to tendermint v0.33 node

a3367f3

update block result response

d0b9039

changes to abci_info and block_results

accb997

Removed CircleCI reference and added codecov reference to Cargo

e4918c8

removing unwanted Option from DeliverTx struct fields

e0bce41

last_block_app_hash is now Option<Vec<u8>>

06d91d4

AbciInfo.last_block_app_hash is base64 encoded instead of hex

af430df

last_block_app_hash changed to Vec<u8>

0ad0537

[88] Fix JSON ID

323b44a

[192] Fix abci_info compatibility for v0.33

95902c6

[4] block_results test fix

c0c3ed7

[184][120] Tendermint v0.33 compatibility fixes + integration tests f…

3659a30

…or CI

Added one abci_query test

6c589c9

FMT + get rid of warnings

2bf6125

Relaxed genesis struct AppHash requirements from strict hex to anything

5e9b62a

Removed unused Protocol struct

0d4952d

Removed unused Protocol struct

58411ac

Merge branch 'tendermint/v0.33' of github.com:informalsystems/tenderm…

2743f8b

…int-rs into tendermint/v0.33

greg-szabo requested a review from liamsi March 29, 2020 08:40

liamsi mentioned this pull request Apr 8, 2020

Update Proof and ProofOp #206

Merged

5 tasks

Update Proof and ProofOp (#206)

51934ba

* manually update Proof and introduce ProofOp to match the actual JSON encoding * 0.33 branch seems out of date with master, committing to switch back * Fix test * Fix rpc endpoint test (how wasn't this detected before by the integration tests)?

greg-szabo changed the title ~~WIP: Tendermint/v0.33~~ Tendermint/v0.33 Apr 9, 2020

romac mentioned this pull request Apr 10, 2020

Perform bisection when verifying headers using the light client informalsystems/hermes#51

Merged

6 tasks

Add missing serializer for TrustThresholdFraction fields (#210)

2993d23

commit followup (#199)

0eae723

* rename iter -> signed_votes and add validation checks - add basic validation to validate method (depending on `BlockIDFlag`) - move check for unknown validator to validate method (for CommitSig)

liamsi mentioned this pull request Apr 10, 2020

tendermint/0.33 / #196 followup issue #211

Closed

9 tasks

liamsi approved these changes Apr 10, 2020

View reviewed changes

melekes approved these changes Apr 16, 2020

View reviewed changes

melekes reviewed Apr 16, 2020

View reviewed changes

liamsi mentioned this pull request Apr 17, 2020

Release 0.12.0 from master #222

Merged

5 tasks

liamsi merged commit 3003fef into master Apr 21, 2020

This was referenced Apr 21, 2020

Add subscriptions support #225

Merged

Fix block_id of nil vote #236

Merged

liamsi mentioned this pull request May 4, 2020

Serialization improvements #248

Closed

5 tasks

greg-szabo mentioned this pull request May 8, 2020

CommitSig Timestamp can be zero time #259

Closed

liamsi mentioned this pull request Jun 3, 2020

Feature guard the networking stuff #289

Closed

liamsi added a commit that referenced this pull request Jun 3, 2020

Drop redundant prefixes in BlockIDFlag enum variants

a544a8c

- #196 (comment)

liamsi mentioned this pull request Jun 3, 2020

0.33 compat followup #296

Merged

5 tasks

liamsi deleted the tendermint/v0.33 branch June 3, 2020 09:51

liamsi added a commit that referenced this pull request Jun 3, 2020

0.33 compat followup (#296)

622166b

* Drop redundant prefixes in BlockIDFlag enum variants - #196 (comment) * improve error message

This was referenced Jun 3, 2020

Add tests for jsonrpc id #298

Merged

json: add Amino-compatible encoder/decoder tendermint/tendermint#4955

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tendermint/v0.33 #196

Tendermint/v0.33 #196

greg-szabo commented Mar 29, 2020 •

edited

Loading

codecov-io commented Mar 29, 2020 •

edited

Loading

liamsi commented Apr 8, 2020

greg-szabo commented Apr 9, 2020

liamsi commented Apr 9, 2020

romac commented Apr 10, 2020 •

edited

Loading

ancazamfir commented Apr 10, 2020

liamsi left a comment

melekes left a comment

melekes Apr 16, 2020

melekes Apr 16, 2020

tarcieri commented Apr 16, 2020 •

edited

Loading

liamsi commented Apr 21, 2020

Tendermint/v0.33 #196

Tendermint/v0.33 #196

Conversation

greg-szabo commented Mar 29, 2020 • edited Loading

codecov-io commented Mar 29, 2020 • edited Loading

Codecov Report

liamsi commented Apr 8, 2020

greg-szabo commented Apr 9, 2020

liamsi commented Apr 9, 2020

romac commented Apr 10, 2020 • edited Loading

ancazamfir commented Apr 10, 2020

liamsi left a comment

Choose a reason for hiding this comment

melekes left a comment

Choose a reason for hiding this comment

melekes Apr 16, 2020

Choose a reason for hiding this comment

melekes Apr 16, 2020

Choose a reason for hiding this comment

tarcieri commented Apr 16, 2020 • edited Loading

liamsi commented Apr 21, 2020

greg-szabo commented Mar 29, 2020 •

edited

Loading

codecov-io commented Mar 29, 2020 •

edited

Loading

romac commented Apr 10, 2020 •

edited

Loading

tarcieri commented Apr 16, 2020 •

edited

Loading