07 Client inherits unnecessary and unverified fields from the Tendermint ValidatorSet

[ebuchman]

Summary of Bug

The 07 client uses the Tendermint Go ValidatorSet defined here. This comes with a bunch of data about proposer selection, including a proposerPriority int64 for each validator and aProposer Validator field. There's also a totalVotingPower. None of these fields are checked or necessary for current IBC, and so they can be set arbitrarily in UpdateClient datagrams. Technically the light client does have enough info to verify them if it wants to run the proposer selection algo itself, but there doesn't seem to be a good use for that yet (until/unless we want to punish proposers for bad blocks over ibc, or other potential changes). I think having unverified fields is generally bad since its a recipe for misuse, so might be worth removing this at some point.

This points to a larger issue in divergence between spec and code due to the use of existing implementation types, eg. tendermint protos. This particular issue could be solved upstream in tendermint by breaking up the ValidatorSet type, or in the SDK by using its own ValidatorSet type that just has the list of pubkeys and voting powers. More generally it seems there may be a strong argument for defining canonical IBC proto files in the spec repo.

For Admin Use

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

[mconcat]

Defining a version of ValidatorSet which removes unnecessary fields from the Tendermint ValidatorSet would also be beneficial for shared security.