Read settings from one storage snapshot - #1600
Conversation
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
CI Agent Review
No security, runtime-correctness, or architectural-regression issues were reported by any agent. The refactor (single atomic storage snapshot in getSettings()) is behavior-preserving: defaults, presence-aware parsing for the address keys, corruption handling, and websiteAccess sanitization are all unchanged. The one minor concern raised is a style/cohesion observation about the new import edge from settings.ts to methodHandlers.js.
| import { mergeStoredWebsiteMetadata, sanitizeWebsiteAccess } from '../utils/websiteIcons.js' | ||
| import type { SigningAddressPreference, SigningAddressPreferences } from '../types/signerTypes.js' | ||
| import { hasOwnKey } from '../utils/methodHandlers.js' | ||
|
|
There was a problem hiding this comment.
Minor cohesion concern (low severity, non-blocking): this import pulls the generic object-own-key guard hasOwnKey from methodHandlers.js, a module whose purpose is RPC-method dispatch. Importing a general-purpose utility from that module into the settings/storage domain extends coupling across module boundaries and is a cohesion smell. This is noted as pre-existing utility misplacement rather than a regression introduced here, but a generic utils module (e.g. utils/typescript.ts) would be a cleaner home for hasOwnKey.
|
/review |
|
/review |
2 participants
Summary
getSettings()field in onebrowser.storage.local.get()callWhy this matters
Settings writers can update simulation mode, address, and RPC together in one storage call. Previously,
getSettings()performed two reads per field across seven parallel getters, so a concurrent mode change or import could return a torn combination of old and new security settings. That mixed state feeds provider routing, access decisions, and simulation behavior.Validation
bun run test(1,260 passed)bun run setup-chromebun run typecheckbun run lint