Skip to content

Commit

Permalink
CI: promote unused linter (#6120)
Browse files Browse the repository at this point in the history
  • Loading branch information
cce authored Sep 19, 2024
1 parent e1eb46a commit 62eb3bf
Show file tree
Hide file tree
Showing 18 changed files with 130 additions and 128 deletions.
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 @@ func creatableKey(cidx basics.CreatableIndex) [11]byte {
return key
}

func creatableMaxRangePrefix(maxIdx basics.CreatableIndex) ([3]byte, [11]byte) {
func creatableMaxRangePrefix(maxIdx basics.CreatableIndex) ([3]byte, [11]byte) { //nolint:unused // TODO
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 @@ type stateProofVerificationWriter struct {
e db.Executable
}

type stateProofVerificationReaderWriter struct {
type stateProofVerificationReaderWriter struct { //nolint:unused // TODO
stateProofVerificationReader
stateProofVerificationWriter
}
Expand All @@ -48,7 +48,7 @@ func makeStateProofVerificationWriter(e db.Executable) *stateProofVerificationWr
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
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 @@ func unmarshallMessageOfInterest(data []byte) (map[protocol.Tag]bool, error) {
if len(tag) != protocol.TagLength {
return nil, errInvalidMessageOfInterestInvalidTag
}
if _, ok := protocol.DeprecatedTagMap[protocol.Tag(tag)]; ok {
continue
}
if _, ok := protocol.TagMap[protocol.Tag(tag)]; !ok {
return nil, errInvalidMessageOfInterestInvalidTag
}
Expand All @@ -58,23 +61,18 @@ func unmarshallMessageOfInterest(data []byte) (map[protocol.Tag]bool, error) {
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 @@ func MessageOfInterestMaxSize() int {
for _, tag := range protocol.TagList {
allTags[tag] = true
}
return len(MarshallMessageOfInterest(protocol.TagList))
for tag := range protocol.DeprecatedTagMap {
allTags[tag] = true
}
return len(marshallMessageOfInterestMap(allTags))
}
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 @@ func (c *CapabilitiesDiscovery) Host() host.Host {
}

// 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
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

0 comments on commit 62eb3bf

Please # to comment.