EOSIO / eosjs

General purpose library for the EOSIO blockchain.
http://eosio.github.io/eosjs
MIT License
1.43k stars 463 forks source link

Important scale request #258

Closed dacom-dark-sun closed 6 years ago

dacom-dark-sun commented 6 years ago

Hello Now in eosjs and eosjs-css 'EOS' prefix is hard-coded.

This is a problem for scale, cause projects need fork libraries, change prefix manual, deploy it to npm and change all libraries on exist project, if they already use them. This is spend developers time and will spend in reason of the need regular fetch changes from main repo.

Hope, for EOS developers will not be difficult add prefix in config and tests. That will reduce a lot of problems for community. Thanks.

jcalfee commented 6 years ago

Looks like the change is needed in eosjs-ecc .. Do you know where one of these patches are?

dacom-dark-sun commented 6 years ago

Easy to find: https://github.com/EOSIO/eosjs-ecc/search?q=EOS&unscoped_q=EOS

jcalfee commented 6 years ago

That is my code .. I'm asking about a use-case in someone else's code..

Did you see the new key format? It does not use EOS at all. This is for legacy keys (although they are still use heavily).

dacom-dark-sun commented 6 years ago

We have already forked library for TravelChain (tcjs-ecc) and will fork it for other DACom Projects, if not resolve it just once. In this reason, we cannot just add our chains to Scatter and must fork it too.

Yes, i see a new key-format. That seems hard to use. Libraries still generate a old format by default and more, we did not know about compatibility with existing applications (Scatter, EOS-Voter, ...) - they may have different internal checks. A quick transition may create a many problems in a relatively flat place.

jcalfee commented 6 years ago

This is a tricky part of the code.. The EOS prefix is static but the library should support public keys from any asset. For this reason, the code allows you to provide the legacy key without any prefix by simply removing EOS if it sees it then not testing for a prefix:

      // legacy
      if(/^EOS/.test(public_key)) {
        public_key = public_key.substring(3)
      }
      return PublicKey.fromBuffer(keyUtils.checkDecode(public_key))

The best bet is to replace the prefix with PUB_K1_ and report any issues to myself, nsjames at scatter or EOS-Voter .. They focus is to remove the token specific prefix for the reason you stated above. If need work-around after a bug is reported, can you simply change this in your fork? I really like the chain agnostic key format (PUB_K1_keys)..

jcalfee commented 6 years ago

I think I'm going to have to change the default format to PUB_K1_. It is not going to help you if I keep creating keys that start with EOS..

jcalfee commented 6 years ago

Will this work?

https://github.com/EOSIO/eosjs-ecc/pull/30

Released in eosjs@16.0.2 and eosjs-ecc@4.0.3