ripple-unmaintained / ripple-lib-java

Java version of ripple-lib (work in progress)
ISC License
126 stars 109 forks source link

Invalid Amount is generated in JSON when AMount#fromString is used with drops #65

Open avalori opened 9 years ago

avalori commented 9 years ago

Ripple-java-lib is not correctly rendering in JSON amount expressed in drop. As an example, an Amount constructed through the following call:

Amount.fromString("1")

renders through ripple-java-lib in

"Amount": {
    "currency": "XRP",
    "issuer": "rrrrrrrrrrrrrrrrrrrrrhoLvTp",
    "value": "0.000001"
}

which is then rejected by the server:

...
"error": "invalidParams",
"error_code": 26,
"error_message": "Invalid field 'tx_json.Amount'.",
...
sublimator commented 9 years ago

I guess you did amount.toJSONObject() or the like afterwards?

Interesting, the JSON format for XRP is actually a string with the amount of drops (a million of make an XRP).

It's unfortunate that XRP is specified in a different manner to issued currencies, and that there are fields, such as Amount in transaction that could be filled with either a native (XRP) amount or otherwise.

Typically you want to use the .toJSON() method which returns an Object, if you aren't sure if the Amount is native or not.

    @Override
    public Object toJSON() {
        if (isNative()) {
            return toDropsString();
        } else {
            return toJSONObject();
        }
    }

    public JSONObject toJSONObject() {
        try {
            JSONObject out = new JSONObject();
            out.put("currency", currencyString());
            out.put("value", valueText());
            out.put("issuer", issuerString());
            return out;
        } catch (JSONException e) {
            throw new RuntimeException(e);
        }
    }

    public String toDropsString() {
        if (!isNative()) {
            throw new RuntimeException("Amount is not native");
        }
        return bigIntegerDrops().toString();
    }

Given that the Amount class must perform twin duties, maybe it makes sense for an exception to be thrown when toJSONObject is called. As you can see above, it seems toDropsString does these sort of sanity checks for you. Not doing the same on toJSONObject was an oversight.

avalori commented 9 years ago

Hi,

your guess was correct. By using "toDropString" everything works fine.

Thanks and Kind regards

On Mon, Nov 24, 2014 at 10:18 AM, sublimator notifications@github.com wrote:

I guess you did amount.toJSONObject() or the like afterwards?

Interesting, the JSON format for XRP is actually a string with the amount of drops (a million of make an XRP).

It's unfortunate that XRP is specified in a different manner to issued currencies, and that there are fields, such as Amount in transaction that could be filled with either a native (XRP) amount or otherwise.

Typically you want to use the .toJSON() method which returns an Object, if you aren't sure if the Amount is native or not.

@Override
public Object toJSON() {
    if (isNative()) {
        return toDropsString();
    } else {
        return toJSONObject();
    }
}

public JSONObject toJSONObject() {
    try {
        JSONObject out = new JSONObject();
        out.put("currency", currencyString());
        out.put("value", valueText());
        out.put("issuer", issuerString());
        return out;
    } catch (JSONException e) {
        throw new RuntimeException(e);
    }
}

public String toDropsString() {
    if (!isNative()) {
        throw new RuntimeException("Amount is not native");
    }
    return bigIntegerDrops().toString();
}

Given that the Amount class must perform twin duties, maybe it makes sense for an exception to be thrown when toJSONObject is called. As you can see above, it seems toDropsString does these sort of sanity checks for you. Not doing the same on toJSONObject was an oversight.

— Reply to this email directly or view it on GitHub https://github.com/ripple/ripple-lib-java/issues/65#issuecomment-64167884 .