routing: fence payment lifecycles by sequence number

[yyforyongyu]

Summary

• bind each payment lifecycle to the DB payment sequence number observed on its first payment reload
• abort stale lifecycles if the same payment hash now points to a newer payment record
• add coverage for same-hash retry races before the stale lifecycle can register more attempts

Testing

• GOWORK=off go test ./payments/db ./routing ./lnrpc/routerrpc
• make lint

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 improves the robustness of the payment routing system by serializing payment lifecycles based on their payment hash. By enforcing that only one lifecycle is active per hash, it prevents race conditions during payment retries and ensures that state transitions are handled correctly until the entire process has finished.

Highlights

• Payment Lifecycle Serialization: Introduced a multimutex to ensure that only one payment lifecycle per payment hash can be active at any given time.
• Lifecycle Release Mechanism: Implemented a release function that must be called after a payment lifecycle has fully exited, ensuring locks are held across both synchronous and asynchronous payment paths.
• Regression Testing: Added a new test case to verify that a retry attempt with the same payment hash cannot initialize a new payment until the previous lifecycle has completed.

🧠 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.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

🟠 PR Severity: HIGH

Classified from changed files | 2 files (excl. tests) | 82 lines changed (excl. tests)

🟠 High (2 files)

• lnrpc/routerrpc/router_server.go - RPC server layer for the router; falls under lnrpc/* (RPC/API definitions)
• routing/router.go - Core payment pathfinding and routing logic; falls under routing/* (Payment pathfinding algorithms)

Analysis

Both modified files belong to HIGH-severity packages. routing/router.go is the core routing engine responsible for payment pathfinding — changes here directly affect how payments are routed across the network. lnrpc/routerrpc/router_server.go is the RPC interface that exposes routing functionality to clients. Neither file is a database migration, and no CRITICAL-tier packages are touched.

Severity bump check:

• Non-test files changed: 2 (threshold: 20) — no bump
• Non-test lines changed: 82 (threshold: 500) — no bump
• Multiple distinct critical packages: none — no bump

A knowledgeable engineer familiar with the routing subsystem and the RPC layer should review this PR.

---

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

[gemini-code-assist]

Code Review

This pull request introduces a payment lifecycle mutex to ensure serialized execution for a given payment hash, preventing concurrent lifecycles. Changes include updating PreparePayment to return a release function and integrating this lock into synchronous, asynchronous, and resumed payment paths. Feedback suggests extending this locking mechanism to SendToRoute and SendToRouteSkipTempErr to prevent potential race conditions and recommends using defer for the release function in SendPaymentV2 to ensure safety during server shutdown or context cancellation.

[gemini-code-assist]

The lockPaymentLifecycle call is correctly placed before InitPayment to ensure serialization. However, the SendToRoute and SendToRouteSkipTempErr methods (which call the internal sendToRoute) also perform payment initialization and attempt registration but do not appear to use this new locking mechanism. This could lead to race conditions when multiple attempts are registered for the same payment hash concurrently via these entry points, violating the serialization requirement mentioned in the ControlTower interface documentation.

References

1. The payment router/controller layer is responsible for ensuring serialized access per payment hash for calls to RegisterAttempt.

[gemini-code-assist]

When subscribePayment fails, releaseLifecycle() is called to release the mutex. This is correct. However, if the server is shutting down or the context is canceled immediately after, there's a small window where the lock might be held longer than necessary if other cleanup logic is triggered. It might be safer to use a defer for the release function once it's successfully obtained from PreparePayment, provided the logic is refactored to handle the async hand-off correctly.