Add domain types (updated) and fix integration tests for Tendermint v0.35.0 #1022

thanethomson · 2021-11-19T14:11:38Z

Builds on and supersedes #862.

Unfortunately the only way to get all the tests to pass here was to effectively do the upgrade to Tendermint v0.35.0. One important ABCI-related change that needed to be incorporated is related to the low-level wire format change in tendermint/tendermint#5783.

As a result, I've had to:

Update the integration tests' Docker images to use Tendermint v0.35.0.
Bump all the versioning in our tools.
Regenerate our Protobuf files.
Regenerate our RPC fixtures.
Make a few minor upgrades to the serialization formats of some data structures that changes from Tendermint v0.34.0 to v0.35.0.

The reason so many lines of code went away was because I removed the rpc/tests/support folder. Many of these files are effectively duplicates of what's tested from rpc/tests/kvstore_fixtures and they were manually extracted, so updating them is a pain. By contrast, the fixtures in rpc/tests/kvstore_fixtures are automatically generated by our rpc-probe tool, so we should look at expanding our RPC testing using the rpc-probe (e.g. by way of probing a running Gaia-based network). This should already be captured in #611.

Referenced an issue explaining the need for the change
Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Added entry in .changelog/

The BlockParams and ConsensusParams structs moved out of the ABCI protos.

This fixes a compile error introduced by upstream proto changes that add an Sr25519 variant.

The previous data modeling allowed a user to construct an `Err(0)` value that would be serialized and deserialized as `Ok`.

This changes the Serialize/Deserialize implementations for Block to convert to/from the proto-generated `RawBlock`, and use the derived serialization for that type. This is much cleaner and more maintainable than keeping Serde annotations for each sub-member of the data structure, because all of the serialization code is kept in one place, and there's only one validation path (the TryFrom conversion from RawBlock to Block) that applies to both kinds of serialization.

This changes the Block type to hold the transactions as a plain `Vec<Vec<u8>>`, rather than as a custom `abci::transaction::Data` type (which is itself a wrapper for an `Option<Vec<Transaction>>`, the `Transaction` type being a wrapper for a `Vec<u8>`). This also updates the `Block` getter functions; it's not clear (to me) why they're there, since they access the public fields and aren't used anywhere else.

The existing contents of the `tendermint::abci` module note that they're only for the purpose of implementing the RPC endpoints, not a general ABCI implementation. Moving that code from the `tendermint` crate to the `tendermint-rpc` crate decouples the RPC interface from improvements to the ABCI domain modeling. Eventually, it would be good to eliminate this code and align it with the new ABCI domain types.

These types mirror the generated types in tendermint_proto, but have better naming. The documentation for each request type is adapted from the ABCI Methods and Types spec document. However, the same logical request may appear three times, as a struct with the request data, as a Request variant, and as a CategoryRequest variant. To avoid duplication, this PR uses the `#[doc = include_str!(...)]` functionality stabilized in Rust 1.54 to keep common definitions of the documentation.

The code in the `abci` module had more complete documentation from the ABCI docs, so I copied it onto the existing structure.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

…0 changes Signed-off-by: Thane Thomson <connect@thanethomson.com>

…int/tendermint#5783 Signed-off-by: Thane Thomson <connect@thanethomson.com>

Signed-off-by: Thane Thomson <connect@thanethomson.com>

codecov-commenter · 2021-11-25T19:54:30Z

Codecov Report

Merging #1022 (07a8efe) into master (9cd12b0) will decrease coverage by 4.0%.
The diff coverage is 13.8%.

@@           Coverage Diff            @@
##           master   #1022     +/-   ##
========================================
- Coverage    66.0%   62.0%   -4.1%     
========================================
  Files         209     237     +28     
  Lines       20760   21169    +409     
========================================
- Hits        13717   13138    -579     
- Misses       7043    8031    +988

Impacted Files	Coverage Δ
abci/src/application.rs	`19.1% <ø> (+1.0%)`	⬆️
abci/src/client.rs	`41.6% <ø> (+1.6%)`	⬆️
light-client/src/evidence.rs	`9.0% <ø> (ø)`
light-client/src/tests.rs	`93.5% <ø> (ø)`
p2p/src/secret_connection.rs	`86.5% <0.0%> (ø)`
proto/src/chrono.rs	`0.0% <0.0%> (ø)`
proto/src/lib.rs	`100.0% <ø> (ø)`
rpc/src/abci/data.rs	`90.9% <ø> (ø)`
rpc/src/abci/gas.rs	`64.2% <ø> (ø)`
rpc/src/abci/info.rs	`40.0% <ø> (ø)`
... and 86 more

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 9cd12b0...07a8efe. Read the comment docs.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

There are two issues to fix: * The original ABCI domain types work used `chrono`; this temporarily re-adds `chrono` as a dependency, so that the changes that eliminated it can be replayed on top of this rebasing. * The original ABCI domain types work was, for some reason, mixed in with changes to the RPC test harness for 0.35 compatibility, and that may break this code. I didn't attempt to fix this, because I didn't touch the RPC tests when writing the ABCI domain types. Original concatenation of commit messages follows: Add domain types (updated) and fix integration tests for Tendermint v0.35.0 (informalsystems#1022) * tendermint: add From<Infallible> for Error * Use hex for AppHash Debug formatting * Regenerate protobuf types using tools/proto-compiler. * Regenerate protos against tendermint v0.35.0-rc3. * Remove SetOption-related code in tendermint-abci. * Update imports to reflect protobuf movements. The BlockParams and ConsensusParams structs moved out of the ABCI protos. * Update tendermint-abci to reflect upstream proto changes. * p2p: treat all non-Ed25519 keys as unsupported This fixes a compile error introduced by upstream proto changes that add an Sr25519 variant. * Improve ABCI response code modeling with NonZeroU32 The previous data modeling allowed a user to construct an `Err(0)` value that would be serialized and deserialized as `Ok`. * Use the Bytes type in ABCI protos. * Add conversions between protobuf and chrono types. * tendermint: define Serde for Block in terms of RawBlock This changes the Serialize/Deserialize implementations for Block to convert to/from the proto-generated `RawBlock`, and use the derived serialization for that type. This is much cleaner and more maintainable than keeping Serde annotations for each sub-member of the data structure, because all of the serialization code is kept in one place, and there's only one validation path (the TryFrom conversion from RawBlock to Block) that applies to both kinds of serialization. * tendermint: simpler transaction modeling in Block This changes the Block type to hold the transactions as a plain `Vec<Vec<u8>>`, rather than as a custom `abci::transaction::Data` type (which is itself a wrapper for an `Option<Vec<Transaction>>`, the `Transaction` type being a wrapper for a `Vec<u8>`). This also updates the `Block` getter functions; it's not clear (to me) why they're there, since they access the public fields and aren't used anywhere else. * rpc: take over tendermint::abci The existing contents of the `tendermint::abci` module note that they're only for the purpose of implementing the RPC endpoints, not a general ABCI implementation. Moving that code from the `tendermint` crate to the `tendermint-rpc` crate decouples the RPC interface from improvements to the ABCI domain modeling. Eventually, it would be good to eliminate this code and align it with the new ABCI domain types. * tendermint: add ABCI domain types. These types mirror the generated types in tendermint_proto, but have better naming. The documentation for each request type is adapted from the ABCI Methods and Types spec document. However, the same logical request may appear three times, as a struct with the request data, as a Request variant, and as a CategoryRequest variant. To avoid duplication, this PR uses the `#[doc = include_str!(...)]` functionality stabilized in Rust 1.54 to keep common definitions of the documentation. * tendermint: eliminate &'static str errors in ABCI domain types. * Merge `abci::params::ConsensusParams` with `consensus::Params`. The code in the `abci` module had more complete documentation from the ABCI docs, so I copied it onto the existing structure. * Add hex encoding Serde attribute to Sr25519 keys. * Replace integers with `block::Height`, `vote::Power` * Replace integer with block::Round * Fix tools build by using correct imports Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy complaints in tools Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy warning Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix more clippy lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix deprecation notices from ed25519 crate Signed-off-by: Thane Thomson <connect@thanethomson.com> * Regenerate protos for Tendermint v0.35.0 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix raw bytes conversion in tests/docs Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add Tendermint v0.35.0 Docker config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add Tendermint v0.35.0 Docker image for ABCI integration testing Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove RPC deserialization tests The support files for these tests were manually generated some time ago. We should rather favour testing with the `kvstore_fixtures` tests, whose fixtures are automatically generated by our rpc-probe tool. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Reformat Docker folder readme Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in ABCI integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in rpc probe Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in kvstore integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in proto-compiler Signed-off-by: Thane Thomson <connect@thanethomson.com> * Regenerate kvstore fixtures with rpc-probe for Tendermint v0.35.0 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update kvstore integration test to accommodate newly generated fixtures Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update RPC tests and data structures to accommodate Tendermint v0.35.0 changes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update ABCI encoding scheme to accommodate breaking change in tendermint/tendermint#5783 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump Tendermint version in GitHub Actions kvstore integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add changelog entries to capture breaking changes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Change tx hash encoding from base64 to hex and update tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add changelog entry for /tx endpoint change Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Henry de Valence <hdevalence@penumbra.zone> Co-authored-by: Henry de Valence <hdevalence@hdevalence.ca>

hdevalence and others added 30 commits November 15, 2021 16:20

tendermint: add From<Infallible> for Error

3fa00dc

Use hex for AppHash Debug formatting

69f0082

Regenerate protobuf types using tools/proto-compiler.

c43a78e

Regenerate protos against tendermint v0.35.0-rc3.

57dd21b

Remove SetOption-related code in tendermint-abci.

d6c8395

Update imports to reflect protobuf movements.

39a9bd4

The BlockParams and ConsensusParams structs moved out of the ABCI protos.

Update tendermint-abci to reflect upstream proto changes.

6e48efa

p2p: treat all non-Ed25519 keys as unsupported

ee4c84a

This fixes a compile error introduced by upstream proto changes that add an Sr25519 variant.

Improve ABCI response code modeling with NonZeroU32

d9ed69a

The previous data modeling allowed a user to construct an `Err(0)` value that would be serialized and deserialized as `Ok`.

Use the Bytes type in ABCI protos.

4f47712

Add conversions between protobuf and chrono types.

156cce9

tendermint: eliminate &'static str errors in ABCI domain types.

0dcdc6f

Merge abci::params::ConsensusParams with consensus::Params.

4dbbbd0

The code in the `abci` module had more complete documentation from the ABCI docs, so I copied it onto the existing structure.

Add hex encoding Serde attribute to Sr25519 keys.

3c5eff5

Replace integers with block::Height, vote::Power

1f1a4c8

Replace integer with block::Round

9660feb

Fix tools build by using correct imports

2a0ccf4

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix clippy complaints in tools

23a05d4

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix clippy warning

6308759

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix clippy lints

48e9f83

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix more clippy lints

f8cf4d2

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix deprecation notices from ed25519 crate

2b4274d

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Regenerate protos for Tendermint v0.35.0

0b13301

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix raw bytes conversion in tests/docs

f3b8924

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Add Tendermint v0.35.0 Docker config

ee8e2e4

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Add Tendermint v0.35.0 Docker image for ABCI integration testing

cc316cf

Signed-off-by: Thane Thomson <connect@thanethomson.com>

thanethomson added 10 commits November 25, 2021 14:16

Bump version of Tendermint used in ABCI integration test

7651ef7

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Bump version of Tendermint used in rpc probe

9e52c5e

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Bump version of Tendermint used in kvstore integration test

56aa3ab

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Bump version of Tendermint used in proto-compiler

8f176e6

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Regenerate kvstore fixtures with rpc-probe for Tendermint v0.35.0

f1a4823

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Update kvstore integration test to accommodate newly generated fixtures

e4d976e

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Update RPC tests and data structures to accommodate Tendermint v0.35.…

673d780

…0 changes Signed-off-by: Thane Thomson <connect@thanethomson.com>

Update ABCI encoding scheme to accommodate breaking change in tenderm…

8abc428

…int/tendermint#5783 Signed-off-by: Thane Thomson <connect@thanethomson.com>

Bump Tendermint version in GitHub Actions kvstore integration test

2a21f38

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Add changelog entries to capture breaking changes

3663c16

Signed-off-by: Thane Thomson <connect@thanethomson.com>

thanethomson added 2 commits November 25, 2021 19:13

Change tx hash encoding from base64 to hex and update tests

60cba9e

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Add changelog entry for /tx endpoint change

b583aa0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

thanethomson changed the title ~~abci: Add domain types (updated)~~ Add domain types (updated) and fix integration tests for Tendermint v0.35.0 Nov 26, 2021

thanethomson marked this pull request as ready for review November 26, 2021 00:34

thanethomson mentioned this pull request Nov 26, 2021

Replace chrono with time 0.3 #1030

Merged

5 tasks

Merge latest changes from master and fix conflicts

07a8efe

Signed-off-by: Thane Thomson <connect@thanethomson.com>

thanethomson merged commit 0b52add into master Nov 27, 2021

thanethomson deleted the penumbra/abci-domain-types branch November 27, 2021 18:59

thanethomson mentioned this pull request Nov 27, 2021

abci: Add domain types #862

Closed

5 tasks

mzabaluev mentioned this pull request Nov 30, 2021

Panicky, non-validating conversions involving protobuf types and/or domain types #1037

Open

8 tasks

hdevalence mentioned this pull request Dec 8, 2021

Reorganize tendermint::abci to clean up module organization and conform to Rust RFC356 #856

Closed

10 tasks

thanethomson mentioned this pull request Feb 11, 2022

rpc: Remove ABCI-related types #1090

Closed

thanethomson mentioned this pull request Aug 17, 2022

Rollback to Tendermint 0.34 penumbra-zone/penumbra#1271

Closed

5 tasks

thanethomson restored the penumbra/abci-domain-types branch August 17, 2022 23:40

thanethomson mentioned this pull request Aug 25, 2022

abci: Port domain types across from master to main #1187

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add domain types (updated) and fix integration tests for Tendermint v0.35.0 #1022

Add domain types (updated) and fix integration tests for Tendermint v0.35.0 #1022

thanethomson commented Nov 19, 2021 •

edited

Loading

codecov-commenter commented Nov 25, 2021 •

edited

Loading

Add domain types (updated) and fix integration tests for Tendermint v0.35.0 #1022

Add domain types (updated) and fix integration tests for Tendermint v0.35.0 #1022

Conversation

thanethomson commented Nov 19, 2021 • edited Loading

codecov-commenter commented Nov 25, 2021 • edited Loading

Codecov Report

thanethomson commented Nov 19, 2021 •

edited

Loading

codecov-commenter commented Nov 25, 2021 •

edited

Loading