cosmos.staking.v1beta1 protos cause duplicate Validators types in prost-generated Rust code

[tony-iqlusion]

NOTE: this is not so much a "bug" as a bad interaction between the Cosmos SDK protos and the Rust tooling we are using to generate code from them.

Summary of Bug

Rust's most mature protobuf library, prost, does not correctly handle the following:

https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/staking/v1beta1/authz.proto

  // validators is the oneof that represents either allow_list or deny_list
  oneof validators {
    [...]
  }
  // Validators defines list of validator addresses.
  message Validators {
    [...]
  }

Namely prost maps the oneof validators onto an enum Validators, resulting in clashing type names in the generated code:

error[E0428]: the name `Validators` is defined multiple times
    --> cosmos-sdk-proto/src/prost/cosmos.staking.v1beta1.rs:1115:5
     |
1109 |     pub struct Validators {
     |     --------------------- previous definition of the type `Validators` here
...
1115 |     pub enum Validators {
     |     ^^^^^^^^^^^^^^^^^^^ `Validators` redefined here
     |
     = note: `Validators` must be defined only once in the type namespace of this module

See our downstream tracking issue here: cosmos/cosmos-rust#154

Suggested resolution

Rename either oneof validators or message Validators.

Since oneof validators expresses a policy which is either allow_list or deny_list, it might make sense to rename it to oneof policy.

Version

We're currently using v0.45.4 although the issue persists on the main branch.

Steps to Reproduce

$ git clone https://github.com/cosmos/cosmos-rust/
$ cd cosmos-rust/prost-build
$ cargo run # requires Rust is installed

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

cc @aaronc for visibility

[beatriz-web3]

What would be best solution for this issue..
Renaming can cause unexpected results.
I think the best way to solve is to change how prost compile proto files.

[tony-iqlusion]

@siulee2 my original suggestion in the comments was to rename oneof validators to oneof policy.

Renaming can cause unexpected results.

Protobufs are specifically designed to allow renaming of fields without "unexpected results", most notably since every field in a protobuf message is identified by a numeric tag, renaming fields has no impact on serialized messages.

It does, however, change the names used by language-specific bindings, but that should be generated code everywhere.

I think the best way to solve is to change how prost compile proto files.

Apparently prost used to namespace oneof in their own module, but changed to putting everything in the same module because it was too annoying and people specifically requested that extra module be removed.

I'm not sure it's going to be possible to convince them to add it back.

[tac0turtle]

for now we will keep the current state, but in staking v2 we will not cause this issue