Validate all relevant square sizes are committed to in MsgWirePayForData

[rootulp]

Closes #620 (after #654 merges)

[rootulp]

Sorry for prematurely moving this from draft to ready for review. Tests passed locally but CI is failing on TestIntegrationTestSuite which presumably doesn't run locally. Will fix and tag reviewers when this PR is ready.

UPDATE: ready for review. The integ test failure was a known flake: #574

[evan-forbes]

left a few comments 🙂

this code is really important because it makes arranging the square way easier when the block proposer knows that all the txs in their mempool are signed over all the relevant square sizes.

[codecov-commenter]

Codecov Report

Merging #666 (e6c18ca) into main (13076a6) will increase coverage by 0.47%.
The diff coverage is 73.52%.

@@            Coverage Diff             @@
##             main     #666      +/-   ##
==========================================
+ Coverage   36.06%   36.54%   +0.47%     
==========================================
  Files          24       24              
  Lines        2623     2657      +34     
==========================================
+ Hits          946      971      +25     
- Misses       1594     1600       +6     
- Partials       83       86       +3     

Impacted Files	Coverage Δ
pkg/shares/parse_message_shares.go	88.60% <ø> (ø)
x/payment/types/wirepayfordata.go	66.35% <73.52%> (+1.33%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[evan-forbes]

just to note that this is consensus breaking

[evan-forbes]

approving due to #677

[evan-forbes]

I'm dumb, this is probably not consensus breaking. It will be nice when we have tests tho

[rootulp]

I think this is consensus breaking b/c prior to this change, validators would've accepted PFDs that didn't sign over all commits. After this change, those PFDs are considered invalid.

Consensus tests would be very nice. +1 to #313

[evan-forbes]

After this change, those PFDs are considered invalid

We didn't actually change anything for the ValidateBasic method of PFD, only the wire message, correct?

[rootulp]

Correct only the wire message, sorry for the lack of precision. Why does that have implications on consensus breaking vs non consensus breaking?

[evan-forbes]

Then that should only affect the mempool, as even if a WireMsg makes it in the block, then all nodes should treat that the same, which is to subtract the minimum amount of gas.