decision: RULES.md states three things the code contradicts #211

Open
opened 2026-08-11 14:33:49 +02:00 by clawbot · 0 comments
Collaborator

Three divergences found by separate workers while verifying documentation against source. RULES.md is owner-only and says the code must change to match it rather than the reverse, so each needs your call on which side gives.

1. External services count. The External Communication checklist says the extension contacts "exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API". The code contacts five — src/shared/phishingDomains.js fetches raw.githubusercontent.com, and src/shared/etherscanLabels.js fetches etherscan.io.

2. Password scope. "Password only required for signing operations" is untrue. src/popup/views/deleteWallet.js and the private-key export in src/popup/views/addressDetail.js both require it, and #161 adds a third.

3. Approval persistence. "TX and sign approvals persist across popup close/reopen (toolbar popup)" no longer describes the code: those approvals use windows.create(), and the windows.onRemoved listener resolves them with EIP-1193 code 4001. A stale comment in src/background/index.js in the port-disconnect branch still says "Keep pending — user can reopen the toolbar popup", describing behaviour that no longer exists.

Options

For each: (a) update RULES.md to describe the shipped behaviour, or (b) change the code to match RULES.md.

Recommendation

  • 1 — (a). The two extra services are deliberate features (phishing blocklist, address labels); the checklist is stale. Worth restating as a maintained list rather than a count, so adding a service forces a doc change.
  • 2 — (a). Password-gating destructive and secret-revealing actions is correct and should stay; the rule is simply narrower than practice.
  • 3 — (b) for the comment, (a) for the rule. The stale comment in src/background/index.js is just wrong and should be deleted regardless. Whether approvals SHOULD survive popup close is a real product question: rejecting on window close is the safer default for a wallet, so I would update the rule to match — but that is your call, not mine.

Tell me which way on each and I will open the PR. Nothing is blocked on this.

Three divergences found by separate workers while verifying documentation against source. `RULES.md` is owner-only and says the code must change to match it rather than the reverse, so each needs your call on which side gives. **1. External services count.** The External Communication checklist says the extension contacts "exactly three external services: configured RPC endpoint, CoinDesk price API, and Blockscout block-explorer API". The code contacts five — `src/shared/phishingDomains.js` fetches `raw.githubusercontent.com`, and `src/shared/etherscanLabels.js` fetches `etherscan.io`. **2. Password scope.** "Password only required for signing operations" is untrue. `src/popup/views/deleteWallet.js` and the private-key export in `src/popup/views/addressDetail.js` both require it, and https://git.eeqj.de/sneak/AutistMask/issues/161 adds a third. **3. Approval persistence.** "TX and sign approvals persist across popup close/reopen (toolbar popup)" no longer describes the code: those approvals use `windows.create()`, and the `windows.onRemoved` listener resolves them with EIP-1193 code 4001. A stale comment in `src/background/index.js` in the port-disconnect branch still says "Keep pending — user can reopen the toolbar popup", describing behaviour that no longer exists. ## Options For each: **(a)** update `RULES.md` to describe the shipped behaviour, or **(b)** change the code to match `RULES.md`. ## Recommendation - **1 — (a).** The two extra services are deliberate features (phishing blocklist, address labels); the checklist is stale. Worth restating as a maintained list rather than a count, so adding a service forces a doc change. - **2 — (a).** Password-gating destructive and secret-revealing actions is correct and should stay; the rule is simply narrower than practice. - **3 — (b) for the comment, (a) for the rule.** The stale comment in `src/background/index.js` is just wrong and should be deleted regardless. Whether approvals SHOULD survive popup close is a real product question: rejecting on window close is the safer default for a wallet, so I would update the rule to match — but that is your call, not mine. Tell me which way on each and I will open the PR. Nothing is blocked on this.
sneak was assigned by clawbot 2026-08-11 14:33:49 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#211