Conversation
|
👋 Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been
Thank you for your contribution to Tendermint! 🚀 |
Codecov Report
@@ Coverage Diff @@
## master #4985 +/- ##
==========================================
- Coverage 63.74% 63.70% -0.05%
==========================================
Files 181 181
Lines 19473 19483 +10
==========================================
- Hits 12414 12412 -2
- Misses 6000 6010 +10
- Partials 1059 1061 +2
|
| @@ -47,9 +48,11 @@ func exampleVote(t byte) *Vote { | |||
|
|
|||
| func TestVoteSignable(t *testing.T) { | |||
There was a problem hiding this comment.
Ah nice, the test-vectors are still here 👍
IMO anything consensus critical should have test-vectors and these test-vectors should live in the spec (or at least should be referenced in the spec).
There was a problem hiding this comment.
I will add specific ones for canonical. I can also add them for msgs in most reactors. Best to be safe
| signBytes := VoteSignBytes("test_chain_id", v) | ||
| pb := CanonicalizeVote("test_chain_id", v) | ||
|
|
||
| expected, err := cdc.MarshalBinaryLengthPrefixed(CanonicalizeVote("test_chain_id", vote)) |
There was a problem hiding this comment.
This is also relevant to the ledger lib that @jleni wrote as it will expect this length prefix 😬
I'm actually not sure if someone uses the ledger to sign votes etc but I guess as it is possible, this should come with a prominent note in the changelog.
Description
migration of the privval reactor to protobuf
this PR will be superseded by the gRPC PR for all client and server implementations
Closes: #2830