ExodusMovement / eosjs

General purpose library for the EOS blockchain.
http://eosio.github.io/eosjs
MIT License
2 stars 0 forks source link

use minimal utf-8-only text-encoding polyfill #1

Open mvayngrib opened 5 years ago

mvayngrib commented 5 years ago

@unidwell if you're not already working on this, i can take it off your hands :)

unidwell commented 5 years ago

Hi @mvayngrib, please do. I checked the following:

mvayngrib commented 5 years ago

@unidwell i see that eos-serialize.ts creates a default encoder/decoder (utf-8) https://github.com/ExodusMovement/eosjs/blob/master/src/eosjs-serialize.ts#L130

in their doc for creating the Api obj, they do the same: https://github.com/ExodusMovement/eosjs#api

the node.js one they use from 'util' only supports utf-8: https://nodejs.org/dist/latest-v10.x/docs/api/util.html#util_class_util_textencoder (node 12 too: https://nodejs.org/dist/latest-v12.x/docs/api/util.html#util_class_util_textencoder)

edit: the node decoder also supports utf-16 as well: https://nodejs.org/dist/latest-v10.x/docs/api/util.html#util_encodings_supported_without_icu (unless built with ICU)

edit: still unclear, will ask on eos slack/open an issue

mvayngrib commented 5 years ago

issue tracker on eosio/eosjs: https://github.com/EOSIO/eosjs/issues/580

btw, the way we're currently using it only supports utf-8: https://github.com/ExodusMovement/eos/search?utf8=%E2%9C%93&q=TextDecoder&type=

mvayngrib commented 5 years ago

@unidwell does the discussion here clarify things for you? https://github.com/EOSIO/eosjs/issues/580 I'm still not sure but you or @feri42 probably have more context. Please feel free to chime in there

feri42 commented 5 years ago

From EOSIO#580 I'm guessing text encoder/decoder is only used for the memo field and for custom action specific text fields. For us this means that on the client we don't do any text decoding because we don't read any transaction data directly from the blockchain. We only do encoding when creating memos for sent transactions and those memos are probably always utf8.

We do the text decoding in eos-server (our indexer), which does not use @exodus/eosjs, it uses the stock eosjs directly from npm. And any strings that we docode on the eos-server are then stored as json and fetched by the client as such.

Taking this into consideration I think we have considerable freedom to replace the encoder/decoder in the client. However this means that in the future the client should not read any transaction data directly from the blockchain. We should always rely on this data from the indexer.

unidwell commented 5 years ago

If I understand https://github.com/EOSIO/eosjs/issues/580 correctly, memos are encoded as UTF-8 on EOS nodes. They are currently not shown in the client. From this and from what @feri42 has written we can do the necessary changes in eos-api, eos-lib or our eosio fork.

mvayngrib commented 5 years ago

From this and from what @feri42 has written we can do the necessary changes in eos-api, eos-lib or our eosio fork.

@unidwell can you clarify a bit? Would those changes necessitate bringing the full text-encoding dep back in? I ask because eos-api and eos-lib are also used on mobile

unidwell commented 5 years ago

Sorry to not be clear enough. I mean it's fine to create a small polyfill instead of using full text-~encoding~ decoding. I believe even if we use the UTF-8 polyfill, the memos (for history logs) if we decide to support them (not in the plans), will work ok with the polyfill.

unidwell commented 5 years ago

To use the polyfill we'll need to update eos-api. It gets TextEncoder and TextDecoder from our eosjs fork. So @exodus/eosjs@^20.0.0-exodus fork will have to be modified too.

TextEncoder and TextDecoder were probably added because nodejs does not support it out of the box, whereas it's supported in Chrome. Don't know if RN has started to support this natively.