Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(crypto): add support for Schnorr crypto (eCash variant) #4559

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Fabcien
Copy link
Contributor

@Fabcien Fabcien commented Jan 30, 2025

Support Schnorr signature and verification defined in https://github.com/Bitcoin-ABC/bitcoin-abc/tree/master/doc/standards/schnorr.md.

The implementation is based on https://github.com/Bitcoin-ABC/secp256k1 as well as the test vectors.

This is a revival of #1509 that was reverted due to not being used after #1591 was rejected. The only difference is that the source files are now located in a ecash/ folder to avoid confusion with BIP340 Schnorr signatures, and a USE_ECASH flag makes it possible to disable the feature to save space.

Now that eCash is supported on Trezor, supporting this signature scheme would be very useful for eCash users.
It can be leveraged to add avalanche stake signature using the Trezor devices.

Support Schnorr signature and verification defined in
https://github.com/Bitcoin-ABC/bitcoin-abc/tree/master/doc/standards/schnorr.md.

The implementation is based on https://github.com/Bitcoin-ABC/secp256k1
as well as the test vectors.

This is a revival of trezor#1509 that was reverted due to not being used after trezor#1591 was rejected.
The only difference is that the source files are now located in a ecash/ folder to avoid confusion with BIP340 Schnorr signatures, and a USE_ECASH flag makes it possible to disable the feature to save space.

Now that eCash is supported on Trezor, supporting this signature scheme would be very useful for eCash users.
@prusnak
Copy link
Member

prusnak commented Jan 30, 2025

This PR adds code which is not used anywhere in the firmware.

@Fabcien
Copy link
Contributor Author

Fabcien commented Jan 30, 2025

This PR adds code which is not used anywhere in the firmware.

Yes. I want to also revive #1591 after this PR, and I split it to make the review easier: because this is crypto it can be more involved to review.
Do you prefer if I submit the whole feature instead ?

@prusnak
Copy link
Member

prusnak commented Jan 30, 2025

I think the position from #1591 has not changed. The coin is irrelevant as ever, adding feature that is needed only for that coin makes very little sense.

@BytesOfMan
Copy link

adding feature that is needed only for that coin makes very little sense.

true, adding this support will probably only result in a few hundred trezor sales if nothing changes about XEC

Some things that could help from trezor perspective

  1. Though a less relevant coin than other utxo projects, XEC is actively maintained. For example, an XEC dev recently found and responsibly disclosed this dogecoin exploit. Trezor would get the benefit of experienced open source dev attention for its other supported popular (but no dev team) coins, like Dogecoin

  2. We have a professional team used to working open source. Because we would use this feature to run our businesses, trezor would not need to worry about maintaining this feature.

  3. We are old school crypto users who have been around for 10+ years. I still have trezors from 2014. Past the point of political arguments about different projects -- understandably trezor does not really have an incentive to add a few hundred sales for a small project. But the additional apolitical dev attention of one of the few competent utxo teams outside of BTC would be a nice boost, and we are happy to help where we can.

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants