Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: promote unused linter #6120

Merged
merged 8 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .golangci-warnings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ linters:
enable:
- gosec
- partitiontest
- unused

linters-settings:
gosec: # we are mostly only interested in G601
Expand Down
11 changes: 11 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ linters:
- staticcheck
- typecheck
- paralleltest
- unused

severity:
default-severity: error
Expand Down Expand Up @@ -115,6 +116,14 @@ issues:
- "^superfluous-else: if block ends with"

exclude-rules:
- path: cmd/algofix/
linters: unused
- path: cmd/algocfg/
linters: unused
- path: cmd/catchpointdump/
linters: unused
- path: tools/
linters: unused
- path: _test\.go
linters:
- errcheck
Expand All @@ -129,6 +138,7 @@ issues:
# - revive
# - staticcheck
- typecheck
- unused
- path: _test\.go
linters:
- staticcheck
Expand Down Expand Up @@ -206,3 +216,4 @@ issues:
- govet
- ineffassign
- misspell
- unused
20 changes: 0 additions & 20 deletions daemon/algod/api/client/restClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,26 +347,6 @@ type pendingTransactionsByAddrParams struct {
Max uint64 `url:"max"`
}

type transactionsByAddrParams struct {
FirstRound uint64 `url:"firstRound"`
LastRound uint64 `url:"lastRound"`
Max uint64 `url:"max"`
}

type assetsParams struct {
AssetIdx uint64 `url:"assetIdx"`
Max uint64 `url:"max"`
}

type appsParams struct {
AppIdx uint64 `url:"appIdx"`
Max uint64 `url:"max"`
}

type rawblockParams struct {
Raw uint64 `url:"raw"`
}

type rawFormat struct {
Format string `url:"format"`
}
Expand Down
1 change: 0 additions & 1 deletion data/transactions/verify/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ type GroupContext struct {
}

var errTxGroupInvalidFee = errors.New("txgroup fee requirement overflow")
var errTxGroupInsuffientLsigBudget = errors.New("txgroup lsig expectations exceed available budget")
var errTxnSigHasNoSig = errors.New("signedtxn has no sig")
var errTxnSigNotWellFormed = errors.New("signedtxn should only have one of Sig or Msig or LogicSig")
var errRekeyingNotSupported = errors.New("nonempty AuthAddr but rekeying is not supported")
Expand Down
2 changes: 1 addition & 1 deletion ledger/store/trackerdb/generickv/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
return key
}

func creatableMaxRangePrefix(maxIdx basics.CreatableIndex) ([3]byte, [11]byte) {
func creatableMaxRangePrefix(maxIdx basics.CreatableIndex) ([3]byte, [11]byte) { //nolint:unused // TODO

Check warning on line 136 in ledger/store/trackerdb/generickv/schema.go

View check run for this annotation

Codecov / codecov/patch

ledger/store/trackerdb/generickv/schema.go#L136

Added line #L136 was not covered by tests
var low [prefixLength + separatorLength]byte

copy(low[0:], kvPrefixCreatorIndex)
Expand Down
4 changes: 2 additions & 2 deletions ledger/store/trackerdb/sqlitedriver/spVerificationAccessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
e db.Executable
}

type stateProofVerificationReaderWriter struct {
type stateProofVerificationReaderWriter struct { //nolint:unused // TODO
stateProofVerificationReader
stateProofVerificationWriter
}
Expand All @@ -48,7 +48,7 @@
return &stateProofVerificationWriter{e: e}
}

func makeStateProofVerificationReaderWriter(q db.Queryable, e db.Executable) *stateProofVerificationReaderWriter {
func makeStateProofVerificationReaderWriter(q db.Queryable, e db.Executable) *stateProofVerificationReaderWriter { //nolint:unused // TODO

Check warning on line 51 in ledger/store/trackerdb/sqlitedriver/spVerificationAccessor.go

View check run for this annotation

Codecov / codecov/patch

ledger/store/trackerdb/sqlitedriver/spVerificationAccessor.go#L51

Added line #L51 was not covered by tests
return &stateProofVerificationReaderWriter{
stateProofVerificationReader{q: q},
stateProofVerificationWriter{e: e},
Expand Down
27 changes: 14 additions & 13 deletions network/msgOfInterest.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
if len(tag) != protocol.TagLength {
return nil, errInvalidMessageOfInterestInvalidTag
}
if _, ok := protocol.DeprecatedTagMap[protocol.Tag(tag)]; ok {
gmalouf marked this conversation as resolved.
Show resolved Hide resolved
continue

Check warning on line 54 in network/msgOfInterest.go

View check run for this annotation

Codecov / codecov/patch

network/msgOfInterest.go#L54

Added line #L54 was not covered by tests
}
if _, ok := protocol.TagMap[protocol.Tag(tag)]; !ok {
return nil, errInvalidMessageOfInterestInvalidTag
}
Expand All @@ -58,23 +61,18 @@
return msgTagsMap, nil
}

// MarshallMessageOfInterest generate a message of interest message body for a given set of message tags.
func MarshallMessageOfInterest(messageTags []protocol.Tag) []byte {
// create a long string with all these messages.
tags := ""
// marshallMessageOfInterest generates a message of interest message body for a given set of message tags.
func marshallMessageOfInterest(messageTags []protocol.Tag) []byte {
m := make(map[protocol.Tag]bool)
for _, tag := range messageTags {
tags += topicsEncodingSeparator + string(tag)
}
if len(tags) > 0 {
tags = tags[len(topicsEncodingSeparator):]
m[tag] = true
}
topics := Topics{Topic{key: "tags", data: []byte(tags)}}
return topics.MarshallTopics()
return marshallMessageOfInterestMap(m)
}

// MarshallMessageOfInterestMap generates a message of interest message body
// marshallMessageOfInterestMap generates a message of interest message body
// for the message tags that map to "true" in the map argument.
func MarshallMessageOfInterestMap(tagmap map[protocol.Tag]bool) []byte {
func marshallMessageOfInterestMap(tagmap map[protocol.Tag]bool) []byte {
tags := ""
for tag, flag := range tagmap {
if flag {
Expand All @@ -95,5 +93,8 @@
for _, tag := range protocol.TagList {
allTags[tag] = true
}
return len(MarshallMessageOfInterest(protocol.TagList))
for tag := range protocol.DeprecatedTagMap {
allTags[tag] = true

Check warning on line 97 in network/msgOfInterest.go

View check run for this annotation

Codecov / codecov/patch

network/msgOfInterest.go#L96-L97

Added lines #L96 - L97 were not covered by tests
}
return len(marshallMessageOfInterestMap(allTags))

Check warning on line 99 in network/msgOfInterest.go

View check run for this annotation

Codecov / codecov/patch

network/msgOfInterest.go#L99

Added line #L99 was not covered by tests
}
6 changes: 3 additions & 3 deletions network/msgOfInterest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,20 @@ func TestUnmarshallMessageOfInterestErrors(t *testing.T) {
func TestMarshallMessageOfInterest(t *testing.T) {
partitiontest.PartitionTest(t)

bytes := MarshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag})
bytes := marshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag})
tags, err := unmarshallMessageOfInterest(bytes)
require.NoError(t, err)
require.Equal(t, tags[protocol.AgreementVoteTag], true)
require.Equal(t, 1, len(tags))

bytes = MarshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag, protocol.NetPrioResponseTag})
bytes = marshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag, protocol.NetPrioResponseTag})
tags, err = unmarshallMessageOfInterest(bytes)
require.NoError(t, err)
require.Equal(t, tags[protocol.AgreementVoteTag], true)
require.Equal(t, tags[protocol.NetPrioResponseTag], true)
require.Equal(t, 2, len(tags))

bytes = MarshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag, protocol.AgreementVoteTag})
bytes = marshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag, protocol.AgreementVoteTag})
tags, err = unmarshallMessageOfInterest(bytes)
require.NoError(t, err)
require.Equal(t, tags[protocol.AgreementVoteTag], true)
Expand Down
2 changes: 1 addition & 1 deletion network/p2p/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
}

// addPeer adds a given peer.AddrInfo to the Host's Peerstore, and the DHT's routing table
func (c *CapabilitiesDiscovery) addPeer(p peer.AddrInfo) (bool, error) {
func (c *CapabilitiesDiscovery) addPeer(p peer.AddrInfo) (bool, error) { //nolint:unused // TODO

Check warning on line 82 in network/p2p/capabilities.go

View check run for this annotation

Codecov / codecov/patch

network/p2p/capabilities.go#L82

Added line #L82 was not covered by tests
c.Host().Peerstore().AddAddrs(p.ID, p.Addrs, libpeerstore.AddressTTL)
return c.dht.RoutingTable().TryAddPeer(p.ID, true, true)
}
Expand Down
6 changes: 3 additions & 3 deletions network/wsNetwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ func (wn *WebsocketNetwork) Start() error {
defer wn.messagesOfInterestMu.Unlock()
wn.messagesOfInterestEncoded = true
if wn.messagesOfInterest != nil {
wn.messagesOfInterestEnc = MarshallMessageOfInterestMap(wn.messagesOfInterest)
wn.messagesOfInterestEnc = marshallMessageOfInterestMap(wn.messagesOfInterest)
}

if wn.config.IsGossipServer() || wn.config.ForceRelayMessages {
Expand Down Expand Up @@ -818,7 +818,7 @@ func (wn *WebsocketNetwork) RegisterHandlers(dispatch []TaggedMessageHandler) {
// ClearHandlers deregisters all the existing message handlers.
func (wn *WebsocketNetwork) ClearHandlers() {
// exclude the internal handlers. These would get cleared out when Stop is called.
wn.handler.ClearHandlers([]Tag{protocol.PingTag, protocol.PingReplyTag, protocol.NetPrioResponseTag})
wn.handler.ClearHandlers([]Tag{protocol.NetPrioResponseTag})
}

// RegisterValidatorHandlers registers the set of given message handlers.
Expand Down Expand Up @@ -2468,7 +2468,7 @@ func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) {

func (wn *WebsocketNetwork) updateMessagesOfInterestEnc() {
// must run inside wn.messagesOfInterestMu.Lock
wn.messagesOfInterestEnc = MarshallMessageOfInterestMap(wn.messagesOfInterest)
wn.messagesOfInterestEnc = marshallMessageOfInterestMap(wn.messagesOfInterest)
wn.messagesOfInterestEncoded = true
wn.messagesOfInterestGeneration.Add(1)
var peers []*wsPeer
Expand Down
7 changes: 3 additions & 4 deletions network/wsNetwork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2904,7 +2904,7 @@ func TestWebsocketNetworkMessageOfInterest(t *testing.T) {
partitiontest.PartitionTest(t)
var (
ft1 = protocol.Tag("AV")
ft2 = protocol.Tag("pj")
ft2 = protocol.Tag("UE")
ft3 = protocol.Tag("NI")
ft4 = protocol.Tag("TX")

Expand All @@ -2924,9 +2924,8 @@ func TestWebsocketNetworkMessageOfInterest(t *testing.T) {
t.Logf("netA %s", addrA)
netB.phonebook.ReplacePeerList([]string{addrA}, "default", phonebook.PhoneBookEntryRelayRole)

// have netB asking netA to send it ft2, deregister ping handler to make sure that we aren't exceeding the maximum MOI messagesize
// have netB asking netA to send it ft2.
// Max MOI size is calculated by encoding all of the valid tags, since we are using a custom tag here we must deregister one in the default set.
netB.DeregisterMessageInterest(protocol.PingTag)
netB.registerMessageInterest(ft2)

netB.Start()
Expand Down Expand Up @@ -3845,7 +3844,7 @@ func (t mockHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

bytes := MarshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag})
bytes := marshallMessageOfInterest([]protocol.Tag{protocol.AgreementVoteTag})
msgBytes := append([]byte(protocol.MsgOfInterestTag), bytes...)
_, err = wr.Write(msgBytes)
if err != nil {
Expand Down
46 changes: 1 addition & 45 deletions network/wsPeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ var defaultSendMessageTags = map[protocol.Tag]bool{
protocol.MsgDigestSkipTag: true,
protocol.NetPrioResponseTag: true,
protocol.NetIDVerificationTag: true,
protocol.PingTag: true,
protocol.PingReplyTag: true,
protocol.ProposalPayloadTag: true,
protocol.TopicMsgRespTag: true,
protocol.MsgOfInterestTag: true,
Expand Down Expand Up @@ -142,7 +140,6 @@ type disconnectReason string

const disconnectReasonNone disconnectReason = ""
const disconnectBadData disconnectReason = "BadData"
const disconnectTooSlow disconnectReason = "TooSlow"
const disconnectReadError disconnectReason = "ReadError"
const disconnectWriteError disconnectReason = "WriteError"
const disconnectIdleConn disconnectReason = "IdleConnection"
Expand Down Expand Up @@ -226,12 +223,6 @@ type wsPeer struct {

processed chan struct{}

pingLock deadlock.Mutex
pingSent time.Time
pingData []byte
pingInFlight bool
lastPingRoundTripTime time.Duration

// Hint about position in wn.peers. Definitely valid if the peer
// is present in wn.peers.
peerIndex int
Expand Down Expand Up @@ -684,8 +675,7 @@ func (wp *wsPeer) readLoop() {
case protocol.ProposalPayloadTag:
wp.ppMessageCount.Add(1)
// the remaining valid tags: no special handling here
case protocol.NetPrioResponseTag, protocol.PingTag, protocol.PingReplyTag,
protocol.StateProofSigTag, protocol.UniEnsBlockReqTag, protocol.VoteBundleTag, protocol.NetIDVerificationTag:
case protocol.NetPrioResponseTag, protocol.StateProofSigTag, protocol.UniEnsBlockReqTag, protocol.VoteBundleTag, protocol.NetIDVerificationTag:
default: // unrecognized tag
unknownProtocolTagMessagesTotal.Inc(nil)
wp.unkMessageCount.Add(1)
Expand Down Expand Up @@ -951,40 +941,6 @@ func (wp *wsPeer) writeNonBlockMsgs(ctx context.Context, data [][]byte, highPrio

// PingLength is the fixed length of ping message, exported to be used in the node.TestMaxSizesCorrect test
const PingLength = 8
const maxPingWait = 60 * time.Second

// sendPing sends a ping block to the peer.
// return true if either a ping request was enqueued or there is already ping request in flight in the past maxPingWait time.
func (wp *wsPeer) sendPing() bool {
wp.pingLock.Lock()
defer wp.pingLock.Unlock()
now := time.Now()
if wp.pingInFlight && (now.Sub(wp.pingSent) < maxPingWait) {
return true
}

tagBytes := []byte(protocol.PingTag)
mbytes := make([]byte, len(tagBytes)+PingLength)
copy(mbytes, tagBytes)
crypto.RandBytes(mbytes[len(tagBytes):])
wp.pingData = mbytes[len(tagBytes):]
sent := wp.writeNonBlock(context.Background(), mbytes, false, crypto.Digest{}, time.Now())

if sent {
wp.pingInFlight = true
wp.pingSent = now
}
return sent
}

// get some times out of the peer while observing the ping data lock
func (wp *wsPeer) pingTimes() (lastPingSent time.Time, lastPingRoundTripTime time.Duration) {
wp.pingLock.Lock()
defer wp.pingLock.Unlock()
lastPingSent = wp.pingSent
lastPingRoundTripTime = wp.lastPingRoundTripTime
return
}

// called when the connection had an error or closed remotely
func (wp *wsPeer) internalClose(reason disconnectReason) {
Expand Down
37 changes: 33 additions & 4 deletions network/wsPeer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,29 +241,58 @@ func getProtocolTags(t *testing.T) []string {
fset := token.NewFileSet()
f, _ := parser.ParseFile(fset, file, nil, parser.ParseComments)

// get deprecated tags
deprecatedTags := make(map[string]bool)
for _, d := range f.Decls {
genDecl, ok := d.(*ast.GenDecl)
if !ok || genDecl.Tok != token.VAR {
continue
}
for _, spec := range genDecl.Specs {
if valueSpec, ok := spec.(*ast.ValueSpec); ok && len(valueSpec.Names) > 0 &&
valueSpec.Names[0].Name == "DeprecatedTagList" {
for _, v := range valueSpec.Values {
cl, ok := v.(*ast.CompositeLit)
if !ok {
continue
}
for _, elt := range cl.Elts {
if ce, ok := elt.(*ast.Ident); ok {
deprecatedTags[ce.Name] = true
}
}
}
}
}
}

// look for const declarations in protocol/tags.go
var declaredTags []string
// Iterate through the declarations in the file
for _, d := range f.Decls {
genDecl, ok := d.(*ast.GenDecl)
// Check if the declaration is a constant and if not, continue
// Check if the declaration is a constant
if !ok || genDecl.Tok != token.CONST {
continue
}
// Iterate through the specs (specifications) in the declaration
for _, spec := range genDecl.Specs {
if valueSpec, ok := spec.(*ast.ValueSpec); ok {
if ident, isIdent := valueSpec.Type.(*ast.Ident); !isIdent || ident.Name != "Tag" {
continue // skip all but Tag constants
}
for _, n := range valueSpec.Names {
if strings.HasSuffix(n.Name, "MaxSize") || n.Name == "TagLength" {
continue
if deprecatedTags[n.Name] {
continue // skip deprecated tags
}
declaredTags = append(declaredTags, n.Name)
}
}
}
}
// assert these AST-discovered tags are complete (match the size of protocol.TagList)
require.Len(t, declaredTags, len(protocol.TagList))
require.Len(t, protocol.TagList, len(declaredTags))
require.Len(t, protocol.DeprecatedTagList, len(deprecatedTags))
return declaredTags
}

Expand Down
Loading
Loading