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.

Sign up for GitHub

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

Jump to bottom

TrezorActions: Allow updating firmware. #2816

Merged
merged 3 commits into from
Nov 12, 2020
Merged

TrezorActions: Allow updating firmware. #2816

merged 3 commits into from
Nov 12, 2020

Conversation

JoeGruffins
Copy link
Member

@JoeGruffins JoeGruffins commented Oct 28, 2020 •
edited
Loading

Part of #2681

We are unable to update firmware from TrezorActions.js. This is believed
to be due to the size of the firmware and the limitations of this
electron child process. We send the firmware to the main process to push
to the device. The main process uses a temporary connection to trezor in
order to accomplish this.

@JoeGruffins JoeGruffins force-pushed the updatefirmware branch 3 times, most recently from adb9459 to 0309b33 Compare October 31, 2020 03:55
@JoeGruffins JoeGruffins marked this pull request as ready for review October 31, 2020 04:05
@JoeGruffins JoeGruffins force-pushed the updatefirmware branch 2 times, most recently from c9602f0 to 2926e85 Compare November 7, 2020 02:58
matheusd
matheusd reviewed Nov 9, 2020
View reviewed changes
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going in the right direction.

Tested with the manually fixed firmware file (stripping the first 256 bytes of the firmware) and it worked on a physical model one.

Only thing I missed, which I'm not sure it's possible, is some feedback that the firmware install process is being performed.

Also, trying to install the wrong firmware makes the trezor set screen hang without detecting a device disconnection.

app/actions/TrezorActions.js Outdated Show resolved Hide resolved
app/actions/TrezorActions.js Outdated Show resolved Hide resolved
@JoeGruffins
TrezorActions: Allow updating firmware.
2b63981 
We are unable to update firmware from TrezorActions.js. This is believed
to be due to the size of the firmware and the limitations of this
electron child process. We send the firmware to the main process to push
to the device. The main process uses a temporary connection to trezor in
order to accomplish this.
@JoeGruffins
Copy link
Member Author

Also, trying to install the wrong firmware makes the trezor set screen hang without detecting a device disconnection.

Even after disconnecting the device? I am not seeing this. Does the trezor have a message about bad headers?

@JoeGruffins
Copy link
Member Author

Making a simple log of progress, but probably not where we want. For trezor 1 only progress 0 and 100 are announced, but I think model T might give increments in between. I would be good to put a progress bar for the user, but can I make an issue for now?

@matheusd
Copy link
Member

Even after disconnecting the device? I am not seeing this. Does the trezor have a message about bad headers?

Correct. The sequence of events is:

  • Put trezor in bootloader mode and connect it to computer
  • Decrediton detects it
  • Start firmware update with an invalid firmware
  • (button to update firmware still enabled instead of disabled with a loading indicator, btw)
  • Trezor complains about invalid header
  • Disconnect trezor from computer
  • Trezor Setup screen is still showing

After that, reconnecting the trezor device will "flash" the setup screen (i.e. it disappears for a moment, then reapears).

@matheusd matheusd closed this Nov 10, 2020
@matheusd matheusd reopened this Nov 10, 2020
@matheusd
Copy link
Member

Sorry for the erroneous close, misclicked the button.

@matheusd
Copy link
Member

I would be good to put a progress bar for the user, but can I make an issue for now?

Fair enough.

@JoeGruffins
Copy link
Member Author

Thank for the process. I understand now.

@JoeGruffins
Copy link
Member Author

Hopefully this handles failures better.

matheusd
matheusd approved these changes Nov 12, 2020
View reviewed changes
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working great now (modulo the header thing)!

@alexlyp alexlyp merged commit bf54bf8 into decred:master Nov 12, 2020
alexlyp added a commit to alexlyp/decrediton that referenced this pull request Nov 13, 2020
@alexlyp
Revert "TrezorActions: Allow updating firmware. (decred#2816)"
78f36a3 
This reverts commit bf54bf8.
alexlyp added a commit that referenced this pull request Nov 13, 2020
@alexlyp
Revert "TrezorActions: Allow updating firmware. (#2816)" (#2931)
1518822 
This reverts commit bf54bf8.
@guilhermemntt guilhermemntt mentioned this pull request Nov 17, 2020
Open
@xaur
Copy link
Contributor

xaur commented Dec 5, 2020

This was reverted by #2931 due to build errors. Fixed version merged in #2932. (posting to link stuff together)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matheusd matheusd matheusd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JoeGruffins @matheusd @xaur @alexlyp