Merge pull request #1561 from NicolasDorier/bp/new

Refactoring of payjoin to support new spec

BTCPayServer.Tests/PayJoinTests.cs

@@ -28,6 +28,7 @@ using NBitcoin.Altcoins;
 using NBitcoin.Payment;
 using NBitpayClient;
 using NBXplorer.Models;
+using Newtonsoft.Json.Linq;
 using OpenQA.Selenium;
 using Xunit;
 using Xunit.Abstractions;
@@ -587,6 +588,14 @@ namespace BTCPayServer.Tests
                 //Let's start the harassment
                 invoice = receiverUser.BitPay.CreateInvoice(
                     new Invoice() {Price = 0.02m, Currency = "BTC", FullNotifications = true});
+                // Bad version should throw incorrect version
+                var endpoint = TestAccount.GetPayjoinEndpoint(invoice, btcPayNetwork.NBitcoinNetwork);
+                var response = await tester.PayTester.HttpClient.PostAsync(endpoint.AbsoluteUri + "?v=2",
+                    new StringContent("", Encoding.UTF8, "text/plain"));
+                Assert.False(response.IsSuccessStatusCode);
+                var error = JObject.Parse(await response.Content.ReadAsStringAsync());
+                Assert.Equal("version-unsupported", error["errorCode"].Value<string>());
+                Assert.Equal(1, ((JArray)error["supported"]).Single().Value<int>());
 
                 var parsedBip21 = new BitcoinUrlBuilder(invoice.CryptoInfo.First().PaymentUrls.BIP21,
                     tester.ExplorerClient.Network.NBitcoinNetwork);
@@ -665,6 +674,7 @@ namespace BTCPayServer.Tests
                 //Attempt 2: Create two transactions using different inputs and send them to the same invoice. 
                 //Result: Second Tx should be rejected. 
                 var Invoice1Coin1ResponseTx = await senderUser.SubmitPayjoin(invoice, Invoice1Coin1, btcPayNetwork);
+
                 await senderUser.SubmitPayjoin(invoice, Invoice1Coin1, btcPayNetwork, "already-paid");
                 var contributedInputsInvoice1Coin1ResponseTx =
                     Invoice1Coin1ResponseTx.Inputs.Where(txin => coin.OutPoint != txin.PrevOut);

BTCPayServer.Tests/TestAccount.cs

@@ -393,7 +393,7 @@ namespace BTCPayServer.Tests
             return response;
         }
 
-        private static Uri GetPayjoinEndpoint(Invoice invoice, Network network)
+        public static Uri GetPayjoinEndpoint(Invoice invoice, Network network)
         {
             var parsedBip21 = new BitcoinUrlBuilder(
                 invoice.CryptoInfo.First(c => c.CryptoCode == network.NetworkSet.CryptoCode).PaymentUrls.BIP21,

BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs

@@ -23,6 +23,7 @@ using NBXplorer.DerivationStrategy;
 using System.Diagnostics.CodeAnalysis;
 using BTCPayServer.Data;
 using NBitcoin.DataEncoders;
+using Amazon.S3.Model;
 
 namespace BTCPayServer.Payments.PayJoin
 {
@@ -115,12 +116,24 @@ namespace BTCPayServer.Payments.PayJoin
         [EnableCors(CorsPolicies.All)]
         [MediaTypeConstraint("text/plain")]
         [RateLimitsFilter(ZoneLimits.PayJoin, Scope = RateLimitsScope.RemoteAddress)]
-        public async Task<IActionResult> Submit(string cryptoCode)
+        public async Task<IActionResult> Submit(string cryptoCode,
+            bool noadjustfee = false,
+            int feebumpindex = -1,
+            int v = 1)
         {
+            if (v != 1)
+            {
+                return BadRequest(new JObject
+                {
+                    new JProperty("errorCode", "version-unsupported"),
+                    new JProperty("supported", new JArray(1)),
+                    new JProperty("message", "This version of payjoin is not supported.")
+                });
+            }
             var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
             if (network == null)
             {
-                return BadRequest(CreatePayjoinError(400, "invalid-network", "Incorrect network"));
+                return BadRequest(CreatePayjoinError("invalid-network", "Incorrect network"));
             }
 
             var explorer = _explorerClientProvider.GetExplorerClient(network);
@@ -128,12 +141,12 @@ namespace BTCPayServer.Payments.PayJoin
             {
                 if (length > 1_000_000)
                     return this.StatusCode(413,
-                        CreatePayjoinError(413, "payload-too-large", "The transaction is too big to be processed"));
+                        CreatePayjoinError("payload-too-large", "The transaction is too big to be processed"));
             }
             else
             {
                 return StatusCode(411,
-                    CreatePayjoinError(411, "missing-content-length",
+                    CreatePayjoinError("missing-content-length",
                         "The http header Content-Length should be filled"));
             }
 
@@ -146,26 +159,28 @@ namespace BTCPayServer.Payments.PayJoin
             Transaction originalTx = null;
             FeeRate originalFeeRate = null;
             bool psbtFormat = true;
-            if (!PSBT.TryParse(rawBody, network.NBitcoinNetwork, out var psbt))
+
+            if (PSBT.TryParse(rawBody, network.NBitcoinNetwork, out var psbt))
+            {
+                if (!psbt.IsAllFinalized())
+                    return BadRequest(CreatePayjoinError("psbt-not-finalized", "The PSBT should be finalized"));
+                originalTx = psbt.ExtractTransaction();
+            }
+            // BTCPay Server implementation support a transaction instead of PSBT
+            else
             {
                 psbtFormat = false;
                 if (!Transaction.TryParse(rawBody, network.NBitcoinNetwork, out var tx))
-                    return BadRequest(CreatePayjoinError(400, "invalid-format", "invalid transaction or psbt"));
+                    return BadRequest(CreatePayjoinError("invalid-format", "invalid transaction or psbt"));
                 originalTx = tx;
                 psbt = PSBT.FromTransaction(tx, network.NBitcoinNetwork);
-                psbt = (await explorer.UpdatePSBTAsync(new UpdatePSBTRequest() {PSBT = psbt})).PSBT;
+                psbt = (await explorer.UpdatePSBTAsync(new UpdatePSBTRequest() { PSBT = psbt })).PSBT;
                 for (int i = 0; i < tx.Inputs.Count; i++)
                 {
                     psbt.Inputs[i].FinalScriptSig = tx.Inputs[i].ScriptSig;
                     psbt.Inputs[i].FinalScriptWitness = tx.Inputs[i].WitScript;
                 }
             }
-            else
-            {
-                if (!psbt.IsAllFinalized())
-                    return BadRequest(CreatePayjoinError(400, "psbt-not-finalized", "The PSBT should be finalized"));
-                originalTx = psbt.ExtractTransaction();
-            }
 
             async Task BroadcastNow()
             {
@@ -173,15 +188,13 @@ namespace BTCPayServer.Payments.PayJoin
             }
 
             var sendersInputType = psbt.GetInputsScriptPubKeyType();
-            if (sendersInputType is null)
-                return BadRequest(CreatePayjoinError(400, "unsupported-inputs", "Payjoin only support segwit inputs (of the same type)"));
             if (psbt.CheckSanity() is var errors && errors.Count != 0)
             {
-                return BadRequest(CreatePayjoinError(400, "insane-psbt", $"This PSBT is insane ({errors[0]})"));
+                return BadRequest(CreatePayjoinError("insane-psbt", $"This PSBT is insane ({errors[0]})"));
             }
             if (!psbt.TryGetEstimatedFeeRate(out originalFeeRate))
             {
-                return BadRequest(CreatePayjoinError(400, "need-utxo-information",
+                return BadRequest(CreatePayjoinError("need-utxo-information",
                     "You need to provide Witness UTXO information to the PSBT."));
             }
 
@@ -189,26 +202,26 @@ namespace BTCPayServer.Payments.PayJoin
             // to leak global xpubs
             if (psbt.GlobalXPubs.Any())
             {
-                return BadRequest(CreatePayjoinError(400, "leaking-data",
+                return BadRequest(CreatePayjoinError("leaking-data",
                     "GlobalXPubs should not be included in the PSBT"));
             }
 
             if (psbt.Outputs.Any(o => o.HDKeyPaths.Count != 0) || psbt.Inputs.Any(o => o.HDKeyPaths.Count != 0))
             {
-                return BadRequest(CreatePayjoinError(400, "leaking-data",
+                return BadRequest(CreatePayjoinError("leaking-data",
                     "Keypath information should not be included in the PSBT"));
             }
 
             if (psbt.Inputs.Any(o => !o.IsFinalized()))
             {
-                return BadRequest(CreatePayjoinError(400, "psbt-not-finalized", "The PSBT Should be finalized"));
+                return BadRequest(CreatePayjoinError("psbt-not-finalized", "The PSBT Should be finalized"));
             }
             ////////////
 
             var mempool = await explorer.BroadcastAsync(originalTx, true);
             if (!mempool.Success)
             {
-                return BadRequest(CreatePayjoinError(400, "invalid-transaction",
+                return BadRequest(CreatePayjoinError("invalid-transaction",
                     $"Provided transaction isn't mempool eligible {mempool.RPCCodeMessage}"));
             }
 
@@ -240,12 +253,12 @@ namespace BTCPayServer.Payments.PayJoin
                 if (!PayjoinClient.SupportedFormats.Contains(receiverInputsType))
                 {
                     //this should never happen, unless the store owner changed the wallet mid way through an invoice
-                    return StatusCode(500, CreatePayjoinError(500, "unavailable", $"This service is unavailable for now"));
+                    return StatusCode(500, CreatePayjoinError("unavailable", $"This service is unavailable for now"));
                 }
-                if (sendersInputType != receiverInputsType)
+                if (sendersInputType is ScriptPubKeyType t && t != receiverInputsType)
                 {
                     return StatusCode(503,
-                        CreatePayjoinError(503, "out-of-utxos",
+                        CreatePayjoinError("out-of-utxos",
                             "We do not have any UTXO available for making a payjoin with the sender's inputs type")); 
                 }
                 var paymentMethod = invoice.GetPaymentMethod(paymentMethodId);
@@ -255,7 +268,7 @@ namespace BTCPayServer.Payments.PayJoin
                     continue;
                 if (invoice.GetAllBitcoinPaymentData().Any())
                 {
-                    return UnprocessableEntity(CreatePayjoinError(422, "already-paid",
+                    return UnprocessableEntity(CreatePayjoinError("already-paid",
                         $"The invoice this PSBT is paying has already been partially or completely paid"));
                 }
 
@@ -268,7 +281,7 @@ namespace BTCPayServer.Payments.PayJoin
 
                 if (!await _payJoinRepository.TryLockInputs(originalTx.Inputs.Select(i => i.PrevOut).ToArray()))
                 {
-                    return BadRequest(CreatePayjoinError(400, "inputs-already-used",
+                    return BadRequest(CreatePayjoinError("inputs-already-used",
                         "Some of those inputs have already been used to make payjoin transaction"));
                 }
 
@@ -293,13 +306,13 @@ namespace BTCPayServer.Payments.PayJoin
 
             if (!paidSomething)
             {
-                return BadRequest(CreatePayjoinError(400, "invoice-not-found",
+                return BadRequest(CreatePayjoinError("invoice-not-found",
                     "This transaction does not pay any invoice with payjoin"));
             }
 
             if (due is null || due > Money.Zero)
             {
-                return BadRequest(CreatePayjoinError(400, "invoice-not-fully-paid",
+                return BadRequest(CreatePayjoinError("invoice-not-fully-paid",
                     "The transaction must pay the whole invoice"));
             }
 
@@ -307,7 +320,7 @@ namespace BTCPayServer.Payments.PayJoin
             {
                 await BroadcastNow();
                 return StatusCode(503,
-                    CreatePayjoinError(503, "out-of-utxos",
+                    CreatePayjoinError("out-of-utxos",
                         "We do not have any UTXO available for making a payjoin for now"));
             }
 
@@ -323,7 +336,7 @@ namespace BTCPayServer.Payments.PayJoin
                 // This should not happen, as we check the existance of private key before creating invoice with payjoin
                 await UnlockUTXOs();
                 await BroadcastNow();
-                return StatusCode(500, CreatePayjoinError(500, "unavailable", $"This service is unavailable for now"));
+                return StatusCode(500, CreatePayjoinError("unavailable", $"This service is unavailable for now"));
             }
             
             Money contributedAmount = Money.Zero;
@@ -331,6 +344,10 @@ namespace BTCPayServer.Payments.PayJoin
             var ourNewOutput = newTx.Outputs[originalPaymentOutput.Index];
             HashSet<TxOut> isOurOutput = new HashSet<TxOut>();
             isOurOutput.Add(ourNewOutput);
+            TxOut preferredFeeBumpOutput = feebumpindex >= 0
+                                            && feebumpindex < newTx.Outputs.Count
+                                            && !isOurOutput.Contains(newTx.Outputs[feebumpindex])
+                                            ? newTx.Outputs[feebumpindex] : null;
             var rand = new Random();
             int senderInputCount = newTx.Inputs.Count;
             foreach (var selectedUTXO in selectedUTXOs.Select(o => o.Value))
@@ -398,7 +415,7 @@ namespace BTCPayServer.Payments.PayJoin
             Money expectedFee = txBuilder.EstimateFees(newTx, originalFeeRate);
             Money actualFee = newTx.GetFee(txBuilder.FindSpentCoins(newTx));
             Money additionalFee = expectedFee - actualFee;
-            if (additionalFee > Money.Zero)
+            if (additionalFee > Money.Zero && !noadjustfee)
             {
                 // If the user overpaid, taking fee on our output (useful if sender dump a full UTXO for privacy)
                 for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && due < Money.Zero; i++)
@@ -418,6 +435,9 @@ namespace BTCPayServer.Payments.PayJoin
                 // The rest, we take from user's change
                 for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero; i++)
                 {
+                    if (preferredFeeBumpOutput is TxOut &&
+                        preferredFeeBumpOutput != newTx.Outputs[i])
+                        continue;
                     if (!isOurOutput.Contains(newTx.Outputs[i]))
                     {
                         var outputContribution = Money.Min(additionalFee, newTx.Outputs[i].Value);
@@ -438,7 +458,7 @@ namespace BTCPayServer.Payments.PayJoin
                     {
                         await UnlockUTXOs();
                         await BroadcastNow();
-                        return UnprocessableEntity(CreatePayjoinError(422, "not-enough-money",
+                        return UnprocessableEntity(CreatePayjoinError("not-enough-money",
                             "Not enough money is sent to pay for the additional payjoin inputs"));
                     }
                 }
@@ -476,7 +496,7 @@ namespace BTCPayServer.Payments.PayJoin
             {
                 await UnlockUTXOs();
                 await BroadcastNow();
-                return UnprocessableEntity(CreatePayjoinError(422, "already-paid",
+                return UnprocessableEntity(CreatePayjoinError("already-paid",
                     $"The original transaction has already been accounted"));
             }
             await _btcPayWalletProvider.GetWallet(network).SaveOffchainTransactionAsync(originalTx);
@@ -492,7 +512,8 @@ namespace BTCPayServer.Payments.PayJoin
                             }))
                     .ToDictionary(pair => pair.Key, pair => pair.Value)
             });
-            
+
+            // BTCPay Server support PSBT set as hex
             if (psbtFormat && HexEncoder.IsWellFormed(rawBody))
             {
                 return Ok(newPsbt.ToHex());
@@ -501,6 +522,7 @@ namespace BTCPayServer.Payments.PayJoin
             {
                 return Ok(newPsbt.ToBase64());
             }
+            // BTCPay Server should returns transaction if received transaction
             else
                 return Ok(newTx.ToHex());
         }
@@ -514,10 +536,9 @@ namespace BTCPayServer.Payments.PayJoin
             return hash;
         }
 
-        private JObject CreatePayjoinError(int httpCode, string errorCode, string friendlyMessage)
+        private JObject CreatePayjoinError(string errorCode, string friendlyMessage)
         {
             var o = new JObject();
-            o.Add(new JProperty("httpCode", httpCode));
             o.Add(new JProperty("errorCode", errorCode));
             o.Add(new JProperty("message", friendlyMessage));
             return o;

BTCPayServer/Services/PayjoinClient.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Configuration;
 using System.Linq;
 using System.Net.Http;
 using System.Text;
@@ -34,7 +35,7 @@ namespace BTCPayServer.Services
             if (i.WitnessUtxo.ScriptPubKey.IsScriptType(ScriptType.P2WPKH))
                 return ScriptPubKeyType.Segwit;
             if (i.WitnessUtxo.ScriptPubKey.IsScriptType(ScriptType.P2SH) &&
-                PayToWitPubKeyHashTemplate.Instance.ExtractWitScriptParameters(i.FinalScriptWitness) is {})
+                PayToWitPubKeyHashTemplate.Instance.ExtractWitScriptParameters(i.FinalScriptWitness) is { })
                 return ScriptPubKeyType.SegwitP2SH;
             return null;
         }
@@ -56,18 +57,22 @@ namespace BTCPayServer.Services
 
         public PayjoinClient(ExplorerClientProvider explorerClientProvider, IHttpClientFactory httpClientFactory)
         {
-            if (httpClientFactory == null) throw new ArgumentNullException(nameof(httpClientFactory));
+            if (httpClientFactory == null)
+                throw new ArgumentNullException(nameof(httpClientFactory));
             _explorerClientProvider =
                 explorerClientProvider ?? throw new ArgumentNullException(nameof(explorerClientProvider));
-            _httpClientFactory =  httpClientFactory;
+            _httpClientFactory = httpClientFactory;
         }
 
         public async Task<PSBT> RequestPayjoin(Uri endpoint, DerivationSchemeSettings derivationSchemeSettings,
             PSBT originalTx, CancellationToken cancellationToken)
         {
-            if (endpoint == null) throw new ArgumentNullException(nameof(endpoint));
-            if (derivationSchemeSettings == null) throw new ArgumentNullException(nameof(derivationSchemeSettings));
-            if (originalTx == null) throw new ArgumentNullException(nameof(originalTx));
+            if (endpoint == null)
+                throw new ArgumentNullException(nameof(endpoint));
+            if (derivationSchemeSettings == null)
+                throw new ArgumentNullException(nameof(derivationSchemeSettings));
+            if (originalTx == null)
+                throw new ArgumentNullException(nameof(originalTx));
             if (originalTx.IsAllFinalized())
                 throw new InvalidOperationException("The original PSBT should not be finalized.");
 
@@ -111,7 +116,7 @@ namespace BTCPayServer.Services
                 try
                 {
                     var error = JObject.Parse(errorStr);
-                    throw new PayjoinReceiverException((int)bpuresponse.StatusCode, error["errorCode"].Value<string>(),
+                    throw new PayjoinReceiverException(error["errorCode"].Value<string>(),
                         error["message"].Value<string>());
                 }
                 catch (JsonReaderException)
@@ -151,7 +156,7 @@ namespace BTCPayServer.Services
             foreach (var output in newPSBT.Outputs)
             {
                 output.HDKeyPaths.Clear();
-                foreach (var originalOutput in  originalTx.Outputs)
+                foreach (var originalOutput in originalTx.Outputs)
                 {
                     if (output.ScriptPubKey == originalOutput.ScriptPubKey)
                         output.UpdateFrom(originalOutput);
@@ -212,10 +217,10 @@ namespace BTCPayServer.Services
             if (sentAfter > sentBefore)
             {
                 var overPaying = sentAfter - sentBefore;
-               
+
                 if (!newPSBT.TryGetEstimatedFeeRate(out var newFeeRate) || !newPSBT.TryGetVirtualSize(out var newVirtualSize))
                     throw new PayjoinSenderException("The payjoin receiver did not included UTXO information to calculate fee correctly");
-                
+
                 var additionalFee = newPSBT.GetFee() - originalFee;
                 if (overPaying > additionalFee)
                     throw new PayjoinSenderException("The payjoin receiver is sending more money to himself");
@@ -250,23 +255,64 @@ namespace BTCPayServer.Services
         }
     }
 
+    public enum PayjoinReceiverWellknownErrors
+    {
+        LeakingData,
+        PSBTNotFinalized,
+        Unavailable,
+        OutOfUTXOS,
+        NotEnoughMoney,
+        InsanePSBT,
+        VersionUnsupported,
+        NeedUTXOInformation
+    }
     public class PayjoinReceiverException : PayjoinException
     {
-        public PayjoinReceiverException(int httpCode, string errorCode, string message) : base(FormatMessage(httpCode,
-            errorCode, message))
+        public PayjoinReceiverException(string errorCode, string receiverDebugMessage) : base(FormatMessage(errorCode, receiverDebugMessage))
         {
-            HttpCode = httpCode;
             ErrorCode = errorCode;
-            ErrorMessage = message;
+            ReceiverDebugMessage = receiverDebugMessage;
+            WellknownError = errorCode switch
+            {
+                "leaking-data" => PayjoinReceiverWellknownErrors.LeakingData,
+                "psbt-not-finalized" => PayjoinReceiverWellknownErrors.PSBTNotFinalized,
+                "unavailable" => PayjoinReceiverWellknownErrors.Unavailable,
+                "out-of-utxos" => PayjoinReceiverWellknownErrors.OutOfUTXOS,
+                "not-enough-money" => PayjoinReceiverWellknownErrors.NotEnoughMoney,
+                "insane-psbt" => PayjoinReceiverWellknownErrors.InsanePSBT,
+                "version-unsupported" => PayjoinReceiverWellknownErrors.VersionUnsupported,
+                "need-utxo-information" => PayjoinReceiverWellknownErrors.NeedUTXOInformation,
+                _ => null
+            };
         }
-
-        public int HttpCode { get; }
         public string ErrorCode { get; }
         public string ErrorMessage { get; }
+        public string ReceiverDebugMessage { get; }
 
-        private static string FormatMessage(in int httpCode, string errorCode, string message)
+        public PayjoinReceiverWellknownErrors? WellknownError
         {
-            return $"{errorCode}: {message} (HTTP: {httpCode})";
+            get;
+        }
+
+        private static string FormatMessage(string errorCode, string receiverDebugMessage)
+        {
+            return $"{errorCode}: {GetMessage(errorCode)}";
+        }
+
+        private static string GetMessage(string errorCode)
+        {
+            return errorCode switch
+            {
+                "leaking-data" => "Key path information or GlobalXPubs should not be included in the original PSBT.",
+                "psbt-not-finalized" => "The original PSBT must be finalized.",
+                "unavailable" => "The payjoin endpoint is not available for now.",
+                "out-of-utxos" => "The receiver does not have any UTXO to contribute in a payjoin proposal.",
+                "not-enough-money" => "The receiver added some inputs but could not bump the fee of the payjoin proposal.",
+                "insane-psbt" => "Some consistency check on the PSBT failed.",
+                "version-unsupported" => "This version of payjoin is not supported.",
+                "need-utxo-information" => "The witness UTXO or non witness UTXO is missing",
+                _ => "Unknown error"
+            };
         }
     }