Add autogenerated protos for `v1.0` by sergio-mena · Pull Request #1381 · cometbft/tendermint-rs

sergio-mena · 2023-11-19T11:28:42Z

This (draft) PR is the first to providing support for CometBFT v1.0.

The PR appears to be massive but the reason is the tons of newly auto-generated code it introduces. For this reason, please don't review it in one shot; rather use the following description of commits:

Commit 1: Ignore. It is the result of running the protobuf generation on my machine, which produces a few changes in the auto-generated code.
Commit 2: Review. First step: add the logic to handle both Tendermint and CometBFT protobufs. We might decide to deprecate the Tendermint part.
Commit 3: Ignore. The effect in auto-generated code of commit 2.
Commit 4: Review. Adapted protobuf generation tweaks to CombetBFT protos. Commit 2, just copy-pasted the Tendermint ones. This commit allows to see how I adapted them.
Commit 5: Ignore. The effect in auto-generated code of commit 4.
Commit 6: Review. As there are several levels of modules now, the preexisting mechanism to generate the modules was too rigid. This commit introduces and extended version
Commit 7: Ignore. The effect in auto-generated code of commit 6.
Commit 8: Review. Fixes needed in the manual code so that the new proto bufs builds correctly
Commit 9: Review. Changelog.

Note: even if this PR is likely to be ready for review, I decided to leave it a draft PR because we need to discuss whether we should be forking tendermint-rs into cometbft-rs and move this PR there.

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/

sergio-mena · 2023-11-19T11:34:32Z

tools/proto-compiler/src/constants.rs

+    (".cometbft.libs.bits.BitArray", SERIALIZED),//TODO Revisit
+    (".cometbft.types.BlockIDFlag", PRIMITIVE_ENUM),
+    (".cometbft.types.Block", SERIALIZED),
+    (".cometbft.types.Data", SERIALIZED),
+    (".cometbft.types.EvidenceParams", SERIALIZED),
+    (".cometbft.types.Evidence.sum", SERIALIZED),
+    (".cometbft.types.Evidence.sum", TYPE_TAG),
+    (".cometbft.types.EvidenceList", SERIALIZED),
+    (".cometbft.types.DuplicateVoteEvidence", SERIALIZED),
+    (".cometbft.types.Vote", SERIALIZED),
+    (".cometbft.types.BlockID", SERIALIZED),
+    (".cometbft.types.PartSetHeader", SERIALIZED),
+    (".cometbft.types.LightClientAttackEvidence", SERIALIZED),
+    (
+        ".cometbft.types.LightClientAttackEvidence",
+        RENAME_ALL_PASCALCASE,
+    ),
+    (".cometbft.types.LightBlock", SERIALIZED),
+    (".cometbft.types.SignedHeader", SERIALIZED),
+    (".cometbft.types.Header", SERIALIZED),
+    (".cometbft.version.Consensus", SERIALIZED),
+    (".cometbft.types.Commit", SERIALIZED),
+    (".cometbft.types.CommitSig", SERIALIZED),
+    (".cometbft.types.ValidatorSet", SERIALIZED),
+    (".cometbft.crypto.PublicKey.sum", SERIALIZED),
+    (".cometbft.crypto.PublicKey.sum", TYPE_TAG),
+    (".cometbft.abci.ResponseInfo", SERIALIZED),
+    (".cometbft.types.CanonicalBlockID", SERIALIZED),
+    (".cometbft.types.CanonicalPartSetHeader", SERIALIZED),
+    (".cometbft.types.Validator", SERIALIZED),
+    (".cometbft.types.CanonicalVote", SERIALIZED),
+    (".cometbft.types.BlockMeta", SERIALIZED),
+    (".cometbft.types.TxProof", SERIALIZED),
+    (".cometbft.crypto.Proof", SERIALIZED),


Please ignore these changes: review them in Commit 4

sergio-mena · 2023-11-19T11:35:07Z

tools/proto-compiler/src/constants.rs

+    (
+        ".cometbft.types.EvidenceParams.max_bytes",
+        QUOTED_WITH_DEFAULT,
+    ), //TODO Revisit
+    (
+        ".cometbft.types.EvidenceParams.max_age_num_blocks",
+        QUOTED_WITH_DEFAULT,
+    ),
+    (".cometbft.version.Consensus.block", QUOTED),
+    (".cometbft.version.Consensus.app", QUOTED_WITH_DEFAULT),
+    (".cometbft.abci.ResponseInfo.data", DEFAULT),
+    (".cometbft.abci.ResponseInfo.version", DEFAULT),
+    (
+        ".cometbft.abci.ResponseInfo.app_version",
+        QUOTED_WITH_DEFAULT,
+    ),
+    (
+        ".cometbft.abci.ResponseInfo.last_block_height",
+        QUOTED_WITH_DEFAULT,
+    ),
+    (".cometbft.abci.ResponseInfo.last_block_app_hash", DEFAULT),
+    (
+        ".cometbft.abci.ResponseInfo.last_block_app_hash",
+        BYTES_SKIP_IF_EMPTY,
+    ),
+    (".cometbft.types.BlockID.hash", HEXSTRING),
+    (".cometbft.types.BlockID.part_set_header", RENAME_PARTS),
+    (
+        ".cometbft.types.PartSetHeader.total",
+        PART_SET_HEADER_TOTAL,
+    ),
+    (".cometbft.types.PartSetHeader.hash", HEXSTRING),
+    (".cometbft.types.Header.height", QUOTED),
+    (".cometbft.types.Header.time", OPTIONAL),
+    (".cometbft.types.Header.last_commit_hash", HEXSTRING),
+    (".cometbft.types.Header.data_hash", HEXSTRING),
+    (".cometbft.types.Header.validators_hash", HEXSTRING),
+    (".cometbft.types.Header.next_validators_hash", HEXSTRING),
+    (".cometbft.types.Header.consensus_hash", HEXSTRING),
+    (".cometbft.types.Header.app_hash", HEXSTRING),
+    (".cometbft.types.Header.last_results_hash", HEXSTRING),
+    (".cometbft.types.Header.evidence_hash", HEXSTRING),
+    (".cometbft.types.Header.proposer_address", HEXSTRING),
+    (".cometbft.types.Data.txs", NULLABLEVECARRAY),
+    (".cometbft.types.EvidenceList.evidence", NULLABLE),
+    (".cometbft.types.Commit.height", QUOTED),
+    (".cometbft.types.Commit.signatures", NULLABLE),
+    (".cometbft.types.CommitSig.validator_address", HEXSTRING),
+    (".cometbft.types.CommitSig.timestamp", OPTIONAL),
+    (".cometbft.types.CommitSig.signature", BASE64STRING),
+    (
+        ".cometbft.types.DuplicateVoteEvidence.total_voting_power",
+        RENAME_TOTAL_VOTING_POWER_QUOTED,
+    ),
+    (
+        ".cometbft.types.DuplicateVoteEvidence.validator_power",
+        RENAME_VALIDATOR_POWER_QUOTED,
+    ),
+    (
+        ".cometbft.types.DuplicateVoteEvidence.timestamp",
+        RENAME_TIMESTAMP,
+    ),
+    (
+        ".cometbft.types.LightClientAttackEvidence.common_height",
+        QUOTED,
+    ),
+    (
+        ".cometbft.types.LightClientAttackEvidence.total_voting_power",
+        QUOTED,
+    ),
+    (".cometbft.types.Vote.height", QUOTED),
+    (".cometbft.types.Vote.validator_address", HEXSTRING),
+    (".cometbft.types.Vote.signature", BASE64STRING),
+    (".cometbft.types.Vote.timestamp", OPTIONAL),
+    (".cometbft.types.Validator.address", HEXSTRING),
+    (
+        ".cometbft.types.Validator.voting_power",
+        ALIAS_POWER_QUOTED,
+    ), // https://github.com/tendermint/tendermint/issues/5549
+    (
+        ".cometbft.types.Validator.proposer_priority",
+        QUOTED_ALLOW_NULL,
+    ), // null occurs in some LightBlock data
+    (".cometbft.types.Validator.proposer_priority", DEFAULT), // Default is for /genesis deserialization
+    (
+        ".cometbft.types.ValidatorSet.total_voting_power",
+        QUOTED_WITH_DEFAULT,
+    ),
+    (
+        ".cometbft.types.ValidatorSet.total_voting_power",
+        SKIP_SERIALIZING,
+    ),
+    (".cometbft.types.BlockMeta.block_size", QUOTED),
+    (".cometbft.types.BlockMeta.num_txs", QUOTED),
+    (".cometbft.crypto.PublicKey.sum.ed25519", RENAME_EDPUBKEY),
+    (
+        ".cometbft.crypto.PublicKey.sum.secp256k1",
+        RENAME_SECPPUBKEY,
+    ),
+    (".cometbft.crypto.PublicKey.sum.sr25519", RENAME_SRPUBKEY),
+    (
+        ".cometbft.types.Evidence.sum.duplicate_vote_evidence",
+        RENAME_DUPLICATEVOTE,
+    ),
+    (
+        ".cometbft.types.Evidence.sum.light_client_attack_evidence",
+        RENAME_LIGHTCLIENTATTACK,
+    ),
+    (".cometbft.types.TxProof.data", BASE64STRING),
+    (".cometbft.types.TxProof.root_hash", HEXSTRING),
+    (".cometbft.crypto.Proof.index", QUOTED),
+    (".cometbft.crypto.Proof.total", QUOTED),
+    (".cometbft.crypto.Proof.aunts", VEC_BASE64STRING),
+    (".cometbft.crypto.Proof.leaf_hash", BASE64STRING),


Please ignore these changes: review them in Commit 4

mzabaluev

Good automation for the module tree.

I don't think we need to namespace the cometbft generated files under module v1_0.
Remember the idea of ADR-107 is to eventually promote the message definitions to *.v1 packages at the proto level. So abci.v1beta4.Request becomes abci.v1.Request and so on. It's better that we generate the root module under proto/cometbft/lib.rs. Further evolution of the protos will add v2 packages alongside if needed, as designed.

I don't see a need to hold on to proto::tendermint, modules either. We're making a switch for cometbft-rs, the legacy generated files will not be used by the rest of the cometbft-rs crates, so they are pretty much dead code. @thanethomson had the idea that the users who need support for tendermint.* protos could still use the tendermint-rs crates as already released, and we can still support those from this current repository in maintenance mode.

mzabaluev · 2023-11-20T10:07:09Z

tools/proto-compiler/src/functions.rs

                && e.file_name()
                    .to_str()
-                    .map(|name| name.starts_with("tendermint."))
+                    .map(|name| name.starts_with(&format!("{project}.")))


Performance nit: it would be better if the prefix string was formatted once, not with every invocation of the closure.

mzabaluev · 2023-11-20T10:07:28Z

tools/proto-compiler/src/functions.rs

        .filter(|e| {
            e.file_type().is_file()
-                && e.file_name().to_str().unwrap().starts_with("tendermint.")
+                && e.file_name().to_str().unwrap().starts_with(&project_dot)


Right, like here.

sergio-mena · 2023-12-07T10:31:17Z

This is continued in cometbft/cometbft-rs#2

sergio-mena added 8 commits November 17, 2023 15:11

Pre: auto changes

99e961d

Added cometbft v1_0 to protobuf generation

a935338

Auto generated protobuf code

95a2621

Adapted custom attributes for cometbft types

659ad9c

Auto generated protobuf code after adapting custom attributes

f60e631

Extended mod generation to support several sub-mods

0e70404

Auto generated protobuf code after extended mod generation

42a3555

Fix build of protos crate

dd1fcb8

sergio-mena self-assigned this Nov 19, 2023

Changelog

03e3aab

sergio-mena commented Nov 19, 2023

View reviewed changes

mzabaluev reviewed Nov 20, 2023

View reviewed changes

Addressed @mzabaluev's comments

f5ddeb1

sergio-mena mentioned this pull request Nov 24, 2023

proto: Coordinate renaming/versioning changes with users cometbft/cometbft#1403

Closed

2 tasks

sergio-mena mentioned this pull request Dec 7, 2023

Add autogenerated protos for v1.0 cometbft/cometbft-rs#2

Merged

5 tasks

sergio-mena closed this Dec 7, 2023

sergio-mena deleted the sergio/proto-renaming branch December 15, 2023 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autogenerated protos for `v1.0`#1381

Add autogenerated protos for `v1.0`#1381
sergio-mena wants to merge 10 commits intomainfrom
sergio/proto-renaming

sergio-mena commented Nov 19, 2023 •

edited

Loading

Uh oh!

sergio-mena Nov 19, 2023

Uh oh!

sergio-mena Nov 19, 2023

Uh oh!

mzabaluev left a comment •

edited

Loading

Uh oh!

mzabaluev Nov 20, 2023

Uh oh!

mzabaluev Nov 20, 2023

Uh oh!

sergio-mena commented Dec 7, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sergio-mena commented Nov 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergio-mena Nov 19, 2023

Choose a reason for hiding this comment

Uh oh!

sergio-mena Nov 19, 2023

Choose a reason for hiding this comment

Uh oh!

mzabaluev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzabaluev Nov 20, 2023

Choose a reason for hiding this comment

Uh oh!

mzabaluev Nov 20, 2023

Choose a reason for hiding this comment

Uh oh!

sergio-mena commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sergio-mena commented Nov 19, 2023 •

edited

Loading

mzabaluev left a comment •

edited

Loading

sergio-mena commented Dec 7, 2023 •

edited

Loading