Skip to content

rpcperms+lncli: return gRPC status codes from RPC state interceptor#10945

Open
kpshukla wants to merge 1 commit into
lightningnetwork:masterfrom
kpshukla:fix/5586-grpc-status-codes-rpc-interceptor
Open

rpcperms+lncli: return gRPC status codes from RPC state interceptor#10945
kpshukla wants to merge 1 commit into
lightningnetwork:masterfrom
kpshukla:fix/5586-grpc-status-codes-rpc-interceptor

Conversation

@kpshukla

@kpshukla kpshukla commented Jul 1, 2026

Copy link
Copy Markdown

Summary

Fixes #5586

The checkRPCState interceptor previously returned plain Go errors, which gRPC forwarded to clients as codes.Unknown. Callers (including lncli) were forced to do brittle string matching against the error message to determine the cause.

This PR replaces each plain error return with a status.Error carrying a semantically correct gRPC code, while keeping the error message text identical to preserve any existing string-match code:

Condition Before After
LND still starting codes.Unknown codes.Unavailable
RPC server starting codes.Unknown codes.Unavailable
No wallet created codes.Unknown codes.FailedPrecondition
Wallet locked codes.Unknown codes.FailedPrecondition
Wallet already unlocked codes.Unknown codes.FailedPrecondition
Unknown state codes.Unknown codes.Unknown (unchanged)

The two lncli call-sites in cmd/commands/commands.go that checked codes.Unknown are updated to use codes.FailedPrecondition.

Test plan

  • New table-driven test TestCheckRPCStateGRPCCodes covers all RPC states and server types (10 subtests, all pass)
  • Existing rpcperms tests still pass
  • make install compiles cleanly

🤖 Generated with Claude Code

The checkRPCState function previously returned plain Go errors which gRPC
forwarded to clients as codes.Unknown. Callers were forced to do brittle
string matching against the error message to identify the cause.

Replace each plain error return with a status.Error carrying a semantically
correct code:

  - ErrWaitingToStart  -> codes.Unavailable
  - ErrRPCStarting     -> codes.Unavailable
  - ErrNoWallet        -> codes.FailedPrecondition
  - ErrWalletLocked    -> codes.FailedPrecondition
  - ErrWalletUnlocked  -> codes.FailedPrecondition
  - unknown state      -> codes.Unknown

The error message text is preserved in every case so existing string-match
code keeps working.

Update the two call-sites in lncli that checked codes.Unknown to use the
correct codes.FailedPrecondition instead.

Fixes lightningnetwork#5586

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the severity-medium Focused review required label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🟡 PR Severity: MEDIUM

gh pr view | 4 files | 146 lines changed (29 additions / 9 deletions excluding tests)

🟡 Medium (2 files)
  • cmd/commands/commands.go - CLI client code (cmd/*); updates two call-sites to check codes.FailedPrecondition instead of codes.Unknown
  • rpcperms/interceptor.go - not in a specially-categorized package; classified as "other Go file"
🟢 Low (2 files)
  • rpcperms/interceptor_test.go - test-only change (new table-driven test, excluded from bump counting)
  • docs/release-notes/release-notes-0.22.0.md - release notes entry

Analysis

The highest-severity files touched are cmd/commands/commands.go (CLI, explicitly MEDIUM per the classification rules for cmd/*) and rpcperms/interceptor.go, which doesn't fall under any of the CRITICAL/HIGH package prefixes (it's not lnwallet, htlcswitch, contractcourt, sweep, peer, keychain, input, channeldb, funding, lnwire, nor server.go/rpcserver.go), so it defaults to MEDIUM ("other Go files not categorized above").

The change itself is narrowly scoped: it maps existing plain errors in the checkRPCState interceptor to semantically correct gRPC status codes while preserving the original error message text, and updates two lncli call-sites to match. No bump conditions apply — only 3 non-test/non-doc files changed, and non-test lines changed (~38) are far below the 500-line bump threshold. No critical packages are touched at all.


To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

severity-medium Focused review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rpcperm: rpc state/status interceptor should return gRPC error codes

1 participant