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

fix: Init wallet device. #2581

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Conversation

yanguoyu
Copy link
Collaborator

@yanguoyu yanguoyu commented Feb 4, 2023

sign-message-by-hardwallet.mov

@Cedar67
Copy link
Contributor

Cedar67 commented Feb 6, 2023

An error is found and needs to be located.

Magickbase/neuron-public-issues#101 (comment)

@yanguoyu
Copy link
Collaborator Author

yanguoyu commented Feb 7, 2023

An error is found and needs to be located.

Magickbase/neuron-public-issues#101 (comment)

Fixed with e9b8939.
After fetching new code, you need to run yarn bootstrap again and retry.

@Cedar67
Copy link
Contributor

Cedar67 commented Feb 9, 2023

An error is found and needs to be located.
Magickbase/neuron-public-issues#101 (comment)

Fixed with e9b8939. After fetching new code, you need to run yarn bootstrap again and retry.

After clicking sign, a new error message is displayed.
The cause of the problem needs to be further located with App v0.5.5 in NanoX.

image

@yanguoyu
Copy link
Collaborator Author

It failed to get the firmware version failed, but it seems not to affect the functions. I have sent an issue to ledger to track it.
LedgerHQ/ledger-live#2583

@Keith-CY
Copy link
Collaborator

An error is found and needs to be located.
Magickbase/neuron-public-issues#101 (comment)

Fixed with e9b8939. After fetching new code, you need to run yarn bootstrap again and retry.

After clicking sign, a new error message is displayed. The cause of the problem needs to be further located with App v0.5.5 in NanoX.

image

Is this error reproducible since we found address for should be set to mainnet? @Cedar67

@yanguoyu
Copy link
Collaborator Author

From LedgerHQ/ledger-live#2583 (comment), I guess getVersion will return success when we don't open any App on ledger. So when we open Nervos, it will throw an exception is normal.

get_version.mov

@Cedar67
Copy link
Contributor

Cedar67 commented Feb 24, 2023

Is this error reproducible since we found address for should be set to mainnet? @Cedar67

This error is not reproducible now.

@Keith-CY
Copy link
Collaborator

Is this error reproducible since we found address for should be set to mainnet? @Cedar67

This error is not reproducible now.

Is this error reproducible since we found address for should be set to mainnet? @Cedar67

This error is not reproducible now.

Cool, then please verify the function, and a conflict should be resolved before being merged @yanguoyu

@Cedar67
Copy link
Contributor

Cedar67 commented Feb 24, 2023

From LedgerHQ/ledger-live#2583 (comment), I guess getVersion will return success when we don't open any App on ledger. So when we open Nervos, it will throw an exception is normal.

Is throwing this exception prompt as expected when the CKB App is not open on Ledger? @yanguoyu

image

@yanguoyu
Copy link
Collaborator Author

From LedgerHQ/ledger-live#2583 (comment), I guess getVersion will return success when we don't open any App on ledger. So when we open Nervos, it will throw an exception is normal.

Is throwing this exception prompt as expected when the CKB App is not open on Ledger? @yanguoyu

image

From my video, I guess yes.

@Keith-CY
Copy link
Collaborator

How is it being tested @Cedar67

Copy link
Contributor

@Cedar67 Cedar67 left a comment

Choose a reason for hiding this comment

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

Verified.
Environment:
Nano X
CKB app v0.5.5

@Keith-CY Keith-CY merged commit 1f19cca into nervosnetwork:develop Feb 28, 2023
@yanguoyu yanguoyu deleted the fix-sign-message branch June 13, 2023 02:24
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.

4 participants