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: align extension storage 'set' and 'observeAll' with pouchdb storage behavior #1597

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

mkazlauskas
Copy link
Member

@mkazlauskas mkazlauskas commented Dec 16, 2024

Checklist


Proposed solution

LW-11986

deleting a wallet does:

  1. delete it from repository
  2. check remaining wallets, activate if some wallet exists

there was a bug where step 2. would find the wallet that was just deleted and try to activate it, because extension storage 'observeAll' emits slightly later

953d550

LW-11993

set()/get() applies toSerializableObject/fromSerializableObject respectively
storageChange$ did not, resulting in emitting slightly different objects
that were not processed by fromSerializableObject util this broke disabling accounts that were just enabled
and possibly some other functionality

487502c

@mkazlauskas mkazlauskas requested a review from a team as a code owner December 16, 2024 15:10
@pczeglik-iohk
Copy link
Contributor

pczeglik-iohk commented Dec 16, 2024

Allure Report

allure-report-publisher generated test report!

processReports: ✅ test report for a657b3a4

passed failed skipped flaky total result
Total 33 0 4 0 37

Copy link
Contributor

@mirceahasegan mirceahasegan left a comment

Choose a reason for hiding this comment

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

Great detective work @mkazlauskas 🧙‍♂️

…age behavior

deleting a wallet does:
1. delete it from repository
2. check remaining wallets, activate if some wallet exists

there was a bug where step 2. would find the wallet that was
just deleted and try to activate it, because extension storage
'observeAll' emits an slightly later
set()/get() applies toSerializableObject/fromSerializableObject respectively
storageChange$ did not, resulting in emitting slightly different objects
that were not processed by fromSerializableObject util

this broke disabling accounts that were just enabled
and possibly some other functionality
in order to recover from wallet activation bugs
for example, a crash after wallet deactivation but before
activating another wallet

fix: do not explicitly deactivate wallet when switching to another

explicitly deactivating will trigger a recovery mechanism
that will activate any wallet
Copy link

Copy link
Member

@rhyslbw rhyslbw left a comment

Choose a reason for hiding this comment

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

Nice work @mkazlauskas

@mkazlauskas mkazlauskas merged commit 6fe9baf into main Dec 18, 2024
43 of 49 checks passed
@mkazlauskas mkazlauskas deleted the fix/delete-wallet branch December 18, 2024 11:15
mkazlauskas added a commit that referenced this pull request Dec 18, 2024
…age behavior (#1597)

* fix: align extension storage 'set' and 'observeAll' with pouchdb storage behavior

deleting a wallet does:
1. delete it from repository
2. check remaining wallets, activate if some wallet exists

there was a bug where step 2. would find the wallet that was
just deleted and try to activate it, because extension storage
'observeAll' emits an slightly later

* fix: apply fromSerializableObject to objects emitted from storageChange$

set()/get() applies toSerializableObject/fromSerializableObject respectively
storageChange$ did not, resulting in emitting slightly different objects
that were not processed by fromSerializableObject util

this broke disabling accounts that were just enabled
and possibly some other functionality

* fix: activate any wallet when none is active

in order to recover from wallet activation bugs
for example, a crash after wallet deactivation but before
activating another wallet

fix: do not explicitly deactivate wallet when switching to another

explicitly deactivating will trigger a recovery mechanism
that will activate any wallet
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