Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
avalori opened this issue Nov 24, 2014 · 2 comments

Comments

@avalori
Copy link

avalori commented Nov 24, 2014

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
Copy link
Contributor

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
Copy link
Author

avalori commented Nov 24, 2014

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 [email protected]
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
#65 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants