Fix: Uploading coldcard in derivation scheme would forget to remember some data

BTCPayServer.Tests/BTCPayServerTester.cs

@@ -144,6 +144,7 @@ namespace BTCPayServer.Tests
             _Host.Start();
             InvoiceRepository = (InvoiceRepository)_Host.Services.GetService(typeof(InvoiceRepository));
             StoreRepository = (StoreRepository)_Host.Services.GetService(typeof(StoreRepository));
+            Networks = (BTCPayNetworkProvider)_Host.Services.GetService(typeof(BTCPayNetworkProvider));
             var dashBoard = (NBXplorerDashboard)_Host.Services.GetService(typeof(NBXplorerDashboard));
             while(!dashBoard.IsFullySynched())
             {
@@ -226,6 +227,7 @@ namespace BTCPayServer.Tests
         }
         public InvoiceRepository InvoiceRepository { get; private set; }
         public StoreRepository StoreRepository { get; private set; }
+        public BTCPayNetworkProvider Networks { get; private set; }
         public Uri IntegratedLightning { get; internal set; }
         public bool InContainer { get; internal set; }
 

BTCPayServer.Tests/UnitTest1.cs

@@ -58,6 +58,8 @@ using System.Runtime.CompilerServices;
 using System.Net;
 using BTCPayServer.Models.AccountViewModels;
 using BTCPayServer.Services.U2F.Models;
+using Microsoft.AspNetCore.Http.Internal;
+using Microsoft.AspNetCore.Http;
 
 namespace BTCPayServer.Tests
 {
@@ -883,14 +885,6 @@ namespace BTCPayServer.Tests
             }
         }
 
-        [Fact]
-        [Trait("Fast", "Fast")]
-        public void CanParseColdcard()
-        {
-            var mnemonic = new Mnemonic("usage fever hen zero slide mammal silent heavy donate budget pulse say brain thank sausage brand craft about save attract muffin advance illegal cabbage");
-            var coldcardWallet = "{\"keystore\": {\"ckcc_xpub\": \"xpub661MyMwAqRbcGVBsTGeNZN6QGVHmMHLdSA4FteGsRrEriu4pnVZMZWnruFFFXkMnyoBjyHndD3Qwcfz4MPzBUxjSevweNFQx7SAYZATtcDw\", \"xpub\": \"ypub6WWc2gWwHbdnAAyJDnR4SPL1phRh7REqrPBfZeizaQ1EmTshieRXJC3Z5YoU4wkcdKHEjQGkh6AYEzCQC1Kz3DNaWSwdc1pc8416hAjzqyD\", \"label\": \"Coldcard Import 0x60d1af8b\", \"ckcc_xfp\": 1624354699, \"type\": \"hardware\", \"hw_type\": \"coldcard\", \"derivation\": \"m/49'/0'/0'\"}, \"wallet_type\": \"standard\", \"use_encryption\": false, \"seed_version\": 17}";
-        }
-
         [Fact]
         [Trait("Fast", "Fast")]
         public void CanParseFilter()
@@ -1517,7 +1511,7 @@ namespace BTCPayServer.Tests
 
         [Fact]
         [Trait("Integration", "Integration")]
-        public void CanDisablePaymentMethods()
+        public void CanAddDerivationSchemes()
         {
             using (var tester = ServerTester.Create())
             {
@@ -1548,16 +1542,18 @@ namespace BTCPayServer.Tests
                 lightningVM = (LightningNodeViewModel)Assert.IsType<ViewResult>(controller.AddLightningNode(user.StoreId, "BTC")).Model;
                 Assert.False(lightningVM.Enabled);
 
+                // Only Enabling/Disabling the payment method must redirect to store page
                 var derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
                 Assert.True(derivationVM.Enabled);
                 derivationVM.Enabled = false;
                 Assert.IsType<RedirectToActionResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult());
                 derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
-                // Confirmation
-                controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult();
                 Assert.False(derivationVM.Enabled);
+
+                // Clicking next without changing anything should send to the confirmation screen
                 derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
-                Assert.False(derivationVM.Enabled);
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult()).Model;
+                Assert.True(derivationVM.Confirmation);
 
                 invoice = user.BitPay.CreateInvoice(new Invoice()
                 {
@@ -1571,6 +1567,45 @@ namespace BTCPayServer.Tests
 
                 Assert.Single(invoice.CryptoInfo);
                 Assert.Equal("LTC", invoice.CryptoInfo[0].CryptoCode);
+
+                // Removing the derivation scheme, should redirect to store page
+                var oldScheme = derivationVM.DerivationScheme;
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
+                derivationVM.DerivationScheme = null;
+                Assert.IsType<RedirectToActionResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult());
+
+                // Setting it again should redirect to the confirmation page
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
+                derivationVM.DerivationScheme = oldScheme;
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult()).Model;
+                Assert.True(derivationVM.Confirmation);
+
+                // Can we upload coldcard settings?
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, "BTC")).Model;
+                string filename = "wallet.json";
+                string content = "{\"keystore\": {\"ckcc_xpub\": \"xpub661MyMwAqRbcGVBsTGeNZN6QGVHmMHLdSA4FteGsRrEriu4pnVZMZWnruFFFXkMnyoBjyHndD3Qwcfz4MPzBUxjSevweNFQx7SAYZATtcDw\", \"xpub\": \"ypub6WWc2gWwHbdnAAyJDnR4SPL1phRh7REqrPBfZeizaQ1EmTshieRXJC3Z5YoU4wkcdKHEjQGkh6AYEzCQC1Kz3DNaWSwdc1pc8416hAjzqyD\", \"label\": \"Coldcard Import 0x60d1af8b\", \"ckcc_xfp\": 1624354699, \"type\": \"hardware\", \"hw_type\": \"coldcard\", \"derivation\": \"m/49'/0'/0'\"}, \"wallet_type\": \"standard\", \"use_encryption\": false, \"seed_version\": 17}";
+                File.WriteAllText(filename, content);
+
+                var fileInfo = new FileInfo(filename);
+                var formFile = new FormFile(
+                    new FileStream(filename, FileMode.OpenOrCreate),
+                    0,
+                    fileInfo.Length, fileInfo.Name, fileInfo.Name)
+                {
+                    Headers = new HeaderDictionary()
+                };
+                formFile.ContentType = "text/plain";
+                formFile.ContentDisposition = $"form-data; name=\"file\"; filename=\"{fileInfo.Name}\"";
+                derivationVM.ColdcardPublicFile = formFile;
+                derivationVM = (DerivationSchemeViewModel)Assert.IsType<ViewResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult()).Model;
+                Assert.True(derivationVM.Confirmation);
+                Assert.IsType<RedirectToActionResult>(controller.AddDerivationScheme(user.StoreId, derivationVM, "BTC").GetAwaiter().GetResult());
+
+                // Now let's check that no data has been lost in the process
+                var store = tester.PayTester.StoreRepository.FindStore(user.StoreId).GetAwaiter().GetResult();
+                var onchainBTC = store.GetSupportedPaymentMethods(tester.PayTester.Networks).OfType<DerivationSchemeSettings>().First(o => o.PaymentId.IsBTCOnChain);
+                DerivationSchemeSettings.TryParseFromColdcard(content, onchainBTC.Network, out var expected);
+                Assert.Equal(expected.ToJson(), onchainBTC.ToJson());
             }
         }
 

BTCPayServer/Controllers/StoresController.BTCLike.cs

@@ -13,6 +13,7 @@ using BTCPayServer.Models.StoreViewModels;
 using BTCPayServer.Payments;
 using BTCPayServer.Services;
 using LedgerWallet;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using NBitcoin;
 using NBXplorer.DerivationStrategy;
@@ -101,7 +102,12 @@ namespace BTCPayServer.Controllers
 
         private void SetExistingValues(StoreData store, DerivationSchemeViewModel vm)
         {
-            vm.DerivationScheme = GetExistingDerivationStrategy(vm.CryptoCode, store)?.AccountDerivation.ToString();
+            var derivation = GetExistingDerivationStrategy(vm.CryptoCode, store);
+            if (derivation != null)
+            {
+                vm.DerivationScheme = derivation.AccountDerivation.ToString();
+                vm.Config = derivation.ToJson();
+            }
             vm.Enabled = !store.GetStoreBlob().IsExcluded(new PaymentMethodId(vm.CryptoCode, PaymentTypes.BTCLike));
         }
 
@@ -135,26 +141,29 @@ namespace BTCPayServer.Controllers
             vm.RootKeyPath = network.GetRootKeyPath();
             DerivationSchemeSettings strategy = null;
             
-            PaymentMethodId paymentMethodId = new PaymentMethodId(network.CryptoCode, PaymentTypes.BTCLike);
-            var exisingStrategy = store.GetSupportedPaymentMethods(_NetworkProvider)
-                .Where(c => c.PaymentId == paymentMethodId)
-                .OfType<DerivationSchemeSettings>()
-                .Select(c => c.AccountDerivation.ToString())
-                .FirstOrDefault();
-            
             var wallet = _WalletProvider.GetWallet(network);
             if (wallet == null)
             {
                 return NotFound();
             }
 
+            if (!string.IsNullOrEmpty(vm.Config))
+            {
+                if (!DerivationSchemeSettings.TryParseFromJson(vm.Config, network, out strategy))
+                {
+                    vm.StatusMessage = new StatusMessageModel()
+                    {
+                        Severity = StatusMessageModel.StatusSeverity.Error,
+                        Message = "Config file was not in the correct format"
+                    }.ToString();
+                    vm.Confirmation = false;
+                    return View(vm);
+                }
+            }
+
             if (vm.ColdcardPublicFile != null)
             {
-                using (var stream = new StreamReader(vm.ColdcardPublicFile.OpenReadStream()))
-                {
-                    var fileContent = await stream.ReadToEndAsync();
-                    if (
-                        !DerivationSchemeSettings.TryParseFromColdcard(fileContent, network, out strategy))
+                if (!DerivationSchemeSettings.TryParseFromColdcard(await ReadAllText(vm.ColdcardPublicFile), network, out strategy))
                 {
                     vm.StatusMessage = new StatusMessageModel()
                     {
@@ -165,15 +174,23 @@ namespace BTCPayServer.Controllers
                     return View(vm);
                 }
             }
-            }
             else
             {
                 try
                 {
                     if (!string.IsNullOrEmpty(vm.DerivationScheme))
                     {
-                        strategy = ParseDerivationStrategy(vm.DerivationScheme, null, network);
-                        vm.DerivationScheme = strategy.ToString();
+                        var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, null, network);
+                        if (newStrategy.AccountDerivation != strategy?.AccountDerivation)
+                        {
+                            strategy = newStrategy;
+                            strategy.AccountKeyPath = vm.KeyPath == null ? null : KeyPath.Parse(vm.KeyPath);
+                            vm.DerivationScheme = strategy.AccountDerivation.ToString();
+                        }
+                    }
+                    else
+                    {
+                        strategy = null;
                     }
                 }
                 catch
@@ -184,6 +201,14 @@ namespace BTCPayServer.Controllers
                 }
             }
 
+            var oldConfig = vm.Config;
+            vm.Config = strategy == null ? null : strategy.ToJson();
+
+            PaymentMethodId paymentMethodId = new PaymentMethodId(network.CryptoCode, PaymentTypes.BTCLike);
+            var exisingStrategy = store.GetSupportedPaymentMethods(_NetworkProvider)
+                .Where(c => c.PaymentId == paymentMethodId)
+                .OfType<DerivationSchemeSettings>()
+                .FirstOrDefault();
             var storeBlob = store.GetStoreBlob();
             var wasExcluded = storeBlob.GetExcludedPaymentMethods().Match(paymentMethodId);
             var willBeExcluded = !vm.Enabled;
@@ -191,10 +216,10 @@ namespace BTCPayServer.Controllers
             var showAddress = // Show addresses if:
                 // - If the user is testing the hint address in confirmation screen
                 (vm.Confirmation && !string.IsNullOrWhiteSpace(vm.HintAddress)) ||
-                // - The user is setting a new derivation scheme
-                (!vm.Confirmation && strategy != null && exisingStrategy != strategy.AccountDerivation.ToString()) ||
-                // - The user is clicking on continue without changing anything   
-                (!vm.Confirmation && willBeExcluded == wasExcluded);
+                // - The user is clicking on continue after changing the config
+                (!vm.Confirmation && oldConfig != vm.Config) ||
+                // - The user is clickingon continue without changing config nor enabling/disabling
+                (!vm.Confirmation && oldConfig == vm.Config && willBeExcluded == wasExcluded);
 
             showAddress = showAddress && strategy != null;
             if (!showAddress)
@@ -203,8 +228,7 @@ namespace BTCPayServer.Controllers
                 {
                     if (strategy != null)
                         await wallet.TrackAsync(strategy.AccountDerivation);
-                    strategy.AccountKeyPath = vm.KeyPath == null ? null : KeyPath.Parse(vm.KeyPath);
-                    store.SetSupportedPaymentMethod(strategy);
+                    store.SetSupportedPaymentMethod(paymentMethodId, strategy);
                     storeBlob.SetExcluded(paymentMethodId, willBeExcluded);
                     store.SetStoreBlob(storeBlob);
                 }
@@ -215,7 +239,13 @@ namespace BTCPayServer.Controllers
                 }
 
                 await _Repo.UpdateStore(store);
-                StatusMessage = $"Derivation scheme for {network.CryptoCode} has been modified.";
+                if (oldConfig != vm.Config)
+                    StatusMessage = $"Derivation settings for {network.CryptoCode} has been modified.";
+                if (willBeExcluded != wasExcluded)
+                {
+                    var label = willBeExcluded ? "disabled" : "enabled";
+                    StatusMessage = $"On-Chain payments for {network.CryptoCode} has been {label}.";
+                }
                 return RedirectToAction(nameof(UpdateStore), new {storeId = storeId});
             }
             else if (!string.IsNullOrEmpty(vm.HintAddress))
@@ -233,7 +263,12 @@ namespace BTCPayServer.Controllers
 
                 try
                 {
-                    strategy = ParseDerivationStrategy(vm.DerivationScheme, address.ScriptPubKey, network);
+                    var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, address.ScriptPubKey, network);
+                    if (newStrategy.AccountDerivation != strategy.AccountDerivation)
+                    {
+                        strategy.AccountDerivation = newStrategy.AccountDerivation;
+                        strategy.AccountOriginal = null;
+                    }
                 }
                 catch
                 {
@@ -251,6 +286,13 @@ namespace BTCPayServer.Controllers
             return ShowAddresses(vm, strategy);
         }
 
+        private async Task<string> ReadAllText(IFormFile file)
+        {
+            using (var stream = new StreamReader(file.OpenReadStream()))
+            {
+                return await stream.ReadToEndAsync();
+            }
+        }
 
         private IActionResult ShowAddresses(DerivationSchemeViewModel vm, DerivationSchemeSettings strategy)
         {

BTCPayServer/Controllers/StoresController.LightningLike.cs

@@ -144,7 +144,7 @@ namespace BTCPayServer.Controllers
                     var storeBlob = store.GetStoreBlob();
                     storeBlob.SetExcluded(paymentMethodId, !vm.Enabled);
                     store.SetStoreBlob(storeBlob);
-                    store.SetSupportedPaymentMethod(paymentMethod);
+                    store.SetSupportedPaymentMethod(paymentMethodId, paymentMethod);
                     await _Repo.UpdateStore(store);
                     StatusMessage = $"Lightning node modified ({network.CryptoCode})";
                     return RedirectToAction(nameof(UpdateStore), new { storeId = storeId });

BTCPayServer/Data/StoreData.cs

@@ -98,13 +98,29 @@ namespace BTCPayServer.Data
 #pragma warning restore CS0618
         }
 
+        public void SetSupportedPaymentMethod(ISupportedPaymentMethod supportedPaymentMethod)
+        {
+            SetSupportedPaymentMethod(null, supportedPaymentMethod);
+        }
+
         /// <summary>
         /// Set or remove a new supported payment method for the store
         /// </summary>
         /// <param name="paymentMethodId">The paymentMethodId</param>
         /// <param name="supportedPaymentMethod">The payment method, or null to remove</param>
-        public void SetSupportedPaymentMethod(ISupportedPaymentMethod supportedPaymentMethod)
+        public void SetSupportedPaymentMethod(PaymentMethodId paymentMethodId, ISupportedPaymentMethod supportedPaymentMethod)
         {
+            if (supportedPaymentMethod != null && paymentMethodId != null && paymentMethodId != supportedPaymentMethod.PaymentId)
+            {
+                throw new InvalidOperationException("Incoherent arguments, this should never happen");
+            }
+            if (supportedPaymentMethod == null && paymentMethodId == null)
+                throw new ArgumentException($"{nameof(supportedPaymentMethod)} or {nameof(paymentMethodId)} should be specified");
+            if (supportedPaymentMethod != null && paymentMethodId == null)
+            {
+                paymentMethodId = supportedPaymentMethod.PaymentId;
+            }
+
 #pragma warning disable CS0618
             JObject strategies = string.IsNullOrEmpty(DerivationStrategies) ? new JObject() : JObject.Parse(DerivationStrategies);
             bool existing = false;
@@ -116,7 +132,7 @@ namespace BTCPayServer.Data
                     // Legacy stuff which should go away
                     DerivationStrategy = null;
                 }
-                if (stratId == supportedPaymentMethod.PaymentId)
+                if (stratId == paymentMethodId)
                 {
                     if (supportedPaymentMethod == null)
                     {

BTCPayServer/DerivationSchemeSettings.cs

@@ -22,6 +22,22 @@ namespace BTCPayServer
             return new DerivationSchemeSettings(result, network) { AccountOriginal = derivationStrategy.Trim() };
         }
 
+        public static bool TryParseFromJson(string config, BTCPayNetwork network, out DerivationSchemeSettings strategy)
+        {
+            if (network == null)
+                throw new ArgumentNullException(nameof(network));
+            if (config == null)
+                throw new ArgumentNullException(nameof(config));
+            strategy = null;
+            try
+            {
+                strategy = network.NBXplorerNetwork.Serializer.ToObject<DerivationSchemeSettings>(config);
+                strategy.Network = network;
+            }
+            catch { }
+            return strategy != null;
+        }
+
         public static bool TryParseFromColdcard(string coldcardExport, BTCPayNetwork network, out DerivationSchemeSettings settings)
         {
             settings = null;
@@ -88,10 +104,6 @@ namespace BTCPayServer
                 }
                 catch { return false; }
             }
-            else
-            {
-                result.AccountKeyPath = new KeyPath();
-            }
             settings = result;
             settings.Network = network;
             return true;
@@ -135,5 +147,10 @@ namespace BTCPayServer
                    !String.IsNullOrEmpty(AccountOriginal) ? AccountOriginal :
                    ToString();
         }
+
+        public string ToJson()
+        {
+            return Network.NBXplorerNetwork.Serializer.ToString(this);
+        }
     }
 }

BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs

@@ -36,5 +36,6 @@ namespace BTCPayServer.Models.StoreViewModels
 
         [Display(Name = "Coldcard Wallet File")]
         public IFormFile ColdcardPublicFile{ get; set; }
+        public string Config { get; set; }
     }
 }

BTCPayServer/Views/Stores/AddDerivationScheme.cshtml

@@ -50,6 +50,7 @@
                 </div>
                 <input id="CryptoCurrency" asp-for="CryptoCode" type="hidden" />
                 <input id="KeyPath" asp-for="KeyPath" type="hidden" />
+                <input id="Config" asp-for="Config" type="hidden" />
                 <div class="form-group">
                     <label asp-for="DerivationScheme"></label>
                     <input asp-for="DerivationScheme" class="form-control" />
@@ -141,6 +142,7 @@
                 <input id="KeyPath" asp-for="KeyPath" type="hidden" />
                 <input type="hidden" asp-for="DerivationScheme" />
                 <input type="hidden" asp-for="Enabled" />
+                <input id="Config" asp-for="Config" type="hidden" />
                 <div class="form-group">
                     <table class="table table-sm table-responsive-md">
                         <thead>