Refactor wallet file parsing (Fix: #5690) (#5692)

BTCPayServer/Controllers/UIStoresController.Onchain.cs

@@ -90,9 +90,17 @@ namespace BTCPayServer.Controllers
 
             if (vm.WalletFile != null)
             {
-                if (!_onChainWalletParsers.TryParseWalletFile(await ReadAllText(vm.WalletFile), network, out strategy, out var error))
+                string fileContent = null;
+                try
                 {
-                    ModelState.AddModelError(nameof(vm.WalletFile), $"Importing wallet failed: {error}");
+                    fileContent = await ReadAllText(vm.WalletFile);
+                }
+                catch
+                {
+                }
+                if (fileContent is null || !_onChainWalletParsers.TryParseWalletFile(fileContent, network, out strategy, out _))
+                {
+                    ModelState.AddModelError(nameof(vm.WalletFile), $"Importing wallet failed");
                     return View(vm.ViewName, vm);
                 }
             }

BTCPayServer/Services/WalletFileParsers.cs

@@ -19,7 +19,7 @@ public class WalletFileParsers
     public bool TryParseWalletFile(string fileContents, BTCPayNetwork network, [MaybeNullWhen(false)] out DerivationSchemeSettings settings, [MaybeNullWhen(true)] out string error)
     {
         settings = null;
-        error = string.Empty;
+        error = null;
         ArgumentNullException.ThrowIfNull(fileContents);
         ArgumentNullException.ThrowIfNull(network);
         if (HexEncoder.IsWellFormed(fileContents))
@@ -29,19 +29,16 @@ public class WalletFileParsers
 
         foreach (IWalletFileParser onChainWalletParser in Parsers)
         {
-            var result = onChainWalletParser.TryParse(network, fileContents);
+            try
-            if (result.DerivationSchemeSettings is not null)
             {
-                settings = result.DerivationSchemeSettings;
+                if (onChainWalletParser.TryParse(network, fileContents, out settings))
-                error = null;
                     return true;
             }
-
+            catch (Exception)
-            if (result.Error is not null)
             {
-                error = result.Error;
             }
         }
+        error = "Unsupported file format";
         return false;
     }
 }

BTCPayServer/Services/WalletFileParsing/BSMSWalletFileParser.cs

@@ -1,5 +1,6 @@
-#nullable enable
+#nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using BTCPayServer;
 using NBitcoin;
@@ -10,34 +11,25 @@ using BTCPayNetwork = BTCPayServer.BTCPayNetwork;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class BSMSWalletFileParser : IWalletFileParser
 {
-    public (BTCPayServer.DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
-        BTCPayNetwork network,
-        string data)
-    {
-        try
     {
+        derivationSchemeSettings = null;
         string[] lines = data.Split(
             new[] { "\r\n", "\r", "\n" },
             StringSplitOptions.None
         );
 
-            if (!lines[0].Trim().Equals("BSMS 1.0"))
+        if (lines.Length < 4 || !lines[0].Trim().Equals("BSMS 1.0"))
-            {
+            return false;
-                return (null, null);
-            }
 
         var descriptor = lines[1];
         var derivationPath = lines[2].Split(',', StringSplitOptions.RemoveEmptyEntries).FirstOrDefault() ??
                              "/0/*";
         if (derivationPath == "No path restrictions")
-            {
             derivationPath = "/0/*";
-            }
 
         if (derivationPath != "/0/*")
-            {
+            return false;
-                return (null, "BTCPay Server can only derive address to the deposit and change paths");
-            }
 
 
         descriptor = descriptor.Replace("/**", derivationPath);
@@ -49,11 +41,9 @@ public class BSMSWalletFileParser : IWalletFileParser
         var line = result.Item1.GetLineFor(deposit).Derive(0);
 
         if (testAddress.ScriptPubKey != line.ScriptPubKey)
-            {
+            return false;
-                return (null, "BSMS test address did not match our generated address");
-            }
 
-            var derivationSchemeSettings = new BTCPayServer.DerivationSchemeSettings()
+        derivationSchemeSettings = new BTCPayServer.DerivationSchemeSettings()
         {
             Network = network,
             Source = "BSMS",
@@ -66,11 +56,6 @@ public class BSMSWalletFileParser : IWalletFileParser
                 AccountKey = result.Item1.GetExtPubKeys().ElementAt(i).GetWif(network.NBitcoinNetwork)
             }).ToArray()
         };
-            return (derivationSchemeSettings, null);
+        return true;
-        }
-        catch (Exception e)
-        {
-            return (null, $"BSMS parse error: {e.Message}");
-        }
     }
 }

BTCPayServer/Services/WalletFileParsing/ElectrumWalletFileParser.cs

@@ -1,88 +1,61 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using BTCPayServer;
 using NBitcoin;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class ElectrumWalletFileParser : IWalletFileParser
 {
-    public (BTCPayServer.DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
+    class ElectrumFormat
-        string data)
     {
-        try
+        internal class KeyStoreFormat
         {
-            var derivationSchemeParser = network.GetDerivationSchemeParser();
+            public string? xpub { get; set; }
-            var jobj = JObject.Parse(data);
+            public string? label { get; set; }
-            var result = new BTCPayServer.DerivationSchemeSettings() {Network = network};
+            public uint? ckcc_xfp { get; set; }
-
+            public string? derivation { get; set; }
-            if (jobj["keystore"] is JObject keyStore)
+            public string? ColdCardFirmwareVersion { get; set; }
-            {
+            public string? CoboVaultFirmwareVersion { get; set; }
-                result.Source = "ElectrumFile";
-                jobj = keyStore;
-
-                if (!jobj.TryGetValue("xpub", StringComparison.InvariantCultureIgnoreCase, out var xpubToken))
-                {
-                    return (null, "no xpub");
         }
-                var strategy = derivationSchemeParser.Parse(xpubToken.Value<string>(), false, false, true);
+        public KeyStoreFormat? keystore { get; set; }
+    }
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
+    {
+        derivationSchemeSettings = null;
+        var jobj = JsonConvert.DeserializeObject<ElectrumFormat>(data);
+        if (jobj?.keystore is null)
+            return false;
+
+        var result = new BTCPayServer.DerivationSchemeSettings() { Network = network };
+        var derivationSchemeParser = network.GetDerivationSchemeParser();
+        result.Source = "ElectrumFile";
+
+        if (jobj.keystore.xpub is null || jobj.keystore.ckcc_xfp is null || jobj.keystore.derivation is null)
+            return false;
+
+        var strategy = derivationSchemeParser.Parse(jobj.keystore.xpub, false, false, true);
         result.AccountDerivation = strategy;
-                result.AccountOriginal = xpubToken.Value<string>();
+        result.AccountOriginal = jobj.keystore.xpub;
         result.GetSigningAccountKeySettings();
 
-                if (jobj["label"]?.Value<string>() is string label)
+        if (jobj.keystore.label is not null)
-                {
+            result.Label = jobj.keystore.label;
-                    try
-                    {
-                        result.Label = label;
-                    }
-                    catch
-                    {
-                        return (null, "Label was not a string");
-                    }
-                }
 
-                if (jobj["ckcc_xfp"]?.Value<uint>() is uint xfp)
+        result.AccountKeySettings[0].RootFingerprint = new HDFingerprint(jobj.keystore.ckcc_xfp.Value);
-                {
+        result.AccountKeySettings[0].AccountKeyPath = new KeyPath(jobj.keystore.derivation);
-                    try
-                    {
-                        result.AccountKeySettings[0].RootFingerprint =
-                            new HDFingerprint(xfp);
-                    }
-                    catch
-                    {
-                        return (null, "fingerprint was not a uint");
-                    }
-                }
-
-                if (jobj["derivation"]?.Value<string>() is string derivation)
-                {
-                    try
-                    {
-                        result.AccountKeySettings[0].AccountKeyPath = new KeyPath(derivation);
-                    }
-                    catch
-                    {
-                        return (null, "derivation keypath was not valid");
-                    }
-                }
 
 
-                if (jobj.ContainsKey("ColdCardFirmwareVersion"))
+        if (jobj.keystore.ColdCardFirmwareVersion is not null)
         {
             result.Source = "ColdCard";
         }
-                else if (jobj.ContainsKey("CoboVaultFirmwareVersion"))
+        else if (jobj.keystore.CoboVaultFirmwareVersion is not null)
         {
             result.Source = "CoboVault";
         }
-                return (result, null);
+        derivationSchemeSettings = result;
-            }
+        return true;
-
-        }
-        catch (FormatException)
-        {
-            return (null, "invalid xpub");
-        }
-        return (null, null);
     }
 }

BTCPayServer/Services/WalletFileParsing/IWalletFileParser.cs

@@ -1,7 +1,8 @@
-#nullable enable
+#nullable enable
+using System.Diagnostics.CodeAnalysis;
 using BTCPayServer;
 namespace BTCPayServer.Services.WalletFileParsing;
 public interface IWalletFileParser
 {
-    (BTCPayServer.DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network, string data);
+    bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings);
 }

BTCPayServer/Services/WalletFileParsing/NBXDerivGenericWalletFileParser.cs

@@ -1,21 +1,14 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using BTCPayServer;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class NBXDerivGenericWalletFileParser : IWalletFileParser
 {
-    public (BTCPayServer.DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
-        string data)
     {
-        try
+        derivationSchemeSettings = BTCPayServer.DerivationSchemeSettings.Parse(data, network);
-        {
+        derivationSchemeSettings.Source = "Generic";
-            var result =  BTCPayServer.DerivationSchemeSettings.Parse(data, network);
+        return true;
-            result.Source = "Generic";
-            return (result, null);
-        }
-        catch (Exception)
-        {
-            return (null, null);
-        }
     }
 }

BTCPayServer/Services/WalletFileParsing/OutputDescriptorJsonWalletFileParser.cs

@@ -1,36 +1,34 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using BTCPayServer;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class OutputDescriptorJsonWalletFileParser : IWalletFileParser
 {
     private readonly OutputDescriptorWalletFileParser _outputDescriptorOnChainWalletParser;
 
+    class OutputDescriptorJsonWalletFileFormat
+    {
+        public string? Descriptor { get; set; }
+        public string? Source { get; set; }
+    }
     public OutputDescriptorJsonWalletFileParser(OutputDescriptorWalletFileParser outputDescriptorOnChainWalletParser)
     {
         _outputDescriptorOnChainWalletParser = outputDescriptorOnChainWalletParser;
     }
-    public (DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
-        string data)
     {
-        try
+        derivationSchemeSettings = null;
-        {
+        var jobj = JsonConvert.DeserializeObject<OutputDescriptorJsonWalletFileFormat>(data);
-            var jobj = JObject.Parse(data);
+        if (jobj?.Descriptor is null)
-            if (!jobj.TryGetValue("Descriptor", StringComparison.InvariantCultureIgnoreCase, out var descriptorToken) ||
+            return false;
-                descriptorToken?.Value<string>() is not string desc)
-                return (null, null);
 
-                
+        if (!_outputDescriptorOnChainWalletParser.TryParse(network, jobj.Descriptor, out derivationSchemeSettings))
-            var result =  _outputDescriptorOnChainWalletParser.TryParse(network, desc);
+            return false;
-            if (result.DerivationSchemeSettings is not null && jobj.TryGetValue("Source", StringComparison.InvariantCultureIgnoreCase, out var sourceToken))
+        if (jobj.Source is not null)
-                result.DerivationSchemeSettings.Source = sourceToken.Value<string>();
+            derivationSchemeSettings.Source = jobj.Source;
-            return result;
+        return true;
-                
-        }
-        catch (Exception)
-        {
-            return (null, null);
-        }
     }
 }

BTCPayServer/Services/WalletFileParsing/OutputDescriptorWalletFileParser.cs

@@ -1,24 +1,20 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using BTCPayServer;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class OutputDescriptorWalletFileParser : IWalletFileParser
 {
-    public (BTCPayServer.DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
-        string data)
-    {
-        try
     {
+        derivationSchemeSettings = null;
         var maybeOutputDesc = !data.Trim().StartsWith("{", StringComparison.OrdinalIgnoreCase);
         if (!maybeOutputDesc)
-                return (null, null);
+            return false;
-
         var derivationSchemeParser = network.GetDerivationSchemeParser();
-
         var descriptor = derivationSchemeParser.ParseOutputDescriptor(data);
-
+        derivationSchemeSettings = new DerivationSchemeSettings()
-            var derivationSchemeSettings = new DerivationSchemeSettings()
         {
             Network = network,
             Source = "OutputDescriptor",
@@ -32,11 +28,6 @@ public class OutputDescriptorWalletFileParser : IWalletFileParser
                     descriptor.Item1.GetExtPubKeys().ElementAt(i).GetWif(derivationSchemeParser.Network)
             }).ToArray()
         };
-            return (derivationSchemeSettings, null);
+        return true;
-        }
-        catch (Exception exception)
-        {
-            return (null, exception.Message);
-        }
     }
 }

BTCPayServer/Services/WalletFileParsing/SpecterWalletFileParser.cs

@@ -1,41 +1,35 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using BTCPayServer;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class SpecterWalletFileParser : IWalletFileParser
 {
     private readonly OutputDescriptorWalletFileParser _outputDescriptorOnChainWalletParser;
 
+    class SpecterFormat
+    {
+        public string? descriptor { get; set; }
+        public int? blockheight { get; set; }
+        public string? label { get; set; }
+    }
     public SpecterWalletFileParser(OutputDescriptorWalletFileParser outputDescriptorOnChainWalletParser)
     {
         _outputDescriptorOnChainWalletParser = outputDescriptorOnChainWalletParser;
     }
-    public (DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
-        string data)
     {
-        try
+        derivationSchemeSettings = null;
-        {
+        var jobj = JsonConvert.DeserializeObject<SpecterFormat>(data);
-            var jobj = JObject.Parse(data);
+        if (jobj?.descriptor is null || jobj.blockheight is null)
-            if (!jobj.TryGetValue("descriptor", StringComparison.InvariantCultureIgnoreCase, out var descriptorObj)
+            return false;
-                || !jobj.ContainsKey("blockheight")
+        if (!_outputDescriptorOnChainWalletParser.TryParse(network, jobj.descriptor, out derivationSchemeSettings))
-                || descriptorObj?.Value<string>() is not string desc)
+            return false;
-                return (null, null);
+        derivationSchemeSettings.Source = "Specter";
-
+        if (jobj.label is not null)
-                
+        derivationSchemeSettings.Label = jobj.label;
-            var result =  _outputDescriptorOnChainWalletParser.TryParse(network, desc);
+        return true;
-            if (result.DerivationSchemeSettings is not null)
-                result.DerivationSchemeSettings.Source = "Specter";
-
-            if (result.DerivationSchemeSettings is not null && jobj.TryGetValue("label",
-                    StringComparison.InvariantCultureIgnoreCase, out var label) && label?.Value<string>() is string labelValue)
-                result.DerivationSchemeSettings.Label = labelValue;
-            return result;
-                
-        }
-        catch (Exception)
-        {
-            return (null, null);
-        }
     }
 }

BTCPayServer/Services/WalletFileParsing/WasabiWalletFileParser.cs

@@ -1,105 +1,70 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using BTCPayServer;
 using NBitcoin;
 using NBitcoin.DataEncoders;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 namespace BTCPayServer.Services.WalletFileParsing;
 public class WasabiWalletFileParser : IWalletFileParser
 {
-
+    class WasabiFormat
-    public (DerivationSchemeSettings? DerivationSchemeSettings, string? Error) TryParse(BTCPayNetwork network,
-        string data)
     {
-        try
+        public string? ExtPubKey { get; set; }
+        public string? MasterFingerprint { get; set; }
+        public string? AccountKeyPath { get; set; }
+        public string? ColdCardFirmwareVersion { get; set; }
+        public string? CoboVaultFirmwareVersion { get; set; }
+        public string? DerivationPath { get; set; }
+        public string? Source { get; set; }
+    }
+    public bool TryParse(BTCPayNetwork network, string data, [MaybeNullWhen(false)] out DerivationSchemeSettings derivationSchemeSettings)
     {
-            var jobj = JObject.Parse(data);
+        derivationSchemeSettings = null;
-            if (jobj["ExtPubKey"]?.Value<string>() is not string extPubKey)
+        var jobj = JsonConvert.DeserializeObject<WasabiFormat>(data);
-                return (null, null);
-
         var derivationSchemeParser = network.GetDerivationSchemeParser();
         var result = new DerivationSchemeSettings()
         {
             Network = network
         };
 
-            if (!derivationSchemeParser.TryParseXpub(extPubKey, ref result, out var error))
+        if (jobj is null || !derivationSchemeParser.TryParseXpub(jobj.ExtPubKey, ref result, out var error))
-            {
+            return false;
-                return (null, error);
-            }
 
-            if (jobj["MasterFingerprint"]?.ToString()?.Trim() is string mfpString)
+        if (jobj.MasterFingerprint is not null)
-            {
-                try
         {
             // https://github.com/zkSNACKs/WalletWasabi/pull/1663#issuecomment-508073066
-                    if (uint.TryParse(mfpString, out var fingerprint))
+            if (uint.TryParse(jobj.MasterFingerprint, out var fingerprint))
             {
                 result.AccountKeySettings[0].RootFingerprint = new HDFingerprint(fingerprint);
             }
             else
             {
-                        var bytes = Encoders.Hex.DecodeData(mfpString);
+                var bytes = Encoders.Hex.DecodeData(jobj.MasterFingerprint);
-                        var shouldReverseMfp = jobj["ColdCardFirmwareVersion"]?.Value<string>() == "2.1.0";
+                var shouldReverseMfp = jobj.ColdCardFirmwareVersion == "2.1.0";
                 if (shouldReverseMfp) // Bug in previous version of coldcard
                     bytes = bytes.Reverse().ToArray();
                 result.AccountKeySettings[0].RootFingerprint = new HDFingerprint(bytes);
             }
         }
 
-                catch
+        if (jobj.AccountKeyPath is not null)
-                {
+            result.AccountKeySettings[0].AccountKeyPath = new KeyPath(jobj.AccountKeyPath);
-                    return (null, "MasterFingerprint was not valid");
-                }
-            }
 
-            if (jobj["AccountKeyPath"]?.Value<string>() is string accountKeyPath)
+        if (jobj.ColdCardFirmwareVersion is not null)
-            {
-                try
-                {
-                    result.AccountKeySettings[0].AccountKeyPath = new KeyPath(accountKeyPath);
-                }
-                catch
-                {
-                    return (null, "AccountKeyPath was not valid");
-                }
-            }
-
-            if (jobj["DerivationPath"]?.Value<string>()?.ToLowerInvariant() is string derivationPath)
-            {
-                try
-                {
-                    result.AccountKeySettings[0].AccountKeyPath = new KeyPath(derivationPath);
-                }
-                catch
-                {
-                    return (null, "Derivation path was not valid");
-                }
-            }
-
-            if (jobj.ContainsKey("ColdCardFirmwareVersion"))
         {
             result.Source = "ColdCard";
         }
-            else if (jobj.ContainsKey("CoboVaultFirmwareVersion"))
+        else if (jobj.CoboVaultFirmwareVersion is not null)
         {
             result.Source = "CoboVault";
         }
-            else if (jobj.TryGetValue("Source", StringComparison.InvariantCultureIgnoreCase, out var source))
-            {
-                result.Source = source.Value<string>();
-            }
         else
-                result.Source = "WasabiFile";
+            result.Source = jobj.Source ?? "WasabiFile";
-
-
-            return (result, null);
-        }
-        catch (Exception)
-        {
-            return (null, null);
-        }
 
+        derivationSchemeSettings = result;
+        return true;
     }
 }