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

Add basic trezor support for Verge (XVG) #816

Closed
wants to merge 610 commits into from
Closed

Add basic trezor support for Verge (XVG) #816

wants to merge 610 commits into from

Conversation

marpme
Copy link
Contributor

@marpme marpme commented Feb 1, 2020

Add basic wallet support for trezor via the electrum wallet supplied by Verge.

This PR is connected to the initial build of our electrum wallet see: vergecurrency/electrum#13

I've tested most stuff manually via the emulator.

@marpme marpme requested a review from prusnak February 1, 2020 11:06
@marpme marpme changed the title Add basic trezor support via electrum wallet Add basic trezor support for Verge (XVG) Feb 1, 2020
@prusnak
Copy link
Member

prusnak commented Feb 11, 2020

Is this issue already fixed? vergecurrency/electrum#12

@marpme
Copy link
Contributor Author

marpme commented Feb 12, 2020

@prusnak It has been fixed with vergecurrency/electrum#13 and this PR. Closed the ticket and attached the tickets for its fix. I've also locally tested receiving and sending XVG via a trezor emulator, so this shouldn't be a problem anymore.

@prusnak prusnak modified the milestones: backlog, 2020-03 Feb 19, 2020
@prusnak prusnak assigned prusnak and unassigned prusnak Feb 19, 2020
@prusnak prusnak removed their request for review February 19, 2020 18:54
@prusnak prusnak added the feature Product related issue visible for end user label Feb 19, 2020
@prusnak prusnak modified the milestones: 2020-03, backlog Feb 19, 2020
@tsusanka tsusanka removed the feature Product related issue visible for end user label Aug 4, 2020
@tsusanka
Copy link
Contributor

tsusanka commented Aug 4, 2020

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.

@tsusanka tsusanka closed this Aug 4, 2020
@marpme
Copy link
Contributor Author

marpme commented Aug 4, 2020

Hi, I'm completely unable to understand your decision. It's 66 lines of code that have been changed overall with the addition of a +4kb PNG. What do you mean by having a "limited capacity"?

I think this is the vaguest answer that I've ever got. I'm totally disappointed to get such an answer after 6 months of waiting.

@justinvforvendetta
Copy link

why would this not be easy to maintain in the longrun? @tsusanka and what kind of firmware storage space does the device have?

@tsusanka
Copy link
Contributor

tsusanka commented Aug 5, 2020

Apologies, we have accidentally included this into a group of PRs which make substantial changes in the code. I have overlooked this changes only common/defs. If you can rebase and squash into one commit we will try to merge this.

@tsusanka tsusanka reopened this Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
altcoin Any non-Bitcoin coins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet