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

fix(connect): emit changed event with state change in keepSession #15276

Merged
merged 2 commits into from
Nov 6, 2024
Merged

fix(connect): emit changed event with state change in keepSession #15276

merged 2 commits into from
Nov 6, 2024

Conversation

martykan
Copy link
Member

@martykan martykan commented Nov 6, 2024 •
edited by mroz22
Loading

Description

We update sessionId in saved device state in Suite from receiving a DEVICE.CHANGED event.
However this event wasn't being sent for calls with keepSession: true, such as during account discovery, this meant that if the passphrase was entered during those calls, it wasn't saved.

Related Issue

Resolve #15226

@martykan martykan added the build-desktop This will trigger the build of desktop apps for your PR label Nov 6, 2024
@mroz22
Copy link
Contributor

mroz22 commented Nov 6, 2024

btw what about skipFinalReload? Does it affect this case too?

@komret komret added the release Will be included in the upcoming release. Needs to be backported to the release branch. label Nov 6, 2024
@martykan
Copy link
Member Author

martykan commented Nov 6, 2024

Cardano doesn't work so I assume there is an issue with skipFinalReload

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 6, 2024 •
edited
Loading

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 10
  • More info

Learn more about 𝝠 Expo Github Action

@martykan
Copy link
Member Author

martykan commented Nov 6, 2024

Nevermind, it's a different issue with Cardano, due to the device state being current on subsequent calls

mroz22
mroz22 approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@mroz22 mroz22 left a comment

Choose a reason for hiding this comment

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

looks legit

@martykan martykan added build-desktop This will trigger the build of desktop apps for your PR and removed build-desktop This will trigger the build of desktop apps for your PR labels Nov 6, 2024
@martykan martykan added build-desktop This will trigger the build of desktop apps for your PR and removed build-desktop This will trigger the build of desktop apps for your PR labels Nov 6, 2024
@martykan martykan added build-desktop This will trigger the build of desktop apps for your PR and removed build-desktop This will trigger the build of desktop apps for your PR labels Nov 6, 2024
@martykan martykan marked this pull request as ready for review November 6, 2024 17:39
@martykan martykan requested review from a team and szymonlesisz as code owners November 6, 2024 17:39
@martykan martykan merged commit e8a0be7 into develop Nov 6, 2024
69 checks passed
@martykan martykan deleted the fix/devicestate-emit-change branch November 6, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroz22 mroz22 mroz22 approved these changes

@szymonlesisz szymonlesisz Awaiting requested review from szymonlesisz szymonlesisz is a code owner

Assignees
No one assigned
Labels
build-desktop This will trigger the build of desktop apps for your PR release Will be included in the upcoming release. Needs to be backported to the release branch.
Projects
Issues Suite
Status: ✅ Approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suite needs passphrase input for every action after reconnecting to view only wallet
3 participants
@martykan @mroz22 @komret