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

Remove older versions of Zcash? #982

Closed
tsusanka opened this issue May 6, 2020 · 7 comments · Fixed by #1146
Closed

Remove older versions of Zcash? #982

tsusanka opened this issue May 6, 2020 · 7 comments · Fixed by #1146
Assignees
Labels
altcoin Any non-Bitcoin coins core Trezor Core firmware. Runs on Trezor Model T and T2B1. good first issue Issue for newbie developers who want to participate
Milestone

Comments

@tsusanka
Copy link
Contributor

tsusanka commented May 6, 2020

In Zcash we currently support both tx.version 3 and 4. 3 should be Overwinter, 4 Sapling. Does it make sense to support both or is one of them obsolete?

We could simplify a few things if we could remove one of the versions. See sign_tx/zcash.py for some context.

@ca333 I believe you did some zcash contributions to our firmware any chance you could shed some light into this?


Also should we call the class Zcash instead of Overwinter?


Related to #617

@tsusanka tsusanka added core Trezor Core firmware. Runs on Trezor Model T and T2B1. altcoin Any non-Bitcoin coins labels May 6, 2020
@tsusanka tsusanka added this to the backlog milestone May 6, 2020
@prusnak
Copy link
Member

prusnak commented May 6, 2020

AFAIK we can nuke tx.version==3 and we can even put hard check whether tx.version==4 for Zcash. I think Komodo is also on version 4 already.

@tsusanka tsusanka added P4 Low and removed W-easy labels May 7, 2020
@tsusanka
Copy link
Contributor Author

tsusanka commented Jul 3, 2020

Since we are now streaming prev tx we need to support also legacy transactions. So it seems we won't be able to do this anytime soon? Closing? cc @matejcik

@matejcik
Copy link
Contributor

matejcik commented Jul 7, 2020

nope, this issue is about removing support for signing v3 transactions.
we already dropped v1 and v2 before the refactor, I think?
to close this, we need to find out if v3 transactions are valid on currently existing Zcash-like networks.
we think not, but it would be super nice to have some confirmation from actual Zcash / Komodo / Koto devs

@tsusanka
Copy link
Contributor Author

tsusanka commented Jul 7, 2020

@matejcik I have confirmed this via email from Zcash. The v4 transaction format was required following the Sapling network upgrade. I do not know about Komodo/Koto though.

@matejcik
Copy link
Contributor

From a random sampling, both Komodo and Koto chains are using v4 since last year. That at minimum means that v4 transactions are valid on the networks, and so we can safely drop v3 support, IMHO.

So let's go ahead and do this.

Heads-up for @ca333 for Komodo and @wo01 for Koto; please let us know if this would pose any problems for you.

@wo01
Copy link
Contributor

wo01 commented Jul 14, 2020

No problem.

@tsusanka tsusanka added the good first issue Issue for newbie developers who want to participate label Jul 16, 2020
@tsusanka tsusanka modified the milestones: backlog, 2020-09 Jul 24, 2020
@tsusanka
Copy link
Contributor Author

We will test Zcash as part of Freeze so nothing specific to test here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
altcoin Any non-Bitcoin coins core Trezor Core firmware. Runs on Trezor Model T and T2B1. good first issue Issue for newbie developers who want to participate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants