multi: fix `staticcheck` flags by hieblmi · Pull Request #1058 · lightninglabs/loop

hieblmi · 2025-12-11T20:10:24Z

fixes issues found with staticcheck

gemini-code-assist · 2025-12-11T20:10:47Z

Summary of Changes

Hello @hieblmi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request systematically addresses and resolves various issues flagged by the staticcheck linter across multiple packages. The changes primarily focus on modernizing Go code by updating deprecated functions, ensuring consistent error handling patterns, aligning with Go's naming conventions, and refactoring function signatures for improved clarity and adherence to best practices. Additionally, an obsolete file has been removed to streamline the codebase.

Highlights

gRPC Client Modernization: Updated gRPC client creation calls from grpc.Dial to the newer grpc.NewClient function in assets/client.go and swap_server_client.go.
File I/O Updates: Replaced deprecated io/ioutil functions like ioutil.ReadFile and ioutil.TempDir with their modern equivalents from the os package, such as os.ReadFile and os.TempDir().
Error Handling Consistency: Corrected the argument order for errors.Is calls to conform to staticcheck recommendations, changing errors.Is(context.Canceled, err) to errors.Is(err, context.Canceled).
Go Naming Conventions: Renamed an error variable from testPublishError to errTestPublish to align with standard Go error naming conventions (ST1012).
Insecure gRPC Transport: Updated the method for specifying insecure gRPC transport from grpc.WithInsecure() to grpc.WithTransportCredentials(insecure.NewCredentials()).
Function Signature Refactoring: Adjusted the return order of the startNewLoopIn function in loopin_test.go to place the error as the last return value, following common Go practices.
File Removal: Removed the uncharge_state.go file, indicating it is no longer needed.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses several issues found by the staticcheck tool, including usage of deprecated functions and incorrect error handling patterns. The changes are mostly correct and improve code quality. However, I've found a critical issue where ioutil.TempDir is incorrectly replaced with os.TempDir. This could lead to the deletion of the system's temporary directory during tests, which is a significant bug. I've provided suggestions to fix this using os.MkdirTemp instead.

loopdb/migration_04_updates_test.go

loopdb/store_test.go

bhandras

Nice, thanks! 🎉

bhandras · 2025-12-17T15:12:57Z

swap_server_client.go

+	case insec:
+		opts = append(opts, grpc.WithTransportCredentials(
+			insecure.NewCredentials(),
+		),


nit: formatting.

starius

LGTM! 🚀
Few nits

starius · 2025-12-19T01:23:07Z

swap_server_client.go

 // proxyAddr indicates that a SOCKS proxy found at the address should be used to
 // establish the connection.
-func getSwapServerConn(address, proxyAddress string, insecure bool,
+func getSwapServerConn(address, proxyAddress string, insec bool,


s/insec/skipCertCheck/

starius · 2025-12-19T01:27:53Z

loopin_test.go

 		ctx.errChan <- err
 	}()
-	return cfg, err, inSwap
+	return cfg, inSwap, err


Add an empty line before return

starius · 2025-12-19T01:36:49Z

loopd/daemon.go


 			err := depositManager.Run(d.mainCtx, initChan)
-			if err != nil && !errors.Is(context.Canceled, err) {
+			if err != nil && !errors.Is(err, context.Canceled) {


Great catch! Can you add tests capturing errors.Is regressions in the file?

I added tests here: hieblmi#5

starius · 2025-12-19T07:22:58Z

assets/client.go


 	// Dial the gRPC server.
-	conn, err := grpc.Dial(config.Host, opts...)
+	conn, err := grpc.NewClient(config.Host, opts...)


Some grpc.Dial calls still live in looprpc/client.pb.gw.go
We can bump protobuf deps and run make rpc to fix that.

That's a great catch!

updated the protobuf dependency, but a single occurrence remains, due to backward-compat issues.

I managed to get rid of it here: hieblmi#4

- Update assets/client.go to use grpc.NewClient - Update swap_server_client.go to use grpc.NewClient - Replace grpc.WithInsecure() with insecure.NewCredentials()

- Change return signature of startNewLoopIn from (*swapConfig, error, *loopInSwap) to (*swapConfig, *loopInSwap, error) - Fixes ST1008: error should be returned as the last argument

- Rename testPublishError to errTestPublish (ST1012) - Add explicit type to all constants in const groups (SA9004) - Fixes staticcheck style warnings

hieblmi · 2025-12-19T09:39:00Z

I added commit 5ec5875 to remove the grpc.DialContext and tested this locally. But I would appreciate a confirmation @starius.

starius

LGTM!
I added regression tests for the errors.Is bug: hieblmi#5

hieblmi self-assigned this Dec 11, 2025

gemini-code-assist bot reviewed Dec 11, 2025

View reviewed changes

loopdb/migration_04_updates_test.go Outdated Show resolved Hide resolved

loopdb/store_test.go Outdated Show resolved Hide resolved

hieblmi force-pushed the minor-fixes branch 5 times, most recently from 184a6c6 to f1dd6fb Compare December 17, 2025 13:36

hieblmi requested review from bhandras and starius December 17, 2025 13:52

bhandras approved these changes Dec 17, 2025

View reviewed changes

loop: delete unused file uncharge_state.go

a3db145

hieblmi force-pushed the minor-fixes branch from f1dd6fb to cbd7ec5 Compare December 18, 2025 07:57

starius approved these changes Dec 19, 2025

View reviewed changes

hieblmi force-pushed the minor-fixes branch 2 times, most recently from 43c8aa7 to 1754b52 Compare December 19, 2025 08:26

loooprpc: replace deprecated grpc.Dial with grpc.NewClient

175b7d6

- Update assets/client.go to use grpc.NewClient - Update swap_server_client.go to use grpc.NewClient - Replace grpc.WithInsecure() with insecure.NewCredentials()

hieblmi force-pushed the minor-fixes branch from 1754b52 to 8086d71 Compare December 19, 2025 09:04

hieblmi added 4 commits December 19, 2025 10:18

cmd: replace grpc.DialContext with grpc.NewClient

5ec5875

loopd: fix errors.Is argument order

f4e7e67

loop: fix error return order in loopin_test.go

76df55b

- Change return signature of startNewLoopIn from (*swapConfig, error, *loopInSwap) to (*swapConfig, *loopInSwap, error) - Fixes ST1008: error should be returned as the last argument

sweepbatcher: fix style issues in sweep_batcher_test.go

417e254

- Rename testPublishError to errTestPublish (ST1012) - Add explicit type to all constants in const groups (SA9004) - Fixes staticcheck style warnings

hieblmi force-pushed the minor-fixes branch from 8086d71 to 417e254 Compare December 19, 2025 09:19

hieblmi requested a review from starius December 19, 2025 09:39

loopdb: test error handling for bbolt.Open

360af61

starius approved these changes Dec 19, 2025

View reviewed changes

loopd: test error reporting logic

d1c4a7d

hieblmi force-pushed the minor-fixes branch from 9797a65 to d1c4a7d Compare December 20, 2025 05:31

hieblmi merged commit 1bdaff8 into lightninglabs:master Dec 20, 2025
9 checks passed

Conversation

hieblmi commented Dec 11, 2025

Uh oh!

gemini-code-assist bot commented Dec 11, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

bhandras left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hieblmi commented Dec 19, 2025

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants