From 9f3a3c5f5191a7bd55cfdeb698dae5b988a94377 Mon Sep 17 00:00:00 2001 From: d11n Date: Tue, 24 Jan 2023 01:44:39 +0100 Subject: [PATCH] BIP21: Uppercase addresses only in QR, not in payment URL (#4553) * BIP21: Uppercase addresses only in QR, not in payment URL The uppercased address/BOLT11 should only be used for the QR code, the payment URI for the link should stay as it is. References: - #2110 - https://bitcoinqr.dev/ * Improve comments * Add comments step by step * Ensure correct delimiter Co-authored-by: nicolas.dorier --- BTCPayServer.Tests/Checkoutv2Tests.cs | 6 +- BTCPayServer.Tests/UnitTest1.cs | 24 +++++--- .../Bitcoin/BitcoinLikePaymentHandler.cs | 55 ++++++++++++------- .../BitcoinLikeMethodCheckout-v2.cshtml | 2 +- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/BTCPayServer.Tests/Checkoutv2Tests.cs b/BTCPayServer.Tests/Checkoutv2Tests.cs index 340f7ddd6..4db107360 100644 --- a/BTCPayServer.Tests/Checkoutv2Tests.cs +++ b/BTCPayServer.Tests/Checkoutv2Tests.cs @@ -152,7 +152,7 @@ namespace BTCPayServer.Tests Assert.Empty(s.Driver.FindElements(By.CssSelector(".payment-method"))); payUrl = s.Driver.FindElement(By.CssSelector(".btn-primary")).GetAttribute("href"); Assert.StartsWith("bitcoin:", payUrl); - Assert.Contains("&LIGHTNING=", payUrl); + Assert.Contains("&lightning=", payUrl); // BIP21 with LN as default payment method s.GoToHome(); @@ -161,7 +161,7 @@ namespace BTCPayServer.Tests Assert.Empty(s.Driver.FindElements(By.CssSelector(".payment-method"))); payUrl = s.Driver.FindElement(By.CssSelector(".btn-primary")).GetAttribute("href"); Assert.StartsWith("bitcoin:", payUrl); - Assert.Contains("&LIGHTNING=", payUrl); + Assert.Contains("&lightning=", payUrl); // BIP21 with topup invoice (which is only available with Bitcoin onchain) s.GoToHome(); @@ -170,7 +170,7 @@ namespace BTCPayServer.Tests Assert.Empty(s.Driver.FindElements(By.CssSelector(".payment-method"))); payUrl = s.Driver.FindElement(By.CssSelector(".btn-primary")).GetAttribute("href"); Assert.StartsWith("bitcoin:", payUrl); - Assert.DoesNotContain("&LIGHTNING=", payUrl); + Assert.DoesNotContain("&lightning=", payUrl); // Expiry message should not show amount for topup invoice expirySeconds = s.Driver.FindElement(By.Id("ExpirySeconds")); diff --git a/BTCPayServer.Tests/UnitTest1.cs b/BTCPayServer.Tests/UnitTest1.cs index 0c8ec461d..9d7775dd3 100644 --- a/BTCPayServer.Tests/UnitTest1.cs +++ b/BTCPayServer.Tests/UnitTest1.cs @@ -1610,20 +1610,28 @@ namespace BTCPayServer.Tests // validate that QR code now has both onchain and offchain payment urls res = await user.GetController().Checkout(invoice.Id); - var paymentMethodSecond = Assert.IsType( + var paymentMethodUnified = Assert.IsType( Assert.IsType(res).Model ); - Assert.Contains("&lightning=", paymentMethodSecond.InvoiceBitcoinUrlQR); - Assert.StartsWith("bitcoin:", paymentMethodSecond.InvoiceBitcoinUrlQR); - var split = paymentMethodSecond.InvoiceBitcoinUrlQR.Split('?')[0]; + Assert.StartsWith("bitcoin:", paymentMethodUnified.InvoiceBitcoinUrl); + Assert.StartsWith("bitcoin:", paymentMethodUnified.InvoiceBitcoinUrlQR); + Assert.Contains("&lightning=", paymentMethodUnified.InvoiceBitcoinUrl); + Assert.Contains("&lightning=", paymentMethodUnified.InvoiceBitcoinUrlQR); + // Check correct casing: Addresses in payment URI need to be … + // - lowercase in link version + // - uppercase in QR version + // Standard for all uppercase characters in QR codes is still not implemented in all wallets // But we're proceeding with BECH32 being uppercase - Assert.True($"bitcoin:{paymentMethodSecond.BtcAddress.ToUpperInvariant()}" == split); + Assert.Equal($"bitcoin:{paymentMethodUnified.BtcAddress}", paymentMethodUnified.InvoiceBitcoinUrl.Split('?')[0]); + Assert.Equal($"bitcoin:{paymentMethodUnified.BtcAddress.ToUpperInvariant()}", paymentMethodUnified.InvoiceBitcoinUrlQR.Split('?')[0]); - // Fallback lightning invoice should be uppercase inside the QR code. - var lightningFallback = paymentMethodSecond.InvoiceBitcoinUrlQR.Split(new[] { "&lightning=" }, StringSplitOptions.None)[1]; - Assert.True(lightningFallback.ToUpperInvariant() == lightningFallback); + // Fallback lightning invoice should be uppercase inside the QR code, lowercase in payment URI + var lightningFallback = paymentMethodUnified.InvoiceBitcoinUrl.Split(new[] { "&lightning=" }, StringSplitOptions.None)[1]; + Assert.NotNull(lightningFallback); + Assert.Contains($"&lightning={lightningFallback}", paymentMethodUnified.InvoiceBitcoinUrl); + Assert.Contains($"&lightning={lightningFallback.ToUpperInvariant()}", paymentMethodUnified.InvoiceBitcoinUrlQR); } [Fact(Timeout = 60 * 2 * 1000)] diff --git a/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs b/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs index ef42a809e..e8669541a 100644 --- a/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs +++ b/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using BTCPayServer.Client.Models; +using BTCPayServer.Common; using BTCPayServer.Data; using BTCPayServer.HostedServices; using BTCPayServer.Logging; @@ -63,44 +64,58 @@ namespace BTCPayServer.Payments.Bitcoin model.FeeRate = ((BitcoinLikeOnChainPaymentMethod)paymentMethod.GetPaymentMethodDetails()).GetFeeRate(); model.PaymentMethodName = GetPaymentMethodName(network); - var lightningFallback = ""; + string lightningFallback = null; if (model.Activated && network.SupportLightning && storeBlob.OnChainWithLnInvoiceFallback) { var lightningInfo = invoiceResponse.CryptoInfo.FirstOrDefault(a => a.GetpaymentMethodId() == new PaymentMethodId(model.CryptoCode, PaymentTypes.LightningLike)); - if (!string.IsNullOrEmpty(lightningInfo?.PaymentUrls?.BOLT11)) - lightningFallback = "&" + lightningInfo.PaymentUrls.BOLT11 - .Replace("lightning:", "lightning=", StringComparison.OrdinalIgnoreCase) - .ToUpperInvariant(); + + + // Turn the colon into an equal sign to trun the whole into the lightning part of the query string + + // lightningInfo?.PaymentUrls?.BOLT11: lightning:lnbcrt440070n1p3ua9np... + lightningFallback = lightningInfo?.PaymentUrls?.BOLT11.Replace("lightning:", "lightning=", StringComparison.OrdinalIgnoreCase); + // lightningFallback: lightning=lnbcrt440070n1p3ua9np... } if (model.Activated) { - model.InvoiceBitcoinUrl = (cryptoInfo.PaymentUrls?.BIP21 ?? "") + lightningFallback; - model.InvoiceBitcoinUrlQR = (cryptoInfo.PaymentUrls?.BIP21 ?? "") + lightningFallback - .Replace("LIGHTNING=", "lightning=", StringComparison.OrdinalIgnoreCase); - - // Most wallets still don't support BITCOIN: schema, so we're leaving this for better days - // Ref: https://github.com/btcpayserver/btcpayserver/pull/2060#issuecomment-723828348 - //model.InvoiceBitcoinUrlQR = cryptoInfo.PaymentUrls.BIP21 - // .Replace("bitcoin:", "BITCOIN:", StringComparison.OrdinalIgnoreCase) - // We're leading the way in Bitcoin community with adding UPPERCASE Bech32 addresses in QR Code + // + // Correct casing: Addresses in payment URI need to be … + // - lowercase in link version + // - uppercase in QR version + // + // The keys (e.g. "bitcoin:" or "lightning=" should be lowercase! + + // cryptoInfo.PaymentUrls?.BIP21: bitcoin:bcrt1qxp2qa5?amount=0.00044007 + model.InvoiceBitcoinUrl = model.InvoiceBitcoinUrlQR = cryptoInfo.PaymentUrls?.BIP21 ?? ""; + // model.InvoiceBitcoinUrl: bitcoin:bcrt1qxp2qa5?amount=0.00044007 + // model.InvoiceBitcoinUrlQR: bitcoin:bcrt1qxp2qa5?amount=0.00044007 + + if (!string.IsNullOrEmpty(lightningFallback)) + { + var delimiterUrl = model.InvoiceBitcoinUrl.Contains("?") ? "&" : "?"; + model.InvoiceBitcoinUrl += $"{delimiterUrl}{lightningFallback}"; + // model.InvoiceBitcoinUrl: bitcoin:bcrt1qxp2qa5dhn7?amount=0.00044007&lightning=lnbcrt440070n1... + + var delimiterUrlQR = model.InvoiceBitcoinUrlQR.Contains("?") ? "&" : "?"; + model.InvoiceBitcoinUrlQR += $"{delimiterUrlQR}{lightningFallback.ToUpperInvariant().Replace("LIGHTNING=", "lightning=", StringComparison.OrdinalIgnoreCase)}"; + // model.InvoiceBitcoinUrlQR: bitcoin:bcrt1qxp2qa5dhn7?amount=0.00044007&lightning=LNBCRT4400... + } + if (network.CryptoCode.Equals("BTC", StringComparison.InvariantCultureIgnoreCase) && _bech32Prefix.TryGetValue(model.CryptoCode, out var prefix) && model.BtcAddress.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) { model.InvoiceBitcoinUrlQR = model.InvoiceBitcoinUrlQR.Replace( $"{network.NBitcoinNetwork.UriScheme}:{model.BtcAddress}", $"{network.NBitcoinNetwork.UriScheme}:{model.BtcAddress.ToUpperInvariant()}", - StringComparison.OrdinalIgnoreCase - ); + StringComparison.OrdinalIgnoreCase); + // model.InvoiceBitcoinUrlQR: bitcoin:BCRT1QXP2QA5DHN...?amount=0.00044007&lightning=LNBCRT4400... } } else { - model.InvoiceBitcoinUrl = ""; - model.InvoiceBitcoinUrlQR = ""; + model.InvoiceBitcoinUrl = model.InvoiceBitcoinUrlQR = string.Empty; } - - } public override string GetCryptoImage(PaymentMethodId paymentMethodId) diff --git a/BTCPayServer/Views/Shared/Bitcoin/BitcoinLikeMethodCheckout-v2.cshtml b/BTCPayServer/Views/Shared/Bitcoin/BitcoinLikeMethodCheckout-v2.cshtml index b86c3b4ca..df4473218 100644 --- a/BTCPayServer/Views/Shared/Bitcoin/BitcoinLikeMethodCheckout-v2.cshtml +++ b/BTCPayServer/Views/Shared/Bitcoin/BitcoinLikeMethodCheckout-v2.cshtml @@ -46,7 +46,7 @@ return this.model.invoiceBitcoinUrl.indexOf('@PayjoinClient.BIP21EndpointKey=') !== -1; }, BOLT11 () { - const match = this.model.invoiceBitcoinUrl.match(/&LIGHTNING=(.*)&?/i); + const match = this.model.invoiceBitcoinUrl.match(/&lightning=(.*)&?/i); return match ? match[1].toLowerCase() : null; } }