Allow multi-step settings in custodian (#4838)

* Allow multi-step settings in custodian

* Fix CustodianAccount.Name not saved

* Reuse TradeQuantity for SimulateTrade

* TradeQuantityJsonConverter accepts numerics

* Fix build

BTCPayServer.Abstractions/Custodians/ICustodian.cs

@@ -1,3 +1,4 @@
+#nullable enable
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
@@ -20,6 +21,6 @@ public interface ICustodian
      */
     Task<Dictionary<string, decimal>> GetAssetBalancesAsync(JObject config, CancellationToken cancellationToken);
 
-    public Task<Form.Form> GetConfigForm(CancellationToken cancellationToken = default);
+    public Task<Form.Form> GetConfigForm(JObject config, CancellationToken cancellationToken = default);
 
 }

BTCPayServer.Abstractions/Form/Form.cs

@@ -32,6 +32,8 @@ public class Form
     // Are all the fields valid in the form?
     public bool IsValid()
     {
+        if (TopMessages?.Any(t => t.Type == AlertMessage.AlertMessageType.Danger) is true)
+            return false;
         return Fields.Select(f => f.IsValid()).All(o => o);
     }
 

BTCPayServer.Client/JsonConverters/NumericStringJsonConverter.cs

@@ -30,9 +30,9 @@ namespace BTCPayServer.JsonConverters
                 case JTokenType.Integer:
                 case JTokenType.String:
                     if (objectType == typeof(decimal) || objectType == typeof(decimal?))
-                        return decimal.Parse(token.ToString(), CultureInfo.InvariantCulture);
+                        return decimal.Parse(token.ToString(), NumberStyles.Any, CultureInfo.InvariantCulture);
                     if (objectType == typeof(double) || objectType == typeof(double?))
-                        return double.Parse(token.ToString(), CultureInfo.InvariantCulture);
+                        return double.Parse(token.ToString(), NumberStyles.Any, CultureInfo.InvariantCulture);
                     throw new JsonSerializationException("Unexpected object type: " + objectType);
                 case JTokenType.Null when objectType == typeof(decimal?) || objectType == typeof(double?):
                     return null;

BTCPayServer.Client/JsonConverters/TradeQuantityJsonConverter.cs

@@ -4,6 +4,7 @@ using BTCPayServer.Client.Models;
 using BTCPayServer.Lightning;
 using NBitcoin.JsonConverters;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 
 namespace BTCPayServer.Client.JsonConverters
 {
@@ -11,13 +12,19 @@ namespace BTCPayServer.Client.JsonConverters
     {
         public override TradeQuantity ReadJson(JsonReader reader, Type objectType, TradeQuantity existingValue, bool hasExistingValue, JsonSerializer serializer)
         {
-            if (reader.TokenType == JsonToken.Null)
-                return null;
-            if (reader.TokenType != JsonToken.String)
-                throw new JsonObjectException("Invalid TradeQuantity, expected string. Expected: \"1.50\" or \"50%\"", reader);
-            if (TradeQuantity.TryParse((string)reader.Value, out var q))
-                return q;
-            throw new JsonObjectException("Invalid format for TradeQuantity. Expected: \"1.50\" or \"50%\"", reader);
+            JToken token = JToken.Load(reader);
+            switch (token.Type)
+            {
+                case JTokenType.Float:
+                case JTokenType.Integer:
+                case JTokenType.String:
+                    if (TradeQuantity.TryParse(token.ToString(), out var q))
+                        return q;
+                    break;
+                case JTokenType.Null:
+                    return null;
+            }
+            throw new JsonObjectException("Invalid TradeQuantity, expected string. Expected: \"1.50\" or \"50%\"", reader);
         }
 
         public override void WriteJson(JsonWriter writer, TradeQuantity value, JsonSerializer serializer)

BTCPayServer.Client/Models/TradeRequestData.cs

@@ -1,8 +1,11 @@
+using Newtonsoft.Json;
+
 namespace BTCPayServer.Client.Models;
 
 public class TradeRequestData
 {
     public string FromAsset { set; get; }
     public string ToAsset { set; get; }
-    public string Qty { set; get; }
+    [JsonConverter(typeof(JsonConverters.TradeQuantityJsonConverter))]
+    public TradeQuantity Qty { set; get; }
 }

BTCPayServer.Tests/FastTests.cs

@@ -134,6 +134,33 @@ namespace BTCPayServer.Tests
             }
         }
 
+
+        [Fact]
+        public void CanParseDecimals()
+        {
+            CanParseDecimalsCore("{\"qty\": 1}", 1.0m);
+            CanParseDecimalsCore("{\"qty\": \"1\"}", 1.0m);
+            CanParseDecimalsCore("{\"qty\": 1.0}", 1.0m);
+            CanParseDecimalsCore("{\"qty\": \"1.0\"}", 1.0m);
+            CanParseDecimalsCore("{\"qty\": 6.1e-7}", 6.1e-7m);
+            CanParseDecimalsCore("{\"qty\": \"6.1e-7\"}", 6.1e-7m);
+
+            var data = JsonConvert.DeserializeObject<TradeRequestData>("{\"qty\": \"6.1e-7\", \"fromAsset\":\"Test\"}");
+            Assert.Equal(6.1e-7m, data.Qty.Value);
+            Assert.Equal("Test", data.FromAsset);
+            data = JsonConvert.DeserializeObject<TradeRequestData>("{\"fromAsset\":\"Test\", \"qty\": \"6.1e-7\"}");
+            Assert.Equal(6.1e-7m, data.Qty.Value);
+            Assert.Equal("Test", data.FromAsset);
+        }
+
+        private void CanParseDecimalsCore(string str, decimal expected)
+        {
+            var d = JsonConvert.DeserializeObject<LedgerEntryData>(str);
+            Assert.Equal(expected, d.Qty);
+            var d2 = JsonConvert.DeserializeObject<TradeRequestData>(str);
+            Assert.Equal(new TradeQuantity(expected, TradeQuantity.ValueType.Exact), d2.Qty);
+        }
+
         [Fact]
         public void CanMergeReceiptOptions()
         {

BTCPayServer.Tests/GreenfieldAPITests.cs

@@ -4001,7 +4001,7 @@ clientBasic.PreviewUpdateStoreRateConfiguration(user.StoreId, new StoreRateConfi
 
 
             // Test: Trade, unauth
-            var tradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = MockCustodian.TradeToAsset, Qty = MockCustodian.TradeQtyBought.ToString(CultureInfo.InvariantCulture) };
+            var tradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = MockCustodian.TradeToAsset, Qty = new TradeQuantity(MockCustodian.TradeQtyBought, TradeQuantity.ValueType.Exact)};
             await AssertHttpError(401, async () => await unauthClient.MarketTradeCustodianAccountAsset(storeId, accountId, tradeRequest));
 
             // Test: Trade, auth, but wrong permission
@@ -4028,17 +4028,13 @@ clientBasic.PreviewUpdateStoreRateConfiguration(user.StoreId, new StoreRateConfi
             Assert.Equal(LedgerEntryData.LedgerEntryType.Fee, newTradeResult.LedgerEntries[2].Type);
 
             // Test: GetTradeQuote, SATS
-            var satsTradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = "SATS", Qty = MockCustodian.TradeQtyBought.ToString(CultureInfo.InvariantCulture) };
+            var satsTradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = "SATS", Qty = new TradeQuantity(MockCustodian.TradeQtyBought, TradeQuantity.ValueType.Exact) };
             await AssertApiError(400, "use-asset-synonym", async () => await tradeClient.MarketTradeCustodianAccountAsset(storeId, accountId, satsTradeRequest));
 
             // TODO Test: Trade with percentage qty
 
-            // Test: Trade with wrong decimal format (example: JavaScript scientific format)
-            var wrongQtyTradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = MockCustodian.TradeToAsset, Qty = "6.1e-7" };
-            await AssertApiError(400, "bad-qty-format", async () => await tradeClient.MarketTradeCustodianAccountAsset(storeId, accountId, wrongQtyTradeRequest));
-
             // Test: Trade, wrong assets method
-            var wrongAssetsTradeRequest = new TradeRequestData { FromAsset = "WRONG", ToAsset = MockCustodian.TradeToAsset, Qty = MockCustodian.TradeQtyBought.ToString(CultureInfo.InvariantCulture) };
+            var wrongAssetsTradeRequest = new TradeRequestData { FromAsset = "WRONG", ToAsset = MockCustodian.TradeToAsset, Qty = new TradeQuantity(MockCustodian.TradeQtyBought, TradeQuantity.ValueType.Exact) };
             await AssertHttpError(WrongTradingPairException.HttpCode, async () => await tradeClient.MarketTradeCustodianAccountAsset(storeId, accountId, wrongAssetsTradeRequest));
 
             // Test: wrong account ID
@@ -4048,7 +4044,7 @@ clientBasic.PreviewUpdateStoreRateConfiguration(user.StoreId, new StoreRateConfi
             await AssertHttpError(403, async () => await tradeClient.MarketTradeCustodianAccountAsset("WRONG-STORE-ID", accountId, tradeRequest));
 
             // Test: Trade, correct assets, wrong amount
-            var insufficientFundsTradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = MockCustodian.TradeToAsset, Qty = "0.01" };
+            var insufficientFundsTradeRequest = new TradeRequestData { FromAsset = MockCustodian.TradeFromAsset, ToAsset = MockCustodian.TradeToAsset, Qty = new TradeQuantity(0.01m, TradeQuantity.ValueType.Exact) };
             await AssertApiError(400, "insufficient-funds", async () => await tradeClient.MarketTradeCustodianAccountAsset(storeId, accountId, insufficientFundsTradeRequest));
 
 

BTCPayServer.Tests/MockCustodian/MockCustodian.cs

@@ -56,7 +56,7 @@ public class MockCustodian : ICustodian, ICanDeposit, ICanTrade, ICanWithdraw
         return Task.FromResult(r);
     }
 
-    public Task<Form> GetConfigForm(CancellationToken cancellationToken = default)
+    public Task<Form> GetConfigForm(JObject config, CancellationToken cancellationToken = default)
     {
         return null;
     }

BTCPayServer/Controllers/GreenField/GreenfieldCustodianAccountController.cs

@@ -255,26 +255,15 @@ namespace BTCPayServer.Controllers.Greenfield
 
             if (custodian is ICanTrade tradableCustodian)
             {
-                bool isPercentage = request.Qty.EndsWith("%", StringComparison.InvariantCultureIgnoreCase);
-                string qtyString = isPercentage ? request.Qty.Substring(0, request.Qty.Length - 1) : request.Qty;
-                bool canParseQty = Decimal.TryParse(qtyString, out decimal qty);
-                if (!canParseQty)
+                decimal qty;
+                try
                 {
-                    return this.CreateAPIError(400, "bad-qty-format",
-                        $"Quantity should be a number or a number ending with '%' for percentages.");
+                    qty = await ParseQty(request.Qty, request.FromAsset, custodianAccount, custodian, cancellationToken);
                 }
-
-                if (isPercentage)
+                catch (Exception ex)
                 {
-                    // Percentage of current holdings => calculate the amount
-                    var config = custodianAccount.GetBlob();
-                    var balances = custodian.GetAssetBalancesAsync(config, cancellationToken).Result;
-                    var fromAssetBalance = balances[request.FromAsset];
-                    var priceQuote =
-                        await tradableCustodian.GetQuoteForAssetAsync(request.FromAsset, request.ToAsset, config, cancellationToken);
-                    qty = fromAssetBalance / priceQuote.Ask * qty / 100;
+                    return UnsupportedAsset(request.FromAsset, ex.Message);
                 }
-
                 try
                 {
                     var result = await tradableCustodian.TradeMarketAsync(request.FromAsset, request.ToAsset, qty,

BTCPayServer/Controllers/UICustodianAccountsController.cs

@@ -221,12 +221,14 @@ namespace BTCPayServer.Controllers
                 return NotFound();
             }
 
-            var configForm = await custodian.GetConfigForm();
-            configForm.SetValues(custodianAccount.GetBlob());
+            var blob = custodianAccount.GetBlob();
+            var configForm = await custodian.GetConfigForm(blob, HttpContext.RequestAborted);
+            configForm.SetValues(blob);
 
             var vm = new EditCustodianAccountViewModel();
             vm.CustodianAccount = custodianAccount;
             vm.ConfigForm = configForm;
+            vm.Config = _formDataService.GetValues(configForm).ToString();
             return View(vm);
         }
 
@@ -244,15 +246,13 @@ namespace BTCPayServer.Controllers
                 // TODO The custodian account is broken. The custodian is no longer available. Maybe delete the custodian account?
                 return NotFound();
             }
-
-            var configForm = await custodian.GetConfigForm();
-            configForm.ApplyValuesFromForm(Request.Form);
-            
+            var configForm = await GetNextForm(custodian, vm.Config);
 
             if (configForm.IsValid())
             {
                 var newData = _formDataService.GetValues(configForm);
                 custodianAccount.SetBlob(newData);
+                custodianAccount.Name = vm.CustodianAccount.Name;
                 custodianAccount = await _custodianAccountRepository.CreateOrUpdate(custodianAccount);
                 return RedirectToAction(nameof(ViewCustodianAccount),
                     new { storeId = custodianAccount.StoreId, accountId = custodianAccount.Id });
@@ -261,9 +261,36 @@ namespace BTCPayServer.Controllers
             // Form not valid: The user must fix the errors before we can save
             vm.CustodianAccount = custodianAccount;
             vm.ConfigForm = configForm;
+            vm.Config = _formDataService.GetValues(configForm).ToString();
             return View(vm);
         }
 
+        private async Task<Form> GetNextForm(ICustodian custodian, string config)
+        {
+            JObject b = null;
+            try
+            {
+                if (config != null)
+                    b = JObject.Parse(config);
+            }
+            catch
+            {
+            }
+            b ??= new JObject();
+            // First, we restore the previous form based on the previous blob that was
+            // stored in config
+            var form = await custodian.GetConfigForm(b, HttpContext.RequestAborted);
+            form.SetValues(b);
+            // Then we apply new values overriding the previous blob from the Form params
+            form.ApplyValuesFromForm(Request.Form);
+            // We extract the new resulting blob, and request what is the next form based on it
+            b = _formDataService.GetValues(form);
+            form = await custodian.GetConfigForm(_formDataService.GetValues(form), HttpContext.RequestAborted);
+            // We set all the values to this blob, and validate the form
+            form.SetValues(b);
+            _formDataService.Validate(form, ModelState);
+            return form;
+        }
 
         [HttpGet("/stores/{storeId}/custodian-accounts/create")]
         public IActionResult CreateCustodianAccount(string storeId)
@@ -301,12 +328,12 @@ namespace BTCPayServer.Controllers
             };
 
 
-            var configForm = await custodian.GetConfigForm();
-            configForm.ApplyValuesFromForm(Request.Form);
+            var configForm = await GetNextForm(custodian, vm.Config);
             if (configForm.IsValid())
             {
                 var configData = _formDataService.GetValues(configForm);
                 custodianAccountData.SetBlob(configData);
+                custodianAccountData.Name = vm.Name;
                 custodianAccountData = await _custodianAccountRepository.CreateOrUpdate(custodianAccountData);
                 TempData[WellKnownTempData.SuccessMessage] = "Custodian account successfully created";
                 CreatedCustodianAccountId = custodianAccountData.Id;
@@ -317,6 +344,7 @@ namespace BTCPayServer.Controllers
 
             // Ask for more data
             vm.ConfigForm = configForm;
+            vm.Config = _formDataService.GetValues(configForm).ToString();
             return View(vm);
         }
 
@@ -574,7 +602,7 @@ namespace BTCPayServer.Controllers
                     }
                     catch (BadConfigException e)
                     {
-                        Form configForm = await custodian.GetConfigForm();
+                        Form configForm = await custodian.GetConfigForm(config);
                         configForm.SetValues(config);
                         string[] badConfigFields = new string[e.BadConfigKeys.Length];
                         int i = 0;

BTCPayServer/Forms/FormComponentProviders.cs

@@ -19,13 +19,13 @@ public class FormComponentProviders
 
     public bool Validate(Form form, ModelStateDictionary modelState)
     {
-        foreach (var field in form.Fields)
+        foreach (var field in form.GetAllFields())
         {
-            if (TypeToComponentProvider.TryGetValue(field.Type, out var provider))
+            if (TypeToComponentProvider.TryGetValue(field.Field.Type, out var provider))
             {
-                provider.Validate(form, field);
-                foreach (var err in field.ValidationErrors)
-                    modelState.TryAddModelError(field.Name, err);
+                provider.Validate(form, field.Field);
+                foreach (var err in field.Field.ValidationErrors)
+                    modelState.TryAddModelError(field.Field.Name, err);
             }
         }
         return modelState.IsValid;

BTCPayServer/Models/CustodianAccountViewModels/CreateCustodianAccountViewModel.cs

@@ -43,6 +43,6 @@ namespace BTCPayServer.Models.CustodianAccountViewModels
         public SelectList Custodians { get; set; }
 
         public Form ConfigForm { get; set; }
-
+        public string Config { get; set; }
     }
 }

BTCPayServer/Models/CustodianAccountViewModels/EditCustodianAccountViewModel.cs

@@ -8,5 +8,6 @@ namespace BTCPayServer.Models.CustodianAccountViewModels
 
         public CustodianAccountData CustodianAccount { get; set; }
         public Form ConfigForm { get; set; }
+        public string Config  { get; set; }
     }
 }

BTCPayServer/Plugins/FakeCustodian/FakeCustodian.cs

@@ -34,7 +34,7 @@ public class FakeCustodian : ICustodian
         return Task.FromResult(r);
     }
 
-    public Task<Form> GetConfigForm(CancellationToken cancellationToken = default)
+    public Task<Form> GetConfigForm(JObject config, CancellationToken cancellationToken = default)
     {
         
         var form = new Form();

BTCPayServer/Views/UICustodianAccounts/CreateCustodianAccount.cshtml

@@ -1,4 +1,4 @@
-@using BTCPayServer.Views.Apps
+@using BTCPayServer.Views.Apps
 @using BTCPayServer.Abstractions.Extensions
 @model BTCPayServer.Models.CustodianAccountViewModels.CreateCustodianAccountViewModel
 @{
@@ -16,6 +16,7 @@
 <div class="row">
     <div class="col-xl-8 col-xxl-constrain">
         <form asp-action="CreateCustodianAccount">
+			<input asp-for="Config" type="hidden" />
             @if (!ViewContext.ModelState.IsValid)
             {
                 <div asp-validation-summary="ModelOnly" class="text-danger"></div>

BTCPayServer/Views/UICustodianAccounts/EditCustodianAccount.cshtml

@@ -17,6 +17,7 @@
 <div class="row">
     <div class="col-xl-8 col-xxl-constrain">
         <form asp-action="EditCustodianAccount" class="mb-5">
+			<input asp-for="Config" type="hidden" />
             @if (!ViewContext.ModelState.IsValid)
             {
                 <div asp-validation-summary="ModelOnly" class="text-danger"></div>