From 5d8a5019cf75bcc3573afd8033225f2afc067033 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 20 Jun 2026 17:55:35 -0500 Subject: [PATCH] lint: enable deprecation checks Enable staticcheck's SA1019 check in golangci-lint so deprecated identifiers are caught in CI. Replace deprecated standard library and bbolt APIs with their current equivalents. Keep intentional compatibility reads and writes of deprecated Loop RPC fields behind narrow nolint annotations, because older clients and persisted liquidity parameters still depend on those fields. --- .golangci.yml | 8 ++++--- cmd/loop/loopin.go | 2 +- cmd/loop/main.go | 3 +-- liquidity/parameters.go | 5 +++- loopd/daemon.go | 8 +++---- loopd/swapclient_server.go | 7 +++--- loopdb/migration_04_updates_test.go | 6 +---- loopdb/store.go | 5 ++-- loopdb/store_test.go | 37 +++++++---------------------- staticaddr/address/manager_test.go | 4 ++++ staticaddr/deposit/manager_test.go | 4 ++++ staticaddr/openchannel/manager.go | 5 ++-- 12 files changed, 42 insertions(+), 52 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 914e5789..5b870ccb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -55,14 +55,16 @@ linters: - wsl_v5 - noinlineerr settings: + staticcheck: + checks: + - all + - -QF* + - -ST* gosec: excludes: - G402 - G306 - G115 - staticcheck: - checks: - - -SA1019 tagliatelle: case: rules: diff --git a/cmd/loop/loopin.go b/cmd/loop/loopin.go index 0f1401d2..d0512040 100644 --- a/cmd/loop/loopin.go +++ b/cmd/loop/loopin.go @@ -207,7 +207,7 @@ func loopIn(ctx context.Context, cmd *cli.Command) error { } fmt.Printf("Swap initiated\n") - fmt.Printf("ID: %v\n", resp.Id) + fmt.Printf("ID: %x\n", resp.IdBytes) if resp.HtlcAddressP2Tr != "" { fmt.Printf("HTLC address (P2TR): %v\n", resp.HtlcAddressP2Tr) diff --git a/cmd/loop/main.go b/cmd/loop/main.go index 113404fd..294095ca 100644 --- a/cmd/loop/main.go +++ b/cmd/loop/main.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "io/ioutil" "os" "os/signal" "path/filepath" @@ -646,7 +645,7 @@ func getClientConn(address, tlsCertPath, macaroonPath string) (daemonConn, // gRPC dial options from it. func readMacaroon(macPath string) (grpc.DialOption, error) { // Load the specified macaroon file. - macBytes, err := ioutil.ReadFile(macPath) + macBytes, err := os.ReadFile(macPath) if err != nil { return nil, fmt.Errorf("unable to read macaroon path : %v", err) } diff --git a/liquidity/parameters.go b/liquidity/parameters.go index 44c555db..9e8e5cc0 100644 --- a/liquidity/parameters.go +++ b/liquidity/parameters.go @@ -559,8 +559,11 @@ func RpcToParameters(req *clientrpc.LiquidityParameters) (*Parameters, req.AutoloopBudgetSat != 0 { params.AutoFeeRefreshPeriod = InfiniteDuration + // Keep reading the legacy start field so old stored + // liquidity parameters migrate to the refresh-period model. + budgetStartSec := req.AutoloopBudgetStartSec //nolint:staticcheck params.AutoloopBudgetLastRefresh = time.Unix( - int64(req.AutoloopBudgetStartSec), 0) + int64(budgetStartSec), 0) } for _, rule := range req.Rules { diff --git a/loopd/daemon.go b/loopd/daemon.go index 241d62e0..91267811 100644 --- a/loopd/daemon.go +++ b/loopd/daemon.go @@ -33,7 +33,7 @@ import ( "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/macaroons" - "go.etcd.io/bbolt" + bbolterrors "go.etcd.io/bbolt/errors" "google.golang.org/grpc" "google.golang.org/protobuf/encoding/protojson" "gopkg.in/macaroon-bakery.v2/bakery" @@ -161,7 +161,7 @@ func (d *Daemon) Start() error { // and error handlers. If this fails, then nothing has been started yet, // and we can just return the error. err = d.initialize(true) - if errors.Is(err, bbolt.ErrTimeout) { + if errors.Is(err, bbolterrors.ErrTimeout) { // We're trying to be started as a standalone Loop daemon, most // likely LiT is already running and blocking the DB return fmt.Errorf("%v: make sure no other loop daemon process "+ @@ -211,7 +211,7 @@ func (d *Daemon) StartAsSubserver(lndGrpc *lndclient.GrpcLndServices, // handlers. If this fails, then nothing has been started yet, and we // can just return the error. err := d.initialize(withMacaroonService) - if errors.Is(err, bbolt.ErrTimeout) { + if errors.Is(err, bbolterrors.ErrTimeout) { // We're trying to be started inside LiT so there most likely is // another standalone Loop process blocking the DB. return fmt.Errorf("%v: make sure no other loop daemon "+ @@ -994,7 +994,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error { d.wg.Go(func() { infof("Starting static address open channel manager") err := openChannelManager.Run(d.mainCtx) - if err != nil && !errors.Is(context.Canceled, err) { + if err != nil && !errors.Is(err, context.Canceled) { d.internalErrChan <- err } infof("Static address open channel manager stopped") diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index bed6efa8..86d44a52 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -251,6 +251,7 @@ func (s *swapClientServer) LoopOut(ctx context.Context, req.AssetSwapRfqId = in.AssetRfqInfo.SwapRfqId } + // Keep accepting the deprecated single-channel field for older clients. switch { case in.LoopOutChannel != 0 && len(in.OutgoingChanSet) > 0: // nolint:staticcheck return nil, errors.New("loop_out_channel and outgoing_" + @@ -273,7 +274,7 @@ func (s *swapClientServer) LoopOut(ctx context.Context, resp := &looprpc.SwapResponse{ Id: info.SwapHash.String(), IdBytes: info.SwapHash[:], - HtlcAddress: htlcAddress, + HtlcAddress: htlcAddress, //nolint:staticcheck ServerMessage: info.ServerMessage, } @@ -1182,11 +1183,11 @@ func (s *swapClientServer) LoopIn(ctx context.Context, if loopdb.CurrentProtocolVersion() < loopdb.ProtocolVersionHtlcV3 { p2wshAddr := swapInfo.HtlcAddressP2WSH.String() - response.HtlcAddress = p2wshAddr + response.HtlcAddress = p2wshAddr //nolint:staticcheck response.HtlcAddressP2Wsh = p2wshAddr } else { p2trAddr := swapInfo.HtlcAddressP2TR.String() - response.HtlcAddress = p2trAddr + response.HtlcAddress = p2trAddr //nolint:staticcheck response.HtlcAddressP2Tr = p2trAddr } diff --git a/loopdb/migration_04_updates_test.go b/loopdb/migration_04_updates_test.go index 37f05466..f0e85f49 100644 --- a/loopdb/migration_04_updates_test.go +++ b/loopdb/migration_04_updates_test.go @@ -2,8 +2,6 @@ package loopdb import ( "context" - "io/ioutil" - "os" "path/filepath" "testing" @@ -48,9 +46,7 @@ func TestMigrationUpdates(t *testing.T) { ctxb := context.Background() // Restore a legacy database. - tempDirName, err := ioutil.TempDir("", "clientstore") - require.NoError(t, err) - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() tempPath := filepath.Join(tempDirName, dbFileName) db, err := bbolt.Open(tempPath, 0600, nil) diff --git a/loopdb/store.go b/loopdb/store.go index fe71c19c..1fec57eb 100644 --- a/loopdb/store.go +++ b/loopdb/store.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/lightningnetwork/lnd/lntypes" "go.etcd.io/bbolt" + bbolterrors "go.etcd.io/bbolt/errors" ) var ( @@ -197,9 +198,9 @@ func NewBoltSwapStore(dbPath string, chainParams *chaincfg.Params) ( bdb, err := bboltOpen(path, 0600, &bbolt.Options{ Timeout: DefaultLoopDBTimeout, }) - if errors.Is(err, bbolt.ErrTimeout) { + if errors.Is(err, bbolterrors.ErrTimeout) { return nil, fmt.Errorf("%w: couldn't obtain exclusive lock on "+ - "%s, timed out after %v", bbolt.ErrTimeout, path, + "%s, timed out after %v", bbolterrors.ErrTimeout, path, DefaultLoopDBTimeout) } if err != nil { diff --git a/loopdb/store_test.go b/loopdb/store_test.go index 4a61062b..ec9feb2a 100644 --- a/loopdb/store_test.go +++ b/loopdb/store_test.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "fmt" - "io/ioutil" "os" "path/filepath" "testing" @@ -18,6 +17,7 @@ import ( "github.com/lightningnetwork/lnd/routing/route" "github.com/stretchr/testify/require" "go.etcd.io/bbolt" + bbolterrors "go.etcd.io/bbolt/errors" ) var ( @@ -62,7 +62,7 @@ func TestNewBoltSwapStoreTimeout(t *testing.T) { bboltOpen = origOpen }) - wrappedErr := fmt.Errorf("wrapped: %w", bbolt.ErrTimeout) + wrappedErr := fmt.Errorf("wrapped: %w", bbolterrors.ErrTimeout) bboltOpen = func(path string, mode os.FileMode, options *bbolt.Options) (*bbolt.DB, error) { @@ -74,7 +74,7 @@ func TestNewBoltSwapStoreTimeout(t *testing.T) { store, err := NewBoltSwapStore(tempDir, &chaincfg.MainNetParams) require.Nil(t, store) - require.ErrorIs(t, err, bbolt.ErrTimeout) + require.ErrorIs(t, err, bbolterrors.ErrTimeout) require.ErrorContains(t, err, "couldn't obtain exclusive lock") } @@ -142,10 +142,7 @@ func TestLoopOutStore(t *testing.T) { // testLoopOutStore tests the basic functionality of the current bbolt // swap store for specific swap parameters. func testLoopOutStore(t *testing.T, pendingSwap *LoopOutContract) { - tempDirName, err := ioutil.TempDir("", "clientstore") - require.NoError(t, err) - - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() store, err := NewBoltSwapStore(tempDirName, &chaincfg.MainNetParams) require.NoError(t, err) @@ -284,9 +281,7 @@ func TestLoopInStore(t *testing.T) { } func testLoopInStore(t *testing.T, pendingSwap LoopInContract) { - tempDirName, err := ioutil.TempDir("", "clientstore") - require.NoError(t, err) - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() store, err := NewBoltSwapStore(tempDirName, &chaincfg.MainNetParams) require.NoError(t, err) @@ -366,11 +361,7 @@ func testLoopInStore(t *testing.T, pendingSwap LoopInContract) { // TestVersionNew tests that a new database is initialized with the current // version. func TestVersionNew(t *testing.T) { - tempDirName, err := ioutil.TempDir("", "clientstore") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() store, err := NewBoltSwapStore(tempDirName, &chaincfg.MainNetParams) if err != nil { @@ -390,11 +381,7 @@ func TestVersionNew(t *testing.T) { // TestVersionMigrated tests that an existing version zero database is migrated // to the latest version. func TestVersionMigrated(t *testing.T) { - tempDirName, err := ioutil.TempDir("", "clientstore") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() createVersionZeroDb(t, tempDirName) @@ -459,11 +446,7 @@ func TestLegacyOutgoingChannel(t *testing.T) { } // Restore a legacy database. - tempDirName, err := ioutil.TempDir("", "clientstore") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() tempPath := filepath.Join(tempDirName, dbFileName) db, err := bbolt.Open(tempPath, 0600, nil) @@ -498,9 +481,7 @@ func TestLegacyOutgoingChannel(t *testing.T) { // TestLiquidityParams checks that reading and writing to liquidty bucket are // as expected. func TestLiquidityParams(t *testing.T) { - tempDirName, err := ioutil.TempDir("", "clientstore") - require.NoError(t, err, "failed to db") - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() ctxb := context.Background() diff --git a/staticaddr/address/manager_test.go b/staticaddr/address/manager_test.go index 5881bf84..b7bbf79a 100644 --- a/staticaddr/address/manager_test.go +++ b/staticaddr/address/manager_test.go @@ -66,6 +66,10 @@ func (m *mockStaticAddressClient) PushStaticAddressHtlcSigs(ctx context.Context, args.Error(1) } +// ServerWithdrawDeposits implements the deprecated RPC required by the +// generated client interface. Production code uses ServerPsbtWithdrawDeposits. +// +//nolint:staticcheck func (m *mockStaticAddressClient) ServerWithdrawDeposits(ctx context.Context, in *swapserverrpc.ServerWithdrawRequest, opts ...grpc.CallOption) (*swapserverrpc.ServerWithdrawResponse, diff --git a/staticaddr/deposit/manager_test.go b/staticaddr/deposit/manager_test.go index 0ab79302..1e798ea3 100644 --- a/staticaddr/deposit/manager_test.go +++ b/staticaddr/deposit/manager_test.go @@ -71,6 +71,10 @@ func (m *mockStaticAddressClient) PushStaticAddressHtlcSigs(ctx context.Context, args.Error(1) } +// ServerWithdrawDeposits implements the deprecated RPC required by the +// generated client interface. Production code uses ServerPsbtWithdrawDeposits. +// +//nolint:staticcheck func (m *mockStaticAddressClient) ServerWithdrawDeposits(ctx context.Context, in *swapserverrpc.ServerWithdrawRequest, opts ...grpc.CallOption) (*swapserverrpc.ServerWithdrawResponse, diff --git a/staticaddr/openchannel/manager.go b/staticaddr/openchannel/manager.go index 21102e69..052c5aca 100644 --- a/staticaddr/openchannel/manager.go +++ b/staticaddr/openchannel/manager.go @@ -310,6 +310,8 @@ func (m *Manager) OpenChannel(ctx context.Context, return nil, err } + // If a local funding amount is set, coin-select deposits to + // cover it. Otherwise fundmax uses all available deposits. if req.LocalFundingAmount != 0 { deposits, err = staticutil.SelectDeposits( deposits, req.LocalFundingAmount, @@ -319,9 +321,6 @@ func (m *Manager) OpenChannel(ctx context.Context, return nil, fmt.Errorf("error selecting "+ "deposits: %w", err) } - } else { - // The fundmax flag is set, hence we select all deposits - // for funding the channel. } }