search

trezor / trezor-common

:lock: Don't post issues/PRs to here, use the new monorepo:
https://github.com/trezor/trezor-firmware
GNU Lesser General Public License v3.0
92 stars 232 forks source link

coin defs: address_prefix ending with ":" #165

Closed matejcik closed 6 years ago

matejcik commented 6 years ago

build_coins.py checks that the address_prefix fields ends with a semicolon (see https://github.com/trezor/trezor-common/blob/master/tools/build_coins.py#L105, https://github.com/trezor/trezor-common/blob/master/defs/coins/crown.json#L33)

That is a weird thing to enforce. ISTM the address_prefix should omit the semicolon and the check could instead match regex ^[a-z0-9]+$, and calling code should add the semicolon. (that is how cashaddr_prefix seems to behave)

prusnak commented 6 years ago

address_prefix is used solely in the web wallet (and probably connect), so this change has to be coordinated with the guys developing it.

I am not against the change and I think it makes sense.

prusnak commented 6 years ago

@mroz22 @mlejva can you please check where in webwallet/connect code do we use address_prefix and whether it is worth changing the logic (i.e. webwallet/connect adds the : character)

mroz22 commented 6 years ago

In webWallet, it appears that address_prefix is used only for qr code parsing:

https://github.com/satoshilabs/mytrezor/blob/e02b270ac5417ace6e6e675b0f80a7d4c10bc7c9/app/scripts/account/AccountSendCtrl.js#L521-L525

So from webwallet perspective it is easy to adapt to such change.

szymonlesisz commented 6 years ago

We don't use it at all inside connect (so far)

prusnak commented 6 years ago

Fixed via https://github.com/trezor/trezor-common/commit/c5045f066729dd3e650c3a61ec2dcfe01bccc8fa (where I renamed address_prefix to uri_prefix, because that's what it is and it will help us detect forgotten usage of this field).

@mroz22 please adapt the code in webwallet to use the uri_prefix field and add ":" at the end of the value