[bisq-network/bisq] Add BTC address param, use String for param value instead of long (#1841)

sqrrm notifications at github.com
Tue Oct 30 22:05:03 UTC 2018


sqrrm commented on this pull request.

utACK

Had a look at the String param changes and added some comments.

> @@ -770,7 +773,22 @@ public void setNewParam(int blockHeight, Param param, long paramValue) {
                 });
     }
 
-    public long getParamValue(Param param, int blockHeight) {
+    public Coin getParamValueAsCoin(Param param, int blockHeight) {

These getParamValueAs...() functions could use a check that they're used correctly.

> +        validateCoinInput(input, btcCoinFormat);
+    }
+
+    public void validateBsqInput(String input) throws ValidationException {
+        validateCoinInput(input, this.coinFormat);
+    }
+
+    private void validateCoinInput(String input, MonetaryFormat coinFormat) throws ValidationException {
+        try {
+            coinFormat.parse(cleanDoubleInput(input));
+        } catch (Throwable t) {
+            throw new ValidationException("Invalid format for a " + coinFormat.code() + " value");
+        }
+    }
+
+    public String formatParamValue(Param param, String value) {

Would be nice if these functions could avoid the large switch blocks and use the ParamType now that it's been added. I think it would make sense, might take a look at that later, I think it would be safer.

> @@ -59,120 +62,118 @@ public void validateDataFields(Proposal proposal) throws ValidationException {
         }
     }
 
-    // TODO
-    public void validateParamValue(Param param, long paramValue) throws ChangeParamValidationException {
-        // max 4 times the current value. min 25% of current value as general boundaries
-        checkMinMax(param, paramValue, 300, -75);
+    public void validateParamValue(Param param, String inputValue) throws ValidationException {

All these values would be nice to have in Param.java to be able to see all the relevant info for each param. Perhaps ParamType could also store the restrictions?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/1841#pullrequestreview-170004816
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20181030/f2cc74a7/attachment.html>


More information about the bisq-github mailing list