Make total_voting_power optional

[rootulp]

Motivated by celestiaorg/celestia-core#1052

I'm a Rust noob so would appreciate a thorough review and/or feedback on if this is desirable.

• Referenced an issue explaining the need for the change
• Updated all relevant documentation in docs (I don't see any references to total_voting_power in docs)
• Updated all code comments where relevant
• Wrote tests
• Added entry in .changelog/

[rootulp]

I can't explicitly request a reviewer but I'd appreciate @thanethomson 's opinion on if it makes sense to do this.

[codecov-commenter]

Codecov Report

Merging #1340 (77b6844) into main (c2b5c9e) will increase coverage by 0.3%.
The diff coverage is 75.0%.

❗ Current head 77b6844 differs from pull request most recent head eab906d. Consider uploading reports for the commit eab906d to get more accurate results

@@           Coverage Diff           @@
##            main   #1340     +/-   ##
=======================================
+ Coverage   59.3%   59.6%   +0.3%     
=======================================
  Files        273     272      -1     
  Lines      27095   26929    -166     
=======================================
+ Hits       16068   16071      +3     
+ Misses     11027   10858    -169     

Files Changed	Coverage Δ
light-client-detector/src/evidence.rs	0.0% <0.0%> (ø)
tendermint/src/validator.rs	72.9% <85.7%> (+0.2%)	⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ratankaliani]

Are there any updates on the review status of this PR?

[ratankaliani]

Bump to above ^

[mzabaluev]

As we are looking closer into this change, it raises some deeper questions about the way this field is serialized.

It's not a good idea to denormalize the total voting power value on the validator::Set domain type. Instead, if the field is not present in the deserialized JSON or protobuf, it should be recomputed from the validators' voting power values. There is also an issue of trust with impications on protocol security: if we accept the total voting power value without checking it against the sum of listed validators' powers, this means validator::Set cannot be assumed correct by construction.

We had to skip the field to match the data where it is not present in at least one place in #1292.
Unfortunately, validator::Set has also had Serialize and Deserialize derived, the total_voting_power field included, and this deserialization schema has probably been in use by parties unknown. I think we can relax this into making total_voting_power optional on deserialization, but this changes the schema of the accepted deserialized data. OTOH taking the total voting value without validating that it equals the sum of validator's voting powers implies trust in the sender computing it right anyway, so I doubt we'd be breaking code that was not broken for other reasons.

[mzabaluev]

Closing to implement a proper fix for #1348.