-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
"book_offers" and "ripple_path_find" return no result for 3 letter currencies with a lower case letter (Version: 1.8.5) #4112
Comments
can confirm.
returns the offer object
returns empty list |
Update: #4112 (comment) |
In which book can the offer then be found? Because it's not in |
Yes, currency codes are case sensitive. But I have to disagree to close the ticket. I stated clearly that issued tokens with (at least) 1 lower case letter (because case sensitive) have no return value on the "book_offers" command. The issued token is "xSD" (lower 'x', capital 'S' and 'D') So using 'xSD' with the "book_offers" command does NOT return any result, even though there are more than enough offers. This is clearly a bug of the "book_offers" command. Here is an example of the book_offers command for the currency "xSD" and an existing offer which should have been returned in the book_offers command, but didn't: |
Reopening this ticket. I misunderstood the issue. To clarify, there are tokens issued with mixed-case three-letter IDs. There are offers on the books for these tokens. The apparent bug is that requesting |
A general request: Instead of or in addition to posting screenshots, please please please copy and paste the text into a code block, and include both the request and the response. |
Preliminary analysis:
I strongly suspect that the transaction from the Issuer that originally "minted" the coins was submitted either as a binary blob or as JSON with the currency code in hex form. (e.g. Workaround:On the command line use
Solution:I have not found conclusive documentation, but I believe that the original designers of the XRPL did not intend to allow for three letter currencies with lower-case letters. I think that they assumed that input would be checked such that it would be difficult to create one, and thus didn't check the output side for upper-case values. Thus, I think the solution is to modify the I could, of course, be wrong. |
Thank you @ximinez for the first analysis. you wrote:
I think the token was issued using a JSON structure and using the actual currency code as "xSD". (through my "Token Creator" service, which allows lower case letters) Here is the issuing transaction for "xSD": based on this documentation: for 3 letter currency codes ("standard currency codes"), the following rule does apply:
So lower case letters are allowed too. And the "book_offers" method does not only return NO result via command line, but also via websocket call. As example:
So the "problem" does not exists exclusively through the command line interface. And while we are on it: I tried to find other methods where you use a currency code as "input" parameter. Issuing the following JSON command via websocket:
returns the following result:
It does NOT return any possible paths (but there are plenty of offers) And using "xSD" encoded as hex in the request like this:
Does return an actual result (paths):
So this seems to be another method not working properly with lower case letters in a 3 letter currency code. "book_offers" and "ripple_path_find" (and possibly "path_find"?) are the only methods not handling tokens with lower case letters properly. All other methods seems to work fine and do return the currency code in proper upper and lower case. |
My pleasure.
Right, so I assume the JSON was not submitted directly to rippled. I assume the "Token Creator" took the "xSD" string, encoded it into the transaction, signed it and returned the blob or submitted the blob to rippled. Serialization bypasses the upper-case conversion that I pointed out above To demonstrate what I mean, I took the transaction you linked to, stripped out the memos for simplicity, and signed it locally using rippled:
Notice that the currency code returned is all upper-case - What this indicates to me is that rippled takes any three-letter currency code that it sees (via command line, RPC, etc.) and deliberately converts it to upper case.
The problem with this example is that is probably showing the same display discrepancy that affects
Yes, the question is: Which is correct: rippled or the documentation? For code this old (the logic was introduced 8 years ago), where the documentation was probably written later, it might be rippled. That's not to say we can't decide to change rippled to match the docs, but that is not a step to be taken lightly, since there's no telling how much existing code relies on the current behavior.
Yes, I should have been more clear about that. rippled converts any three letter currency code in any JSON to upper-case, regardless of where it came from. The command-line client simply converts the parameters into an appropriate JSON representation before submitting to the rippled RPC server.
Exactly. I would expect that all methods convert the 3 letter codes to upper-case.
I can't find any methods that don't do this conversion. Could you provide an example? |
Not to take away from the great work done on this so far BUT, there seems to be another case worth considering; All upper-case but six characters including a symbol.
|
@NetScr1be The case you are referring is not relevant to this issue. Anything other than the Standard Currency Code is only possible if something other than the |
Does that mean the fix for this issue might orphan non-compliant currencies (even though it allowed their creation)? I realize I'm not a developer but we have to interpret these situations for our clients and, so far, looking into it only creates more confusion. For example; Searching for 'xSD' on https://xrpl.services/tokens returns both 'XSD' and 'xSD'. There is no hexadecimal currency code at all when looking at the details. The same with 'mXm' the support ticket for which triggered this iteration of this issue. Searching for $Ghost returns; Currency: $GHOST $Ghost seems to comply with specification for Non-standard currency codes
|
@NetScr1be Your example, This issue is only related to 3 char (non HEX) currency codes, containing one or more lowercase characters. |
Nothing will be orphaned. The transactions and ledger objects will still be on the ledger. The consequence will be that any non-compliant currencies will need to use their full hex code instead of the three letter version.
Bugs suck. Bugs that people start using as if they're "correct" behavior suck worse. (I'm not trying to be dismissive here. My point is that sometimes you just have to rip off the bandaid and deal with the consequences.)
xrpl.services is a 3rd party service, AFAIK, and I have no idea how it's interfacing with the rippled API, or interpreting the data from it. |
That is interesting. xrpl.services is actually built by me and what I do is to scan a full ledger, using the ledger_data method and iterating through the markers. (directly connected to rippled via admin port)
I use "binary":false to get the plain JSON objects. I filter for all RippleState objects and collect information about issued currencies. To determine the currency code, I simple access the RippleState object's "Balance" field and then the "currency" field. And for "xSD", the currency field actually contains "xSD" in the JSON response and not a HEX encoded currency code (and neither an upper case 3 letter code). I do not execute any conversion of currency codes in my API code and here is the result for "xSD": And here is an example where rippled actually returns a HEX encoded currency code: (currency code > 3 letters) so, while rippled seems to run every "input´" currency code through the "toUpperCase" function (if submitted via JSON and not blob), it does not run the currency code through a "toUpperCase" function when providing currency data. This behavior and the text in the documentation give the impression that for 3 letter currency codes, lower and upper case letters are allowed. I can submit a transaction, issuing a 3 letter currency with lower case letters. And I can query that very same transaction (Payment) and the underlying Trustline (RippleState) and the currency code is also provided in lower case. So there was no way to see as a developer/user that lower case letters in 3 letter currency codes should not be used. |
Thank you for the great work!
This is, unfortunately, true. I got into more detail about that above.
Can you though? Everything I've seen so far indicates that if you submit the transaction as JSON with a 3-letter code including lower case letters, the code will be converted to upper case before the transaction is signed. Of course, signing is disabled on most servers now, particularly the public-facing ones, so unless the issuer is running their own node and signing there, the transaction is being signed and encoded by another tool/library (possibly even one of "ours"), so rippled never sees the currency code as a string... The only real question in my mind is how to fix it. On the input side, and risk breaking older apps that counted on the case being changed. Or on the output side, and confused everyone when their currency codes go from 3 letters to serialized hex... |
Also seeing this or a related issue with currency codes containing a number: Spec: https://xrpl.org/currency-formats.html#standard-currency-codes Does not match used regex: https://github.com/XRPLF/rippled/blob/develop/src/ripple/net/impl/RPCCall.cpp#L124
|
Took a look at this. Here is the way I see it: The XRPL doesn't care if the letters are lowercase or uppercase. It correctly permits all and only characters from the alphabet documented on xrpl.org, and defined in I think rippled should just get out of the way and let clients submit 3-character currency codes that include characters that are not uppercase letters. It requires only two changes: |
A few methods, including `book_offers`, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL. This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet. Fixes XRPLF#4112
Proposed solution: #4566 |
A few methods, including `book_offers`, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL. This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet. Fixes #4112
* Add unit test to validate rippled bug (See XRPLF/rippled#4112) * Update test to cover binary codec
A few methods, including `book_offers`, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL. This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet. Fixes XRPLF#4112
Issue Description
The rippled method to get existing offers ("book_offers") does not return any offers for tokens with a 3 letter currency code where, at least, one letter is lower case.
This does NOT affect currencies where all letters are capital or the currency code is > 3 letters (HEX code)
Steps to Reproduce
You can simply reproduce this behavior by calling the "book_offers" method with one of the following tokens in the "TakerGets" field:
Example 1:
Issuer: rGbNLwrYNuPim39crh1bjyyauefF1VHPoj
Currency: xSD
Current DEX offers: 97
Example 2:
Issuer: raZ87rZoVjgg8oc5LijSigyN8rbutXGdbw
Currency: Lil
Current DEX offers: 10
Example 3:
Issuer: rJXesdkxGHDtzTEgxxbo7h9C1WyVr8NNgw
Currency: Vtc
Current DEX offers: 3
Expected Result
Rippled should return the offers for these tokens when calling the "book_offers" method.
Actual Result
Calling the "book_offers" method results in an empty response:
Environment
I am using Ubuntu 20.04 with rippled 1.8.5
The text was updated successfully, but these errors were encountered: