Add BEAM support

[beam-mw]

Related to issue: #338

[prusnak]

There is lots of new code being added to crypto/beam/lib/. Can we just remove that and use the code from vendor/secp256k1-zkp?

[beam-mw]

That's a good point indeed. When we began our development process, secp256k1-zkp lib hadn't been a part of the Trezor's dependencies yet.
So we cut out only the mandatory parts for us. Now we can't use secp256k1-zkp lib without any changes applied, as many low-level parts (i.e. scalars) are defined there as static functions.
So we should decide together, how to proceed over it: whether keep our shortened library or change some files in secp256k1-zkp dependency.

[prusnak]

The commits add the file core/src/apps/management/recovery_device.py which is weird. Can you do a rebase on top of current master?

Also, does your protocol as it is designed now work for Grin as well?

[vldmkr]

Yes, core/src/apps/management/recovery_device.py appeared here by mistake as a result of a merge. It has been fixed and updated.

No, at this stage, we did not work on the integration of Grin.

[vldmkr]

There are conflicts appeared. Do you want us to resolve it, or is it better to let you review the code in its current state?

[vldmkr]

There are conflict resolution and code refactoring in this update.

[prusnak]

Can you please start a discussion with secp256k1-zkp how to make static optional in their upstream code? Usually you can do so by replacing static with STATIC and then doing #define STATIC static. If you then define STATIC as empty in project then you would get the behaviour you'd need. I'd suggest we all start working on upstreaming the changes you need in the library before even trying to merge this code. Feel free to start the issue here and I'll chime in: https://github.com/ElementsProject/secp256k1-zkp

Once this is done we can work on removing the bundled secp256k1-zkp dependency and use the new version with upstreamed changes of yours.

[vldmkr]

I managed to use secp256k1-zkp without changing it... almost without changes. I have noticed that you locked to commit BlockstreamResearch/secp256k1-zkp@9ecd8bf (BlockstreamResearch/secp256k1-zkp#68). But it is not up to date and does not have the necessary changes. So, I have added git cherry-pick to commit which solves the problem (BlockstreamResearch/secp256k1-zkp@496c5b4) to Makefile. This is a temporary fix. Please suggest more elegant one. Also, there is separate commit with these changes to simplify code review. Overall, solution is a little bit tricky but it works :)

[vldmkr]

Oh.. Travis CI build failed on style check. I will fix it after your comments.

[vldmkr]

Code updated and conflicts resolved according to the current master branch' state. Waiting for your review. Thanks!

[tsusanka]

Hi there, thank you for your PR. After a thorough discussion in our Product team, we have decided we do not wish to include this PR in our firmware. Due to our limited capacity, we need to restrict our support to coins that we can afford to maintain in the long run. We hope you’ll understand this decision.

[Megawatt7]

Hi there, thank you for your PR. After a thorough discussion in our Product team, we have decided we do not wish to include this PR in our firmware. Due to our limited capacity, we need to restrict our support to coins that we can afford to maintain in the long run. We hope you’ll understand this decision.

Hello take a look at Beam's github activity. This is a serious project that actually has a good use case - proven privacy through mathematical proofs while providing decentralization.

[leifliddy]

Hi there, thank you for your PR. After a thorough discussion in our Product team, we have decided we do not wish to include this PR in our firmware. Due to our limited capacity, we need to restrict our support to coins that we can afford to maintain in the long run. We hope you’ll understand this decision.

What exactly is meant by limited capacity? Are you referring to resource constraints on your hardware? Or the limited capacity of your developers to maintain your firmware? I'm pretty sure the BEAM team would be willing and able to maintain the BEAM code base on your firmware. It's just a bit odd to me that you didn't have any issues supporting coins like EOS, Klaytn, and Theta Fuel --when BEAM is older then all of those coins. Like what sorts of metrics did you discuss in your "thorough discussion"? Stability, security, open source development model, audited code base, community? Seems like BEAM checks all those boxes....