Skip to content

Commit 937f440

Browse files
ccecce
committed
increase code coverage for vpack package
1 parent dda786e commit 937f440

File tree

8 files changed

+256
-32
lines changed

8 files changed

+256
-32
lines changed

‎agreement/vote.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type (
4040

4141
// unauthenticatedVote is a vote which has not been verified
4242
unauthenticatedVote struct {
43-
_struct struct{} `codec:",omitempty,omitemptyarray" vpack_size:"3"`
43+
_struct struct{} `codec:",omitempty,omitemptyarray" vpack_assert_size:"3"`
4444
R rawVote `codec:"r"`
4545
Cred committee.UnauthenticatedCredential `codec:"cred"`
4646
Sig crypto.OneTimeSignature `codec:"sig,omitempty,omitemptycheckstruct"`

‎crypto/onetimesig.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type OneTimeSignature struct {
3737
// Unfortunately we forgot to mark this struct as omitempty at
3838
// one point, and now it's hard to change if we want to preserve
3939
// encodings.
40-
_struct struct{} `codec:"" vpack_size:"6"`
40+
_struct struct{} `codec:"" vpack_assert_size:"6"`
4141

4242
// Sig is a signature of msg under the key PK.
4343
Sig ed25519Signature `codec:"s" vpack:"literal"`

‎data/committee/credential.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type (
3333
// An UnauthenticatedCredential is a Credential which has not yet been
3434
// authenticated.
3535
UnauthenticatedCredential struct {
36-
_struct struct{} `codec:",omitempty,omitemptyarray" vpack_size:"1"`
36+
_struct struct{} `codec:",omitempty,omitemptyarray" vpack_assert_size:"1"`
3737
Proof crypto.VrfProof `codec:"pf" vpack:"literal"`
3838
}
3939

‎network/vpack/gen.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type parseFuncData struct {
6161
TypeName string
6262
CodecName string
6363
MaxFieldCount int
64-
FixedSize int // If > 0, indicates a fixed size from vpack_size tag
64+
FixedSize int // If > 0, indicates a fixed size from vpack_assert_size tag
6565
Fields []fieldData
6666
}
6767

@@ -341,10 +341,10 @@ func (g *codeGenerator) analyzeType(t reflect.Type) error {
341341
return getCodecTagName(fields[i]) < getCodecTagName(fields[j])
342342
})
343343

344-
// Check for fixed-size structs using the vpack_size tag on _struct field
344+
// Check for fixed-size structs using the vpack_assert_size tag on _struct field
345345
fixedSize := 0
346346
if structField, found := findStructField(t, "_struct"); found {
347-
vpackSizeTag := structField.Tag.Get("vpack_size")
347+
vpackSizeTag := structField.Tag.Get("vpack_assert_size")
348348
if vpackSizeTag != "" {
349349
if size, err := strconv.Atoi(vpackSizeTag); err == nil {
350350
fixedSize = size

‎network/vpack/msgp.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,16 @@ func isfixint(b byte) bool {
157157
// It will return a zero-length/nil slice iff err != nil.
158158
func (p *parser) readUintBytes() ([]byte, error) {
159159
startPos := p.pos
160-
160+
// read marker byte
161161
b, err := p.readByte()
162162
if err != nil {
163163
return nil, err
164164
}
165-
if isfixint(b) { // 1-byte unsigned int encoding
165+
// fixint is a single byte containing marker and value
166+
if isfixint(b) {
166167
return p.data[startPos : startPos+1], nil
167168
}
169+
// otherwise, we expect a tag byte followed by the value
168170
var dataSize int
169171
switch b {
170172
case uint8tag:

‎network/vpack/parse_test.go‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
// Copyright (C) 2019-2025 Algorand, Inc.
2+
// This file is part of go-algorand
3+
//
4+
// go-algorand is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as
6+
// published by the Free Software Foundation, either version 3 of the
7+
// License, or (at your option) any later version.
8+
//
9+
// go-algorand is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.
16+
17+
package vpack
18+
19+
import (
20+
"testing"
21+
22+
"github.com/algorand/go-algorand/agreement"
23+
"github.com/algorand/go-algorand/crypto"
24+
"github.com/algorand/go-algorand/protocol"
25+
"github.com/algorand/go-algorand/test/partitiontest"
26+
"github.com/stretchr/testify/require"
27+
)
28+
29+
// a string that is greater than the max 5-bit fixmap size
30+
const gtFixMapString = "12345678901234567890123456789012"
31+
32+
// TestParseVoteErrors tests error cases of the parseVote function
33+
func TestParseVoteErrors(t *testing.T) {
34+
partitiontest.PartitionTest(t)
35+
36+
for _, tc := range []struct {
37+
obj any
38+
errContains string
39+
}{
40+
// vote
41+
{map[string]string{"a": "1", "b": "2"},
42+
"expected fixed map size 3 for unauthenticatedVote, got 2"},
43+
{map[string]any{"a": 1, "b": 2, "c": 3},
44+
"unexpected field in unauthenticatedVote"},
45+
{[]int{1, 2, 3},
46+
"reading map for unauthenticatedVote"},
47+
{map[string]string{"a": "1", "b": "2", "c": "3", "d": "4", "e": "5", "f": "6", "g": "7"},
48+
"expected fixed map size 3 for unauthenticatedVote, got 7"},
49+
{map[string]string{gtFixMapString: "1", "b": "2", "c": "3"},
50+
"reading key for unauthenticatedVote"},
51+
52+
// cred
53+
{map[string]string{"cred": "1", "d": "2", "e": "3"},
54+
"reading map for UnauthenticatedCredential"},
55+
{map[string]any{"cred": map[string]int{"pf": 1, "q": 2}, "d": "2", "e": "3"},
56+
"expected fixed map size 1 for UnauthenticatedCredential, got 2"},
57+
{map[string]any{"cred": map[string]int{gtFixMapString: 1}, "d": "2", "e": "3"},
58+
"reading key for UnauthenticatedCredential"},
59+
{map[string]any{"cred": map[string]string{"invalid": "1"}, "r": "2", "sig": "3"},
60+
"unexpected field in UnauthenticatedCredential"},
61+
{map[string]any{"cred": map[string]any{"pf": []byte{1, 2, 3}}, "r": "2", "sig": "3"},
62+
"reading pf"},
63+
64+
// rawVote
65+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": []int{1, 2, 3}, "sig": "3"},
66+
"reading map for rawVote"},
67+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]string{}, "sig": "3"},
68+
"expected fixmap size for rawVote 1 <= cnt <= 5, got 0"},
69+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]string{"a": "1", "b": "2", "c": "3", "d": "4", "e": "5", "f": "6"}, "sig": "3"},
70+
"expected fixmap size for rawVote 1 <= cnt <= 5, got 6"},
71+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]string{gtFixMapString: "1"}, "sig": "3"},
72+
"reading key for rawVote"},
73+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]string{"invalid": "1"}, "sig": "3"},
74+
"unexpected field in rawVote"},
75+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"per": "not-a-number"}, "sig": "3"},
76+
"reading per"},
77+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": "not-a-number"}, "sig": "3"},
78+
"reading rnd"},
79+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"step": "not-a-number"}, "sig": "3"},
80+
"reading step"},
81+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": "not-a-map"}, "sig": "3"},
82+
"reading map for proposalValue"},
83+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"snd": []int{1, 2, 3}}, "sig": "3"},
84+
"reading snd"},
85+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]string{"snd": "1"}, "sig": []int{1, 2, 3}},
86+
"reading snd: expected bin8 length 32"},
87+
88+
// proposalValue
89+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]string{"invalid": "1"}}, "sig": "3"},
90+
"unexpected field in proposalValue"},
91+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]string{gtFixMapString: "1"}}, "sig": "3"},
92+
"reading key for proposalValue"},
93+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]any{"dig": []int{1, 2, 3}}}, "sig": "3"},
94+
"reading dig"},
95+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]any{"encdig": []int{1, 2, 3}}}, "sig": "3"},
96+
"reading encdig"},
97+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]any{"oper": "not-a-number"}}, "sig": "3"},
98+
"reading oper"},
99+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]any{"oprop": []int{1, 2, 3}}}, "sig": "3"},
100+
"reading oprop"},
101+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"prop": map[string]any{"a": 1, "b": 2, "c": 3, "d": 4, "e": 5}}, "sig": "3"},
102+
"expected fixmap size for proposalValue 1 <= cnt <= 4, got 5"},
103+
104+
// sig
105+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": []int{1, 2, 3}},
106+
"reading map for OneTimeSignature"},
107+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{}},
108+
"expected fixed map size 6 for OneTimeSignature, got 0"},
109+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{"p": []int{1}}},
110+
"expected fixed map size 6 for OneTimeSignature, got 1"},
111+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
112+
gtFixMapString: "1", "a": 1, "b": 2, "c": 3, "d": 4, "e": 5}},
113+
"reading key for OneTimeSignature"},
114+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
115+
"a": 1, "b": 2, "c": 3, "d": 4, "e": 5, "f": 6}},
116+
"unexpected field in OneTimeSignature"},
117+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
118+
"p": []int{1}, "p1s": [64]byte{}, "p2": [32]byte{}, "p2s": [64]byte{}, "ps": [64]byte{}, "s": [64]byte{}}},
119+
"reading p: expected bin8 length 32"},
120+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
121+
"p": [32]byte{}, "p1s": []int{1}, "p2": [32]byte{}, "p2s": [64]byte{}, "ps": [64]byte{}, "s": [64]byte{}}},
122+
"reading p1s: expected bin8 length 64"},
123+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
124+
"p": [32]byte{}, "p1s": [64]byte{}, "p2": []int{1}, "p2s": [64]byte{}, "ps": [64]byte{}, "s": [64]byte{}}},
125+
"reading p2: expected bin8 length 32"},
126+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
127+
"p": [32]byte{}, "p1s": [64]byte{}, "p2": [32]byte{}, "p2s": []int{1}, "ps": [64]byte{}, "s": [64]byte{}}},
128+
"reading p2s: expected bin8 length 64"},
129+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
130+
"p": [32]byte{}, "p1s": [64]byte{}, "p2": [32]byte{}, "p2s": [64]byte{}, "ps": []int{1}, "s": [64]byte{}}},
131+
"reading ps: expected bin8 length 64"},
132+
{map[string]any{"cred": map[string]any{"pf": crypto.VrfProof{1}}, "r": map[string]any{"rnd": 1}, "sig": map[string]any{
133+
"p": [32]byte{}, "p1s": [64]byte{}, "p2": [32]byte{}, "p2s": [64]byte{}, "ps": [64]byte{}, "s": []int{1}}},
134+
"reading s: expected bin8 length 64"},
135+
} {
136+
mock := &mockCompressWriter{}
137+
var buf []byte
138+
// protocol.Encode and protocol.EncodeReflect encode keys in alphabetical order
139+
if v, ok := tc.obj.(*agreement.UnauthenticatedVote); ok {
140+
buf = protocol.Encode(v)
141+
} else {
142+
buf = protocol.EncodeReflect(tc.obj)
143+
}
144+
err := parseVote(buf, mock)
145+
require.Error(t, err)
146+
t.Logf("For test with %T, got error: %v, expected to contain: %v", tc.obj, err, tc.errContains)
147+
require.Contains(t, err.Error(), tc.errContains)
148+
}
149+
}
150+
151+
// mockCompressWriter implements compressWriter for testing
152+
type mockCompressWriter struct{ writes []any }
153+
154+
func (m *mockCompressWriter) writeStatic(idx uint8) { m.writes = append(m.writes, idx) }
155+
156+
func (m *mockCompressWriter) writeLiteralBin64(val [64]byte) { m.writes = append(m.writes, val) }
157+
158+
func (m *mockCompressWriter) writeLiteralBin80(val [80]byte) { m.writes = append(m.writes, val) }
159+
160+
func (m *mockCompressWriter) writeDynamicBin32(val [32]byte) { m.writes = append(m.writes, val) }
161+
162+
func (m *mockCompressWriter) writeDynamicVaruint(valBytes []byte) error {
163+
m.writes = append(m.writes, valBytes)
164+
return nil
165+
}

‎network/vpack/vpack_test.go‎

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,51 +22,86 @@ import (
2222
"github.com/algorand/go-algorand/agreement"
2323
"github.com/algorand/go-algorand/protocol"
2424
"github.com/algorand/go-algorand/test/partitiontest"
25-
"github.com/algorand/msgp/msgp"
2625
"github.com/stretchr/testify/require"
2726
)
2827

2928
// based on RunEncodingTest from protocol/codec_tester.go
3029
func TestEncodingTest(t *testing.T) {
3130
partitiontest.PartitionTest(t)
3231

33-
for i := 0; i < 10000; i++ {
34-
v0, err := protocol.RandomizeObject(&agreement.UnauthenticatedVote{}, protocol.RandomizeObjectWithZeroesEveryN(10))
32+
var errorCount int
33+
const iters = 10000
34+
for range iters {
35+
v0obj, err := protocol.RandomizeObject(&agreement.UnauthenticatedVote{},
36+
protocol.RandomizeObjectWithZeroesEveryN(10),
37+
protocol.RandomizeObjectWithAllUintSizes(),
38+
)
3539
require.NoError(t, err)
3640

37-
v0vote := v0.(*agreement.UnauthenticatedVote)
38-
if *v0vote == (agreement.UnauthenticatedVote{}) {
41+
v0 := v0obj.(*agreement.UnauthenticatedVote)
42+
if *v0 == (agreement.UnauthenticatedVote{}) {
3943
continue // don't try to encode or compress empty votes (a single byte, 0x80)
4044
}
41-
expectFailMissing := false
42-
// top-level should have 3 fields, all of which are required
43-
if v0vote.R.MsgIsZero() || v0vote.Cred.MsgIsZero() || v0vote.Sig.MsgIsZero() {
44-
expectFailMissing = true
45+
var expectError string
46+
// Expect errors when random vote doesn't match vpack_assert_size
47+
if v0.Cred.Proof.MsgIsZero() {
48+
expectError = "expected fixed map size 1 for UnauthenticatedCredential"
4549
}
46-
// Cred must be non-zero
47-
if v0vote.Cred.Proof.MsgIsZero() {
48-
expectFailMissing = true
50+
if v0.R.MsgIsZero() || v0.Cred.MsgIsZero() || v0.Sig.MsgIsZero() {
51+
expectError = "expected fixed map size 3 for unauthenticatedVote"
4952
}
5053

51-
msgpBuf := protocol.EncodeMsgp(v0.(msgp.Marshaler))
54+
msgpBuf := protocol.EncodeMsgp(v0)
5255
enc := NewStaticEncoder()
5356
encBuf, err := enc.CompressVote(nil, msgpBuf)
54-
if expectFailMissing {
55-
// We are strict, and won't process votes with missing fields (where vpack_size is set)
56-
require.ErrorContains(t, err, "expected fixed map size")
57+
if expectError != "" {
58+
// skip expected errors
59+
require.ErrorContains(t, err, expectError)
60+
require.Nil(t, encBuf)
61+
errorCount++
5762
continue
58-
} else {
59-
require.NoError(t, err)
6063
}
64+
require.NoError(t, err)
6165

66+
// decompress and compare to original
6267
dec := NewStaticDecoder()
6368
decMsgpBuf, err := dec.DecompressVote(nil, encBuf)
6469
require.NoError(t, err)
65-
66-
require.Equal(t, msgpBuf, decMsgpBuf)
70+
require.Equal(t, msgpBuf, decMsgpBuf) // msgp encoding matches
6771
var v1 agreement.UnauthenticatedVote
68-
protocol.Decode(decMsgpBuf, &v1)
72+
err = protocol.Decode(decMsgpBuf, &v1)
73+
require.NoError(t, err)
74+
require.Equal(t, *v0, v1) // vote objects match
75+
}
76+
t.Logf("TestEncodingTest: %d expected errors out of %d iterations", errorCount, iters)
77+
}
6978

70-
require.Equal(t, *v0vote, v1)
79+
// TestEncodeStaticSteps asserts that table entries for step:1, step:2, step:3 are encoded
80+
func TestEncodeStaticSteps(t *testing.T) {
81+
partitiontest.PartitionTest(t)
82+
v := agreement.UnauthenticatedVote{}
83+
v.Cred.Proof[0] = 1 // not empty
84+
v.R.Round = 1
85+
v.Sig.PK[0] = 1 // not empty
86+
87+
for i := 1; i <= 3; i++ {
88+
var expectedStaticIdx uint8
89+
switch i {
90+
case 1:
91+
v.R.Step = 1
92+
expectedStaticIdx = staticIdxStepVal1Field
93+
case 2:
94+
v.R.Step = 2
95+
expectedStaticIdx = staticIdxStepVal2Field
96+
case 3:
97+
v.R.Step = 3
98+
expectedStaticIdx = staticIdxStepVal3Field
99+
}
100+
101+
msgpbuf := protocol.Encode(&v)
102+
w := &mockCompressWriter{}
103+
err := parseVote(msgpbuf, w)
104+
require.NoError(t, err)
105+
require.Contains(t, w.writes, expectedStaticIdx)
71106
}
72107
}

‎protocol/codec_tester.go‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ func oneOf(n int) bool {
5050
}
5151

5252
type randomizeObjectCfg struct {
53-
// MoreZeros will increase the chance of zero values being generated.
53+
// ZeroesEveryN will increase the chance of zero values being generated.
5454
ZeroesEveryN int
55+
// AllUintSizes will be equally likely to generate 8-bit, 16-bit, 32-bit, or 64-bit uints.
56+
AllUintSizes bool
5557
}
5658

5759
// RandomizeObjectOption is an option for RandomizeObject
@@ -62,6 +64,11 @@ func RandomizeObjectWithZeroesEveryN(n int) RandomizeObjectOption {
6264
return func(cfg *randomizeObjectCfg) { cfg.ZeroesEveryN = n }
6365
}
6466

67+
// RandomizeObjectWithAllUintSizes will be equally likely to generate 8-bit, 16-bit, 32-bit, or 64-bit uints.
68+
func RandomizeObjectWithAllUintSizes() RandomizeObjectOption {
69+
return func(cfg *randomizeObjectCfg) { cfg.AllUintSizes = true }
70+
}
71+
6572
// RandomizeObject returns a random object of the same type as template
6673
func RandomizeObject(template interface{}, opts ...RandomizeObjectOption) (interface{}, error) {
6774
cfg := randomizeObjectCfg{}
@@ -256,7 +263,22 @@ func randomizeValue(v reflect.Value, depth int, datapath string, tag string, rem
256263
// generate value that will avoid protocol.ErrInvalidObject from HashType.Validate()
257264
v.SetUint(rand.Uint64() % 3) // 3 is crypto.MaxHashType
258265
} else {
259-
v.SetUint(rand.Uint64())
266+
var num uint64
267+
if cfg.AllUintSizes {
268+
switch rand.Intn(4) {
269+
case 0: // fewer than 8 bits
270+
num = uint64(rand.Intn(1 << 8)) // 0 to 255
271+
case 1: // fewer than 16 bits
272+
num = uint64(rand.Intn(1 << 16)) // 0 to 65535
273+
case 2: // fewer than 32 bits
274+
num = uint64(rand.Uint32()) // 0 to 2^32-1
275+
case 3: // fewer than 64 bits
276+
num = rand.Uint64() // 0 to 2^64-1
277+
}
278+
} else {
279+
num = rand.Uint64()
280+
}
281+
v.SetUint(num)
260282
}
261283
*remainingChanges--
262284
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:

0 commit comments

Comments
 (0)