Add --build-tonic flag to proto-compiler, sync protobuf script, and generate protobuf for std and no_std mode

[soareschen]

Part of #1158

Description

• Add a --build-tonic flag to proto-compiler to allow generation of protobuf definitions without tonic client for no_std.
• Add ci/sync-protobuf.sh script to reliably re-generate the protobuf Rust definitions. Note however that the current protobuf-compiler is non-reproducible, so multiple calls to ci/sync-protobuf.sh may yield different results.
• Add std feature flag in ibc-proto to include tonic as dependency when activated.
• Make ibc-proto load protobuf definitions from either proto/src/prost/std or proto/src/prost/no_std depending on the std feature flag.
• The commits document detailed step to arrive to the current protobuf generation, to convince the reviewer that there is indeed no change in the protobuf definition. This is necessary because the proto-compiler being non-reproducible makes it difficult to check the diff and verify that there is indeed no changes in the generated files.

---

For contributor use:

• Added a changelog entry, using unclog.
• If applicable: Unit tests written, added test to CI.
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation (docs/) and code comments.
• Re-reviewed Files changed in the Github PR explorer.

[romac]

Very nice! Thanks for including the script as well, which allows us to skip the slow libgit2 clone step and call compile directly :)

[romac]

@adizere @soareschen Do we want to include this in 0.8.0?

[soareschen]

Do we want to include this in 0.8.0?

I think this should be merged after we switched to the official versions of prost and tonic in #1502. Right now it seems like the protobuf definitions regenerated using our forked tonic-build contain errors like:

error[E0277]: the trait bound `tendermint_proto::types::Header: informalsystems_prost::Message` is not satisfied                                                             
    --> proto/src/prost/std/cosmos.staking.v1beta1.rs:5:28                                                                                                                   
     |                                                                                                                                                                       
5    | #[derive(Clone, PartialEq, ::prost::Message)]                                                                                                                         
     |                            ^^^^^^^^^^^^^^^^ the trait `informalsystems_prost::Message` is not implemented for `tendermint_proto::types::Header`                       
     |                                                                                                                                                                       
note: required by a bound in `informalsystems_prost::encoding::message::encode`                                                                                              
    --> /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/informalsystems-prost-0.8.1/src/encoding.rs:1043:12                                                     
     |                                                                                                                                                                       
1043 |         M: Message,
     |            ^^^^^^^ required by this bound in `informalsystems_prost::encoding::message::encode`
     = note: this error originates in the derive macro `::prost::Message` (in Nightly builds, run with -Z macro-backtrace for more info)

[adizere]

Leaving this as draft until it gets prioritized again.

[mzabaluev]

Ah, I've read further to understand that these are just mirrors and they are fetched from the remote every time the script is invoked. Still, the point about trusting well-known file paths in /tmp stands. Maybe mirror these repos under ${XDG_CACHE_HOME:-$HOME/.cache}/ibc-rs.build?

[soareschen]

That could work as well. Compared to the original, we want to use a local git bare repository as cache instead of trying to clone the entire remote repository every time the command is run.

Right now I still have the problem that if a bare repository has already been cloned before, I can't find simple way to update it other than having to reclone the entire bare repository. So it is still a little problematic to set a persistent location at $HOME by default.

[mzabaluev]

Use of non-randomized paths in /tmp also assumes that only a single user will ever run this script (over the host's uptime before /tmp is cleared). This is probably OK for a typical developer's workstation, but using a location at $HOME by convention gives you better isolation.

Right now I still have the problem that if a bare repository has already been cloned before, I can't find simple way to update it

I've tried locally and git fetch or git fetch origin seem to work. What's the problem?

[soareschen]

I've tried locally and git fetch or git fetch origin seem to work. What's the problem?

Hmm I might have cloned the bare repository inappropriately. That seems to work now!

I have updated the script as you suggested. It stores the git cache at ~/.cache/cosmos.

[mzabaluev]

It stores the git cache at ~/.cache/cosmos.

This might conceivably clash with some Cosmos client in the future (or just an unrelated app named cosmos), I'd rather use something identifying the ibc-rs build scripts as the well-known directory under ~/.cache and put everything cached by the scripts in there. But this is mostly pedantry, thanks for fixing the main issue.

[adizere]

It would be preferable to merge this after #1582.

Not sure what our merge strategy could be to help prevent regressions or problems, since manually reviewing it is impossible. Any thoughts?

[adizere]

Do we need to maintain this file under revision?

[romac]

Are we ready to merge this?

[mzabaluev]

Needs a merge from master at least.