From 39a4be56416861fa38d1521e10ffac1b2f37a68b Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 17 Jun 2021 14:11:01 +0900 Subject: [PATCH 1/5] Remove HintAddress --- .../AltcoinTests/AltcoinTests.cs | 22 -------- .../Controllers/StoresController.Onchain.cs | 40 +-------------- BTCPayServer/Controllers/StoresController.cs | 3 +- BTCPayServer/DerivationSchemeParser.cs | 50 +------------------ .../DerivationSchemeViewModel.cs | 2 - .../ImportWallet/ConfirmAddresses.cshtml | 15 ------ 6 files changed, 4 insertions(+), 128 deletions(-) diff --git a/BTCPayServer.Tests/AltcoinTests/AltcoinTests.cs b/BTCPayServer.Tests/AltcoinTests/AltcoinTests.cs index 8ec19f555..da38b633f 100644 --- a/BTCPayServer.Tests/AltcoinTests/AltcoinTests.cs +++ b/BTCPayServer.Tests/AltcoinTests/AltcoinTests.cs @@ -954,28 +954,6 @@ normal: result = testnetParser.Parse(tpub); Assert.Equal(tpub, result.ToString()); - testnetParser.HintScriptPubKey = BitcoinAddress - .Create("tb1q4s33amqm8l7a07zdxcunqnn3gcsjcfz3xc573l", testnetParser.Network).ScriptPubKey; - result = testnetParser.Parse(tpub); - Assert.Equal(tpub, result.ToString()); - - testnetParser.HintScriptPubKey = BitcoinAddress - .Create("2N2humNio3YTApSfY6VztQ9hQwDnhDvaqFQ", testnetParser.Network).ScriptPubKey; - result = testnetParser.Parse(tpub); - Assert.Equal($"{tpub}-[p2sh]", result.ToString()); - - testnetParser.HintScriptPubKey = BitcoinAddress - .Create("mwD8bHS65cdgUf6rZUUSoVhi3wNQFu1Nfi", testnetParser.Network).ScriptPubKey; - result = testnetParser.Parse(tpub); - Assert.Equal($"{tpub}-[legacy]", result.ToString()); - - testnetParser.HintScriptPubKey = BitcoinAddress - .Create("2N2humNio3YTApSfY6VztQ9hQwDnhDvaqFQ", testnetParser.Network).ScriptPubKey; - result = testnetParser.Parse($"{tpub}-[legacy]"); - Assert.Equal($"{tpub}-[p2sh]", result.ToString()); - - result = testnetParser.Parse(tpub); - Assert.Equal($"{tpub}-[p2sh]", result.ToString()); var regtestParser = new DerivationSchemeParser(regtestNetworkProvider.GetNetwork("BTC")); var parsed = diff --git a/BTCPayServer/Controllers/StoresController.Onchain.cs b/BTCPayServer/Controllers/StoresController.Onchain.cs index 4cfc9e1ae..bd7ee0326 100644 --- a/BTCPayServer/Controllers/StoresController.Onchain.cs +++ b/BTCPayServer/Controllers/StoresController.Onchain.cs @@ -114,7 +114,7 @@ namespace BTCPayServer.Controllers { try { - var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, null, network); + var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, network); if (newStrategy.AccountDerivation != strategy?.AccountDerivation) { var accountKey = string.IsNullOrEmpty(vm.AccountKey) @@ -160,8 +160,6 @@ namespace BTCPayServer.Controllers var willBeExcluded = !vm.Enabled; 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 clicking on continue after changing the config (!vm.Confirmation && configChanged); @@ -191,42 +189,6 @@ namespace BTCPayServer.Controllers // This is success case when derivation scheme is added to the store return RedirectToAction(nameof(UpdateStore), new {storeId = vm.StoreId}); } - - if (!string.IsNullOrEmpty(vm.HintAddress)) - { - BitcoinAddress address; - try - { - address = BitcoinAddress.Create(vm.HintAddress, network.NBitcoinNetwork); - } - catch - { - ModelState.AddModelError(nameof(vm.HintAddress), "Invalid hint address"); - return ConfirmAddresses(vm, strategy); - } - - try - { - var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, address.ScriptPubKey, network); - if (newStrategy.AccountDerivation != strategy.AccountDerivation) - { - strategy.AccountDerivation = newStrategy.AccountDerivation; - strategy.AccountOriginal = null; - } - } - catch - { - ModelState.AddModelError(nameof(vm.HintAddress), "Impossible to find a match with this address. Are you sure the wallet and address provided are correct and from the same source?"); - return ConfirmAddresses(vm, strategy); - } - - vm.HintAddress = ""; - TempData[WellKnownTempData.SuccessMessage] = - "Address successfully found, please verify that the rest is correct and click on \"Confirm\""; - ModelState.Remove(nameof(vm.HintAddress)); - ModelState.Remove(nameof(vm.DerivationScheme)); - } - return ConfirmAddresses(vm, strategy); } diff --git a/BTCPayServer/Controllers/StoresController.cs b/BTCPayServer/Controllers/StoresController.cs index d95cfd37d..1c270f7b6 100644 --- a/BTCPayServer/Controllers/StoresController.cs +++ b/BTCPayServer/Controllers/StoresController.cs @@ -689,10 +689,9 @@ namespace BTCPayServer.Controllers } - private DerivationSchemeSettings ParseDerivationStrategy(string derivationScheme, Script hint, BTCPayNetwork network) + private DerivationSchemeSettings ParseDerivationStrategy(string derivationScheme, BTCPayNetwork network) { var parser = new DerivationSchemeParser(network); - parser.HintScriptPubKey = hint; try { var derivationSchemeSettings = new DerivationSchemeSettings(); diff --git a/BTCPayServer/DerivationSchemeParser.cs b/BTCPayServer/DerivationSchemeParser.cs index bd79c9634..7e8d9845f 100644 --- a/BTCPayServer/DerivationSchemeParser.cs +++ b/BTCPayServer/DerivationSchemeParser.cs @@ -13,8 +13,6 @@ namespace BTCPayServer public Network Network => BtcPayNetwork.NBitcoinNetwork; - public Script HintScriptPubKey { get; set; } - public DerivationSchemeParser(BTCPayNetwork expectedNetwork) { if (expectedNetwork == null) @@ -131,19 +129,6 @@ namespace BTCPayServer HashSet hintedLabels = new HashSet(); - var hintDestination = HintScriptPubKey?.GetDestination(); - if (hintDestination != null) - { - if (hintDestination is KeyId) - { - hintedLabels.Add("legacy"); - } - if (hintDestination is ScriptId) - { - hintedLabels.Add("p2sh"); - } - } - if (!Network.Consensus.SupportSegwit) { hintedLabels.Add("legacy"); @@ -152,8 +137,7 @@ namespace BTCPayServer try { - var result = BtcPayNetwork.NBXplorerNetwork.DerivationStrategyFactory.Parse(str); - return FindMatch(hintedLabels, result); + return BtcPayNetwork.NBXplorerNetwork.DerivationStrategyFactory.Parse(str); } catch { @@ -205,22 +189,13 @@ namespace BTCPayServer catch { continue; } } - if (hintDestination != null) - { - if (hintDestination is WitKeyId) - { - hintedLabels.Remove("legacy"); - hintedLabels.Remove("p2sh"); - } - } - str = string.Join('-', parts.Where(p => !IsLabel(p))); foreach (var label in hintedLabels) { str = $"{str}-[{label}]"; } - return FindMatch(hintedLabels, BtcPayNetwork.NBXplorerNetwork.DerivationStrategyFactory.Parse(str)); + return BtcPayNetwork.NBXplorerNetwork.DerivationStrategyFactory.Parse(str); } public static BitcoinExtPubKey GetBitcoinExtPubKeyByNetwork(Network network, byte[] data) @@ -235,27 +210,6 @@ namespace BTCPayServer } } - private DerivationStrategyBase FindMatch(HashSet hintLabels, DerivationStrategyBase result) - { - var firstKeyPath = new KeyPath("0/0"); - if (HintScriptPubKey == null) - return result; - if (HintScriptPubKey == result.GetDerivation(firstKeyPath).ScriptPubKey) - return result; - - if (result is MultisigDerivationStrategy) - hintLabels.Add("keeporder"); - - var resultNoLabels = result.ToString(); - resultNoLabels = string.Join('-', resultNoLabels.Split('-').Where(p => !IsLabel(p))); - foreach (var labels in ItemCombinations(hintLabels.ToList())) - { - var hinted = BtcPayNetwork.NBXplorerNetwork.DerivationStrategyFactory.Parse(resultNoLabels + '-' + string.Join('-', labels.Select(l => $"[{l}]").ToArray())); - if (HintScriptPubKey == hinted.GetDerivation(firstKeyPath).ScriptPubKey) - return hinted; - } - throw new FormatException("Could not find any match"); - } private static bool IsLabel(string v) { diff --git a/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs b/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs index a88b277ef..1b0408548 100644 --- a/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs +++ b/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs @@ -20,8 +20,6 @@ namespace BTCPayServer.Models.StoreViewModels public string KeyPath { get; set; } [Display(Name = "Root fingerprint")] public string RootFingerprint { get; set; } - [Display(Name = "Hint address")] - public string HintAddress { get; set; } public bool Confirmation { get; set; } public bool Enabled { get; set; } = true; diff --git a/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml b/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml index cbdc1a32d..f795e4243 100644 --- a/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml +++ b/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml @@ -83,21 +83,6 @@ -
- -
-
-
- - - -
-
-
-
-
From afd479ac69cb76b0926c314e4fa92d8fa247c8c7 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 17 Jun 2021 14:17:14 +0900 Subject: [PATCH 2/5] Remove the DerivationSchemeViewmodel.Enabled property --- BTCPayServer.Tests/TestAccount.cs | 1 - BTCPayServer/Controllers/StoresController.Onchain.cs | 6 +----- .../Models/StoreViewModels/DerivationSchemeViewModel.cs | 1 - .../Views/Stores/ImportWallet/ConfirmAddresses.cshtml | 1 - 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/BTCPayServer.Tests/TestAccount.cs b/BTCPayServer.Tests/TestAccount.cs index 66771847d..0bd3ad91b 100644 --- a/BTCPayServer.Tests/TestAccount.cs +++ b/BTCPayServer.Tests/TestAccount.cs @@ -191,7 +191,6 @@ namespace BTCPayServer.Tests { StoreId = StoreId, Method = importKeysToNBX ? WalletSetupMethod.HotWallet : WalletSetupMethod.WatchOnly, - Enabled = true, CryptoCode = cryptoCode, Network = SupportedNetwork, RootFingerprint = GenerateWalletResponseV.AccountKeyPath.MasterFingerprint.ToString(), diff --git a/BTCPayServer/Controllers/StoresController.Onchain.cs b/BTCPayServer/Controllers/StoresController.Onchain.cs index bd7ee0326..4403d3b71 100644 --- a/BTCPayServer/Controllers/StoresController.Onchain.cs +++ b/BTCPayServer/Controllers/StoresController.Onchain.cs @@ -157,7 +157,6 @@ namespace BTCPayServer.Controllers var configChanged = oldConfig != vm.Config; PaymentMethodId paymentMethodId = new PaymentMethodId(network.CryptoCode, PaymentTypes.BTCLike); var storeBlob = store.GetStoreBlob(); - var willBeExcluded = !vm.Enabled; var showAddress = // Show addresses if: // - The user is clicking on continue after changing the config @@ -171,7 +170,7 @@ namespace BTCPayServer.Controllers if (strategy != null) await wallet.TrackAsync(strategy.AccountDerivation); store.SetSupportedPaymentMethod(paymentMethodId, strategy); - storeBlob.SetExcluded(paymentMethodId, willBeExcluded); + storeBlob.SetExcluded(paymentMethodId, false); storeBlob.Hints.Wallet = false; store.SetStoreBlob(storeBlob); } @@ -215,7 +214,6 @@ namespace BTCPayServer.Controllers vm.Config = derivation.ToJson(); } - vm.Enabled = !store.GetStoreBlob().IsExcluded(new PaymentMethodId(vm.CryptoCode, PaymentTypes.BTCLike)); vm.CanUseHotWallet = hotWallet; vm.CanUseRPCImport = rpcImport; vm.RootKeyPath = network.GetRootKeyPath(); @@ -259,7 +257,6 @@ namespace BTCPayServer.Controllers Confirmation = string.IsNullOrEmpty(request.ExistingMnemonic), Network = network, RootKeyPath = network.GetRootKeyPath(), - Enabled = !store.GetStoreBlob().IsExcluded(new PaymentMethodId(cryptoCode, PaymentTypes.BTCLike)), Source = "NBXplorer", DerivationSchemeFormat = "BTCPay", CanUseHotWallet = true, @@ -371,7 +368,6 @@ namespace BTCPayServer.Controllers vm.DerivationScheme = derivation.AccountDerivation.ToString(); vm.KeyPath = derivation.GetSigningAccountKeySettings().AccountKeyPath?.ToString(); vm.Config = derivation.ToJson(); - vm.Enabled = !store.GetStoreBlob().IsExcluded(new PaymentMethodId(vm.CryptoCode, PaymentTypes.BTCLike)); vm.IsHotWallet = isHotWallet; return View(vm); diff --git a/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs b/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs index 1b0408548..14d40d918 100644 --- a/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs +++ b/BTCPayServer/Models/StoreViewModels/DerivationSchemeViewModel.cs @@ -21,7 +21,6 @@ namespace BTCPayServer.Models.StoreViewModels [Display(Name = "Root fingerprint")] public string RootFingerprint { get; set; } public bool Confirmation { get; set; } - public bool Enabled { get; set; } = true; public KeyPath RootKeyPath { get; set; } diff --git a/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml b/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml index f795e4243..11577a00c 100644 --- a/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml +++ b/BTCPayServer/Views/Stores/ImportWallet/ConfirmAddresses.cshtml @@ -50,7 +50,6 @@ -
From 956370592f67213e21a776977dffa061a2ab3395 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 17 Jun 2021 15:36:22 +0900 Subject: [PATCH 3/5] Create a dedicated IsHotwalletProperty in the DerivationSchemeSettings --- .../Controllers/StoresController.Onchain.cs | 4 +++- BTCPayServer/DerivationSchemeSettings.cs | 4 ++-- BTCPayServer/Hosting/MigrationStartupTask.cs | 20 +++++++++++++++++++ BTCPayServer/Services/MigrationSettings.cs | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/BTCPayServer/Controllers/StoresController.Onchain.cs b/BTCPayServer/Controllers/StoresController.Onchain.cs index 4403d3b71..7e64119a4 100644 --- a/BTCPayServer/Controllers/StoresController.Onchain.cs +++ b/BTCPayServer/Controllers/StoresController.Onchain.cs @@ -14,6 +14,7 @@ using BTCPayServer.Services; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using NBitcoin; using NBXplorer; using NBXplorer.DerivationStrategy; @@ -257,7 +258,8 @@ namespace BTCPayServer.Controllers Confirmation = string.IsNullOrEmpty(request.ExistingMnemonic), Network = network, RootKeyPath = network.GetRootKeyPath(), - Source = "NBXplorer", + Source = isImport ? "SeedImported" : "NBXplorerGenerated", + IsHotWallet = isImport ? request.SavePrivateKeys : method == WalletSetupMethod.HotWallet, DerivationSchemeFormat = "BTCPay", CanUseHotWallet = true, CanUseRPCImport = rpcImport diff --git a/BTCPayServer/DerivationSchemeSettings.cs b/BTCPayServer/DerivationSchemeSettings.cs index ba496826f..8dad2ef22 100644 --- a/BTCPayServer/DerivationSchemeSettings.cs +++ b/BTCPayServer/DerivationSchemeSettings.cs @@ -276,8 +276,8 @@ namespace BTCPayServer [JsonIgnore] public BTCPayNetwork Network { get; set; } public string Source { get; set; } - [JsonIgnore] - public bool IsHotWallet => Source == "NBXplorer"; + + public bool IsHotWallet { get; set; } [Obsolete("Use GetSigningAccountKeySettings().AccountKeyPath instead")] [JsonProperty(DefaultValueHandling = DefaultValueHandling.Ignore)] diff --git a/BTCPayServer/Hosting/MigrationStartupTask.cs b/BTCPayServer/Hosting/MigrationStartupTask.cs index 7294403bd..de84b6448 100644 --- a/BTCPayServer/Hosting/MigrationStartupTask.cs +++ b/BTCPayServer/Hosting/MigrationStartupTask.cs @@ -128,6 +128,12 @@ namespace BTCPayServer.Hosting settings.MigrateU2FToFIDO2 = true; await _Settings.UpdateSetting(settings); } + if (!settings.MigrateHotwalletProperty) + { + await MigrateHotwalletProperty(); + settings.MigrateHotwalletProperty = true; + await _Settings.UpdateSetting(settings); + } } catch (Exception ex) { @@ -136,6 +142,20 @@ namespace BTCPayServer.Hosting } } + private async Task MigrateHotwalletProperty() + { + await using var ctx = _DBContextFactory.CreateContext(); + foreach (var store in await ctx.Stores.AsQueryable().ToArrayAsync()) + { + foreach (var paymentMethod in store.GetSupportedPaymentMethods(_NetworkProvider).OfType()) + { + paymentMethod.IsHotWallet = paymentMethod.Source == "NBXplorer"; + paymentMethod.Source = "NBXplorerGenerated"; + } + } + await ctx.SaveChangesAsync(); + } + private async Task MigrateU2FToFIDO2() { await using var ctx = _DBContextFactory.CreateContext(); diff --git a/BTCPayServer/Services/MigrationSettings.cs b/BTCPayServer/Services/MigrationSettings.cs index 56d63ff68..c8bec9c30 100644 --- a/BTCPayServer/Services/MigrationSettings.cs +++ b/BTCPayServer/Services/MigrationSettings.cs @@ -2,6 +2,7 @@ namespace BTCPayServer.Services { public class MigrationSettings { + public bool MigrateHotwalletProperty { get; set; } public bool MigrateU2FToFIDO2{ get; set; } public bool UnreachableStoreCheck { get; set; } public bool DeprecatedLightningConnectionStringCheck { get; set; } From 8a1d5bbc57176377a3e76fe3f55f6942bc3fe457 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 17 Jun 2021 16:02:47 +0900 Subject: [PATCH 4/5] Remove outdated code in UpdateWallet --- BTCPayServer.Tests/TestAccount.cs | 27 ++-- .../Controllers/StoresController.Onchain.cs | 129 +++++++++--------- BTCPayServer/Controllers/WalletsController.cs | 4 +- 3 files changed, 76 insertions(+), 84 deletions(-) diff --git a/BTCPayServer.Tests/TestAccount.cs b/BTCPayServer.Tests/TestAccount.cs index 0bd3ad91b..d96376c2f 100644 --- a/BTCPayServer.Tests/TestAccount.cs +++ b/BTCPayServer.Tests/TestAccount.cs @@ -179,29 +179,18 @@ namespace BTCPayServer.Tests if (StoreId is null) await CreateStoreAsync(); SupportedNetwork = parent.NetworkProvider.GetNetwork(cryptoCode); - var store = parent.PayTester.GetController(UserId, StoreId); - GenerateWalletResponseV = await parent.ExplorerClient.GenerateWalletAsync(new GenerateWalletRequest() + var store = parent.PayTester.GetController(UserId, StoreId, true); + + var generateRequest = new GenerateWalletRequest() { ScriptPubKeyType = segwit, SavePrivateKeys = importKeysToNBX, ImportKeysToRPC = importsKeysToBitcoinCore - }); - await store.UpdateWallet( - new WalletSetupViewModel - { - StoreId = StoreId, - Method = importKeysToNBX ? WalletSetupMethod.HotWallet : WalletSetupMethod.WatchOnly, - CryptoCode = cryptoCode, - Network = SupportedNetwork, - RootFingerprint = GenerateWalletResponseV.AccountKeyPath.MasterFingerprint.ToString(), - RootKeyPath = SupportedNetwork.GetRootKeyPath(), - Source = "NBXplorer", - AccountKey = GenerateWalletResponseV.AccountHDKey.Neuter().ToWif(), - DerivationSchemeFormat = "BTCPay", - KeyPath = GenerateWalletResponseV.AccountKeyPath.KeyPath.ToString(), - DerivationScheme = DerivationScheme.ToString(), - Confirmation = true - }); + }; + + await store.GenerateWallet(StoreId, cryptoCode, WalletSetupMethod.HotWallet, generateRequest); + Assert.NotNull(store.GenerateWalletResponseV); + GenerateWalletResponseV = store.GenerateWalletResponseV; return new WalletId(StoreId, cryptoCode); } diff --git a/BTCPayServer/Controllers/StoresController.Onchain.cs b/BTCPayServer/Controllers/StoresController.Onchain.cs index 7e64119a4..a4ff4a355 100644 --- a/BTCPayServer/Controllers/StoresController.Onchain.cs +++ b/BTCPayServer/Controllers/StoresController.Onchain.cs @@ -11,6 +11,7 @@ using BTCPayServer.Models; using BTCPayServer.Models.StoreViewModels; using BTCPayServer.Payments; using BTCPayServer.Services; +using ExchangeSharp; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -86,15 +87,6 @@ namespace BTCPayServer.Controllers return NotFound(); } - if (!string.IsNullOrEmpty(vm.Config)) - { - if (!DerivationSchemeSettings.TryParseFromJson(vm.Config, network, out strategy)) - { - ModelState.AddModelError(nameof(vm.Config), "Config file was not in the correct format"); - return View(vm.ViewName, vm); - } - } - if (vm.WalletFile != null) { if (!DerivationSchemeSettings.TryParseFromWalletFile(await ReadAllText(vm.WalletFile), network, out strategy)) @@ -115,31 +107,25 @@ namespace BTCPayServer.Controllers { try { - var newStrategy = ParseDerivationStrategy(vm.DerivationScheme, network); - if (newStrategy.AccountDerivation != strategy?.AccountDerivation) + strategy = ParseDerivationStrategy(vm.DerivationScheme, network); + strategy.Source = "ManualDerivationScheme"; + if (!string.IsNullOrEmpty(vm.AccountKey)) { - var accountKey = string.IsNullOrEmpty(vm.AccountKey) - ? null - : new BitcoinExtPubKey(vm.AccountKey, network.NBitcoinNetwork); - if (accountKey != null) + var accountKey = new BitcoinExtPubKey(vm.AccountKey, network.NBitcoinNetwork); + var accountSettings = + strategy.AccountKeySettings.FirstOrDefault(a => a.AccountKey == accountKey); + if (accountSettings != null) { - var accountSettings = - newStrategy.AccountKeySettings.FirstOrDefault(a => a.AccountKey == accountKey); - if (accountSettings != null) - { - accountSettings.AccountKeyPath = - vm.KeyPath == null ? null : KeyPath.Parse(vm.KeyPath); - accountSettings.RootFingerprint = string.IsNullOrEmpty(vm.RootFingerprint) - ? (HDFingerprint?)null - : new HDFingerprint( - NBitcoin.DataEncoders.Encoders.Hex.DecodeData(vm.RootFingerprint)); - } + accountSettings.AccountKeyPath = + vm.KeyPath == null ? null : KeyPath.Parse(vm.KeyPath); + accountSettings.RootFingerprint = string.IsNullOrEmpty(vm.RootFingerprint) + ? (HDFingerprint?)null + : new HDFingerprint( + NBitcoin.DataEncoders.Encoders.Hex.DecodeData(vm.RootFingerprint)); } - - strategy = newStrategy; - strategy.Source = vm.Source; - vm.DerivationScheme = strategy.AccountDerivation.ToString(); } + vm.DerivationScheme = strategy.AccountDerivation.ToString(); + ModelState.Remove(nameof(vm.DerivationScheme)); } catch { @@ -147,29 +133,31 @@ namespace BTCPayServer.Controllers return View(vm.ViewName, vm); } } - else + else if (!string.IsNullOrEmpty(vm.Config)) + { + if (!DerivationSchemeSettings.TryParseFromJson(vm.Config, network, out strategy)) + { + ModelState.AddModelError(nameof(vm.Config), "Config file was not in the correct format"); + return View(vm.ViewName, vm); + } + } + + if (strategy is null) { ModelState.AddModelError(nameof(vm.DerivationScheme), "Please provide your extended public key"); return View(vm.ViewName, vm); } - var oldConfig = vm.Config; - vm.Config = strategy?.ToJson(); - var configChanged = oldConfig != vm.Config; + vm.Config = strategy.ToJson(); + ModelState.Remove(nameof(vm.Config)); + PaymentMethodId paymentMethodId = new PaymentMethodId(network.CryptoCode, PaymentTypes.BTCLike); var storeBlob = store.GetStoreBlob(); - - var showAddress = // Show addresses if: - // - The user is clicking on continue after changing the config - (!vm.Confirmation && configChanged); - - showAddress = showAddress && strategy != null; - if (!showAddress) + if (vm.Confirmation) { try { - if (strategy != null) - await wallet.TrackAsync(strategy.AccountDerivation); + await wallet.TrackAsync(strategy.AccountDerivation); store.SetSupportedPaymentMethod(paymentMethodId, strategy); storeBlob.SetExcluded(paymentMethodId, false); storeBlob.Hints.Wallet = false; @@ -182,12 +170,12 @@ namespace BTCPayServer.Controllers } await _Repo.UpdateStore(store); - _EventAggregator.Publish(new WalletChangedEvent {WalletId = new WalletId(vm.StoreId, vm.CryptoCode)}); + _EventAggregator.Publish(new WalletChangedEvent { WalletId = new WalletId(vm.StoreId, vm.CryptoCode) }); TempData[WellKnownTempData.SuccessMessage] = $"Derivation settings for {network.CryptoCode} have been updated."; // This is success case when derivation scheme is added to the store - return RedirectToAction(nameof(UpdateStore), new {storeId = vm.StoreId}); + return RedirectToAction(nameof(UpdateStore), new { storeId = vm.StoreId }); } return ConfirmAddresses(vm, strategy); } @@ -208,13 +196,6 @@ namespace BTCPayServer.Controllers return NotFound(); } - var derivation = GetExistingDerivationStrategy(vm.CryptoCode, store); - if (derivation != null) - { - vm.DerivationScheme = derivation.AccountDerivation.ToString(); - vm.Config = derivation.ToJson(); - } - vm.CanUseHotWallet = hotWallet; vm.CanUseRPCImport = rpcImport; vm.RootKeyPath = network.GetRootKeyPath(); @@ -231,7 +212,7 @@ namespace BTCPayServer.Controllers return View(vm.ViewName, vm); } - + internal GenerateWalletResponse GenerateWalletResponseV; [HttpPost("{storeId}/onchain/{cryptoCode}/generate/{method}")] public async Task GenerateWallet(string storeId, string cryptoCode, WalletSetupMethod method, GenerateWalletRequest request) { @@ -249,6 +230,7 @@ namespace BTCPayServer.Controllers var client = _ExplorerProvider.GetExplorerClient(cryptoCode); var isImport = method == WalletSetupMethod.Seed; + var vm = new WalletSetupViewModel { StoreId = storeId, @@ -290,11 +272,26 @@ namespace BTCPayServer.Controllers return View(vm.ViewName, vm); } + var derivationSchemeSettings = new DerivationSchemeSettings(response.DerivationScheme, network); + if (method == WalletSetupMethod.Seed) + { + derivationSchemeSettings.Source = "ImportedSeed"; + derivationSchemeSettings.IsHotWallet = request.SavePrivateKeys; + } + else + { + derivationSchemeSettings.Source = "NBXplorerGenerated"; + derivationSchemeSettings.IsHotWallet = method == WalletSetupMethod.HotWallet; + } + + var accountSettings = derivationSchemeSettings.GetSigningAccountKeySettings(); + accountSettings.AccountKeyPath = response.AccountKeyPath.KeyPath; + accountSettings.RootFingerprint = response.AccountKeyPath.MasterFingerprint; + derivationSchemeSettings.AccountOriginal = response.DerivationScheme.ToString(); + // Set wallet properties from generate response - vm.RootFingerprint = response.AccountKeyPath.MasterFingerprint.ToString(); - vm.DerivationScheme = response.DerivationScheme.ToString(); - vm.AccountKey = response.AccountHDKey.Neuter().ToWif(); - vm.KeyPath = response.AccountKeyPath.KeyPath.ToString(); + vm.Config = derivationSchemeSettings.ToJson(); + var result = await UpdateWallet(vm); @@ -314,8 +311,12 @@ namespace BTCPayServer.Controllers Mnemonic = response.Mnemonic, Passphrase = response.Passphrase, IsStored = request.SavePrivateKeys, - ReturnUrl = Url.Action(nameof(GenerateWalletConfirm), new {storeId, cryptoCode}) + ReturnUrl = Url.Action(nameof(GenerateWalletConfirm), new { storeId, cryptoCode }) }; + if (this._BTCPayEnv.IsDeveloping) + { + GenerateWalletResponseV = response; + } return this.RedirectToRecoverySeedBackup(seedVm); } @@ -340,7 +341,7 @@ namespace BTCPayServer.Controllers TempData[WellKnownTempData.SuccessMessage] = $"Derivation settings for {network.CryptoCode} have been updated."; - return RedirectToAction(nameof(UpdateStore), new {storeId}); + return RedirectToAction(nameof(UpdateStore), new { storeId }); } [HttpGet("{storeId}/onchain/{cryptoCode}/modify")] @@ -420,7 +421,7 @@ namespace BTCPayServer.Controllers return NotFound(); } - return RedirectToAction(nameof(SetupWallet), new {storeId, cryptoCode}); + return RedirectToAction(nameof(SetupWallet), new { storeId, cryptoCode }); } [HttpGet("{storeId}/onchain/{cryptoCode}/delete")] @@ -479,12 +480,12 @@ namespace BTCPayServer.Controllers storeBlob.SetExcluded(paymentMethodId, !enabled); store.SetStoreBlob(storeBlob); await _Repo.UpdateStore(store); - _EventAggregator.Publish(new WalletChangedEvent {WalletId = new WalletId(storeId, cryptoCode)}); + _EventAggregator.Publish(new WalletChangedEvent { WalletId = new WalletId(storeId, cryptoCode) }); TempData[WellKnownTempData.SuccessMessage] = $"{network.CryptoCode} on-chain payments are now {(enabled ? "enabled" : "disabled")} for this store."; - return RedirectToAction(nameof(UpdateStore), new {storeId}); + return RedirectToAction(nameof(UpdateStore), new { storeId }); } [HttpPost("{storeId}/onchain/{cryptoCode}/delete")] @@ -506,12 +507,12 @@ namespace BTCPayServer.Controllers store.SetSupportedPaymentMethod(paymentMethodId, null); await _Repo.UpdateStore(store); - _EventAggregator.Publish(new WalletChangedEvent {WalletId = new WalletId(storeId, cryptoCode)}); + _EventAggregator.Publish(new WalletChangedEvent { WalletId = new WalletId(storeId, cryptoCode) }); TempData[WellKnownTempData.SuccessMessage] = $"On-Chain payment for {network.CryptoCode} has been removed."; - return RedirectToAction(nameof(UpdateStore), new {storeId}); + return RedirectToAction(nameof(UpdateStore), new { storeId }); } private IActionResult ConfirmAddresses(WalletSetupViewModel vm, DerivationSchemeSettings strategy) diff --git a/BTCPayServer/Controllers/WalletsController.cs b/BTCPayServer/Controllers/WalletsController.cs index bf21c9045..2d9de3fb4 100644 --- a/BTCPayServer/Controllers/WalletsController.cs +++ b/BTCPayServer/Controllers/WalletsController.cs @@ -1089,7 +1089,9 @@ namespace BTCPayServer.Controllers DerivationScheme = derivationSchemeSettings.AccountDerivation.ToString(), DerivationSchemeInput = derivationSchemeSettings.AccountOriginal, SelectedSigningKey = derivationSchemeSettings.SigningKey.ToString(), - NBXSeedAvailable = await CanUseHotWallet() && !string.IsNullOrEmpty(await ExplorerClientProvider.GetExplorerClient(walletId.CryptoCode) + NBXSeedAvailable = derivationSchemeSettings.IsHotWallet && + await CanUseHotWallet() && + !string.IsNullOrEmpty(await ExplorerClientProvider.GetExplorerClient(walletId.CryptoCode) .GetMetadataAsync(GetDerivationSchemeSettings(walletId).AccountDerivation, WellknownMetadataKeys.MasterHDKey)) }; From 70f56d5920ad24499d8a0d59fb9319fdea483ffc Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 17 Jun 2021 18:27:17 +0900 Subject: [PATCH 5/5] Encrypt WalletSetupViewModel.Config --- BTCPayServer.Tests/TestAccount.cs | 4 ++-- .../Controllers/StoresController.Onchain.cs | 22 ++++++++++++++----- BTCPayServer/Controllers/StoresController.cs | 6 ++++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/BTCPayServer.Tests/TestAccount.cs b/BTCPayServer.Tests/TestAccount.cs index d96376c2f..095919234 100644 --- a/BTCPayServer.Tests/TestAccount.cs +++ b/BTCPayServer.Tests/TestAccount.cs @@ -189,8 +189,8 @@ namespace BTCPayServer.Tests }; await store.GenerateWallet(StoreId, cryptoCode, WalletSetupMethod.HotWallet, generateRequest); - Assert.NotNull(store.GenerateWalletResponseV); - GenerateWalletResponseV = store.GenerateWalletResponseV; + Assert.NotNull(store.GenerateWalletResponse); + GenerateWalletResponseV = store.GenerateWalletResponse; return new WalletId(StoreId, cryptoCode); } diff --git a/BTCPayServer/Controllers/StoresController.Onchain.cs b/BTCPayServer/Controllers/StoresController.Onchain.cs index a4ff4a355..7b15fb2e7 100644 --- a/BTCPayServer/Controllers/StoresController.Onchain.cs +++ b/BTCPayServer/Controllers/StoresController.Onchain.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Linq; +using System.Text; using System.Threading.Tasks; using BTCPayServer.Abstractions.Extensions; using BTCPayServer.Abstractions.Models; @@ -135,7 +136,7 @@ namespace BTCPayServer.Controllers } else if (!string.IsNullOrEmpty(vm.Config)) { - if (!DerivationSchemeSettings.TryParseFromJson(vm.Config, network, out strategy)) + if (!DerivationSchemeSettings.TryParseFromJson(UnprotectString(vm.Config), network, out strategy)) { ModelState.AddModelError(nameof(vm.Config), "Config file was not in the correct format"); return View(vm.ViewName, vm); @@ -148,7 +149,7 @@ namespace BTCPayServer.Controllers return View(vm.ViewName, vm); } - vm.Config = strategy.ToJson(); + vm.Config = ProtectString(strategy.ToJson()); ModelState.Remove(nameof(vm.Config)); PaymentMethodId paymentMethodId = new PaymentMethodId(network.CryptoCode, PaymentTypes.BTCLike); @@ -180,6 +181,15 @@ namespace BTCPayServer.Controllers return ConfirmAddresses(vm, strategy); } + private string ProtectString(string str) + { + return Convert.ToBase64String(DataProtector.Protect(Encoding.UTF8.GetBytes(str))); + } + private string UnprotectString(string str) + { + return Encoding.UTF8.GetString(DataProtector.Unprotect(Convert.FromBase64String(str))); + } + [HttpGet("{storeId}/onchain/{cryptoCode}/generate/{method?}")] public async Task GenerateWallet(WalletSetupViewModel vm) { @@ -212,7 +222,7 @@ namespace BTCPayServer.Controllers return View(vm.ViewName, vm); } - internal GenerateWalletResponse GenerateWalletResponseV; + internal GenerateWalletResponse GenerateWalletResponse; [HttpPost("{storeId}/onchain/{cryptoCode}/generate/{method}")] public async Task GenerateWallet(string storeId, string cryptoCode, WalletSetupMethod method, GenerateWalletRequest request) { @@ -290,7 +300,7 @@ namespace BTCPayServer.Controllers derivationSchemeSettings.AccountOriginal = response.DerivationScheme.ToString(); // Set wallet properties from generate response - vm.Config = derivationSchemeSettings.ToJson(); + vm.Config = ProtectString(derivationSchemeSettings.ToJson()); var result = await UpdateWallet(vm); @@ -315,7 +325,7 @@ namespace BTCPayServer.Controllers }; if (this._BTCPayEnv.IsDeveloping) { - GenerateWalletResponseV = response; + GenerateWalletResponse = response; } return this.RedirectToRecoverySeedBackup(seedVm); } @@ -370,7 +380,7 @@ namespace BTCPayServer.Controllers vm.RootFingerprint = derivation.GetSigningAccountKeySettings().RootFingerprint.ToString(); vm.DerivationScheme = derivation.AccountDerivation.ToString(); vm.KeyPath = derivation.GetSigningAccountKeySettings().AccountKeyPath?.ToString(); - vm.Config = derivation.ToJson(); + vm.Config = ProtectString(derivation.ToJson()); vm.IsHotWallet = isHotWallet; return View(vm); diff --git a/BTCPayServer/Controllers/StoresController.cs b/BTCPayServer/Controllers/StoresController.cs index 1c270f7b6..4076049c7 100644 --- a/BTCPayServer/Controllers/StoresController.cs +++ b/BTCPayServer/Controllers/StoresController.cs @@ -27,6 +27,7 @@ using BTCPayServer.Services.Stores; using BTCPayServer.Services.Wallets; using BundlerMinifier.TagHelpers; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; @@ -69,7 +70,8 @@ namespace BTCPayServer.Controllers AppService appService, IWebHostEnvironment webHostEnvironment, WebhookNotificationManager webhookNotificationManager, - IOptions lightningNetworkOptions) + IOptions lightningNetworkOptions, + IDataProtectionProvider dataProtector) { _RateFactory = rateFactory; _Repo = repo; @@ -85,6 +87,7 @@ namespace BTCPayServer.Controllers _appService = appService; _webHostEnvironment = webHostEnvironment; _lightningNetworkOptions = lightningNetworkOptions; + DataProtector = dataProtector.CreateProtector("ConfigProtector"); WebhookNotificationManager = webhookNotificationManager; _EventAggregator = eventAggregator; _NetworkProvider = networkProvider; @@ -826,6 +829,7 @@ namespace BTCPayServer.Controllers public string GeneratedPairingCode { get; set; } public WebhookNotificationManager WebhookNotificationManager { get; } + public IDataProtector DataProtector { get; } [HttpGet] [Route("{storeId}/Tokens/Create")]