A0-3946: Fix password saving function not working properly

[rAskVAL]

Issue 1: Password saving feature was not consistent. ✅
Issue 2: If checkbox "Extend the period without password by 15 minutes" was not checked, it locks account and on next sign in asks for password, even if saved password was still in the memory and it didn't expire yet. ✅

Tested in Chrome and FF.

P.S. I edited some code but were not able to test them yet, since nightly wallet was not responding. I will test it again in the morning.

[rAskVAL]

Nightly is down since yesterdays evening :(( I was not able to test new code yet...

[youPickItUp]

For now it seems to fix a half of the issue - the #cachedUnlocks. It seems to me that the other part of it is unlocking a pair. From what I understand a pair is unlocked only in memory. Because of that, when the extension is killed and revived the pair becomes locked again even though the expiry time haven't yet passed.

What still needs to be done (correct me if I'm wrong):

• save a passphrase in the session storage
• on sign request, if the expiration time hasn't passed, unlock a pair
• remove the passphrase when time passes (I think it should be enough to remove it whenever refreshAccountPasswordCache gets called, though if we have time it would be better to use some alarm feature - not sure how supported it is)

One more thing to consider (I'm not sure if it's a must or if we want to ignore it):

https://stackoverflow.com/a/73804636
the bug he mentions seems to be fixed, but we should probably double check if passphrase is accessible from content script and if it is - make it unavailable.
Or maybe just follow the trick he mentions... just in case?

Not sure if the above is doable in our set up 🤷

Note about testing:
It would be nice to make sure that when testing the extension background script gets killed. In FF there is this debugging view:
about:debugging#/runtime/this-firefox

where I can check if background script is running. I don't see an option to kill it, but usually doing something CPU heavy (and / or waiting a bit) makes browser kill it 🤷

[jalooc]

This code would be a nice utility if it was only related to the password cache, but as for the overall storage utility, it's got a bit too much logic related to password saving only. Why: imagine you need the storage for other use cases, so you add business logic of all those use cases to this module; suddenly you end up with a module with business logic mixed up from all sorts of different parts of the application. This is hard to maintain = does not scale well.

I see 2 solutions:

1. either rename this module to indicate it handles the passwords only (you'll also have to add a new utility for the storage because we still want the storage to bo managed from one central place to avoid namespace collisions),
2. [probably my choice] rework this utility to be more generic, business-logic-agnostic - sth like this:

// pseudo-code, not sure about the precise names for the zod types - you'll have to check it; also not sure if I reflected your logic correctly

const namespaces = {
 // the key of the record is the account address
 'passoword-cache': z.record(z.string(), z.object({ password: z.string(), expiresAt: z.number() })
} as const satisfies Record<string, z.ZodShape>

export const setItem = <N extends keyof typeof namespaces>(namespace: N, data: z.TypeOf<typeof namespaces[N]>) => {
  await chrome.storage.session.set({ [namespace]: data });
}

export const getItem = ...

export const removeItem = ...

so you have a type-safe namespace declaration with the shapes definition for each of those namespaces.

The drawback is that you won't have the logic for appending addresses inside of it. So you can still create yet another utility using the session utility or just use the session utility directly (whatever's cleaner). While it might sound like more work than necessary, it's not really - it'll amount to the same volume of code, but we'll have it nicely decoupled based on the jobs it does.

[jalooc]

I'd switch to the if statement - it's more descriptive and does not really take more code. Let's leave short-circuiting expressions for cases in which if is not allowed.

[youPickItUp]

It seems that you duplicate changes already included in main. In commit f7b7a83 you change ts-node to tsx. I think the clean solution would be to rebase this branch on main. The change should not pop up in diff

[youPickItUp]

A few improvements I noticed, looks good apart from that 🚀

[youPickItUp]

More nits