Update Proof and ProofOp #206

liamsi · 2020-04-08T11:40:29Z

Note: changes from 79bc33a can be ignored when reviewing (they are from master),

relevant changes here: https://github.com/informalsystems/tendermint-rs/pull/206/files#diff-fb2318e2d9a483eac0f27bed09958988

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

…encoding

…f_ops

…tion tests)?

liamsi · 2020-04-08T11:56:53Z

tendermint/tests/support/rpc/abci_query.json

+                        "data": "CvEECjAKBGJhbmsSKAomCIjYAxIg2MEyyonbZButYnvSRkf2bPQg+nqA+Am1MeDxG6F4p1UKLwoDYWNjEigKJgiI2AMSIN2YHczeuXNvyetrSFQpkCcJzfB6PXVCw0i/XShMgPnIChEKB3VwZ3JhZGUSBgoECIjYAwovCgNnb3YSKAomCIjYAxIgYM0TfBli7KxhY4nWgDSDPykhUJwtKFql9RU5l86WinQKLwoDaWJjEigKJgiI2AMSIFp6aJASeInQKF8y824zjmgcFORN6M+ECbgFfJkobKs8CjAKBG1haW4SKAomCIjYAxIgsZzwmLQ7PH1UeZ/vCUSqlQmfgt3CGfoMgJLkUqKCv0EKMwoHc3Rha2luZxIoCiYIiNgDEiCiBZoBLyDGj5euy3n33ik+SpqYK9eB5xbI+iY8ycYVbwo0CghzbGFzaGluZxIoCiYIiNgDEiAJz3gEYuIhdensHU3b5qH5ons2quepd6EaRgCHXab6PQoyCgZzdXBwbHkSKAomCIjYAxIglWLA5/THPTiTxAlaLHOBYFIzEJTmKPznItUwAc8zD+AKEgoIZXZpZGVuY2USBgoECIjYAwowCgRtaW50EigKJgiI2AMSIMS8dZ1j8F6JVVv+hB1rHBZC+gIFJxHan2hM8qDC64n/CjIKBnBhcmFtcxIoCiYIiNgDEiB8VIzExUHX+SvHZFz/P9NM9THnw/gTDDLVReuZX8htLgo4CgxkaXN0cmlidXRpb24SKAomCIjYAxIg3u/Nd4L+8LT8OXJCh14o8PHIJ/GLQwsmE7KYIl1GdSYKEgoIdHJhbnNmZXISBgoECIjYAw=="
+                    }
+                ]
+            },


Surprised the integration tests didn't kick in here.

all are ignored. But something else is wrong there (testing with null key). the rpc abci_query test is passing in parent but the tests/support/rpc/abci_query.json file has the old proof format there. Will fail without your changes and new proof format in the .json file.

The integration tests should be spun up in CI and specifically run the ignored ones.

tendermint/src/abci/proof.rs

ancazamfir

Looks good I think for now. Will try to run IBC with this. I am wondering though why is proof.rs in .../abci/.. directory and module and not in merkle?

liamsi · 2020-04-08T14:58:32Z

Looks good I think for now. Will try to run IBC with this. I am wondering though why is proof.rs in .../abci/.. directory and module and not in merkle?

Hmm, that is a good point. I left it this way because it was already there. I'll go on and merge now but I think you are right, should be in merkle.

* initial new commit struct * update functions * fixed deserialization for new commit struct * updated links to generator + val_set_test file * Ismail/commit struct followup (#182) * 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 <Ismail.Khoffi@gmail.com> * 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 <joshi.shivani912@gmail.com> Co-authored-by: Shivani Joshi <46731446+Shivani912@users.noreply.github.com> * fix error * more json bisection tests * abci_info struct app_version implemented * Fixed fmt * Updated integration tests to tendermint v0.33 node * update block result response * changes to abci_info and block_results * Removed CircleCI reference and added codecov reference to Cargo * removing unwanted Option from DeliverTx struct fields * last_block_app_hash is now Option<Vec<u8>> * AbciInfo.last_block_app_hash is base64 encoded instead of hex * last_block_app_hash changed to Vec<u8> * Cherry-pick from 87e888b (Liamsi) 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 * [88] Fix JSON ID * [192] Fix abci_info compatibility for v0.33 * [4] block_results test fix * [184][120] Tendermint v0.33 compatibility fixes + integration tests for CI * Added one abci_query test * FMT + get rid of warnings * Relaxed genesis struct AppHash requirements from strict hex to anything * Removed unused Protocol struct * Removed unused Protocol struct * IdType removed and JSON ID made as enum * Duration looks better now * fix fmt * Update tendermint/src/lite_impl/signed_header.rs Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Update tendermint/src/lite_impl/signed_header.rs Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Removed AsRef from JSON ID * Update Proof and ProofOp (#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)? * Add missing serializer for TrustThresholdFraction fields (#210) * commit followup (#199) * 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) Co-authored-by: Shivani Joshi <joshi.shivani912@gmail.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> Co-authored-by: Shivani Joshi <46731446+Shivani912@users.noreply.github.com> Co-authored-by: Romain Ruetschi <romain@informal.systems>

* initial new commit struct * update functions * fixed deserialization for new commit struct * updated links to generator + val_set_test file * Ismail/commit struct followup (#182) * 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 <Ismail.Khoffi@gmail.com> * 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 <joshi.shivani912@gmail.com> Co-authored-by: Shivani Joshi <46731446+Shivani912@users.noreply.github.com> * fix error * more json bisection tests * abci_info struct app_version implemented * Fixed fmt * Updated integration tests to tendermint v0.33 node * update block result response * changes to abci_info and block_results * Removed CircleCI reference and added codecov reference to Cargo * removing unwanted Option from DeliverTx struct fields * last_block_app_hash is now Option<Vec<u8>> * AbciInfo.last_block_app_hash is base64 encoded instead of hex * last_block_app_hash changed to Vec<u8> * Cherry-pick from 87e888b (Liamsi) 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 * [88] Fix JSON ID * [192] Fix abci_info compatibility for v0.33 * [4] block_results test fix * [184][120] Tendermint v0.33 compatibility fixes + integration tests for CI * Added one abci_query test * FMT + get rid of warnings * Relaxed genesis struct AppHash requirements from strict hex to anything * Removed unused Protocol struct * Removed unused Protocol struct * IdType removed and JSON ID made as enum * Duration looks better now * fix fmt * Update tendermint/src/lite_impl/signed_header.rs Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Update tendermint/src/lite_impl/signed_header.rs Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Removed AsRef from JSON ID * Update Proof and ProofOp (#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)? * Add missing serializer for TrustThresholdFraction fields (#210) * commit followup (#199) * 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) * Initial support for websocket subscriptions * Websocker error * Fix formatting * Intitial integration test * Fmt * Missed adding file * Fix url scheme * fix url scheme * Format * Debugged event subscription with unit tests * Fix clippy lint * Add back in the integration test ignore * Refactor the websocket listener into a seperate struct from the RPC client * Cargo fmt * Cargo fmt * Fix integration tests * Remove the Box<Errors> * Update tendermint/src/rpc/endpoint/subscribe.rs Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Fix the doc comment for subscribe * Moved a TODO comment to correct place * Create a more specific type for JSON RPC TX Result responses * Add a convinience function for extracting the event hashmap * add an an additional query for helping type the event responses * switch to borrowed types * Remove extra borrow * Remove borrow * Fmt and fix clippy issues * Improve the extract events function so that we treat any matching query in the vectors as valid * Derive Clone for the Event type * Prevent panics in extract events * Fix module query * Type events only on message.action * Ensures all that events end up in the hashmap. A little unclear if this was happening * Revert "Type events only on message.action" This reverts commit 97f5c9b. * Revert "Type events only on message.action" This reverts commit 97f5c9b. * accept periods in the chain-id * Fmt and clippy * review comments * fix build: dbg! macro and catch up with renaming * simplify match in integration test * Fix typo in doc comment * Make queries an enum handle block events and tx events * Refactor Enum to remove data field * Update tendermint/src/rpc/event_listener.rs Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * Explain why we throw away subscription responses * DRY the top level JSON response type * Update tendermint/src/rpc/endpoint/subscribe.rs Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * DRY some of the Block types * Fmt * return `Box<dyn stdError>` instead of `&'static str` * Fix clippy warnings and errs * Fix typo * DRY Block, BlockData, Evidence, Parts * Remove raw json responses from the public API * Please clippy * Box large variant * Box the other variant * Update the RPCTxResult to the new api * clarify some local var names * use rpc::response::Response and improve err handling - unexport Wrapper fields * Fix hyperlink in doc comment * Update link on error codes * Make error codes symmetric * Fix panic string * Add some debug output * Add some more debug output * Type alias instead of wrapping the wrapper * Revert "DRY Block, BlockData, Evidence, Parts" This reverts commit 3269d58 * DRY Block, BlockData, Evidence, Parts * Let's try an Option * Another Option? * These events are actually optional * typo * Let it be * Fix match arm Co-authored-by: Shivani Joshi <joshi.shivani912@gmail.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> Co-authored-by: Shivani Joshi <46731446+Shivani912@users.noreply.github.com> Co-authored-by: Greg Szabo <greg@philosobear.com> Co-authored-by: Greg Szabo <16846635+greg-szabo@users.noreply.github.com> Co-authored-by: Romain Ruetschi <romain@informal.systems> Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>

liamsi added 4 commits April 8, 2020 12:34

manually update Proof and introduce ProofOp to match the actual JSON …

b971211

…encoding

0.33 branch seems out of date with master, committing to switch back

236b436

Merge remote-tracking branch 'remotes/origin/master' into ismail/proo…

79bc33a

…f_ops

Fix test

30a228e

liamsi requested a review from ancazamfir April 8, 2020 11:44

Fix rpc endpoint test (how wasn't this detected before by the integra…

beedac3

…tion tests)?

liamsi commented Apr 8, 2020

View reviewed changes

tendermint/src/abci/proof.rs Show resolved Hide resolved

liamsi mentioned this pull request Apr 8, 2020

Tendermint/v0.33 #196

Merged

5 tasks

liamsi requested review from greg-szabo and Shivani912 April 8, 2020 12:59

ancazamfir approved these changes Apr 8, 2020

View reviewed changes

liamsi merged commit 51934ba into tendermint/v0.33 Apr 8, 2020

liamsi deleted the ismail/proof_ops branch April 8, 2020 14:59

liamsi mentioned this pull request Apr 8, 2020

Move Proof and ProofOp out of abci module #207

Closed

ancazamfir mentioned this pull request Apr 10, 2020

ABCI Merkle Proof struct does not match Go counterpart #198

Closed

liamsi mentioned this pull request Apr 10, 2020

tendermint/0.33 / #196 followup issue #211

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Proof and ProofOp #206

Update Proof and ProofOp #206

liamsi commented Apr 8, 2020 •

edited

Loading

liamsi Apr 8, 2020

ancazamfir Apr 8, 2020

liamsi Apr 8, 2020

ancazamfir left a comment

liamsi commented Apr 8, 2020

Update Proof and ProofOp #206

Update Proof and ProofOp #206

Conversation

liamsi commented Apr 8, 2020 • edited Loading

liamsi Apr 8, 2020

Choose a reason for hiding this comment

ancazamfir Apr 8, 2020

Choose a reason for hiding this comment

liamsi Apr 8, 2020

Choose a reason for hiding this comment

ancazamfir left a comment

Choose a reason for hiding this comment

liamsi commented Apr 8, 2020

liamsi commented Apr 8, 2020 •

edited

Loading