non breaking signbytes (#5008)

* test-vectors for backwards compatibility:

 - copy & paste test-vectors from v0.33.5 to ensure
 backwards compatibility for vote's SignBytes

* WIP: everything besides time seems to match :-/

* almost

* Found the culprit: field nums weren't consecutive ints ...

* fix order of partset header too

* this last votes-related test can easily be fixed

* some minor changes and fix last failing test

* move proto types back to stdtime, fix various linting

* use libs/protoio

* remvoe commented code

* add comments

* fix tests

* uncomment testscases

* dont ignore error panic

* fix signable test

* fix happy path testing

* fix comment

Co-authored-by: Marko Baricevic <marbar3778@yahoo.com>
This commit is contained in:
Ismail Khoffi
2020-06-15 21:54:58 +02:00
committed by GitHub
co-authored by Marko Baricevic
parent b8b50733f0
commit 4774a8ec61
20 changed files with 365 additions and 273 deletions
+1
View File
@@ -1306,6 +1306,7 @@ func BlockIDFromProto(bID *tmproto.BlockID) (*BlockID, error) {
if bID == nil {
return nil, errors.New("nil BlockID")
}
blockID := new(BlockID)
ph, err := PartSetHeaderFromProto(&bID.PartsHeader)
if err != nil {
+20 -12
View File
@@ -15,25 +15,33 @@ const TimeFormat = time.RFC3339Nano
//-----------------------------------
// Canonicalize the structs
func CanonicalizeBlockID(blockID tmproto.BlockID) tmproto.CanonicalBlockID {
return tmproto.CanonicalBlockID{
Hash: blockID.Hash,
PartsHeader: CanonicalizePartSetHeader(blockID.PartsHeader),
func CanonicalizeBlockID(bid tmproto.BlockID) *tmproto.CanonicalBlockID {
rbid, err := BlockIDFromProto(&bid)
if err != nil {
panic(err)
}
var cbid *tmproto.CanonicalBlockID
if rbid == nil || rbid.IsZero() {
cbid = nil
} else {
cbid = &tmproto.CanonicalBlockID{
Hash: bid.Hash,
PartsHeader: CanonicalizePartSetHeader(bid.PartsHeader),
}
}
return cbid
}
func CanonicalizePartSetHeader(psh tmproto.PartSetHeader) tmproto.CanonicalPartSetHeader {
return tmproto.CanonicalPartSetHeader{
Hash: psh.Hash,
Total: psh.Total,
}
return tmproto.CanonicalPartSetHeader(psh)
}
func CanonicalizeProposal(chainID string, proposal *tmproto.Proposal) tmproto.CanonicalProposal {
return tmproto.CanonicalProposal{
Type: tmproto.ProposalType,
Height: proposal.Height,
Round: int64(proposal.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
Height: proposal.Height, // encoded as sfixed64
Round: int64(proposal.Round), // encoded as sfixed64
POLRound: int64(proposal.PolRound),
BlockID: CanonicalizeBlockID(proposal.BlockID),
Timestamp: proposal.Timestamp,
@@ -44,8 +52,8 @@ func CanonicalizeProposal(chainID string, proposal *tmproto.Proposal) tmproto.Ca
func CanonicalizeVote(chainID string, vote *tmproto.Vote) tmproto.CanonicalVote {
return tmproto.CanonicalVote{
Type: vote.Type,
Height: vote.Height,
Round: int64(vote.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
Height: vote.Height, // encoded as sfixed64
Round: int64(vote.Round), // encoded as sfixed64
BlockID: CanonicalizeBlockID(vote.BlockID),
Timestamp: vote.Timestamp,
ChainID: chainID,
+39
View File
@@ -0,0 +1,39 @@
package types
import (
"reflect"
"testing"
"github.com/tendermint/tendermint/crypto/tmhash"
tmrand "github.com/tendermint/tendermint/libs/rand"
tmproto "github.com/tendermint/tendermint/proto/types"
)
func TestCanonicalizeBlockID(t *testing.T) {
randhash := tmrand.Bytes(tmhash.Size)
block1 := tmproto.BlockID{Hash: randhash,
PartsHeader: tmproto.PartSetHeader{Total: 5, Hash: randhash}}
block2 := tmproto.BlockID{Hash: randhash,
PartsHeader: tmproto.PartSetHeader{Total: 10, Hash: randhash}}
cblock1 := tmproto.CanonicalBlockID{Hash: randhash,
PartsHeader: tmproto.CanonicalPartSetHeader{Total: 5, Hash: randhash}}
cblock2 := tmproto.CanonicalBlockID{Hash: randhash,
PartsHeader: tmproto.CanonicalPartSetHeader{Total: 10, Hash: randhash}}
tests := []struct {
name string
args tmproto.BlockID
want *tmproto.CanonicalBlockID
}{
{"first", block1, &cblock1},
{"second", block2, &cblock2},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
if got := CanonicalizeBlockID(tt.args); !reflect.DeepEqual(got, tt.want) {
t.Errorf("CanonicalizeBlockID() = %v, want %v", got, tt.want)
}
})
}
}
+5 -5
View File
@@ -5,9 +5,8 @@ import (
"fmt"
"time"
"github.com/gogo/protobuf/proto"
"github.com/tendermint/tendermint/libs/bytes"
tmbytes "github.com/tendermint/tendermint/libs/bytes"
"github.com/tendermint/tendermint/libs/protoio"
tmproto "github.com/tendermint/tendermint/proto/types"
tmtime "github.com/tendermint/tendermint/types/time"
)
@@ -87,17 +86,18 @@ func (p *Proposal) String() string {
p.Round,
p.BlockID,
p.POLRound,
bytes.Fingerprint(p.Signature),
tmbytes.Fingerprint(p.Signature),
CanonicalTime(p.Timestamp))
}
// ProposalSignBytes returns the Proposal bytes for signing
func ProposalSignBytes(chainID string, p *tmproto.Proposal) []byte {
pb := CanonicalizeProposal(chainID, p)
bz, err := proto.Marshal(&pb)
bz, err := protoio.MarshalDelimited(&pb)
if err != nil {
panic(err)
}
return bz
}
+8 -5
View File
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto/tmhash"
"github.com/tendermint/tendermint/libs/protoio"
tmrand "github.com/tendermint/tendermint/libs/rand"
tmproto "github.com/tendermint/tendermint/proto/types"
)
@@ -25,9 +26,10 @@ func init() {
panic(err)
}
testProposal = &Proposal{
Height: 12345,
Round: 23456,
BlockID: BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}},
Height: 12345,
Round: 23456,
BlockID: BlockID{Hash: []byte("--June_15_2020_amino_was_removed"),
PartsHeader: PartSetHeader{Total: 111, Hash: []byte("--June_15_2020_amino_was_removed")}},
POLRound: -1,
Timestamp: stamp,
}
@@ -38,14 +40,15 @@ func TestProposalSignable(t *testing.T) {
chainID := "test_chain_id"
signBytes := ProposalSignBytes(chainID, pbp)
pb := CanonicalizeProposal(chainID, pbp)
expected, err := proto.Marshal(&pb)
expected, err := protoio.MarshalDelimited(&pb)
require.NoError(t, err)
require.Equal(t, expected, signBytes, "Got unexpected sign bytes for Proposal")
}
func TestProposalString(t *testing.T) {
str := testProposal.String()
expected := `Proposal{12345/23456 (010203:111:626C6F636B70, -1) 000000000000 @ 2018-02-11T07:09:22.765Z}`
expected := `Proposal{12345/23456 (2D2D4A756E655F31355F323032305F616D696E6F5F7761735F72656D6F766564:111:2D2D4A756E65, -1) 000000000000 @ 2018-02-11T07:09:22.765Z}` //nolint:lll // ignore line length for tests
if str != expected {
t.Errorf("got unexpected string for Proposal. Expected:\n%v\nGot:\n%v", expected, str)
}
+3 -3
View File
@@ -6,10 +6,9 @@ import (
"fmt"
"time"
"github.com/gogo/protobuf/proto"
"github.com/tendermint/tendermint/crypto"
tmbytes "github.com/tendermint/tendermint/libs/bytes"
"github.com/tendermint/tendermint/libs/protoio"
tmproto "github.com/tendermint/tendermint/proto/types"
)
@@ -87,10 +86,11 @@ func (vote *Vote) CommitSig() CommitSig {
// If any error arises this will panic
func VoteSignBytes(chainID string, vote *tmproto.Vote) []byte {
pb := CanonicalizeVote(chainID, vote)
bz, err := proto.Marshal(&pb)
bz, err := protoio.MarshalDelimited(&pb)
if err != nil {
panic(err)
}
return bz
}
+47 -13
View File
@@ -12,6 +12,7 @@ import (
"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/ed25519"
"github.com/tendermint/tendermint/crypto/tmhash"
"github.com/tendermint/tendermint/libs/protoio"
tmproto "github.com/tendermint/tendermint/proto/types"
)
@@ -51,8 +52,7 @@ func TestVoteSignable(t *testing.T) {
v := vote.ToProto()
signBytes := VoteSignBytes("test_chain_id", v)
pb := CanonicalizeVote("test_chain_id", v)
expected, err := proto.Marshal(&pb)
expected, err := protoio.MarshalDelimited(&pb)
require.NoError(t, err)
require.Equal(t, expected, signBytes, "Got unexpected sign bytes for Vote.")
@@ -68,38 +68,72 @@ func TestVoteSignBytesTestVectors(t *testing.T) {
0: {
"", &Vote{},
// NOTE: Height and Round are skipped here. This case needs to be considered while parsing.
[]byte{0x2a, 0x2, 0x12, 0x0, 0x32, 0xb, 0x8, 0x80, 0x92, 0xb8,
0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
[]byte{0xd, 0x2a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
},
// with proper (fixed size) height and round (PreCommit):
1: {
"", &Vote{Height: 1, Round: 1, Type: tmproto.PrecommitType},
[]byte{0x8, 0x2, 0x10, 0x1, 0x18, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x32,
[]byte{
0x21, // length
0x8, // (field_number << 3) | wire_type
0x2, // PrecommitType
0x11, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
0x19, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
0x2a, // (field_number << 3) | wire_type
// remaining fields (timestamp):
0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
},
// with proper (fixed size) height and round (PreVote):
2: {
"", &Vote{Height: 1, Round: 1, Type: tmproto.PrevoteType},
[]byte{0x8, 0x1, 0x10, 0x1, 0x18, 0x1, 0x2a, 0x2, 0x12, 0x0,
0x32, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
[]byte{
0x21, // length
0x8, // (field_number << 3) | wire_type
0x1, // PrevoteType
0x11, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
0x19, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
0x2a, // (field_number << 3) | wire_type
// remaining fields (timestamp):
0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
},
3: {
"", &Vote{Height: 1, Round: 1},
[]byte{0x10, 0x1, 0x18, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x32, 0xb,
0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
[]byte{
0x1f, // length
0x11, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
0x19, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
// remaining fields (timestamp):
0x2a,
0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
},
// containing non-empty chain_id:
4: {
"test_chain_id", &Vote{Height: 1, Round: 1},
[]byte{0x10, 0x1, 0x18, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x32, 0xb, 0x8,
0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x3a, 0xd,
0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64},
[]byte{
0x2e, // length
0x11, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
0x19, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
// remaining fields:
0x2a, // (field_number << 3) | wire_type
0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, // timestamp
// (field_number << 3) | wire_type
0x32,
0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64}, // chainID
},
}
for i, tc := range tests {
v := tc.vote.ToProto()
got := VoteSignBytes(tc.chainID, v)
require.Equal(t, tc.want, got, "test case #%v: got unexpected sign bytes for Vote.", i)
assert.Equal(t, len(tc.want), len(got), "test case #%v: got unexpected sign bytes length for Vote.", i)
assert.Equal(t, tc.want, got, "test case #%v: got unexpected sign bytes for Vote.", i)
}
}