From 21f7ca810b2a087ba5f1dc121693646a7bc87f7e Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 11:18:13 +0200 Subject: [PATCH 1/6] feat: mandatory SourceChanId and targetPubkey to avoid creating a same peer rebalance --- src/Proto/nodeguard.proto | 6 +- src/Rpc/NodeGuardService.cs | 10 +- src/Services/RebalanceService.cs | 37 +++- src/Shared/NewRebalanceModal.razor | 52 +++-- .../Services/RebalanceServiceTests.cs | 183 ++++++++++++++++-- 5 files changed, 240 insertions(+), 48 deletions(-) diff --git a/src/Proto/nodeguard.proto b/src/Proto/nodeguard.proto index fdce92ee..59934937 100644 --- a/src/Proto/nodeguard.proto +++ b/src/Proto/nodeguard.proto @@ -512,10 +512,10 @@ message RequestRebalanceRequest { optional double max_fee_pct = 3; // Optional max fee cap for retry attempts, expressed as a percentage of the rebalanced amount (not a fraction of the outbound rate). e.g. 0.075 means 0.075% = 750 ppm. If unset, falls back to REBALANCE_DEFAULT_RETRY_MAX_FEE_PCT optional double retry_max_fee_pct = 4; - // Optional NG channel id to use as the outbound channel + // NG channel id to use as the outbound channel. Required. optional int32 source_channel_id = 5; - // Optional explicit last-hop peer pubkey (33-byte hex). If unset, LND picks any - // inbound channel that satisfies the cost cap. + // Last-hop peer pubkey (33-byte hex) that constrains the receiving peer of the + // circular payment. Required. optional string target_pubkey = 6; // Pathfinding/payment timeout in seconds; 0 → server default (60s) int32 timeout_seconds = 7; diff --git a/src/Rpc/NodeGuardService.cs b/src/Rpc/NodeGuardService.cs index 7046aad1..7860291f 100644 --- a/src/Rpc/NodeGuardService.cs +++ b/src/Rpc/NodeGuardService.cs @@ -1212,14 +1212,20 @@ public override async Task RequestRebalance(RequestReb if (request.AmountSats <= 0) throw new RpcException(new Status(StatusCode.InvalidArgument, "amount_sats must be > 0")); + if (!request.HasSourceChannelId || request.SourceChannelId <= 0) + throw new RpcException(new Status(StatusCode.InvalidArgument, "source_channel_id is required")); + + if (!request.HasTargetPubkey || string.IsNullOrWhiteSpace(request.TargetPubkey)) + throw new RpcException(new Status(StatusCode.InvalidArgument, "target_pubkey is required")); + var node = await _nodeRepository.GetByPubkey(request.NodePubkey); if (node == null) throw new RpcException(new Status(StatusCode.NotFound, $"Node with pubkey {request.NodePubkey} not found")); var domainRequest = new RebalanceRequest( NodeId: node.Id, - SourceChannelId: request.HasSourceChannelId ? request.SourceChannelId : null, - TargetPubkey: request.HasTargetPubkey ? request.TargetPubkey : null, + SourceChannelId: request.SourceChannelId, + TargetPubkey: request.TargetPubkey, AmountSats: request.AmountSats, MaxFeePct: request.HasMaxFeePct ? request.MaxFeePct : null, TimeoutSeconds: request.TimeoutSeconds, diff --git a/src/Services/RebalanceService.cs b/src/Services/RebalanceService.cs index 0452d2fc..a929d801 100644 --- a/src/Services/RebalanceService.cs +++ b/src/Services/RebalanceService.cs @@ -28,8 +28,8 @@ namespace NodeGuard.Services; public record RebalanceRequest( int NodeId, - int? SourceChannelId, - string? TargetPubkey, + int SourceChannelId, + string TargetPubkey, long AmountSats, double? MaxFeePct, int TimeoutSeconds = 60, @@ -104,18 +104,35 @@ public async Task RebalanceAsync(RebalanceRequest request, Cancellati "Retry max fee % must be greater than 0.", nameof(request.RetryMaxFeePct)); + if (request.SourceChannelId <= 0) + throw new ArgumentException( + "Source channel id is required.", + nameof(request.SourceChannelId)); + + if (string.IsNullOrWhiteSpace(request.TargetPubkey)) + throw new ArgumentException( + "Target pubkey is required.", + nameof(request.TargetPubkey)); + var node = await _nodeRepository.GetById(request.NodeId); if (node == null) throw new ArgumentException($"Node {request.NodeId} not found", nameof(request.NodeId)); - ulong? sourceChanIdLnd = null; - if (request.SourceChannelId.HasValue) - { - var sourceChannel = await _channelRepository.GetById(request.SourceChannelId.Value); - if (sourceChannel == null) - throw new ArgumentException($"Source channel {request.SourceChannelId} not found"); - sourceChanIdLnd = sourceChannel.ChanId; - } + var sourceChannel = await _channelRepository.GetById(request.SourceChannelId); + if (sourceChannel == null) + throw new ArgumentException( + $"Source channel {request.SourceChannelId} not found", + nameof(request.SourceChannelId)); + var sourceChanIdLnd = sourceChannel.ChanId; + + var counterpartyPeerPubkey = sourceChannel.SourceNodeId == node.Id + ? sourceChannel.DestinationNode?.PubKey + : sourceChannel.SourceNode?.PubKey; + + if (counterpartyPeerPubkey == request.TargetPubkey) + throw new ArgumentException( + "Target pubkey is the same as the source channel's counterparty peer; rebalance would be a no-op.", + nameof(request.TargetPubkey)); // LastHopPubkey constrains the receiving peer, not a specific channel — LND picks // which of that peer's channels to use. We don't accept or persist a target chan_id. diff --git a/src/Shared/NewRebalanceModal.razor b/src/Shared/NewRebalanceModal.razor index fcdb80d3..a43ad843 100644 --- a/src/Shared/NewRebalanceModal.razor +++ b/src/Shared/NewRebalanceModal.razor @@ -58,26 +58,28 @@ - Receiving peer (optional) - + Receiving peer + + DefaultItemText="Choose a receiving peer"> - @if (string.IsNullOrEmpty(_selectedTargetPubkey)) + @if (!string.IsNullOrEmpty(SourceChannelCounterpartyPubkey)) { - No receiving peer pinned — LND will pick any inbound peer that satisfies the cost cap. +
+ Source channel's counterparty (@SourceChannelCounterpartyPubkey.Substring(0, 10)…) is excluded — pinning the last hop to that peer would be a no-op. +
} - else if (_targetOutboundPpm.HasValue) + @if (!string.IsNullOrEmpty(_selectedTargetPubkey) && _targetOutboundPpm.HasValue) { Outbound fee rate to this peer: @_targetOutboundPpm.Value ppm (for reference only). } @@ -107,12 +109,9 @@ @RenderBalanceBar("Before", _sourceLocalSats.Value, _sourceRemoteSats.Value) @RenderBalanceBar("After", SourceLocalAfter, SourceRemoteAfter, isProjection: true) - @if (!string.IsNullOrEmpty(_selectedTargetPubkey)) - { -
- Receiving peer pinned: @_selectedTargetPubkey.Substring(0, 10)…. The exact channel that lands the sats is LND's call — no per-channel projection possible. -
- } +
+ Receiving peer pinned: @_selectedTargetPubkey?.Substring(0, 10)…. The exact channel that lands the sats is LND's call — no per-channel projection possible. +
@{ @@ -301,6 +300,21 @@ (c.SourceNode?.PubKey == _selectedNode?.PubKey ? c.DestinationNode?.PubKey : c.SourceNode?.PubKey) == _selectedSourcePubkey ).ToList(); + // Pubkey of the peer on the OTHER side of the selected source channel. Pinning the + // last-hop to this same peer would route the sats straight back to where they came from + // (no-op rebalance). The service rejects this combination; the modal mirrors the guard. + private string? SourceChannelCounterpartyPubkey => + _selectedSourceChannel == null || _selectedNode == null + ? null + : _selectedSourceChannel.SourceNodeId == _selectedNode.Id + ? _selectedSourceChannel.DestinationNode?.PubKey + : _selectedSourceChannel.SourceNode?.PubKey; + + private List AvailableReceivingPeers => + string.IsNullOrEmpty(SourceChannelCounterpartyPubkey) + ? _peerOptions + : _peerOptions.Where(p => p.Pubkey != SourceChannelCounterpartyPubkey).ToList(); + private decimal CurrentInboundPct => _sourceLocalSats.HasValue && _sourceRemoteSats.HasValue && (_sourceLocalSats.Value + _sourceRemoteSats.Value) > 0 @@ -325,6 +339,8 @@ private bool CanSubmit => _selectedNode != null && _selectedSourceChannel != null + && !string.IsNullOrEmpty(_selectedTargetPubkey) + && _selectedTargetPubkey != SourceChannelCounterpartyPubkey && _sourceLocalSats.HasValue && ComputeAmountSats() > 0 && _maxFeePct > 0m @@ -474,6 +490,14 @@ _selectedSourceChannel = _nodeChannels.FirstOrDefault(c => c.Id == channelId); _sourceLocalSats = null; _sourceRemoteSats = null; + // If the user had already picked a receiving peer and the newly-chosen source channel + // makes that peer invalid (it's the channel's counterparty), drop the selection so + // CanSubmit doesn't lie and the user re-picks against the filtered list. + if (!string.IsNullOrEmpty(_selectedTargetPubkey) && _selectedTargetPubkey == SourceChannelCounterpartyPubkey) + { + _selectedTargetPubkey = null; + _targetOutboundPpm = null; + } if (_selectedSourceChannel != null && _selectedNode != null) { var (local, remote) = await TryGetLiveBalance(_selectedNode, _selectedSourceChannel.ChanId); @@ -556,7 +580,7 @@ var request = new RebalanceRequest( NodeId: _selectedNode!.Id, SourceChannelId: _selectedSourceChannel!.Id, - TargetPubkey: _selectedTargetPubkey, + TargetPubkey: _selectedTargetPubkey!, AmountSats: amountSats, MaxFeePct: _maxFeePct > 0m ? (double)_maxFeePct : (double?)null, TimeoutSeconds: _timeoutSeconds, diff --git a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs index 1004aa69..5ac17712 100644 --- a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs +++ b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs @@ -44,6 +44,10 @@ public RebalanceServiceTests() { _schedulerFactory.Setup(x => x.GetScheduler(It.IsAny())) .ReturnsAsync(_scheduler.Object); + // SourceChannelId is now mandatory on RebalanceRequest, so any test that reaches the + // source-channel lookup needs a Channel back. Pre-stub here so individual tests don't + // each have to repeat it. Tests that explicitly want the lookup to fail can override. + StubChannelRepo(); } private RebalanceService CreateService() => new( @@ -65,6 +69,29 @@ private static Node CreateNode(int id = 1, string pubkey = "03000000000000000000 ChannelAdminMacaroon = "mac", }; + // SourceChannelId and TargetPubkey are required on the domain request; tests that don't care + // about either still need to supply something that satisfies the validators (and, for flow + // tests, something the channel repo can resolve via StubChannelRepo). + private const int ValidChannelId = 1; + private const string ValidTargetPubkey = "030000000000000000000000000000000000000000000000000000000000000099"; + + /// + /// Stubs ChannelRepository.GetById so the source-channel lookup inside RebalanceAsync resolves + /// to a real-looking Channel instead of throwing. Flow tests (anything that reaches the node + /// lookup) must call this — validation-throw tests can skip it because the throw fires earlier. + /// + private void StubChannelRepo(int channelId = ValidChannelId, ulong chanIdLnd = 12345UL) + { + _channelRepo.Setup(x => x.GetById(channelId)).ReturnsAsync(new Channel + { + Id = channelId, + ChanId = chanIdLnd, + SatsAmount = 1_000_000, + Status = Channel.ChannelStatus.Open, + FundingTx = "tx", + }); + } + /// /// Wires up the repository mocks so that AddAsync stamps an Id and GetById returns the /// same instance — mimicking what EF would do without an actual database. @@ -88,7 +115,7 @@ public async Task RebalanceAsync_AmountZero_ThrowsWithTargetEqualsCurrentMessage // Amount=0 is what the modal computes when target inbound % equals (or is below) current inbound %. // The service must reject this with a clear, user-facing message. var service = CreateService(); - var request = new RebalanceRequest(NodeId: 1, null, null, AmountSats: 0, MaxFeePct: null); + var request = new RebalanceRequest(NodeId: 1, ValidChannelId, ValidTargetPubkey, AmountSats: 0, MaxFeePct: null); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) .Should().ThrowAsync() @@ -100,7 +127,7 @@ public async Task RebalanceAsync_ProbeBackoffRatioOutOfRange_Throws() { // Ratio must be in (0, 1) exclusive. 1.0 never shrinks; 0 zeroes the next try. var service = CreateService(); - var request = new RebalanceRequest(NodeId: 1, null, null, + var request = new RebalanceRequest(NodeId: 1, ValidChannelId, ValidTargetPubkey, AmountSats: 1000, MaxFeePct: null, ProbeBackoffRatio: 1.0); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) @@ -112,7 +139,7 @@ await FluentActions.Awaiting(() => service.RebalanceAsync(request)) public async Task RebalanceAsync_MaxAttemptsZeroOrNegative_Throws() { var service = CreateService(); - var request = new RebalanceRequest(NodeId: 1, null, null, + var request = new RebalanceRequest(NodeId: 1, ValidChannelId, ValidTargetPubkey, AmountSats: 1000, MaxFeePct: null, MaxAttempts: 0); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) @@ -124,7 +151,7 @@ await FluentActions.Awaiting(() => service.RebalanceAsync(request)) public async Task RebalanceAsync_RetryMaxFeePctOutOfRange_Throws() { var service = CreateService(); - var request = new RebalanceRequest(NodeId: 1, null, null, + var request = new RebalanceRequest(NodeId: 1, ValidChannelId, ValidTargetPubkey, AmountSats: 1000, MaxFeePct: null, RetryMaxFeePct: -0.1); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) @@ -137,12 +164,130 @@ public async Task RebalanceAsync_NodeNotFound_Throws() { _nodeRepo.Setup(x => x.GetById(99, It.IsAny())).ReturnsAsync((Node?)null); var service = CreateService(); - var request = new RebalanceRequest(NodeId: 99, null, null, AmountSats: 1000, MaxFeePct: null); + var request = new RebalanceRequest(NodeId: 99, ValidChannelId, ValidTargetPubkey, AmountSats: 1000, MaxFeePct: null); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) .Should().ThrowAsync(); } + [Fact] + public async Task RebalanceAsync_SourceChannelIdMissing_Throws() + { + var service = CreateService(); + var request = new RebalanceRequest(NodeId: 1, SourceChannelId: 0, ValidTargetPubkey, + AmountSats: 1000, MaxFeePct: null); + + await FluentActions.Awaiting(() => service.RebalanceAsync(request)) + .Should().ThrowAsync() + .WithMessage("*Source channel id is required*"); + } + + [Fact] + public async Task RebalanceAsync_TargetPubkeyEmpty_Throws() + { + var service = CreateService(); + var request = new RebalanceRequest(NodeId: 1, ValidChannelId, TargetPubkey: " ", + AmountSats: 1000, MaxFeePct: null); + + await FluentActions.Awaiting(() => service.RebalanceAsync(request)) + .Should().ThrowAsync() + .WithMessage("*Target pubkey is required*"); + } + + [Fact] + public async Task RebalanceAsync_SourceChannelNotFound_Throws() + { + // ValidChannelId is pre-stubbed in the constructor; this test overrides to null so the + // source-channel lookup fails after the args pass the cheap validators. + var node = CreateNode(); + _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); + _channelRepo.Setup(x => x.GetById(ValidChannelId)).ReturnsAsync((Channel?)null); + + var service = CreateService(); + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, + AmountSats: 100_000, MaxFeePct: null); + + await FluentActions.Awaiting(() => service.RebalanceAsync(request)) + .Should().ThrowAsync() + .WithMessage($"*Source channel {ValidChannelId} not found*"); + } + + [Fact] + public async Task RebalanceAsync_TargetPubkeyEqualsSourceCounterparty_Throws() + { + // Pinning the LastHopPubkey to the same peer that's already the source channel's + // counterparty would route the sats straight back to where they came from — a no-op + // rebalance. The service must reject this. + var node = CreateNode(); + var peerPubkey = "030000000000000000000000000000000000000000000000000000000000000002"; + _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); + _channelRepo.Setup(x => x.GetById(ValidChannelId)).ReturnsAsync(new Channel + { + Id = ValidChannelId, + ChanId = 12345UL, + SatsAmount = 1_000_000, + Status = Channel.ChannelStatus.Open, + FundingTx = "tx", + // Our node opened the channel; the counterparty is the destination. + SourceNodeId = node.Id, + SourceNode = node, + DestinationNodeId = 2, + DestinationNode = new Node + { + Id = 2, + Name = "peer", + PubKey = peerPubkey, + Endpoint = "peer:10009", + ChannelAdminMacaroon = "mac", + }, + }); + + var service = CreateService(); + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: peerPubkey, + AmountSats: 100_000, MaxFeePct: null); + + await FluentActions.Awaiting(() => service.RebalanceAsync(request)) + .Should().ThrowAsync() + .WithMessage("*same as the source channel's counterparty peer*"); + } + + [Fact] + public async Task RebalanceAsync_TargetPubkeyEqualsSourceCounterparty_NodeIsDestination_Throws() + { + // Same guard, mirrored: the local node is the channel's DESTINATION (peer opened the + // channel to us). Counterparty is then the SourceNode. + var node = CreateNode(); + var peerPubkey = "030000000000000000000000000000000000000000000000000000000000000003"; + _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); + _channelRepo.Setup(x => x.GetById(ValidChannelId)).ReturnsAsync(new Channel + { + Id = ValidChannelId, + ChanId = 67890UL, + SatsAmount = 1_000_000, + Status = Channel.ChannelStatus.Open, + FundingTx = "tx", + SourceNodeId = 2, + SourceNode = new Node + { + Id = 2, + Name = "peer", + PubKey = peerPubkey, + Endpoint = "peer:10009", + ChannelAdminMacaroon = "mac", + }, + DestinationNodeId = node.Id, + DestinationNode = node, + }); + + var service = CreateService(); + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: peerPubkey, + AmountSats: 100_000, MaxFeePct: null); + + await FluentActions.Awaiting(() => service.RebalanceAsync(request)) + .Should().ThrowAsync() + .WithMessage("*same as the source channel's counterparty peer*"); + } + [Fact] public async Task RebalanceAsync_UserSuppliedFeePct_PersistedAsIs() { @@ -158,7 +303,7 @@ public async Task RebalanceAsync_UserSuppliedFeePct_PersistedAsIs() .ReturnsAsync(new ProbeResult.NoRoute("test")); var service = CreateService(); - var request = new RebalanceRequest(NodeId: node.Id, null, null, + var request = new RebalanceRequest(NodeId: node.Id, ValidChannelId, ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: 0.1234); var result = await service.RebalanceAsync(request); @@ -183,7 +328,7 @@ public async Task RebalanceAsync_UserSuppliedProbeBackoffRatio_PersistedAndPasse .ReturnsAsync(new ProbeResult.NoRoute("test")); var service = CreateService(); - var request = new RebalanceRequest(NodeId: node.Id, null, null, + var request = new RebalanceRequest(NodeId: node.Id, ValidChannelId, ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: 0.025, ProbeBackoffRatio: 0.8); var result = await service.RebalanceAsync(request); @@ -208,7 +353,7 @@ public async Task RebalanceAsync_NullProbeBackoffRatio_FallsBackToConstant() .ReturnsAsync(new ProbeResult.NoRoute("test")); var service = CreateService(); - var request = new RebalanceRequest(NodeId: node.Id, null, null, + var request = new RebalanceRequest(NodeId: node.Id, ValidChannelId, ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: 0.025, ProbeBackoffRatio: null); var result = await service.RebalanceAsync(request); @@ -238,7 +383,7 @@ public async Task RebalanceAsync_NoUserFeePct_FallsBackToDefaultFeePct() .ReturnsAsync(new Payment { Status = Payment.Types.PaymentStatus.Succeeded, FeeMsat = 1000 }); var service = CreateService(); - var request = new RebalanceRequest(NodeId: node.Id, null, TargetPubkey: peerPubkey, + var request = new RebalanceRequest(NodeId: node.Id, ValidChannelId, TargetPubkey: peerPubkey, AmountSats: 100_000, MaxFeePct: null); var result = await service.RebalanceAsync(request); @@ -277,7 +422,7 @@ public async Task ExecuteAsync_ProbeSucceeds_PaymentSucceeds_StatusSucceeded() }); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05); + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05); var result = await service.RebalanceAsync(request); result.Status.Should().Be(RebalanceStatus.Succeeded); @@ -306,7 +451,7 @@ public async Task ExecuteAsync_InvoiceExpiryCoversFullRetryWindowPlusBuffer() var service = CreateService(); // TimeoutSeconds=60, MaxAttempts=3 → backoff = 60 + 120 = 180 → 60 + 180 + buffer. - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05, + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05, TimeoutSeconds: 60, MaxAttempts: 3); await service.RebalanceAsync(request); @@ -335,7 +480,7 @@ public async Task ExecuteAsync_InvoiceExpiry_MaxAttempts1_OmitsBackoffSum() .ReturnsAsync(new ProbeResult.NoRoute("stop")); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05, + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05, TimeoutSeconds: 45, MaxAttempts: 1); await service.RebalanceAsync(request); @@ -376,7 +521,7 @@ public async Task ExecuteAsync_PersistsPaymentHashHexBeforePayment() .ReturnsAsync(new ProbeResult.NoRoute("test")); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05); + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05); var result = await service.RebalanceAsync(request); result.PaymentHashHex.Should().Be("abcdef01"); @@ -396,7 +541,7 @@ public async Task ExecuteAsync_ProbeNoRoute_StatusNoRoute_RetryScheduled() .ReturnsAsync(new ProbeResult.NoRoute("exhausted")); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05); + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05); var result = await service.RebalanceAsync(request); // After scheduling a retry, AttemptNumber is bumped to 2 and Status is reset to Pending. @@ -426,7 +571,7 @@ public async Task ExecuteAsync_PaymentInsufficientBalance_NoRetryScheduled() }); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, null, 100_000, MaxFeePct: 0.05); + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, 100_000, MaxFeePct: 0.05); var result = await service.RebalanceAsync(request); result.Status.Should().Be(RebalanceStatus.InsufficientBalance); @@ -449,7 +594,7 @@ public async Task ExecuteAsync_ScheduleRetry_EscalatesFeePctFromInitialToRetry() .ReturnsAsync(new ProbeResult.NoRoute()); var service = CreateService(); - var request = new RebalanceRequest(node.Id, null, TargetPubkey: null, + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: null); var result = await service.RebalanceAsync(request); @@ -476,7 +621,7 @@ public async Task ExecuteAsync_PerRowRetryMaxFeePct_OverridesConstantOnEscalatio // Per-row retry pct = 0.09%, overriding the constant default // (REBALANCE_DEFAULT_RETRY_MAX_FEE_PCT = 0.05%). After one NoRoute the // escalated cap should be max(initial=0.05%, retry=0.09%) = 0.09%. - var request = new RebalanceRequest(node.Id, null, TargetPubkey: null, + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: null, RetryMaxFeePct: 0.09); var result = await service.RebalanceAsync(request); @@ -499,7 +644,7 @@ public async Task ExecuteAsync_PerRowMaxAttempts1_NoRetryScheduled() var service = CreateService(); // MaxAttempts=1 → first attempt is also the last; no Quartz retry should be scheduled. - var request = new RebalanceRequest(node.Id, null, null, + var request = new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: 0.025, MaxAttempts: 1); var result = await service.RebalanceAsync(request); @@ -559,7 +704,7 @@ public async Task RebalanceAsync_AuditsInitiation() .ReturnsAsync(new ProbeResult.NoRoute()); var service = CreateService(); - await service.RebalanceAsync(new RebalanceRequest(node.Id, null, null, AmountSats: 100_000, MaxFeePct: 0.1, TimeoutSeconds: 500)); + await service.RebalanceAsync(new RebalanceRequest(node.Id, ValidChannelId, ValidTargetPubkey, AmountSats: 100_000, MaxFeePct: 0.1, TimeoutSeconds: 500)); _audit.Verify(a => a.LogAsync( AuditActionType.RebalanceInitiated, From 6fc8777abdde879bc62f0682fe7b7de42316084e Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 11:46:04 +0200 Subject: [PATCH 2/6] test: fix --- .../Rpc/NodeGuardServiceTests.cs | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs index 1a19f574..70be76e3 100644 --- a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs +++ b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs @@ -1435,7 +1435,15 @@ public async Task RequestRebalance_NodeNotFound_ThrowsNotFound() nodeRepoMock.Setup(r => r.GetByPubkey("pubkey1")).ReturnsAsync((Node?)null); var service = CreateNodeGuardService(nodeRepository: nodeRepoMock.Object); - var request = new RequestRebalanceRequest { NodePubkey = "pubkey1", AmountSats = 1000 }; + // source_channel_id and target_pubkey are required guards that fire BEFORE the node + // lookup; supply both so this test actually exercises the node-not-found path. + var request = new RequestRebalanceRequest + { + NodePubkey = "pubkey1", + AmountSats = 1000, + SourceChannelId = 1, + TargetPubkey = "peer-pubkey", + }; var act = () => service.RequestRebalance(request, TestServerCallContext.Create()); @@ -1443,6 +1451,42 @@ public async Task RequestRebalance_NodeNotFound_ThrowsNotFound() .Which.StatusCode.Should().Be(StatusCode.NotFound); } + [Fact] + public async Task RequestRebalance_MissingSourceChannelId_ThrowsInvalidArgument() + { + var service = CreateNodeGuardService(); + var request = new RequestRebalanceRequest + { + NodePubkey = "pubkey1", + AmountSats = 1000, + TargetPubkey = "peer-pubkey", + // SourceChannelId intentionally unset. + }; + + var act = () => service.RequestRebalance(request, TestServerCallContext.Create()); + + (await act.Should().ThrowAsync()) + .Which.StatusCode.Should().Be(StatusCode.InvalidArgument); + } + + [Fact] + public async Task RequestRebalance_MissingTargetPubkey_ThrowsInvalidArgument() + { + var service = CreateNodeGuardService(); + var request = new RequestRebalanceRequest + { + NodePubkey = "pubkey1", + AmountSats = 1000, + SourceChannelId = 1, + // TargetPubkey intentionally unset. + }; + + var act = () => service.RequestRebalance(request, TestServerCallContext.Create()); + + (await act.Should().ThrowAsync()) + .Which.StatusCode.Should().Be(StatusCode.InvalidArgument); + } + [Fact] public async Task RequestRebalance_Success_PassesOptionalsAndReturnsResponse() { @@ -1523,7 +1567,15 @@ public async Task RequestRebalance_DomainArgumentException_ThrowsInvalidArgument nodeRepository: nodeRepoMock.Object, rebalanceService: rebalanceServiceMock.Object); - var request = new RequestRebalanceRequest { NodePubkey = "pubkey1", AmountSats = 1000 }; + // Required gRPC guards must pass so the request actually reaches the service and we + // can verify the domain ArgumentException → StatusCode.InvalidArgument mapping. + var request = new RequestRebalanceRequest + { + NodePubkey = "pubkey1", + AmountSats = 1000, + SourceChannelId = 1, + TargetPubkey = "peer-pubkey", + }; var act = () => service.RequestRebalance(request, TestServerCallContext.Create()); From 1e760d0acb31309c6ff4fb87a275acb3deb7e5cc Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 12:06:05 +0200 Subject: [PATCH 3/6] fix: validate counterparty peer existence and prevent rebalance to the same peer --- src/Services/RebalanceService.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Services/RebalanceService.cs b/src/Services/RebalanceService.cs index a929d801..b441cfd5 100644 --- a/src/Services/RebalanceService.cs +++ b/src/Services/RebalanceService.cs @@ -125,11 +125,16 @@ public async Task RebalanceAsync(RebalanceRequest request, Cancellati nameof(request.SourceChannelId)); var sourceChanIdLnd = sourceChannel.ChanId; - var counterpartyPeerPubkey = sourceChannel.SourceNodeId == node.Id - ? sourceChannel.DestinationNode?.PubKey - : sourceChannel.SourceNode?.PubKey; + var counterpartyPeerId = sourceChannel.SourceNodeId == node.Id + ? sourceChannel.DestinationNodeId + : sourceChannel.SourceNodeId; - if (counterpartyPeerPubkey == request.TargetPubkey) + var counterpartyPeer = await _nodeRepository.GetById(counterpartyPeerId); + if (counterpartyPeer == null) + throw new InvalidOperationException( + $"Counterparty peer node {counterpartyPeerId} not found for source channel {request.SourceChannelId}"); + + if (counterpartyPeer.PubKey == request.TargetPubkey) throw new ArgumentException( "Target pubkey is the same as the source channel's counterparty peer; rebalance would be a no-op.", nameof(request.TargetPubkey)); From 9cc89b721b9d7eef5ed55ef786ee94386932624b Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 12:13:30 +0200 Subject: [PATCH 4/6] fix: conditionally render receiving peer information based on target pubkey presence --- src/Shared/NewRebalanceModal.razor | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Shared/NewRebalanceModal.razor b/src/Shared/NewRebalanceModal.razor index a43ad843..bdc1d549 100644 --- a/src/Shared/NewRebalanceModal.razor +++ b/src/Shared/NewRebalanceModal.razor @@ -109,9 +109,13 @@ @RenderBalanceBar("Before", _sourceLocalSats.Value, _sourceRemoteSats.Value) @RenderBalanceBar("After", SourceLocalAfter, SourceRemoteAfter, isProjection: true) -
- Receiving peer pinned: @_selectedTargetPubkey?.Substring(0, 10)…. The exact channel that lands the sats is LND's call — no per-channel projection possible. -
+ @if (!string.IsNullOrEmpty(_selectedTargetPubkey)) + { +
+ Receiving peer pinned: @_selectedTargetPubkey.Substring(0, 10)…. The exact + channel that lands the sats is LND's call — no per-channel projection possible. +
+ }
@{ From c08bfce319cdd1e8f021798b58ac6705f717b500 Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 12:13:39 +0200 Subject: [PATCH 5/6] test: enhance rebalance tests to enforce counterparty peer validation and mandatory channel resolution --- .../Services/RebalanceServiceTests.cs | 85 ++++++++++--------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs index 5ac17712..e29ea2e8 100644 --- a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs +++ b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs @@ -45,9 +45,12 @@ public RebalanceServiceTests() _schedulerFactory.Setup(x => x.GetScheduler(It.IsAny())) .ReturnsAsync(_scheduler.Object); // SourceChannelId is now mandatory on RebalanceRequest, so any test that reaches the - // source-channel lookup needs a Channel back. Pre-stub here so individual tests don't - // each have to repeat it. Tests that explicitly want the lookup to fail can override. + // source-channel lookup needs a Channel back. The service also resolves the channel's + // counterparty peer via _nodeRepository.GetById to enforce the "TargetPubkey != counterparty" + // guard, so the counterparty node has to resolve too. Pre-stub both so individual tests + // don't repeat it. Tests that explicitly want either lookup to fail can override. StubChannelRepo(); + StubCounterpartyNode(); } private RebalanceService CreateService() => new( @@ -74,11 +77,17 @@ private static Node CreateNode(int id = 1, string pubkey = "03000000000000000000 // tests, something the channel repo can resolve via StubChannelRepo). private const int ValidChannelId = 1; private const string ValidTargetPubkey = "030000000000000000000000000000000000000000000000000000000000000099"; + // Local node id (matches CreateNode() default) is 1; the source channel's counterparty is + // node id 2 with this pubkey — different from ValidTargetPubkey so the no-op guard passes + // by default. Tests that exercise the guard override the counterparty's pubkey to match. + private const int CounterpartyNodeId = 2; + private const string CounterpartyPubkey = "030000000000000000000000000000000000000000000000000000000000000002"; /// /// Stubs ChannelRepository.GetById so the source-channel lookup inside RebalanceAsync resolves - /// to a real-looking Channel instead of throwing. Flow tests (anything that reaches the node - /// lookup) must call this — validation-throw tests can skip it because the throw fires earlier. + /// to a real-looking Channel instead of throwing. The local node (id 1 via CreateNode()) is + /// set as the SourceNode and CounterpartyNodeId as the DestinationNode, mirroring the + /// "we opened this channel" case. /// private void StubChannelRepo(int channelId = ValidChannelId, ulong chanIdLnd = 12345UL) { @@ -89,6 +98,24 @@ private void StubChannelRepo(int channelId = ValidChannelId, ulong chanIdLnd = 1 SatsAmount = 1_000_000, Status = Channel.ChannelStatus.Open, FundingTx = "tx", + SourceNodeId = 1, + DestinationNodeId = CounterpartyNodeId, + }); + } + + /// + /// Stubs NodeRepository.GetById for the source channel's counterparty peer. The service + /// looks this up to evaluate the "TargetPubkey != counterparty" guard. + /// + private void StubCounterpartyNode(string pubkey = CounterpartyPubkey) + { + _nodeRepo.Setup(x => x.GetById(CounterpartyNodeId, It.IsAny())).ReturnsAsync(new Node + { + Id = CounterpartyNodeId, + Name = "counterparty", + PubKey = pubkey, + Endpoint = "peer:10009", + ChannelAdminMacaroon = "mac", }); } @@ -219,31 +246,13 @@ public async Task RebalanceAsync_TargetPubkeyEqualsSourceCounterparty_Throws() // counterparty would route the sats straight back to where they came from — a no-op // rebalance. The service must reject this. var node = CreateNode(); - var peerPubkey = "030000000000000000000000000000000000000000000000000000000000000002"; _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); - _channelRepo.Setup(x => x.GetById(ValidChannelId)).ReturnsAsync(new Channel - { - Id = ValidChannelId, - ChanId = 12345UL, - SatsAmount = 1_000_000, - Status = Channel.ChannelStatus.Open, - FundingTx = "tx", - // Our node opened the channel; the counterparty is the destination. - SourceNodeId = node.Id, - SourceNode = node, - DestinationNodeId = 2, - DestinationNode = new Node - { - Id = 2, - Name = "peer", - PubKey = peerPubkey, - Endpoint = "peer:10009", - ChannelAdminMacaroon = "mac", - }, - }); - + // The default StubChannelRepo + StubCounterpartyNode in the ctor already sets up a + // channel where node 1 is the source and node 2 (with CounterpartyPubkey) is the + // destination. Asking the service to pin the last hop to CounterpartyPubkey hits the + // guard. var service = CreateService(); - var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: peerPubkey, + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: CounterpartyPubkey, AmountSats: 100_000, MaxFeePct: null); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) @@ -255,9 +264,10 @@ await FluentActions.Awaiting(() => service.RebalanceAsync(request)) public async Task RebalanceAsync_TargetPubkeyEqualsSourceCounterparty_NodeIsDestination_Throws() { // Same guard, mirrored: the local node is the channel's DESTINATION (peer opened the - // channel to us). Counterparty is then the SourceNode. + // channel to us). Counterparty is then the SourceNode side. Override the channel stub + // to flip the orientation; the counterparty node lookup still returns CounterpartyPubkey + // via the constructor's StubCounterpartyNode. var node = CreateNode(); - var peerPubkey = "030000000000000000000000000000000000000000000000000000000000000003"; _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); _channelRepo.Setup(x => x.GetById(ValidChannelId)).ReturnsAsync(new Channel { @@ -266,21 +276,13 @@ public async Task RebalanceAsync_TargetPubkeyEqualsSourceCounterparty_NodeIsDest SatsAmount = 1_000_000, Status = Channel.ChannelStatus.Open, FundingTx = "tx", - SourceNodeId = 2, - SourceNode = new Node - { - Id = 2, - Name = "peer", - PubKey = peerPubkey, - Endpoint = "peer:10009", - ChannelAdminMacaroon = "mac", - }, + // Peer opened the channel to us: counterparty sits on the source side. + SourceNodeId = CounterpartyNodeId, DestinationNodeId = node.Id, - DestinationNode = node, }); var service = CreateService(); - var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: peerPubkey, + var request = new RebalanceRequest(node.Id, ValidChannelId, TargetPubkey: CounterpartyPubkey, AmountSats: 100_000, MaxFeePct: null); await FluentActions.Awaiting(() => service.RebalanceAsync(request)) @@ -368,7 +370,8 @@ public async Task RebalanceAsync_NoUserFeePct_FallsBackToDefaultFeePct() // When no MaxFeePct is supplied the service must derive it from // Constants.REBALANCE_DEFAULT_MAX_FEE_PCT (0.05). No LND outbound-rate call should be made. var node = CreateNode(); - var peerPubkey = "030000000000000000000000000000000000000000000000000000000000000002"; + // Use a pubkey distinct from CounterpartyPubkey so the no-op guard doesn't trip. + var peerPubkey = ValidTargetPubkey; _nodeRepo.Setup(x => x.GetById(node.Id, It.IsAny())).ReturnsAsync(node); StubRepoForCapture(); _lightning.Setup(x => x.AddInvoiceAsync(node, It.IsAny(), It.IsAny(), It.IsAny())) From 20c37991c0a75fcfbc11c0519052145eb70e587d Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 19 Jun 2026 12:16:28 +0200 Subject: [PATCH 6/6] fix: clarify comments regarding counterparty pubkey handling in rebalance tests --- test/NodeGuard.Tests/Services/RebalanceServiceTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs index e29ea2e8..920ac433 100644 --- a/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs +++ b/test/NodeGuard.Tests/Services/RebalanceServiceTests.cs @@ -79,7 +79,8 @@ private static Node CreateNode(int id = 1, string pubkey = "03000000000000000000 private const string ValidTargetPubkey = "030000000000000000000000000000000000000000000000000000000000000099"; // Local node id (matches CreateNode() default) is 1; the source channel's counterparty is // node id 2 with this pubkey — different from ValidTargetPubkey so the no-op guard passes - // by default. Tests that exercise the guard override the counterparty's pubkey to match. + // by default. Tests that exercise the guard pass CounterpartyPubkey as the request's + // TargetPubkey to make it collide with the (default) counterparty pubkey. private const int CounterpartyNodeId = 2; private const string CounterpartyPubkey = "030000000000000000000000000000000000000000000000000000000000000002";