Skip to content

Restore session message is sent to trade key#540

Merged
grunch merged 2 commits into
mainfrom
fix-restore-receiver-key
Oct 24, 2025
Merged

Restore session message is sent to trade key#540
grunch merged 2 commits into
mainfrom
fix-restore-receiver-key

Conversation

@arkanoider

@arkanoider arkanoider commented Oct 23, 2025

Copy link
Copy Markdown
Collaborator

@grunch ,

here is the fix for the restore session message, now is sent correctly to the trade key!

Summary by CodeRabbit

  • Refactor
    • Internal session restoration workflow optimizations and parameter alignment
    • Background communication channel efficiency improvements

@coderabbitai

coderabbitai Bot commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The pull request refactors the restore session workflow by replacing master_key with trade_key terminology throughout internal function parameters and variable names. Additionally, the RestoreSessionManager's background channel buffer capacity is reduced from 100 to 10, altering backpressure characteristics during result delivery.

Changes

Cohort / File(s) Summary
Restore session key terminology update
src/app/restore_session.rs
Renamed master_key parameter to trade_key across restore_session_action, handle_restore_session_results, send_restore_session_response, and helper functions; all derived PublicKey references and log messages updated accordingly. External public API signatures unchanged.
Channel buffer capacity reduction
src/db.rs
Reduced RestoreSessionManager background channel buffer from 100 to 10, tightening inter-task communication capacity and increasing potential sender blocking under load.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • grunch

Poem

🐰 A rabbit hops through code so neat,
Swapping terms to make logic complete,
Trade keys sparkle where masters once stood,
Buffers tightened—backpressure feels good!
Internal tweaks, yet the API still sings,
Simple refactors on elegant wings. 🌿

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Restore session message is sent to trade key" directly and accurately describes the primary change in the changeset. The core modification replaces master_key with trade_key throughout the restore session workflow, fundamentally changing where the restore session message is routed. The title is concise, clear, and specific enough that a teammate reviewing the commit history would immediately understand the key change without relying on additional context. It avoids vague language and accurately reflects the developer's intent as stated in the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-restore-receiver-key

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4498ac8 and 22161c9.

📒 Files selected for processing (2)
  • src/app/restore_session.rs (4 hunks)
  • src/db.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/app/**

📄 CodeRabbit inference engine (AGENTS.md)

src/app/**: Place order flow runtime code under src/app/
Mirror fixtures used by tests under src/app/

Files:

  • src/app/restore_session.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Run cargo fmt and adhere to the Rustfmt profile
Keep cargo clippy --all-targets --all-features clean
Use Rust 2021 defaults with four-space indentation
Use snake_case for function names
Use PascalCase for type names
Use SCREAMING_SNAKE_CASE for constants
Document non-obvious public APIs with triple-slash (///) doc comments
Favor tracing spans over ad-hoc logging
Co-locate tests within their modules inside a mod tests
Name test cases descriptively (e.g., handles_expired_hold_invoice)

Files:

  • src/app/restore_session.rs
  • src/db.rs
🧬 Code graph analysis (1)
src/app/restore_session.rs (1)
src/util.rs (1)
  • enqueue_restore_session_msg (912-920)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: clippy
🔇 Additional comments (6)
src/db.rs (1)

1421-1421: Verify the channel buffer size reduction.

The buffer capacity has been reduced from 100 to 10. While this reduces memory usage, it may increase sender blocking if more than 10 restore sessions complete before the consumer processes results.

Can you confirm this change is intentional and that a buffer of 10 is sufficient for the expected concurrent restore session load? If this was reduced to address a specific issue, please clarify the reasoning.

src/app/restore_session.rs (5)

16-27: LGTM! Proper extraction and validation of trade key.

The code correctly extracts the trade key from event.rumor.pubkey and validates it with the same checks as the master key. This ensures the response is sent to the correct ephemeral session key rather than the master key.


45-45: LGTM! Core fix correctly implemented.

The spawned task now receives trade_key for sending responses, while the database query (line 40) continues to use master_key for lookup. This correctly fixes the bug where restore session messages were sent to the wrong key.


52-79: LGTM! Results handler correctly uses trade key.

The function parameter has been correctly renamed to trade_key, and all response paths (success and timeout) now send messages to the trade key. The error handling remains appropriate.


83-105: LGTM! Response function correctly sends to trade key.

The function properly converts the trade key string to a PublicKey and uses it as the destination for the restore session response. Error handling for invalid public keys is appropriate.


107-118: LGTM! Timeout function correctly sends to trade key.

The timeout handler mirrors the response function pattern, properly converting the trade key to a PublicKey and using it as the destination for the timeout message.


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

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

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@grunch grunch merged commit 8460e71 into main Oct 24, 2025
6 checks passed
@grunch grunch deleted the fix-restore-receiver-key branch October 24, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants