search

ripple / ripple-binary-codec

Convert between json and hex representations of transactions and ledger entries on the XRP Ledger. Moved to: https://github.com/XRPLF/xrpl.js/tree/develop/packages/ripple-binary-codec
https://github.com/XRPLF/xrpl.js/tree/develop/packages/ripple-binary-codec
ISC License
19 stars 45 forks source link

Encode any 3 character ASCII currency code #106

Closed natenichols closed 4 years ago

natenichols commented 4 years ago

Allow for the encoding of all 3 letter ASCII currency codes.

Only decode into ASCII codes if they are valid ISO 4217 codes.

intelliot commented 4 years ago

Just for the record, here is the current behavior as of ripple-binary-codec 1.0.1, e.g. when encoding UsD which has a lowercase s:

Error: Unsupported Currency representation: UsD
    at bytesFromRepresentation (/Users/user/project/node_modules/ripple-binary-codec/src/types/currency.ts:59:11)
    at Function.Currency.from (/Users/user/project/node_modules/ripple-binary-codec/src/types/currency.ts:122:27)
    at Function.Amount.from (/Users/user/project/node_modules/ripple-binary-codec/src/types/amount.ts:109:33)
    at /Users/user/project/node_modules/ripple-binary-codec/src/types/st-object.ts:127:52
    at Array.forEach (<anonymous>)
    at Function.STObject.from (/Users/user/project/node_modules/ripple-binary-codec/src/types/st-object.ts:126:12)
    at serializeObject (/Users/user/project/node_modules/ripple-binary-codec/src/binary.ts:67:22)
    at signingData (/Users/user/project/node_modules/ripple-binary-codec/src/binary.ts:87:10)
    at Object.encodeForSigning (/Users/user/project/node_modules/ripple-binary-codec/src/index.ts:47:10)
    at computeSignature (/Users/user/project/node_modules/ripple-lib/src/transaction/sign.ts:15:19)

The goal with this PR is to mimic the behavior that we had with 0.2.x, where lowercase and symbols were allowed in currency

sublimator commented 4 years ago

Why have an asymmetrical codec ?

intelliot commented 4 years ago

The main reason is that it's the existing behavior of 0.2.x and we didn't expect to change it with 1.0. I can certainly imagine changing the behavior with version 2.0. The other reason is that the use of non-standard currency codes is discouraged. For example, "xrp" is currently permitted, but it is misleading/dangerous. So the fact that the codec makes such currency codes more difficult to use could be considered a good thing.