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

CTID in tx command returns invalidParams on lowercase hex #4776

Closed
mDuo13 opened this issue Oct 18, 2023 · 4 comments · Fixed by #5049
Closed

CTID in tx command returns invalidParams on lowercase hex #4776

mDuo13 opened this issue Oct 18, 2023 · 4 comments · Fixed by #5049
Assignees

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Oct 18, 2023

Issue Description

The ctid field of the tx command, added in #4418, does not work if the given CTID contains lowercase hex.

Steps to Reproduce

Send this request to a rippled 1.12 server.

{
  "id": "example lowercase in CTID",
  "command": "tx",
  "ctid": "C005523e00000000"
}

Expected Result

Request should successfully find transaction E08D6E9754025BA2534A78707605E0601F03ACE063687A0CA1BDDACFCD1698C7 like this example. Aside from the ID, the difference is the e vs E in the CTID.

Actual Result

Response is an invalidParams error message.

{
  "error": "invalidParams",
  "error_code": 31,
  "error_message": "Invalid parameters.",
  "id": "example lowercase CTID",
  "request": {
    "command": "tx",
    "ctid": "C005523e00000000",
    "id": "example lowercase CTID"
  },
  "status": "error",
  "type": "response"
}

Environment

rippled 1.12.0 via xrplcluster.com

@intelliot
Copy link
Collaborator

Could we just document that the ctid must be in uppercase?

@intelliot
Copy link
Collaborator

nit: I noticed under "Actual Result", you have: "ctid": "c005523e00000000", (lowercase leading c) but when I tried it in the websocket tool, it correctly echoes back the provided capitalization - i.e. "ctid": "C005523e00000000",

@mDuo13
Copy link
Collaborator Author

mDuo13 commented Oct 18, 2023

I was planning on documenting the behavior, but case-sensitive hexadecimal is annoying and easy to get wrong/forget. For example, I constructed a CTID partly using Python's binascii.hexlify standard lib function and it generated lowercase hex by default, which I didn't realize would be a problem. And the "invalid params" error is unhelpfully vague since it also applies if you're on a server that doesn't support CTIDs at all (e.g. the Clio servers behind s1), or if you used the wrong field or whatever else.

re: the "actual result" example, I accidentally pasted the wrong example response (as you noted, it appropriately echoes back what you put in)

@intelliot
Copy link
Collaborator

Definitely agree that the error message isn't helpful. I guess the purist in me would prefer that everyone just use uppercase hex for consistency, but I think it should also be fine to be permissive here.

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

Successfully merging a pull request may close this issue.

4 participants