Skip to content

Commit 875a70b

Browse files
ccecce
committed
CR feedback
1 parent c76eb31 commit 875a70b

File tree

3 files changed

+43
-48
lines changed

3 files changed

+43
-48
lines changed

‎network/wsNetwork.go‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -882,28 +882,29 @@ func setHeaders(header http.Header, netProtoVer string, meta peerMetadataProvide
882882
header.Set(GenesisHeader, meta.GetGenesisID())
883883

884884
// set the features header (comma-separated list)
885-
features := []string{PeerFeatureProposalCompression}
885+
features := []string{peerFeatureProposalCompression}
886886
if meta.Config().EnableVoteCompression {
887-
features = append(features, PeerFeatureVoteVpackCompression)
887+
features = append(features, peerFeatureVoteVpackCompression)
888888

889889
// Announce our maximum supported dynamic table size
890890
// Both sides will independently calculate min(ourSize, theirSize)
891891
// Only advertise dynamic features if stateless compression is enabled
892+
// Supported values: 16, 32, 64, 128, 256, 512, 1024 (or higher, which advertises 1024)
892893
switch dtSize := uint32(meta.Config().VoteCompressionDynamicTableSize); {
893894
case dtSize >= 1024:
894-
features = append(features, PeerFeatureVoteVpackDynamic1024)
895+
features = append(features, peerFeatureVoteVpackDynamic1024)
895896
case dtSize >= 512:
896-
features = append(features, PeerFeatureVoteVpackDynamic512)
897+
features = append(features, peerFeatureVoteVpackDynamic512)
897898
case dtSize >= 256:
898-
features = append(features, PeerFeatureVoteVpackDynamic256)
899+
features = append(features, peerFeatureVoteVpackDynamic256)
899900
case dtSize >= 128:
900-
features = append(features, PeerFeatureVoteVpackDynamic128)
901+
features = append(features, peerFeatureVoteVpackDynamic128)
901902
case dtSize >= 64:
902-
features = append(features, PeerFeatureVoteVpackDynamic64)
903+
features = append(features, peerFeatureVoteVpackDynamic64)
903904
case dtSize >= 32:
904-
features = append(features, PeerFeatureVoteVpackDynamic32)
905+
features = append(features, peerFeatureVoteVpackDynamic32)
905906
case dtSize >= 16:
906-
features = append(features, PeerFeatureVoteVpackDynamic16)
907+
features = append(features, peerFeatureVoteVpackDynamic16)
907908
}
908909
}
909910
header.Set(PeerFeaturesHeader, strings.Join(features, ","))
@@ -1965,23 +1966,23 @@ const UserAgentHeader = "User-Agent"
19651966
// PeerFeaturesHeader is the HTTP header listing features
19661967
const PeerFeaturesHeader = "X-Algorand-Peer-Features"
19671968

1968-
// PeerFeatureProposalCompression is a value for PeerFeaturesHeader indicating peer
1969+
// peerFeatureProposalCompression is a value for PeerFeaturesHeader indicating peer
19691970
// supports proposal payload compression with zstd
1970-
const PeerFeatureProposalCompression = "ppzstd"
1971+
const peerFeatureProposalCompression = "ppzstd"
19711972

1972-
// PeerFeatureVoteVpackCompression is a value for PeerFeaturesHeader indicating peer
1973+
// peerFeatureVoteVpackCompression is a value for PeerFeaturesHeader indicating peer
19731974
// supports agreement vote message compression with vpack
1974-
const PeerFeatureVoteVpackCompression = "avvpack"
1975+
const peerFeatureVoteVpackCompression = "avvpack"
19751976

1976-
// PeerFeatureVoteVpackDynamic* are values for PeerFeaturesHeader indicating peer
1977+
// peerFeatureVoteVpackDynamic* are values for PeerFeaturesHeader indicating peer
19771978
// supports specific dynamic table sizes for vpack compression
1978-
const PeerFeatureVoteVpackDynamic1024 = "avvpack1024"
1979-
const PeerFeatureVoteVpackDynamic512 = "avvpack512"
1980-
const PeerFeatureVoteVpackDynamic256 = "avvpack256"
1981-
const PeerFeatureVoteVpackDynamic128 = "avvpack128"
1982-
const PeerFeatureVoteVpackDynamic64 = "avvpack64"
1983-
const PeerFeatureVoteVpackDynamic32 = "avvpack32"
1984-
const PeerFeatureVoteVpackDynamic16 = "avvpack16"
1979+
const peerFeatureVoteVpackDynamic1024 = "avvpack1024"
1980+
const peerFeatureVoteVpackDynamic512 = "avvpack512"
1981+
const peerFeatureVoteVpackDynamic256 = "avvpack256"
1982+
const peerFeatureVoteVpackDynamic128 = "avvpack128"
1983+
const peerFeatureVoteVpackDynamic64 = "avvpack64"
1984+
const peerFeatureVoteVpackDynamic32 = "avvpack32"
1985+
const peerFeatureVoteVpackDynamic16 = "avvpack16"
19851986

19861987
var websocketsScheme = map[string]string{"http": "ws", "https": "wss"}
19871988

‎network/wsPeer.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,31 +1232,31 @@ func decodePeerFeatures(version string, announcedFeatures string) peerFeatureFla
12321232
parts := strings.Split(announcedFeatures, ",")
12331233
for _, part := range parts {
12341234
part = strings.TrimSpace(part)
1235-
if part == PeerFeatureProposalCompression {
1235+
if part == peerFeatureProposalCompression {
12361236
features |= pfCompressedProposal
12371237
}
1238-
if part == PeerFeatureVoteVpackCompression {
1238+
if part == peerFeatureVoteVpackCompression {
12391239
features |= pfCompressedVoteVpack
12401240
}
1241-
if part == PeerFeatureVoteVpackDynamic1024 {
1241+
if part == peerFeatureVoteVpackDynamic1024 {
12421242
features |= pfCompressedVoteVpackDynamic1024
12431243
}
1244-
if part == PeerFeatureVoteVpackDynamic512 {
1244+
if part == peerFeatureVoteVpackDynamic512 {
12451245
features |= pfCompressedVoteVpackDynamic512
12461246
}
1247-
if part == PeerFeatureVoteVpackDynamic256 {
1247+
if part == peerFeatureVoteVpackDynamic256 {
12481248
features |= pfCompressedVoteVpackDynamic256
12491249
}
1250-
if part == PeerFeatureVoteVpackDynamic128 {
1250+
if part == peerFeatureVoteVpackDynamic128 {
12511251
features |= pfCompressedVoteVpackDynamic128
12521252
}
1253-
if part == PeerFeatureVoteVpackDynamic64 {
1253+
if part == peerFeatureVoteVpackDynamic64 {
12541254
features |= pfCompressedVoteVpackDynamic64
12551255
}
1256-
if part == PeerFeatureVoteVpackDynamic32 {
1256+
if part == peerFeatureVoteVpackDynamic32 {
12571257
features |= pfCompressedVoteVpackDynamic32
12581258
}
1259-
if part == PeerFeatureVoteVpackDynamic16 {
1259+
if part == peerFeatureVoteVpackDynamic16 {
12601260
features |= pfCompressedVoteVpackDynamic16
12611261
}
12621262
}

‎network/wsPeer_test.go‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"io"
2626
"net"
2727
"path/filepath"
28+
"slices"
2829
"sort"
2930
"strings"
3031
"sync/atomic"
@@ -159,16 +160,16 @@ func TestVersionToFeature(t *testing.T) {
159160
{"1.2.3", "", peerFeatureFlag(0)},
160161
{"a.b", "", peerFeatureFlag(0)},
161162
{"2.1", "", peerFeatureFlag(0)},
162-
{"2.1", PeerFeatureProposalCompression, peerFeatureFlag(0)},
163+
{"2.1", peerFeatureProposalCompression, peerFeatureFlag(0)},
163164
{"2.2", "", peerFeatureFlag(0)},
164165
{"2.2", "test", peerFeatureFlag(0)},
165166
{"2.2", strings.Join([]string{"a", "b"}, ","), peerFeatureFlag(0)},
166-
{"2.2", PeerFeatureProposalCompression, pfCompressedProposal},
167-
{"2.2", strings.Join([]string{PeerFeatureProposalCompression, "test"}, ","), pfCompressedProposal},
168-
{"2.2", strings.Join([]string{PeerFeatureProposalCompression, "test"}, ", "), pfCompressedProposal},
169-
{"2.2", strings.Join([]string{PeerFeatureProposalCompression, PeerFeatureVoteVpackCompression}, ","), pfCompressedVoteVpack | pfCompressedProposal},
170-
{"2.2", PeerFeatureVoteVpackCompression, pfCompressedVoteVpack},
171-
{"2.3", PeerFeatureProposalCompression, pfCompressedProposal},
167+
{"2.2", peerFeatureProposalCompression, pfCompressedProposal},
168+
{"2.2", strings.Join([]string{peerFeatureProposalCompression, "test"}, ","), pfCompressedProposal},
169+
{"2.2", strings.Join([]string{peerFeatureProposalCompression, "test"}, ", "), pfCompressedProposal},
170+
{"2.2", strings.Join([]string{peerFeatureProposalCompression, peerFeatureVoteVpackCompression}, ","), pfCompressedVoteVpack | pfCompressedProposal},
171+
{"2.2", peerFeatureVoteVpackCompression, pfCompressedVoteVpack},
172+
{"2.3", peerFeatureProposalCompression, pfCompressedProposal},
172173
}
173174
for i, test := range tests {
174175
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
@@ -233,19 +234,12 @@ func TestPeerReadLoopSwitchAllTags(t *testing.T) {
233234
})
234235
require.True(t, readLoopFound)
235236
require.NotEmpty(t, foundTags)
236-
ignored := map[string]struct{}{
237-
"VotePackedTag": {}, // normalized to AgreementVoteTag before the switch
238-
}
239-
filtered := allTags[:0]
240-
for _, tag := range allTags {
241-
if _, ok := ignored[tag]; ok {
242-
continue
243-
}
244-
filtered = append(filtered, tag)
245-
}
246-
allTags = filtered
237+
// Filter out VP, it's normalized to AV before the switch statement
238+
allTags = slices.DeleteFunc(allTags, func(tag string) bool { return tag == "VotePackedTag" })
247239
sort.Strings(allTags)
248240
sort.Strings(foundTags)
241+
t.Logf("All tags: %v", allTags)
242+
t.Logf("Found tags: %v", foundTags)
249243
require.Equal(t, allTags, foundTags)
250244
}
251245

0 commit comments

Comments
 (0)