Payout Destination Handling (#2985)

* Payout Destination Handling

fixes #2765
This PR:
* reactivates the BIP21 support for payouts.
* allows LNUrl destinations to be reusable.
* allows addresses to be reused in claims as long as the other claims are in a final state

* Ensure bolt amount matches the payout amount

* fixes

* reduce duplicate parsing of bolt

* make hash the id of bolt

* better bolt11 tostring

* use cached payment request from lnurl

BTCPayServer.Data/Data/PayoutData.cs

@@ -1,5 +1,6 @@
 using System;
 using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations.Schema;
 using BTCPayServer.Client.Models;
 using Microsoft.EntityFrameworkCore;
 using NBitcoin;
@@ -19,9 +20,9 @@ namespace BTCPayServer.Data
         [MaxLength(20)]
         [Required]
         public string PaymentMethodId { get; set; }
-        public string Destination { get; set; }
         public byte[] Blob { get; set; }
         public byte[] Proof { get; set; }
+        public string? Destination { get; set; }
 
 
         internal static void OnModelCreating(ModelBuilder builder)
@@ -29,15 +30,13 @@ namespace BTCPayServer.Data
             builder.Entity<PayoutData>()
                 .HasOne(o => o.PullPaymentData)
                 .WithMany(o => o.Payouts).OnDelete(DeleteBehavior.Cascade);
-
             builder.Entity<PayoutData>()
                 .Property(o => o.State)
                 .HasConversion<string>();
-            builder.Entity<PayoutData>()
-                .HasIndex(o => o.Destination)
-                .IsUnique();
             builder.Entity<PayoutData>()
                 .HasIndex(o => o.State);
+            builder.Entity<PayoutData>()
+                .HasIndex(x => new { DestinationId = x.Destination, x.State});
         }
 
         // utility methods

BTCPayServer.Data/Migrations/20211021085011_RemovePayoutDestinationConstraint.cs

@@ -0,0 +1,41 @@
+// <auto-generated />
+using System;
+using BTCPayServer.Data;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Migrations;
+using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata;
+
+namespace BTCPayServer.Migrations
+{
+    [DbContext(typeof(ApplicationDbContext))]
+    [Migration("20211021085011_RemovePayoutDestinationConstraint")]
+    public partial class RemovePayoutDestinationConstraint : Migration
+    {
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropIndex(
+                name: "IX_Payouts_Destination",
+                table: "Payouts");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_Payouts_Destination_State",
+                table: "Payouts",
+                columns: new[] { "Destination", "State" });
+        }
+
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropIndex(
+                name: "IX_Payouts_Destination_State",
+                table: "Payouts");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_Payouts_Destination",
+                table: "Payouts",
+                column: "Destination",
+                unique: true);
+        }
+    }
+}

BTCPayServer.Data/Migrations/ApplicationDbContextModelSnapshot.cs

@@ -15,7 +15,7 @@ namespace BTCPayServer.Migrations
         {
 #pragma warning disable 612, 618
             modelBuilder
-                .HasAnnotation("ProductVersion", "3.1.4");
+                .HasAnnotation("ProductVersion", "3.1.19");
 
             modelBuilder.Entity("BTCPayServer.Data.APIKeyData", b =>
                 {
@@ -527,13 +527,12 @@ namespace BTCPayServer.Migrations
 
                     b.HasKey("Id");
 
-                    b.HasIndex("Destination")
-                        .IsUnique();
-
                     b.HasIndex("PullPaymentDataId");
 
                     b.HasIndex("State");
 
+                    b.HasIndex("Destination", "State");
+
                     b.ToTable("Payouts");
                 });
 

BTCPayServer.Tests/GreenfieldAPITests.cs

@@ -440,9 +440,11 @@ namespace BTCPayServer.Tests
                 Assert.Null(payout.PaymentMethodAmount);
 
                 Logs.Tester.LogInformation("Can't overdraft");
+                
+                var destination2 = (await tester.ExplorerNode.GetNewAddressAsync()).ToString();
                 await this.AssertAPIError("overdraft", async () => await unauthenticated.CreatePayout(pps[0].Id, new CreatePayoutRequest()
                 {
-                    Destination = destination,
+                    Destination = destination2,
                     Amount = 0.00001m,
                     PaymentMethod = "BTC"
                 }));
@@ -450,7 +452,7 @@ namespace BTCPayServer.Tests
                 Logs.Tester.LogInformation("Can't create too low payout");
                 await this.AssertAPIError("amount-too-low", async () => await unauthenticated.CreatePayout(pps[0].Id, new CreatePayoutRequest()
                 {
-                    Destination = destination,
+                    Destination = destination2,
                     PaymentMethod = "BTC"
                 }));
 

BTCPayServer/BTCPayServer.csproj

@@ -46,7 +46,7 @@
   <ItemGroup>
     <PackageReference Include="BIP78.Sender" Version="0.2.2" />
     <PackageReference Include="BTCPayServer.Hwi" Version="2.0.2" />
-    <PackageReference Include="BTCPayServer.Lightning.All" Version="1.2.12" />
+    <PackageReference Include="BTCPayServer.Lightning.All" Version="1.2.13" />
     <PackageReference Include="BuildBundlerMinifier" Version="3.2.449" />
     <PackageReference Include="BundlerMinifier.Core" Version="3.2.435" />
     <PackageReference Include="BundlerMinifier.TagHelpers" Version="3.2.435" />
@@ -54,7 +54,7 @@
     <PackageReference Include="Fido2" Version="2.0.1" />
     <PackageReference Include="Fido2.AspNet" Version="2.0.1" />
     <PackageReference Include="HtmlSanitizer" Version="5.0.372" />
-    <PackageReference Include="LNURL" Version="0.0.7" />
+    <PackageReference Include="LNURL" Version="0.0.8" />
     <PackageReference Include="McMaster.NETCore.Plugins.Mvc" Version="1.4.0" />
     <PackageReference Include="Microsoft.Extensions.Logging.Filter" Version="1.1.2" />
     <PackageReference Include="Microsoft.NetCore.Analyzers" Version="3.3.2">

BTCPayServer/Data/Payouts/BitcoinLike/AddressClaimDestination.cs

@@ -19,6 +19,7 @@ namespace BTCPayServer.Data
             return _bitcoinAddress.ToString();
         }
 
+        public string Id => ToString();
         public decimal? Amount => null;
     }
 }

BTCPayServer/Data/Payouts/BitcoinLike/BitcoinLikePayoutHandler.cs

@@ -70,11 +70,10 @@ public class BitcoinLikePayoutHandler : IPayoutHandler
         destination = destination.Trim();
         try
         {
-            // This doesn't work properly, (payouts are not detected), we can reactivate later when we fix the bug https://github.com/btcpayserver/btcpayserver/issues/2765
+            if (destination.StartsWith($"{network.UriScheme}:", StringComparison.OrdinalIgnoreCase))
-            //if (destination.StartsWith($"{network.UriScheme}:", StringComparison.OrdinalIgnoreCase))
+            {
-            //{
+                return Task.FromResult<(IClaimDestination, string)>((new UriClaimDestination(new BitcoinUrlBuilder(destination, network.NBitcoinNetwork)), null));
-            //    return Task.FromResult<IClaimDestination>(new UriClaimDestination(new BitcoinUrlBuilder(destination, network.NBitcoinNetwork)));
+            }
-            //}
 
             return Task.FromResult<(IClaimDestination, string)>((new AddressClaimDestination(BitcoinAddress.Create(destination, network.NBitcoinNetwork)), null));
         }
@@ -248,7 +247,17 @@ public class BitcoinLikePayoutHandler : IPayoutHandler
             var blob = payout.GetBlob(_jsonSerializerSettings);
             if (payout.GetPaymentMethodId() != paymentMethodId)
                 continue;
-            bip21.Add(network.GenerateBIP21(payout.Destination, new Money(blob.CryptoAmount.Value, MoneyUnit.BTC)).ToString());          
+            var claim = await ParseClaimDestination(paymentMethodId, blob.Destination, false);
+            switch (claim.destination)
+            {
+                case UriClaimDestination uriClaimDestination:
+                    uriClaimDestination.BitcoinUrl.Amount = new Money(blob.CryptoAmount.Value, MoneyUnit.BTC);
+                    bip21.Add(uriClaimDestination.ToString());
+                    break;
+                case AddressClaimDestination addressClaimDestination:
+                    bip21.Add(network.GenerateBIP21(addressClaimDestination.Address.ToString(), new Money(blob.CryptoAmount.Value, MoneyUnit.BTC)).ToString());
+                    break;
+            }      
         }
         if(bip21.Any())
             return  new RedirectToActionResult("WalletSend", "Wallets", new {walletId = new WalletId(storeId, paymentMethodId.CryptoCode).ToString(), bip21});
@@ -289,7 +298,6 @@ public class BitcoinLikePayoutHandler : IPayoutHandler
                     {
                         payout.State = PayoutState.Completed;
                         proof.TransactionId = tx.TransactionHash;
-                        payout.Destination = null;
                         break;
                     }
                     else

BTCPayServer/Data/Payouts/BitcoinLike/UriClaimDestination.cs

@@ -25,6 +25,7 @@ namespace BTCPayServer.Data
             return _bitcoinUrl.ToString();
         }
 
+        public string Id => Address.ToString();
         public decimal? Amount => _bitcoinUrl.Amount?.ToDecimal(MoneyUnit.BTC);
     }
 }

BTCPayServer/Data/Payouts/IClaimDestination.cs

@@ -5,6 +5,7 @@ namespace BTCPayServer.Data
 {
     public interface IClaimDestination
     {
+        public string? Id { get; }
         decimal? Amount { get; }
     }
 }

BTCPayServer/Data/Payouts/LightningLike/BoltInvoiceClaimDestination.cs

@@ -6,26 +6,22 @@ namespace BTCPayServer.Data.Payouts.LightningLike
 {
     public class BoltInvoiceClaimDestination : ILightningLikeLikeClaimDestination
     {
-        private readonly string _bolt11;
+        public BoltInvoiceClaimDestination(string bolt11, BOLT11PaymentRequest paymentRequest)
-        private readonly decimal _amount;
-
-        public BoltInvoiceClaimDestination(string bolt11, Network network)
         {
-            _bolt11 = bolt11 ?? throw new ArgumentNullException(nameof(bolt11));
+            Bolt11 = bolt11 ?? throw new ArgumentNullException(nameof(bolt11));
-            _amount = BOLT11PaymentRequest.Parse(bolt11, network).MinimumAmount.ToDecimal(LightMoneyUnit.BTC);
+            PaymentRequest = paymentRequest;
-        }
+            PaymentHash = paymentRequest.Hash;
-
+            Amount = paymentRequest.MinimumAmount.ToDecimal(LightMoneyUnit.BTC);
-        public BoltInvoiceClaimDestination(string bolt11, BOLT11PaymentRequest invoice)
-        {
-            _bolt11 = bolt11 ?? throw new ArgumentNullException(nameof(bolt11));
-            _amount = invoice?.MinimumAmount.ToDecimal(LightMoneyUnit.BTC) ?? throw new ArgumentNullException(nameof(invoice));
         }
 
         public override string ToString()
         {
-            return _bolt11;
+            return Bolt11;
         }
-
+        public string Bolt11 { get; }
-        public decimal? Amount => _amount;
+        public BOLT11PaymentRequest PaymentRequest { get; }
+        public uint256 PaymentHash { get; }
+        public string Id => PaymentHash.ToString();
+        public decimal? Amount { get; }
     }
 }

BTCPayServer/Data/Payouts/LightningLike/LNURLPayClaimDestinaton.cs

@@ -7,6 +7,7 @@
             LNURL = lnurl;
         }
 
+        public string Id => null; //lnurls are reusable
         public decimal? Amount { get; } = null;
         public string LNURL { get; set; }
 

BTCPayServer/Data/Payouts/LightningLike/LightningLikePayoutController.cs

@@ -113,10 +113,21 @@ namespace BTCPayServer.Data.Payouts.LightningLike
             var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(pmi.CryptoCode);
 
             //we group per store and init the transfers by each
-            async Task TrypayBolt(ILightningClient lightningClient, PayoutBlob payoutBlob, PayoutData payoutData,
+            async Task TrypayBolt(ILightningClient lightningClient, PayoutBlob payoutBlob, PayoutData payoutData, BOLT11PaymentRequest bolt11PaymentRequest)
-                string destination)
             {
-                var result = await lightningClient.Pay(destination);
+                var boltAmount = bolt11PaymentRequest.MinimumAmount.ToDecimal(LightMoneyUnit.BTC);
+                if (boltAmount != payoutBlob.CryptoAmount)
+                {
+                    results.Add(new ResultVM()
+                    {
+                        PayoutId = payoutData.Id, 
+                        Result = PayResult.Error,
+                        Message = $"The BOLT11 invoice amount did not match the payout's amount ({boltAmount} instead of {payoutBlob.CryptoAmount})", 
+                        Destination = payoutBlob.Destination
+                    });
+                    return;
+                }
+                var result = await lightningClient.Pay(bolt11PaymentRequest.ToString());
                 if (result.Result == PayResult.Ok)
                 {
                     results.Add(new ResultVM()
@@ -176,9 +187,8 @@ namespace BTCPayServer.Data.Payouts.LightningLike
                                     {
                                         var lnurlPayRequestCallbackResponse =
                                             await lnurlInfo.SendRequest(lm, network.NBitcoinNetwork, httpClient);
-
+                                        
-
+                                        await TrypayBolt(client, blob, payoutData, lnurlPayRequestCallbackResponse.GetPaymentRequest(network.NBitcoinNetwork));
-                                        await TrypayBolt(client, blob, payoutData, lnurlPayRequestCallbackResponse.Pr);
                                     }
                                     catch (LNUrlException e)
                                     {
@@ -195,7 +205,7 @@ namespace BTCPayServer.Data.Payouts.LightningLike
                                 break;
 
                             case BoltInvoiceClaimDestination item1:
-                                await TrypayBolt(client, blob, payoutData, payoutData.Destination);
+                                await TrypayBolt(client, blob, payoutData, item1.PaymentRequest);
 
                                 break;
                             default:

BTCPayServer/HostedServices/PullPaymentHostedService.cs

@@ -377,6 +377,20 @@ namespace BTCPayServer.HostedServices
                     req.Completion.TrySetResult(new ClaimRequest.ClaimResponse(ClaimRequest.ClaimResult.PaymentMethodNotSupported));
                     return;
                 }
+
+                if (req.ClaimRequest.Destination.Id != null)
+                {
+                    if (await ctx.Payouts.AnyAsync(data =>
+                        data.Destination.Equals(req.ClaimRequest.Destination.Id) &&
+                        data.State != PayoutState.Completed && data.State != PayoutState.Cancelled
+                        ))
+                    {
+                        
+                        req.Completion.TrySetResult(new ClaimRequest.ClaimResponse(ClaimRequest.ClaimResult.Duplicate));
+                        return;
+                    }
+                }
+
                 var payouts = (await ctx.Payouts.GetPayoutInPeriod(pp, now)
                                                 .Where(p => p.State != PayoutState.Cancelled)
                                                 .ToListAsync())
@@ -400,7 +414,7 @@ namespace BTCPayServer.HostedServices
                     State = PayoutState.AwaitingApproval,
                     PullPaymentDataId = req.ClaimRequest.PullPaymentId,
                     PaymentMethodId = req.ClaimRequest.PaymentMethodId.ToString(),
-                    Destination = req.ClaimRequest.Destination.ToString()
+                    Destination = req.ClaimRequest.Destination.Id
                 };
                 if (claimed < ppBlob.MinimumClaim || claimed == 0.0m)
                 {
@@ -463,7 +477,6 @@ namespace BTCPayServer.HostedServices
                 {
                     if (payout.State != PayoutState.Completed && payout.State != PayoutState.InProgress)
                         payout.State = PayoutState.Cancelled;
-                    payout.Destination = null;
                 }
                 await ctx.SaveChangesAsync();
                 cancel.Completion.TrySetResult(true);

BTCPayServer/Hosting/MigrationStartupTask.cs

@@ -36,6 +36,8 @@ namespace BTCPayServer.Hosting
         private readonly BTCPayNetworkProvider _NetworkProvider;
         private readonly SettingsRepository _Settings;
         private readonly AppService _appService;
+        private readonly IEnumerable<IPayoutHandler> _payoutHandlers;
+        private readonly BTCPayNetworkJsonSerializerSettings _btcPayNetworkJsonSerializerSettings;
         private readonly UserManager<ApplicationUser> _userManager;
 
         public IOptions<LightningNetworkOptions> LightningOptions { get; }
@@ -47,13 +49,17 @@ namespace BTCPayServer.Hosting
             UserManager<ApplicationUser> userManager,
             IOptions<LightningNetworkOptions> lightningOptions,
             SettingsRepository settingsRepository,
-            AppService appService)
+            AppService appService, 
+            IEnumerable<IPayoutHandler> payoutHandlers,
+            BTCPayNetworkJsonSerializerSettings btcPayNetworkJsonSerializerSettings)
         {
             _DBContextFactory = dbContextFactory;
             _StoreRepository = storeRepository;
             _NetworkProvider = networkProvider;
             _Settings = settingsRepository;
             _appService = appService;
+            _payoutHandlers = payoutHandlers;
+            _btcPayNetworkJsonSerializerSettings = btcPayNetworkJsonSerializerSettings;
             _userManager = userManager;
             LightningOptions = lightningOptions;
         }
@@ -147,6 +153,12 @@ namespace BTCPayServer.Hosting
                     settings.MigrateAppCustomOption = true;
                     await _Settings.UpdateSetting(settings);
                 }
+                if (!settings.MigratePayoutDestinationId)
+                {
+                    await MigratePayoutDestinationId();
+                    settings.MigratePayoutDestinationId = true;
+                    await _Settings.UpdateSetting(settings);
+                }
             }
             catch (Exception ex)
             {
@@ -154,6 +166,28 @@ namespace BTCPayServer.Hosting
                 throw;
             }
         }
+        
+        private async Task MigratePayoutDestinationId()
+        {
+            await using var ctx = _DBContextFactory.CreateContext();
+            foreach (var payoutData in await ctx.Payouts.AsQueryable().ToArrayAsync())
+            {
+                    var pmi = payoutData.GetPaymentMethodId();
+                    if (pmi is null)
+                    {
+                        continue;
+                    }
+                    var handler = _payoutHandlers
+                        .FindPayoutHandler(pmi);
+                    if (handler is null)
+                    {
+                        continue;
+                    }
+                    var claim = await handler?.ParseClaimDestination(pmi, payoutData.GetBlob(_btcPayNetworkJsonSerializerSettings).Destination, false);
+                    payoutData.Destination = claim.destination?.Id;
+            }
+            await ctx.SaveChangesAsync();
+        }
 
         private async Task MigrateAppCustomOption()
         {

BTCPayServer/Services/MigrationSettings.cs

@@ -26,5 +26,6 @@ namespace BTCPayServer.Services
         // Done in DbMigrationsHostedService
         public int? MigratedInvoiceTextSearchPages { get; set; }
         public bool MigrateAppCustomOption { get; set; }
+        public bool MigratePayoutDestinationId { get; set; }
     }
 }