plasma-contracts rename currency to token

May 04 '20 09:05 InoMurko

maybe you guys know what this python error is?

ImportError while loading conftest '/home/circleci/repo/plasma_framework/python_tests/tests/conftest.py'.
 
tests/conftest.py:15: in <module>
 
    from tests.tests_utils.constants import (
 
E   ModuleNotFoundError: No module named 'tests.tests_utils'

May 04 '20 17:05 InoMurko

Thanks for this @InoMurko, I thought we'd already done this 🤦

Unfortunately, changing the eip712 hash will have an impact on any code that does signing, such as omg-js, etc. Let's discuss with the relevant owners and see how to best sync this change

May 05 '20 07:05 kevsul

Why didn't we stay with currency? Suits better. For example ETH is not a token.

May 05 '20 13:05 pgebal

Ethereum is a shit token. I have heard this many many times :)

May 05 '20 13:05 InoMurko

The reason I didn't go with currency is simply because of the impact. If I would rename token2currency it would have touched more files.

May 05 '20 13:05 InoMurko

Oops, did not notice we use currency in vault. I did not change the wording in EIP712 as the reason mentioned above....but if we are scoping to make it truly consistent across all repo it will be 💯

As for CI, I am asking if gold team have some resource to help on this 🙏 (@pik694 )

May 11 '20 09:05 boolafish

how do we want to proceed? @kevsul @boolafish ?

May 20 '20 10:05 InoMurko

hmm....how about we do the change on the vault in this PR only as it would not break anything outside.

For the change to EIP712, we create another issue and pull that into gold team, I don't think any feature team has extra engineer budget at this moment? The change has to cover all repos: contract, elixir-omg, omg-js (anything more? BE and wallet??).

Alternatively we try to ask to pull this in next cycle. Though I think if no emergent issue, this is sth gold team can take care of.

May 21 '20 01:05 boolafish

I'm still a bit worried about the impact - this is a breaking change, which means all currently deployed envs will be incompatible. We'll have to redeploy staging, integration, etc.

May 21 '20 08:05 kevsul

This is 70% true for any merge into plasma-contracts repo :)

For the mainnet release, are we bundling any other PRs? Something like plasma-contracts 2.0 perhaps? Then this is where this PR should go.

May 21 '20 08:05 InoMurko

Agreed. I think it's too much to be handle by the gold team - let's try to bundle it into the next breaking release.

May 21 '20 08:05 kevsul