lnwallet+rpc: add label filter to GetTransactions#10806
Conversation
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 introduces label-based filtering for chain transactions across the RPC, CLI, and internal wallet interfaces. By allowing users to filter transactions by a specific label, it improves the discoverability and management of wallet activity. The changes include updates to the gRPC definition, internal wallet controller logic, and command-line interface, accompanied by comprehensive integration tests to ensure correctness. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an optional "label" filter to the GetTransactions RPC and the listchaintxns CLI command, allowing users to filter transactions by their exact label. The changes span the API definitions, the wallet controller interface, and the btcwallet implementation, supported by new integration tests. Review feedback highlighted a placeholder PR number in the release notes that needs updating and noted significant formatting regressions in the generated gRPC code, suggesting the file be regenerated using the standard project toolchain to reduce diff noise.
github-actions
Bot
added
the
severity-critical
PR Severity: CRITICAL - lnwallet and rpcserver.go changes require expert review. <!-- pr-severity-bot --> |
|
PR Severity: CRITICAL Automated classification | 9 files | 210 lines changed (excluding auto-generated and test files) Critical (5 files):
High (2 files):
Medium (1 file): cmd/commands/commands.go Analysis: This PR modifies lnwallet/* and rpcserver.go, both CRITICAL. The wallet interface change (lnwallet/interface.go) affects channel funding, signing, and commitment transactions. The lnrpc proto addition adds new RPC surface. No severity bump: 9 files (threshold 20) and 210 lines (threshold 500). To override: add a severity-override-{critical,high,medium,low} label. |
Adds an optional `label` string field to `GetTransactionsRequest` (proto field 6) so callers can filter chain transactions by label. - `WalletController.ListTransactionDetails` gains a `labelFilter string` parameter; when non-empty, only transactions whose `Label` field exactly matches are returned (client-side filter after the full fetch). - `rpcserver.GetTransactions` threads `req.Label` through to the wallet call. - `lncli listchaintxns` gains a `--label` flag. - All mocks and the integration-test harness are updated to pass the new parameter. - New integration test `testListTransactionDetailsLabelFilter` covers: exact match by label, no-filter returns all, and unknown label returns empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
saraogiraj94
force-pushed
the
label-filter-get-transactions
branch
from
639f5a4 to
34c70da
Compare
|
Looks good. Nice use of the in place filter! |
|
|
||
| // If a label filter is specified, only include transactions whose label | ||
| // matches exactly. | ||
| if labelFilter != "" { |
There was a problem hiding this comment.
This look solid!
However, there might be a little bug here with how pagination interacts with this filter. Since the code above this block populates txDetails from the database using the raw maxTransactions limit, the result set is already truncated before it hits this loop.
If a node has thousands of transactions, but the client queries with a default maxTransactions window (like 20 or 50) along with a specific label, the filter will only check that initial batch. If the matching transaction sits further down in the history, it'll be missed entirely. The label filter likely needs to be applied to the raw results before the pagination slices and offsets are handled.
| require.NoError(t, err) | ||
| require.Len(t, txDetails, 0) | ||
| } | ||
|
|
There was a problem hiding this comment.
The test cases cover the exact matching nicely.
One minor thing: all the test queries here pass 0 for maxTransactions, which fetches the whole history at once and disables pagination boundaries. Because of that, it doesn't catch the edge case where a client passes both a label filter and a tight max_transactions limit at the same time.
It would be great to add one more test assertion in here that uses a small maxTransactions limit to verify that pagination doesn't accidentally chop off matching records from the results.
3 participants
Closes #10785
Summary
labelfield (proto field 6) toGetTransactionsRequestso callers can filter chain transactions by their labelWalletController.ListTransactionDetailsgains alabelFilter stringparameter; when non-empty, only transactions whoseLabelfield exactly matches are returnedlncli listchaintxnsgains a--labelflagTesting
testListTransactionDetailsLabelFiltercovers: exact match by label A, exact match by label B, no filter returns all, unknown label returns emptyRelease Notes
Updates
docs/release-notes/release-notes-0.22.0.mdunder RPC Additions and lncli Additions.🤖 Generated with Claude Code