XRPLF / clio

An XRP Ledger API Server
https://xrpl.org
ISC License
61 stars 52 forks source link

“ledger_entry” with request improperly specified an Address field returns "invalidParams" instead of "malformedAddress". #272

Closed mounikakun closed 1 year ago

mounikakun commented 2 years ago

account_root is an invalid address in the request. Expected response: malformedAddress Actual response: invalidParams

Sample request: { "method": "ledger_entry", "params": [ { "account_root": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpna", "ledger_index": "validated" } ] }

mounikakun commented 2 years ago

“ledger_entry” of directory with invalid owner in the request. Expected response: malformedAddress Actual response: invalidParams

Sample request: { "method": "ledger_entry", "params": [ { "directory": { "owner": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpna", "sub_index": 0 }, "ledger_index": "validated" } ] }

mounikakun commented 2 years ago

“ledger_entry” of offer with invalid account in the request. Expected response: malformedAddress Actual response: invalidParams

Sample request: { "method": "ledger_entry", "params": [ { "offer": { "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpna", "seq": 359 }, "ledger_index": "validated" } ] }

mounikakun commented 2 years ago

“ledger_entry” of ripple_state with invalid account in the request. Expected response: malformedAddress Actual response: invalidParams

Sample request: { "method": "ledger_entry", "params": [{ "ripple_state": { "accounts": [ "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpna", "rsA2LpzuawewSBQXkiju3YQTMzW13pAAdW" ], "currency": "USD" }, "ledger_index": "validated" }] }

mounikakun commented 2 years ago

“ledger_entry” of escrow with invalid account in the request. Expected response: malformedAddress Actual response: invalidParams Suggested fix: malformedRequest

Sample request: { "method": "ledger_entry", "params": [{ "escrow": { "account": "rL4fPHi2FWGwRGRQSH7gBcxkuo2b9NTjKKa", "seq": 126 }, "ledger_index": "validated" }] }

officialfrancismendoza commented 1 year ago

Noticed these have similar issues to #276, where malformedAddress does not possess it's own error code in ErrorCodes.h in rippled. Much like malformedRequest in #276, rippled only returns a string literal for the error but doesn't have a dedicated error code.

Also, there is a slight discrepancy in documentation. For "escrow" in ledger_entry, "owner" and "account" seem to be synonymous and refer to the same functionality. Suggest we pick one.

godexsoft commented 1 year ago

“ledger_entry” of escrow with invalid account in the request. Expected response: malformedAddress Actual response: invalidParams Suggested fix: malformedRequest

Sample request: { "method": "ledger_entry", "params": [{ "escrow": { "account": "rL4fPHi2FWGwRGRQSH7gBcxkuo2b9NTjKKa", "seq": 126 }, "ledger_index": "validated" }] }

@ManiMounikaKunasani this seems like a bug with the webapi. I can see that here it clearly says that the argument is supposed to be owner, yet the example rpc is using account. I don't think we should support account in escrow as per documentation.

legleux commented 1 year ago

Last case still returns invalidParams This syntax is incorrect but rippled returns malformedOwner

curl 10.0.0.2:8080 -d ' { "method": "ledger_entry", "params": [{ "escrow": { "account": "rL4fPHi2FWGwRGRQSH7gBcxkuo2b9NTjKKa", "seq": 126 }, "ledger_index": "validated" }] }'

  "result": {
    "error": "invalidParams",
    "error_code": 31,
    "error_message": "malformedOwner",
...
godexsoft commented 1 year ago

@legleux so what's your suggestion? to support both account and owner for escrow? Since account is not a documented way to do this, why are we even testing for it?

However, i get malformedAddress, not invalidParams from clio if I change account to owner in your example but rippled responds with malformedOwner.

I'm a bit confused what exactly we are trying to achieve rn. Thoughts? cc @cjcobb23 @legleux

legleux commented 1 year ago

Not sure. Seems to me either way is fine, just mentioning it's not 100% same as rippled. malformedOwner - ok, yes, total lack of "owner" could be construed as "malformed", the error_message, I suppose but then the error is "invalidParams" so... Submitted a PR to fix the docs so https://github.com/XRPLF/xrpl-dev-portal/pull/1653