Re-built tendermint-proto with serialization annotations

[greg-szabo]

Closes #634.

This is a research for #638 that adds Serialization support for protobuf 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 CHANGELOG.md

[greg-szabo]

All right, I think this research is a success. In this PR you can see the following:

• JSON serializers are moved into the tendermint-proto crate (proto/src/serializers.rs copied from tendermint) and hidden from the outside (serialization is a similar problem as protobuf encoding: noone cares, it just needs to work)
• protobuf types are annotated so all of them can be JSON serialized (

  tendermint-rs/proto-compiler/build.rs

  Line 23 in 356324a

  (".", "#[derive(::serde::Deserialize, ::serde::Serialize)]"), /* All types should be

  )
• custom serialization annotations are implemented in the proto-compiler crate in an easily expandable way (

  tendermint-rs/proto-compiler/build.rs

  Line 28 in 356324a

  static CUSTOM_FIELD_ATTRIBUTES: &[(&str, &str)] = &[

  ) This is useful for fields that use some weird rule for serialization, for example serializing integers as strings.
• Finally, an example re-implementation of the AbciInfo type with the new serialization annotation (and as a domaintype on the side) shows how serialization is much less of a deal (

  tendermint-rs/tendermint/tests/integration.rs

  Line 69 in 356324a

  #[serde(default, try_from = "ResponseInfo", into = "ResponseInfo")]

  ) This is implemented in a compact way in a test, so it can be compared with the old implementation. Obviously, these will go into the respective files, instead of tests.

If I get a "hey this works" from the team, I'll start revamping other types too, focusing on the types listed in #638 for IBC.

[greg-szabo]

I gave up on trying to organize JSON serialization for now. I've implemented a bunch of extra domain type conversions and moved a lot of the serialization into tendermint-proto, but we'll have to get back and do more structued thinking about JSON serialization.

I need to fix the light-client test failures and I'll implement the last domaintype conversion for Anca and wrap it up for review.

[greg-szabo]

The last test that fails is the model-based single-step tests. There were a few changes in the JSON encoding that doesn't seem to be in testgen. The most prominent one is that the "parts" field was renamed to "part_set_header" both in Go and in this branch in Rust but testgen still seem to generate "parts".

I've added time serialization annotation to some of the testgen tests where I saw it's having a hard time deserializing.

Validator sets now contain a new "total_voting_power" field, but I think testgen doesn't generate that either.

I also found error messages missing "description" and "model" fields in the testgen structs but I don't know exactly what's missing there.

I'll need @andrey-kuprianov 's help to fix the remaining issues.

[codecov-io]

Codecov Report

Merging #639 into master will decrease coverage by 6.3%.
The diff coverage is 48.8%.

@@           Coverage Diff            @@
##           master    #639     +/-   ##
========================================
- Coverage    45.2%   38.9%   -6.4%     
========================================
  Files         175     184      +9     
  Lines       12113   12800    +687     
  Branches     2724    2929    +205     
========================================
- Hits         5480    4984    -496     
- Misses       6350    7572   +1222     
+ Partials      283     244     -39     

Impacted Files	Coverage Δ
light-client/src/builder/light_client.rs	0.0% <0.0%> (ø)
light-client/src/components/io.rs	0.0% <0.0%> (ø)
light-client/src/predicates/errors.rs	19.0% <ø> (-33.4%)	⬇️
light-client/src/supervisor.rs	28.6% <0.0%> (ø)
light-client/src/tests.rs	47.4% <ø> (-16.6%)	⬇️
light-client/src/types.rs	28.4% <0.0%> (-4.3%)	⬇️
light-client/tests/model_based.rs	0.2% <0.0%> (-83.6%)	⬇️
light-node/src/rpc.rs	53.5% <ø> (ø)
proto-compiler/src/constants.rs	0.0% <0.0%> (ø)
proto/src/lib.rs	100.0% <ø> (ø)
... and 114 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 e5484f5...5f8fe33. Read the comment docs.

[ancazamfir]

Can we specify the revision to a commit hash in the proto-compiler?. Current ref used by gaia and go relayer stargate-4 release is:
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6

[ancazamfir]

Missed this one:
We have some Merkle related structs here:

tendermint-rs/proto/src/prost/tendermint.crypto.rs

Lines 34 to 47 in 820b634

	pub struct ProofOp {
	#[prost(string, tag="1")]
	pub r#type: std::string::String,
	#[prost(bytes, tag="2")]
	pub key: std::vec::Vec<u8>,
	#[prost(bytes, tag="3")]
	pub data: std::vec::Vec<u8>,
	}
	/// ProofOps is Merkle proof defined by the list of ProofOps
	#[derive(Clone, PartialEq, ::prost::Message)]
	pub struct ProofOps {
	#[prost(message, repeated, tag="1")]
	pub ops: ::std::vec::Vec<ProofOp>,
	}

And similar here:

tendermint-rs/tendermint/src/merkle/proof.rs

Lines 9 to 29 in 820b634

	pub struct Proof {
	/// The list of ProofOps
	pub ops: Vec<ProofOp>,
	}
	/// ProofOp defines an operation used for calculating Merkle root
	/// The data could be arbitrary format, providing necessary data
	/// for example neighbouring node hash
	/// <https://github.com/tendermint/tendermint/blob/c8483531d8e756f7fbb812db1dd16d841cdf298a/crypto/merkle/merkle.proto#L19>
	#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
	pub struct ProofOp {
	/// Type of the ProofOp
	#[serde(alias = "type")]
	pub field_type: String,
	/// Key of the ProofOp
	#[serde(default, with = "serializers::bytes::base64string")]
	pub key: Vec<u8>,
	/// Actual data
	#[serde(default, with = "serializers::bytes::base64string")]
	pub data: Vec<u8>,
	}

Could tendermint use the proto ones? Or impl the domain types?
IBC gets the domain types from abci queries and needs to serialize into the protobuf ones.

[thanethomson]

So, a lot of work and thought and deliberation have gone into this PR and what the next steps from here should be. We're not yet where we need to be yet for v0.17.0, but I think it's important to at least push out a v0.17.0-rc2 release with the disclaimer that some more data structure changes are coming in v0.17.0-rc3. At the very least, rc2 will cater to IBC's immediate needs. Other consumers may want to wait until rc3 to upgrade.

The main thing that'd be good to fix right now before the v0.17.0-rc2 release is to get the Light Client's model-based single-step tests working with the new data structures introduced in this PR. cc @Shivani912 and @andrey-kuprianov

I'm keeping this branch around for a bit until IBC can base off of v0.17.0-rc2 (or master).

[adizere]

FYI -- ibc-rs master no longer depends on branch greg/research-json, so it should be safe to delete from that perspective!