routing: add mission control namespace support to SendPaymentV2 #9907
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This commit adds a new optional mission_control_namespace field to the SendPaymentRequest RPC message. This field allows callers to specify a custom namespace for mission control, enabling different routing strategies and payment history isolation for different payment types. The namespace field is added as field number 26 in the SendPaymentRequest message. When not specified, the default mission control namespace will be used, maintaining backward compatibility.
This commit enhances the routing package to support namespaced mission control for payment routing. The changes include: - Added MissionControlNamespace field to LightningPayment struct to propagate namespace through the routing layer - Modified SessionSource to accept a GetMissionControl function that retrieves namespaced MissionControl instances - Updated NewPaymentSession to use the namespaced MissionControl when a custom namespace is specified in the payment These changes enable different payment types to maintain separate routing histories and success/failure statistics through mission control namespaces. Unit tests are included to verify the namespace functionality.
This commit connects the RPC layer to the routing layer for mission control namespace support: - Updated extractIntentFromSendRequest in router_backend.go to propagate the mission_control_namespace from the RPC request to the payment intent - Modified server.go to provide a GetMissionControl function to the SessionSource that retrieves namespaced MissionControl instances from the MissionController These changes complete the integration between the RPC interface and the routing engine, allowing SendPaymentV2 callers to specify custom mission control namespaces for their payments.
This commit adds integration test coverage for the mission control namespace feature. The test verifies that: - Payments can be sent using the default namespace - Payments can be sent using a custom namespace - Both namespace types work correctly and maintain isolation - Mission control records payment attempts in the appropriate namespace The test creates a simple two-node network, sends payments with different namespaces, and verifies that mission control is tracking the payment attempts correctly. This ensures end-to-end functionality of the namespace feature from the RPC layer through to mission control.
In this commit, we enhance the itess to exercise the new query+reset+import namespace features. This also rounds out the test as well to make sure that the mc payment portion works properly.
|
Rounded things out by adding the mc namespace support to query+reset+import mc. I think there's a gap here though: we don't persist the mc namespace for a payment, so on restart, if we resume, it'll fall back to the default mc. |
Roasbeef
force-pushed
the
mission-control-path-finding
branch
from
926290f to
c34c35d
Compare
There was a problem hiding this comment.
Awesome change. Been hanging out in routing and lnrpc/routerrpc packages so this was a pretty straightforward read. Just a couple comments otherwise this looks good to me. utACK, but will give this a try locally.
Re: the gap after restart - currently we retrieve a list of channeldb.MPPayment from disk and spin up "empty" sessions when resuming payments. We might have to include the mission control namespace in the channeldb.PaymentCreationInfo we write to disk in InitPayment.
| @@ -1192,6 +1213,22 @@ func (s *Server) XImportMissionControl(_ context.Context, | |||
| return nil, err | |||
| } | |||
|
|
|||
| namespace := req.MissionControlNamespace | |||
There was a problem hiding this comment.
With this new logic for namespaced import, we can probably omit the call to s.cfg.RouterBackend.MissionControl.ImportHistory() on line 1209?
| func (a *missionControllerAdapter) GetNamespacedStore(namespace string) (routerrpc.MissionControl, error) { | ||
| // The underlying GetNamespacedStore returns *routing.MissionControl. | ||
| // Since *routing.MissionControl implements routerrpc.MissionControl, | ||
| // this direct return is valid. |
There was a problem hiding this comment.
nit: The comment on GetNamespacedStore together with the the comment on this function call might over explain the validity of the return type.
| // Create two nodes. Alice gets funded. | ||
|
|
||
| // We'll create a simple two node network for this tesat. |
There was a problem hiding this comment.
nit: small typos/tesat/test
| FromNode: fromNode[:], | ||
| ToNode: toNode[:], | ||
| AmtMsat: int64(amtMsat), | ||
| MissionControlNamespace: ctx.String("mission_control_namespace"), |
There was a problem hiding this comment.
Interestingly the RPC is marked deprecated. It is hidden from the lncli help output, but is still available for use. In any case, we might need to update the server side implementation to actually make use of this new request parameter.
|
|
||
| // An optional mission control namespace to query. If not specified, the | ||
| // default mission control namespace will be used. | ||
| string mission_control_namespace = 4; |
There was a problem hiding this comment.
Mentioned this in previous comment, but I think we're missing the corresponding update to the server side implementation to actually make use of this new request parameter.
| // payment session. | ||
| // | ||
| // NOTE: Part of the PaymentSession interface. | ||
| func (p *paymentSession) MissionControl() fn.Option[MissionControlQuerier] { |
There was a problem hiding this comment.
nit: move this further down the file so that the constructor/newPaymentSession immediately follows its type definition.
|
@Roasbeef, remember to re-request review from reviewers when ready |
This PR adds the ability for callers of SendPaymentV2 to specify a custom mission control namespace for their payments. This enables applications to maintain separate routing histories and
success/failure statistics for different types of payments.
Motivation
Currently, all payments share the same mission control state, which can be problematic when an application needs different routing strategies for different payment types. For example, you might want
high-value payments to use more conservative routing while micropayments could be more experimental. Or you might want to isolate routing data between different services using the same LND node.
With this change, applications can specify a namespace when making payments, and each namespace maintains its own view of channel reliability and routing success rates.
Changes
The implementation threads a new optional
mission_control_namespacefield through the payment flow:When no namespace is specified, the default mission control is used, maintaining full backward compatibility.
The changes have been tested with both unit tests for the routing logic and an integration test that verifies end-to-end functionality of payments with different namespaces.