Addressing Coderabbit nits

BTCPayServer.Tests/PullPaymentsTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Security.Cryptography;
 using System.Text.RegularExpressions;
@@ -86,11 +87,11 @@ public class PullPaymentsTests(ITestOutputHelper helper) : UnitTestBase(helper)
             await ClickClaimAmount();
             await s.FindAlertMessage();
 
-            // We should not be able to use an address already used
+            address = await s.Server.ExplorerNode.GetNewAddressAsync();
             await s.Page.FillAsync("#Destination", address.ToString());
             await s.Page.FillAsync("#ClaimedAmount", "20");
-            await s.Page.PressAsync("#ClaimedAmount", "Enter");
-            await s.FindAlertMessage(StatusMessageModel.StatusSeverity.Error);
+            await ClickClaimAmount();
+            await s.FindAlertMessage();
 
             address = await s.Server.ExplorerNode.GetNewAddressAsync();
             await s.Page.FillAsync("#Destination", address.ToString());
@@ -112,7 +113,7 @@ public class PullPaymentsTests(ITestOutputHelper helper) : UnitTestBase(helper)
         await s.GoToStore(s.StoreId, StoreNavPages.PullPayments);
         payouts = s.Page.Locator(".pp-payout");
         await payouts.First.ClickAsync();
-        await Expect(s.Page.Locator(".payout")).ToHaveCountAsync(2);
+        await Expect(s.Page.Locator(".payout")).ToHaveCountAsync(3);
 
         await s.Page.CheckAsync(".mass-action-select-all");
         await s.Page.ClickAsync($"#{PayoutState.AwaitingApproval}-approve-pay");
@@ -129,10 +130,10 @@ public class PullPaymentsTests(ITestOutputHelper helper) : UnitTestBase(helper)
         await s.GoToStore(s.StoreId, StoreNavPages.Payouts);
         await s.Page.ClickAsync($"#{PayoutState.InProgress}-view");
 
-        await Expect(s.Page.Locator(".transaction-link")).ToHaveCountAsync(2);
+        await Expect(s.Page.Locator(".transaction-link")).ToHaveCountAsync(3);
 
         await s.GoToUrl(viewPullPaymentUrl);
-        await Expect(s.Page.Locator(".transaction-link")).ToHaveCountAsync(2);
+        await Expect(s.Page.Locator(".transaction-link")).ToHaveCountAsync(3);
         await Expect(s.Page.Locator("body")).ToContainTextAsync(PayoutState.InProgress.GetStateString());
 
         await s.Server.ExplorerNode.GenerateAsync(1);
@@ -1064,4 +1065,270 @@ public class PullPaymentsTests(ITestOutputHelper helper) : UnitTestBase(helper)
             }
         }
     }
+
+    [Fact]
+    [Trait("Integration", "Integration")]
+    public async Task CanCreateDuplicateDestinationPayouts()
+    {
+        using var tester = CreateServerTester();
+        await tester.StartAsync();
+        var acc = tester.NewAccount();
+        await acc.GrantAccessAsync(true);
+        var storeId = (await acc.RegisterDerivationSchemeAsync("BTC", importKeysToNBX: true)).StoreId;
+        var client = await acc.CreateClient();
+
+        var pp = await client.CreatePullPayment(storeId, new CreatePullPaymentRequest()
+        {
+            Name = "Duplicate Destination Test",
+            Amount = 1.0m,
+            Currency = "BTC",
+            PayoutMethods = new[] { "BTC" }
+        });
+
+        var destination = (await tester.ExplorerNode.GetNewAddressAsync()).ToString();
+
+        // Create first payout to the address
+        var payout1 = await client.CreatePayout(pp.Id, new CreatePayoutRequest()
+        {
+            Destination = destination,
+            Amount = 0.001m,
+            PayoutMethodId = "BTC"
+        });
+        Assert.Equal(PayoutState.AwaitingApproval, payout1.State);
+
+        // Create second payout to the SAME address -- should succeed, not throw duplicate-destination
+        var payout2 = await client.CreatePayout(pp.Id, new CreatePayoutRequest()
+        {
+            Destination = destination,
+            Amount = 0.002m,
+            PayoutMethodId = "BTC"
+        });
+        Assert.Equal(PayoutState.AwaitingApproval, payout2.State);
+        Assert.NotEqual(payout1.Id, payout2.Id);
+
+        // Approve both
+        payout1 = await client.ApprovePayout(storeId, payout1.Id, new ApprovePayoutRequest());
+        payout2 = await client.ApprovePayout(storeId, payout2.Id, new ApprovePayoutRequest());
+        Assert.Equal(PayoutState.AwaitingPayment, payout1.State);
+        Assert.Equal(PayoutState.AwaitingPayment, payout2.State);
+
+        // Verify both exist
+        var payouts = await client.GetPayouts(pp.Id);
+        Assert.Equal(2, payouts.Length);
+        Assert.All(payouts, p => Assert.Equal(PayoutState.AwaitingPayment, p.State));
+    }
+
+    [Fact]
+    [Trait("Integration", "Integration")]
+    public async Task CanMatchPayoutsToSameAddressByAmount()
+    {
+        using var tester = CreateServerTester();
+        await tester.StartAsync();
+        var acc = tester.NewAccount();
+        await acc.GrantAccessAsync(true);
+        var storeId = (await acc.RegisterDerivationSchemeAsync("BTC", importKeysToNBX: true)).StoreId;
+        var client = await acc.CreateClient();
+
+        // Fund the store wallet so we can send internal payments
+        var storeAddress = (await client.GetOnChainWalletReceiveAddress(storeId, "BTC", true)).Address;
+        await tester.ExplorerNode.SendToAddressAsync(
+            BitcoinAddress.Create(storeAddress, tester.ExplorerClient.Network.NBitcoinNetwork),
+            Money.Coins(1m));
+        await tester.ExplorerNode.GenerateAsync(1);
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var utxos = await client.GetOnChainWalletUTXOs(storeId, "BTC");
+            Assert.NotEmpty(utxos);
+        });
+
+        var destination = (await tester.ExplorerNode.GetNewAddressAsync()).ToString();
+
+        // Create the larger payout first so amount matching, not FIFO-by-address,
+        // is required to pick the right payout.
+        var payout1 = await client.CreatePayout(storeId, new CreatePayoutThroughStoreRequest()
+        {
+            Destination = destination,
+            Amount = 0.002m,
+            Approved = true,
+            PayoutMethodId = "BTC"
+        });
+        var payout2 = await client.CreatePayout(storeId, new CreatePayoutThroughStoreRequest()
+        {
+            Destination = destination,
+            Amount = 0.001m,
+            Approved = true,
+            PayoutMethodId = "BTC"
+        });
+
+        Assert.Equal(PayoutState.AwaitingPayment, payout1.State);
+        Assert.Equal(PayoutState.AwaitingPayment, payout2.State);
+
+        // Send 0.001 BTC first. This should match payout2, not the first-created payout.
+        await client.CreateOnChainTransaction(storeId, "BTC",
+            new CreateOnChainTransactionRequest()
+            {
+                Destinations = new List<CreateOnChainTransactionRequest.CreateOnChainTransactionRequestDestination>()
+                {
+                    new() { Destination = destination, Amount = 0.001m }
+                },
+                FeeRate = new FeeRate(5m)
+            });
+        await tester.ExplorerNode.GenerateAsync(1);
+
+        // Payout2 (0.001) should be matched, payout1 (0.002) should stay AwaitingPayment
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var payouts = await client.GetStorePayouts(storeId);
+            var p1 = payouts.First(p => p.Id == payout1.Id);
+            var p2 = payouts.First(p => p.Id == payout2.Id);
+            Assert.Equal(PayoutState.AwaitingPayment, p1.State);
+            Assert.True(p2.State == PayoutState.InProgress || p2.State == PayoutState.Completed,
+                $"Expected payout2 to be InProgress or Completed, but was {p2.State}");
+        });
+
+        // Send 0.002 BTC. The remaining payout1 should now be matched.
+        await client.CreateOnChainTransaction(storeId, "BTC",
+            new CreateOnChainTransactionRequest()
+            {
+                Destinations = new List<CreateOnChainTransactionRequest.CreateOnChainTransactionRequestDestination>()
+                {
+                    new() { Destination = destination, Amount = 0.002m }
+                },
+                FeeRate = new FeeRate(5m)
+            });
+        await tester.ExplorerNode.GenerateAsync(1);
+
+        // Payout1 (0.002) should now also be matched
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var payouts = await client.GetStorePayouts(storeId);
+            var p1 = payouts.First(p => p.Id == payout1.Id);
+            Assert.True(p1.State == PayoutState.InProgress || p1.State == PayoutState.Completed,
+                $"Expected payout1 to be InProgress or Completed, but was {p1.State}");
+        });
+    }
+
+    [Fact]
+    [Trait("Integration", "Integration")]
+    public async Task CanProcessThreePayoutsToSameAddress()
+    {
+        using var tester = CreateServerTester();
+        await tester.StartAsync();
+        var acc = tester.NewAccount();
+        await acc.GrantAccessAsync(true);
+        var storeId = (await acc.RegisterDerivationSchemeAsync("BTC", importKeysToNBX: true)).StoreId;
+        var client = await acc.CreateClient();
+
+        // Fund the store wallet
+        var storeAddress = (await client.GetOnChainWalletReceiveAddress(storeId, "BTC", true)).Address;
+        await tester.ExplorerNode.SendToAddressAsync(
+            BitcoinAddress.Create(storeAddress, tester.ExplorerClient.Network.NBitcoinNetwork),
+            Money.Coins(1m));
+        await tester.ExplorerNode.GenerateAsync(1);
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var utxos = await client.GetOnChainWalletUTXOs(storeId, "BTC");
+            Assert.NotEmpty(utxos);
+        });
+
+        var destination = (await tester.ExplorerNode.GetNewAddressAsync()).ToString();
+        var amount = 0.001m;
+
+        // Create 3 payouts to the same address, each for 0.001 BTC
+        var payout1 = await client.CreatePayout(storeId, new CreatePayoutThroughStoreRequest()
+        {
+            Destination = destination,
+            Amount = amount,
+            Approved = true,
+            PayoutMethodId = "BTC"
+        });
+        var payout2 = await client.CreatePayout(storeId, new CreatePayoutThroughStoreRequest()
+        {
+            Destination = destination,
+            Amount = amount,
+            Approved = true,
+            PayoutMethodId = "BTC"
+        });
+        var payout3 = await client.CreatePayout(storeId, new CreatePayoutThroughStoreRequest()
+        {
+            Destination = destination,
+            Amount = amount,
+            Approved = true,
+            PayoutMethodId = "BTC"
+        });
+
+        Assert.All(new[] { payout1, payout2, payout3 }, p => Assert.Equal(PayoutState.AwaitingPayment, p.State));
+
+        // Send first payment from the store wallet (internal)
+        await client.CreateOnChainTransaction(storeId, "BTC",
+            new CreateOnChainTransactionRequest()
+            {
+                Destinations = new List<CreateOnChainTransactionRequest.CreateOnChainTransactionRequestDestination>()
+                {
+                    new() { Destination = destination, Amount = amount }
+                },
+                FeeRate = new FeeRate(5m)
+            });
+        await tester.ExplorerNode.GenerateAsync(1);
+
+        // Payout1 should be matched (FIFO), payout2 and payout3 still awaiting
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var payouts = await client.GetStorePayouts(storeId, false);
+            var p1 = payouts.Single(p => p.Id == payout1.Id);
+            var p2 = payouts.Single(p => p.Id == payout2.Id);
+            var p3 = payouts.Single(p => p.Id == payout3.Id);
+            Assert.True(p1.State == PayoutState.InProgress || p1.State == PayoutState.Completed,
+                $"Expected payout1 to be InProgress or Completed, but was {p1.State}");
+            Assert.Equal(PayoutState.AwaitingPayment, p2.State);
+            Assert.Equal(PayoutState.AwaitingPayment, p3.State);
+        });
+
+        // Send second payment from the store wallet (internal)
+        await client.CreateOnChainTransaction(storeId, "BTC",
+            new CreateOnChainTransactionRequest()
+            {
+                Destinations = new List<CreateOnChainTransactionRequest.CreateOnChainTransactionRequestDestination>()
+                {
+                    new() { Destination = destination, Amount = amount }
+                },
+                FeeRate = new FeeRate(5m)
+            });
+        await tester.ExplorerNode.GenerateAsync(1);
+
+        // Payout1 and payout2 should be matched (FIFO), payout3 still awaiting
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var payouts = await client.GetStorePayouts(storeId, false);
+            var p1 = payouts.Single(p => p.Id == payout1.Id);
+            var p2 = payouts.Single(p => p.Id == payout2.Id);
+            var p3 = payouts.Single(p => p.Id == payout3.Id);
+            Assert.True(p1.State == PayoutState.InProgress || p1.State == PayoutState.Completed,
+                $"Expected payout1 to be InProgress or Completed, but was {p1.State}");
+            Assert.True(p2.State == PayoutState.InProgress || p2.State == PayoutState.Completed,
+                $"Expected payout2 to be InProgress or Completed, but was {p2.State}");
+            Assert.Equal(PayoutState.AwaitingPayment, p3.State);
+        });
+
+        // Send third payment from the store wallet (internal)
+        await client.CreateOnChainTransaction(storeId, "BTC",
+            new CreateOnChainTransactionRequest()
+            {
+                Destinations = new List<CreateOnChainTransactionRequest.CreateOnChainTransactionRequestDestination>()
+                {
+                    new() { Destination = destination, Amount = amount }
+                },
+                FeeRate = new FeeRate(5m)
+            });
+        await tester.ExplorerNode.GenerateAsync(1);
+
+        // All 3 payouts should be matched
+        await TestUtils.EventuallyAsync(async () =>
+        {
+            var payouts = await client.GetStorePayouts(storeId, false);
+            Assert.All(payouts, p => Assert.True(
+                p.State == PayoutState.InProgress || p.State == PayoutState.Completed,
+                $"Expected payout to be InProgress or Completed, but was {p.State}"));
+        });
+    }
 }

BTCPayServer/Data/Payouts/BitcoinLike/BitcoinLikePayoutHandler.cs

@@ -381,7 +381,30 @@ public class BitcoinLikePayoutHandler : IPayoutHandler, IHasNetwork
 
 
             await using var ctx = _dbContextFactory.CreateContext();
-            var payout = await ctx.Payouts
+            var txId = newTransaction.NewTransactionEvent.TransactionData.TransactionHash;
+
+            // Check if this transaction is already claimed by any non-AwaitingPayment payout for this destination.
+            // This prevents a single transaction from matching multiple payouts when the event fires
+            // more than once (e.g. mempool detection then block confirmation).
+            // We check both InProgress (Candidates) and Completed (TransactionId, since Candidates is cleared on completion).
+            var claimedPayouts = await ctx.Payouts
+                .Where(p => p.State == PayoutState.InProgress || p.State == PayoutState.Completed)
+                .Where(p => p.PayoutMethodId == PaymentMethodId.ToString())
+#pragma warning disable CA1307 // Specify StringComparison
+                .Where(p => destination.Equals(p.DedupId))
+#pragma warning restore CA1307 // Specify StringComparison
+                .ToListAsync();
+
+            foreach (var existing in claimedPayouts)
+            {
+                var existingProof = ParseProof(existing) as PayoutTransactionOnChainBlob;
+                if (existingProof?.Candidates?.Contains(txId) == true)
+                    return;
+                if (existingProof?.TransactionId == txId)
+                    return;
+            }
+
+            var payouts = await ctx.Payouts
                 .Include(o => o.StoreData)
                 .Include(o => o.PullPaymentData)
                 .Where(p => p.State == PayoutState.AwaitingPayment)
@@ -389,16 +412,26 @@ public class BitcoinLikePayoutHandler : IPayoutHandler, IHasNetwork
 #pragma warning disable CA1307 // Specify StringComparison
                 .Where(p => destination.Equals(p.DedupId))
 #pragma warning restore CA1307 // Specify StringComparison
-                .FirstOrDefaultAsync();
+                .OrderBy(p => p.Date)
+                .ToListAsync();
+
+            // Also check AwaitingPayment payouts for already-claimed txIds.
+            // This covers external payments where the payout stays in AwaitingPayment
+            // but already had its proof updated by a previous event (e.g. mempool detection).
+            foreach (var p in payouts)
+            {
+                var existingProof = ParseProof(p) as PayoutTransactionOnChainBlob;
+                if (existingProof?.Candidates.Contains(txId) == true)
+                    return;
+            }
+
+            var payout = payouts.FirstOrDefault(p =>
+                p.Amount is not null &&
+                destinationSum ==
+                BTCPayServer.Extensions.RoundUp(p.Amount.Value, Network.Divisibility));
 
             if (payout is null)
                 return;
-            if (payout.Amount is null ||
-                // The round up here is not strictly necessary, this is temporary to fix existing payout before we
-                // were properly roundup the crypto amount
-                destinationSum !=
-                BTCPayServer.Extensions.RoundUp(payout.Amount.Value, Network.Divisibility))
-                return;
 
             var derivationSchemeSettings = payout.StoreData
                 .GetDerivationSchemeSettings(_paymentHandlers, Network.CryptoCode)?.AccountDerivation;
@@ -413,7 +446,6 @@ public class BitcoinLikePayoutHandler : IPayoutHandler, IHasNetwork
 
             var proof = ParseProof(payout) as PayoutTransactionOnChainBlob ??
                         new PayoutTransactionOnChainBlob() { Accounted = isInternal };
-            var txId = newTransaction.NewTransactionEvent.TransactionData.TransactionHash;
             if (!proof.Candidates.Add(txId))
                 return;
             if (isInternal)

BTCPayServer/HostedServices/PullPaymentHostedService.cs

@@ -641,18 +641,6 @@ namespace BTCPayServer.HostedServices
                     return;
                 }
 
-                if (req.ClaimRequest.Destination.Id != null)
-                {
-                    if (await ctx.Payouts.AnyAsync(data =>
-                            data.DedupId.Equals(req.ClaimRequest.Destination.Id) &&
-                            data.State != PayoutState.Completed && data.State != PayoutState.Cancelled
-                        ))
-                    {
-                        req.Completion.TrySetResult(new ClaimRequest.ClaimResponse(ClaimRequest.ClaimResult.Duplicate));
-                        return;
-                    }
-                }
-
                 var payoutsRaw = withoutPullPayment
                     ? null
                     : await ctx.Payouts.Where(p => p.PullPaymentDataId == pp.Id)