Replace network with protocol magic on cardano

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

91 stars 232 forks source link

Replace network with protocol magic on cardano #244

Closed refi93 closed 5 years ago

refi93 commented 5 years ago

See https://github.com/trezor/connect/pull/272 and https://github.com/trezor/trezor-core/pull/417

prusnak commented 5 years ago

It's a very bad idea to change the behavior of the field. Let's introduce a new field (5) and drop the old field (4) instead.

-optional uint32 network = 4;                // network number
+optional uint32 protocol_magic = 5;         // network's protocol magic

refi93 commented 5 years ago

@prusnak thanks for pointing out, fixed

prusnak commented 5 years ago

@jpochyla @matejcik are you OK with this change?

matejcik commented 5 years ago

there should be a corresponding PR to python-trezor, which is using the field otherwise LGTM, except that I don't know how Cardano works so I trust @refi93 that this is correct from that angle :)

refi93 commented 5 years ago

@matejcik there already is one: https://github.com/trezor/python-trezor/pull/342 I just need to fix the clicks emulation so the test does not hang. Will do probably tomorrow.