switchrpc: wire MacService and NetworkDir in config setup#10737
Conversation
The switchrpc configuration in PopulateDependencies was missing the MacService and NetworkDir fields, so switch.macaroon was never written to disk. This adds the two missing config setup steps, bringing switchrpc setup in line with the pattern used by the other lnd sub-servers.
Summary of ChangesHello, I'm Gemini Code Assist1! 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 ensures that the switchrpc sub-server configuration is properly initialized by including necessary dependencies that were previously omitted. This change aligns the switchrpc setup with the standard pattern used by other sub-servers in the project, ensuring consistent behavior and correct file system operations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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
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 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. Footnotes
|
|
🟡 PR Severity: MEDIUM | subrpcserver_config.go (1 file, 7 lines) | Root-level Go file not matching CRITICAL (server.go/rpcserver.go) or HIGH (lnrpc/*) packages. No bump: 1 file, 7 lines, single package. <!-- pr-severity-bot --> |
There was a problem hiding this comment.
Code Review
This pull request updates the dependency population for the switchrpc.Config sub-server by adding NetworkDir and MacService fields. Feedback was provided to replace the use of reflection with direct field assignments, as the configuration object is already type-asserted, which would improve type safety and maintainability.
| subCfgValue.FieldByName("NetworkDir").Set( | ||
| reflect.ValueOf(networkDir), | ||
| ) | ||
| subCfgValue.FieldByName("MacService").Set( | ||
| reflect.ValueOf(macService), | ||
| ) |
There was a problem hiding this comment.
While this is consistent with the rest of the function, using reflection with FieldByName is brittle and can lead to runtime panics if the field names in the switchrpc.Config struct are ever changed.
Since the subCfg variable is already typed as *switchrpc.Config, you can set the fields directly for better type safety and readability.
This would be a good refactoring to apply to the other cases in this function as well.
subCfg.NetworkDir = networkDir
subCfg.MacService = macServicec6be683
into
lightningnetwork:elle-base-branch-payment-service
Change Description
The switchrpc config dispatcher in
PopulateDependencieswas missing theMacServiceandNetworkDirfields, which meantcfg.MacServicewas always nil whenswitch_server.go'sNew()function ran, soswitch.macaroonwas never written to disk.This brings switchrpc server in line with the pattern used by every other sub-server (
signrpc,walletrpc,chainrpc,invoicesrpc, etc.).Steps to Test
switchrpcbuild tag and observe creation of file.