M-H03: fix(clearnode): log correct fields on failure

[nksazonov]

Description

RequestCreation checks asset support against the home ledger, but on the rejection path it formats the error using incomingState.EscrowLedger, which can be nil for normal channel creation requests.

    if !ok {
        c.Fail(rpc.Errorf(
            "asset %s is not supported on blockchain %d with token address %s",
            incomingState.Asset,
            incomingState.EscrowLedger.BlockchainID,
            incomingState.EscrowLedger.TokenAddress), "")
        return
    }

Because toCoreState explicitly allows EscrowLedger to be absent, a request that omits EscrowLedger and uses an unsupported home token/chain pair will trigger a nil-pointer panic before the handler can return a normal RPC error.
This panic happens before c.Fail can populate a response. The RPC server executes handlers in a separate goroutine and does not recover panics in that path, so a single malformed  channels.v1.request_creation request can terminate the entire clearnode process.
Consequently, a caller can crash the off-chain node by submitting a channel creation request with an unsupported home asset and no EscrowLedger, causing a service-wide denial of service for all users.

Summary by CodeRabbit

• Bug Fixes
  • Corrected diagnostic error messages to report accurate asset information when unsupported assets are encountered.
  • Enhanced system resilience by implementing panic recovery during request processing, enabling the system to gracefully handle unexpected errors and return proper error responses to clients instead of terminating.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5474533d-82c1-458d-aac0-2c5a88eb1422

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Two independent fixes: correcting diagnostic message parameters from EscrowLedger to HomeLedger references in request creation error handling, and adding panic recovery to RPC node request handling to prevent unhandled panics from terminating execution.

Changes

Cohort / File(s)	Summary
Diagnostic Message Fix clearnode/api/channel_v1/request_creation.go	Updated error message to reference correct ledger fields (incomingState.HomeLedger.BlockchainID and incomingState.HomeLedger.TokenAddress) instead of escrow ledger fields in unsupported asset validation error.
Panic Recovery pkg/rpc/node.go	Added inline defer/recover() wrapper around handler chain execution to catch panics, log error details with method information, and return error response instead of propagating panic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A message now points to the right ledger home,
And panics? They're caught before making us roam!
Two bugs gently fixed with surgical care,
Diagnostics and safety—a well-matched pair! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions fixing incorrect fields but does not reflect the primary objective of adding panic recovery to prevent DoS.	Update the title to emphasize the main change: adding panic recovery in the handler, not just the field reference fix. Consider: 'fix(clearnode): recover from handler panics to prevent DoS' or similar.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/audit-m-h03

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/rpc/node.go`:
- Around line 315-323: The panic recover block inside the anonymous goroutine
should stop further processing so we don't marshal/send a second response or
re-panic: import runtime/debug, capture and log the stack via
wn.cfg.Logger.Error(..., "stack", string(debug.Stack())), and set a local flag
(e.g., recovered := true) or return from the outer function path so that after
calling wn.sendErrorResponse(...) the code does not continue to the ctx.Next()
result handling/marshaling; then guard the subsequent marshal/send logic (the
code using ctx.Response and marshalling) with a check that no panic was
recovered before proceeding.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bf388ee5-5620-460e-aaba-84c750644a35

📥 Commits

Reviewing files that changed from the base of the PR and between e85051a and 29e161e.

📒 Files selected for processing (2)

• clearnode/api/channel_v1/request_creation.go
• pkg/rpc/node.go