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

add go linter - "unused" #11235

Merged
merged 15 commits into from
Sep 5, 2023
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ linters:
- varcheck
- deadcode
- scopelint
- unused

# We don't want to skip builtin/
skip-dirs-use-default: false
Expand Down
1 change: 0 additions & 1 deletion blockstore/splitstore/splitstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ type SplitStore struct {

compactionIndex int64
pruneIndex int64
onlineGCCnt int64

ctx context.Context
cancel func()
Expand Down
13 changes: 0 additions & 13 deletions chain/beacon/drand/drand.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,6 @@ import (

var log = logging.Logger("drand")

type drandPeer struct {
addr string
tls bool
}

func (dp *drandPeer) Address() string {
return dp.addr
}

func (dp *drandPeer) IsTLS() bool {
return dp.tls
}

// DrandBeacon connects Lotus with a drand network in order to provide
// randomness to the system in a way that's aligned with Filecoin rounds/epochs.
//
Expand Down
25 changes: 0 additions & 25 deletions chain/events/state/ctxstore.go

This file was deleted.

17 changes: 0 additions & 17 deletions chain/market/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,6 @@ func (ps *Store) save(ctx context.Context, state *FundedAddressState) error {
return ps.ds.Put(ctx, k, b)
}

// get the state for the given address
func (ps *Store) get(ctx context.Context, addr address.Address) (*FundedAddressState, error) {
k := dskeyForAddr(addr)

data, err := ps.ds.Get(ctx, k)
if err != nil {
return nil, err
}

var state FundedAddressState
err = cborrpc.ReadCborRPC(bytes.NewReader(data), &state)
if err != nil {
return nil, err
}
return &state, nil
}

// forEach calls iter with each address in the datastore
func (ps *Store) forEach(ctx context.Context, iter func(*FundedAddressState)) error {
res, err := ps.ds.Query(ctx, dsq.Query{Prefix: dsKeyAddr})
Expand Down
5 changes: 0 additions & 5 deletions chain/stmgr/forks.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ func (sm *StateManager) hasExpensiveForkBetween(parent, height abi.ChainEpoch) b
return false
}

func (sm *StateManager) hasExpensiveFork(height abi.ChainEpoch) bool {
_, ok := sm.expensiveUpgrades[height]
return ok
}

func runPreMigration(ctx context.Context, sm *StateManager, fn PreMigrationFunc, cache *nv16.MemMigrationCache, ts *types.TipSet) {
height := ts.Height()
parent := ts.ParentState()
Expand Down
61 changes: 61 additions & 0 deletions chain/sub/ratelimit/queue_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package ratelimit

import (
"testing"
)

func TestQueue(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objection to this test, obviously, but I'm surprised it was needed to make the queue lok "used" -- it should already look used because of its use in rate limiting.

Do you understand why this was the case?

Copy link
Contributor Author

@snissn snissn Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I sort of understand, for some reason pop is unused:

chain/sub/ratelimit/queue.go:38:17: func `(*queue).pop` is unused (unused)
func (q *queue) pop() int64 {

it's pretty common to write a little datastructure library, write out a bunch of api/funcs for it, and then not use a few when you have your final implementation. I think there's some combination of peeking and truncating that happens in ./chain/sub/ratelimit/window.go instead of pop, but i haven't done a thorough dive into it. Given my expectation of this pattern of building a general tool and then just using some of it, the test made more sense than deleting pop in case in future we wanted to reorganize this code. also just a simple parallel example, might make sense to write a queue library that lets you take out values from the top or bottom, but so far only use one of them

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the explanation! Makes total sense.

const size = 3
q := &queue{buf: make([]int64, size)}

if q.len() != 0 {
t.Fatalf("q.len() = %d, expect 0", q.len())
}

if q.cap() != size {
t.Fatalf("q.cap() = %d, expect %d", q.cap(), size)
}

for i := int64(0); i < int64(size); i++ {
err := q.push(i)
if err != nil {
t.Fatalf("cannot push element %d", i)
}
}

if q.len() != size {
t.Fatalf("q.len() = %d, expect %d", q.len(), size)
}

err := q.push(int64(size))
if err != ErrRateLimitExceeded {
t.Fatalf("pushing element beyond capacity should have failed with err: %s, got %s", ErrRateLimitExceeded, err)
}

if q.front() != 0 {
t.Fatalf("q.front() = %d, expect 0", q.front())
}

if q.back() != int64(size-1) {
t.Fatalf("q.back() = %d, expect %d", q.back(), size-1)
}

popVal := q.pop()
if popVal != 0 {
t.Fatalf("q.pop() = %d, expect 0", popVal)
}

if q.len() != size-1 {
t.Fatalf("q.len() = %d, expect %d", q.len(), size-1)
}

// Testing truncation.
threshold := int64(1)
q.truncate(threshold)
if q.len() != 1 {
t.Fatalf("q.len() after truncate = %d, expect 1", q.len())
}
if q.front() != 2 {
t.Fatalf("q.front() after truncate = %d, expect 2", q.front())
}
}
8 changes: 0 additions & 8 deletions chain/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/ipfs/go-cid"
ds "github.com/ipfs/go-datastore"
logging "github.com/ipfs/go-log/v2"
"github.com/libp2p/go-libp2p/core/peer"
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -344,13 +343,6 @@ func (tu *syncTestUtil) addClientNode() int {
return len(tu.nds) - 1
}

func (tu *syncTestUtil) pid(n int) peer.ID {
nal, err := tu.nds[n].NetAddrsListen(tu.ctx)
require.NoError(tu.t, err)

return nal.ID
}

func (tu *syncTestUtil) connect(from, to int) {
toPI, err := tu.nds[to].NetAddrsListen(tu.ctx)
require.NoError(tu.t, err)
Expand Down
3 changes: 0 additions & 3 deletions chain/types/blockmsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import (
)

func TestDecodeBlockMsg(t *testing.T) {
type args struct {
b []byte
}
tests := []struct {
name string
data []byte
Expand Down
21 changes: 0 additions & 21 deletions chain/types/vmcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,3 @@ type StateTree interface {

Version() StateTreeVersion
}

type storageWrapper struct {
s Storage
}

func (sw *storageWrapper) Put(i cbg.CBORMarshaler) (cid.Cid, error) {
c, err := sw.s.Put(i)
if err != nil {
return cid.Undef, err
}

return c, nil
}

func (sw *storageWrapper) Get(c cid.Cid, out cbg.CBORUnmarshaler) error {
if err := sw.s.Get(c, out); err != nil {
return err
}

return nil
}
15 changes: 0 additions & 15 deletions cmd/lotus-bench/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,21 +497,6 @@ type Invocation struct {

const GasPerNs = 10

func countGasCosts(et *types.ExecutionTrace) int64 {
var cgas int64

for _, gc := range et.GasCharges {
cgas += gc.ComputeGas
}

for _, sub := range et.Subcalls {
c := countGasCosts(&sub) //nolint
cgas += c
}

return cgas
}

type stats struct {
timeTaken meanVar
gasRatio meanVar
Expand Down
30 changes: 0 additions & 30 deletions lib/consensus/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,33 +561,3 @@ func find(s []string, elem string) bool {
}
return false
}

func (rw *raftWrapper) observePeers() {
obsCh := make(chan hraft.Observation, 1)
defer close(obsCh)

observer := hraft.NewObserver(obsCh, true, func(o *hraft.Observation) bool {
po, ok := o.Data.(hraft.PeerObservation)
return ok && po.Removed
})

rw.raft.RegisterObserver(observer)
defer rw.raft.DeregisterObserver(observer)

for {
select {
case obs := <-obsCh:
pObs := obs.Data.(hraft.PeerObservation)
logger.Info("raft peer departed. Removing from peerstore: ", pObs.Peer.ID)
pID, err := peer.Decode(string(pObs.Peer.ID))
if err != nil {
logger.Error(err)
continue
}
rw.host.Peerstore().ClearAddrs(pID)
case <-rw.ctx.Done():
logger.Debug("stopped observing raft peers")
return
}
}
}
4 changes: 2 additions & 2 deletions lib/shardedmutex/shardedmutex.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const cacheline = 64
// name old time/op new time/op delta
// Locks-8 74.6ns ± 7% 12.3ns ± 2% -83.54% (p=0.000 n=20+18)
type paddedMutex struct {
mt sync.Mutex
pad [cacheline - 8]uint8
mt sync.Mutex
_ [cacheline - 8]uint8
}

type ShardedMutex struct {
Expand Down
2 changes: 0 additions & 2 deletions node/modules/actorevent.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ func EthEventAPI(cfg config.FevmConfig) func(helpers.MetricsCtx, repo.LockedRepo
MaxFilterResults: cfg.Events.MaxFilterResults,
}

const ChainHeadConfidence = 1

lc.Append(fx.Hook{
OnStart: func(context.Context) error {
ev, err := events.NewEvents(ctx, &evapi)
Expand Down
20 changes: 0 additions & 20 deletions paychmgr/paych_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,26 +805,6 @@ func createTestVoucher(t *testing.T, ch address.Address, voucherLane uint64, non
return sv
}

func createTestVoucherWithExtra(t *testing.T, ch address.Address, voucherLane uint64, nonce uint64, voucherAmount big.Int, key []byte) *paychtypes.SignedVoucher { //nolint:deadcode
sv := &paychtypes.SignedVoucher{
ChannelAddr: ch,
Lane: voucherLane,
Nonce: nonce,
Amount: voucherAmount,
Extra: &paychtypes.ModVerifyParams{
Actor: tutils.NewActorAddr(t, "act"),
},
}

signingBytes, err := sv.SigningBytes()
require.NoError(t, err)
sig, err := sigs.Sign(crypto.SigTypeSecp256k1, key, signingBytes)
require.NoError(t, err)
sv.Signature = sig

return sv
}

type mockBestSpendableAPI struct {
mgr *Manager
}
Expand Down
2 changes: 2 additions & 0 deletions storage/wdpost/wdpost_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (
)

// recordPoStFailure records a failure in the journal.
//
//nolint:unused
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these false positives?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they're called over an API.

func (s *WindowPoStScheduler) recordPoStFailure(err error, ts *types.TipSet, deadline *dline.Info) {
s.journal.RecordEvent(s.evtTypes[evtTypeWdPoStScheduler], func() interface{} {
c := evtCommon{Error: err}
Expand Down
Loading