fix(extension): #207: fix resetting the fullSyncHeight in local storage after cache clearing #217
Conversation
…ge after cache clearing
|
|
||
| const handleCacheClear = () => { | ||
| setLoading(true); | ||
|
|
||
| void (async function () { | ||
| await setFullSyncHeight(0); |
There was a problem hiding this comment.
question: Does a height synced of 0 mean that the genesis state has been synced? I actually think the data structure is more like:
fullSyncHeight = undefined --> no syncing has been completed
fullSyncHeight = 0 --> genesis state synced
fullSyncHeight = <number> --> that block number has been synced
Can you validate this? May help checking the logic of the loading bar to see what text shows under which condition.
There was a problem hiding this comment.
Both 0 and undefined mean loading, number means syncing, and latestKnownBlockHeight - fullSyncHeight <= 10 means synced
There was a problem hiding this comment.
Updated zeros to undefined
There was a problem hiding this comment.
to reiterate, this sufficiently addresses the symptom, but doesn't fully resolve the issue as the root cause is deeper. Alternatively, we could modify the promise in the message listener with await Promise.allSettled([localExtStorage.remove('params'), indexedDb.clear(), chrome.storage.local.remove('fullSyncHeight')]), but they're functionally identical.
Specifically, the deeper problem lies in the fact that the listeners responsible for initializing the content scripts are blocked by the service worker while it synchronizes the genesis block. I believe there's no way to persist the wallet's connection state across page refreshes or extension reloads because the MessagePort is an in-memory object. So when the service worker restarts via chrome.runtime.reload(), it's waiting for the MessagePort to be initialized, and redirects to the onboarding screen until that's finished.
|
|
||
| const handleCacheClear = () => { | ||
| setLoading(true); | ||
|
|
||
| void (async function () { | ||
| await setFullSyncHeight(undefined); |
There was a problem hiding this comment.
issue: I don't think this will work as doing chrome.storage.set({ "fullSyncHeight": undefined }) is a no-op. Can you confirm with a test? May help reviewing the docs as well: https://developer.chrome.com/docs/extensions/reference/api/storage#type-StorageArea.
There was a problem hiding this comment.
Truly has some problems, reverted to the Tal's initial solution with allSettled
| await Promise.allSettled([ | ||
| localExtStorage.remove('params'), | ||
| indexedDb.clear(), | ||
| chrome.storage.local.remove('fullSyncHeight'), |
There was a problem hiding this comment.
Ah, unfortunately using the chrome storage api directly is not typesafe and could cause bugs with old Prax versions that have not undergone db migrations. Can you follow up with a PR to do:
- chrome.storage.local.remove('fullSyncHeight'),
+ localExtStorage.remove('fullSyncHeight'),
Fixes #207
I understand that the issue was bigger but at least this PR fixes the symptom of incorrect cache clearing, which is the sync bar getting stuck in the incorrect state.
Read Tal's comment #207 (comment) on the "data objects could not be cloned" error – we should perhaps plan the specific set tasks to deal with it.
However, for now I decided that it might not hurt to add a "reload page" button next to the error messages in Minifront: penumbra-zone/web#1861