-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New rpc GetConfigInfo added#10608
Conversation
Change Description
Fixes #10508
Adds a new rpc GetConfigInfo
Steps to Test
- Build
make build - Run
make rpc-check
Verifies the generated proto code matches the proto definitions - Unit tests
Run the root package tests to ensure nothing is broken:
make unit pkg=. timeout=5m - Manual testing
Start lnd (with any backend), then run:
lncli getconfiginfo
Expected output (varies by config):
{
"db_backend": "bolt",
"native_sql": false
}
Test with different configurations:
- Default (bolt): Should return "db_backend": "bolt"
- With --db.backend=sqlite: Should return "db_backend": "sqlite"
- With --db.use-native-sql: Should return "native_sql": true
- REST endpoint
curl -s --cacert ~/.lnd/tls.cert
-H "Grpc-Metadata-macaroon: $(xxd -ps -u -c 1000 ~/.lnd/data/chain/bitcoin/mainnet/admin.macaroon)"
https://localhost:8080/v1/getconfiginfo - Macaroon permissions
Verify it works with a readonly.macaroon (since it uses info:read) and fails with an invoice-only macaroon.
Pull Request Checklist
Testing
- Your PR passes all CI checks.
- Tests covering the positive and negative (error paths) are included.
- Bug fixes contain tests triggering the bug to prevent regressions.
Code Style and Documentation
- The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
- The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
- Commits follow the Ideal Git Commit Structure.
- Any new logging statements use an appropriate subsystem and logging level.
- Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
- There is a change description in the release notes, or
[skip ci]in the commit message for small changes.
Please see our Contribution Guidelines for further guidance.
Summary of ChangesHello @saubyk, 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 enhances the LND daemon's introspection capabilities by providing a new RPC and CLI command. This addition allows users and applications to programmatically and directly query the operational configuration of the LND node, specifically regarding its database setup. This improves transparency and aids in debugging or operational monitoring by exposing key internal settings. 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. Changelog
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. 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. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new GetConfigInfo RPC and its corresponding lncli command getconfiginfo. The changes are well-structured and cover all necessary parts, including protobuf definitions, RPC server implementation, lncli command, and release notes. I've found a couple of minor issues: a missing function comment in cmd/commands/commands.go and a broken markdown link in the release notes. After addressing these, the PR should be in good shape.
787f1c9 to
ca0b107
Compare
PR Severity: CRITICAL
Critical (1 file)
High (2 files)
Medium (3 files)
Low (1 file)
Excluded from counting (4 auto-generated files)
AnalysisThis PR is classified as CRITICAL because it modifies The bulk of the line changes (3,590 additions / 3,454 deletions) are in auto-generated protobuf files ( The PR appears to add new RPC functionality (additions only, no deletions in non-generated files), touching the proto definition, swagger spec, gRPC generated code, CLI commands, and the RPC server itself. Expert review of the To override, add a |
ca0b107 to
b046fd6
Compare
b046fd6 to
6bee8a7
Compare
6bee8a7 to
4180c48
Compare
ellemouton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this info is already present in lncli getdebuginfo?
| "description": "An unexpected error response.", | ||
| "schema": { | ||
| "$ref": "#/definitions/rpcStatus" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls squash into previous commit
| /* lncli: `getconfiginfo` | ||
| GetConfigInfo returns configuration information about the running | ||
| LND node. This includes details about the database backend and | ||
| other operational settings. | ||
| */ | ||
| rpc GetConfigInfo (GetConfigInfoRequest) returns (GetConfigInfoResponse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this if we already have GetDebugInfo?
That's a good callout. I wasn't even aware of this rpc! |
|
yeah could do! |
|
Closing this one. Will open a new one which will add a |