sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

decision: Sepolia support contradicts "Non-Goals for 1.0", and isMetaMask names a competitor in shipped code #165

New Issue
Open
opened 2026-08-09 03:47:31 +02:00 by clawbot · 0 comments
clawbot commented 2026-08-09 03:47:31 +02:00
Collaborator
Copy Link

Two questions I can't resolve from the repo's own documents, because in each
case the code and a stated policy contradict each other and either side could
legitimately be the one that gives. Both need your call before I dispatch work.
Everything else in the 1.0.0 milestone is unblocked and proceeding.

Question 1 — Sepolia versus "Non-Goals for 1.0: Multi-chain support"

README.md:871 states, under Non-Goals for 1.0:

> Multi-chain support (Ethereum mainnet only)

But multi-chain is shipped and user-facing. There is a network selector in
Settings (src/popup/views/settings.js:150-153, :263-272), Sepolia is a
first-class network (src/shared/networks.js:19-28), there is a whole
src/shared/chainSwitch.js, and the background implements the dApp-facing
wallet_switchEthereumChain (src/background/index.js:343-375). Testnet
support landed deliberately in #137, with a TESTNET tag and debug banner in
#143, and USD display is suppressed on testnets (#142).

So the feature is intentional and recent; the Non-Goal line is what looks
stale. But "Ethereum mainnet only" may have meant "no additional mainnets"
all along, with testnets always considered in scope.

Options

  • (a) Update the README. Reword the Non-Goal to something like "Additional
    mainnet chains (Ethereum mainnet plus Sepolia testnet only)", making explicit
    that testnets are in scope and other mainnets are not. Nothing else changes.
  • (b) Remove Sepolia for 1.0. Rip out the network selector, networks.js
    testnet entries, chainSwitch.js, and the wallet_switchEthereumChain
    handler; ship mainnet-only and reintroduce testnets after 1.0.
  • (c) Keep both as-is and accept the documentation contradiction until
    after 1.0.

My recommendation: (a). The feature is deliberate, useful for testing the
wallet itself, and was built across three separate issues. The Non-Goal was
about not becoming a multi-chain portfolio wallet, and a testnet does not
violate that spirit. (b) throws away working, recently-landed code and would
make the wallet harder to test safely; (c) leaves the authoritative document
knowingly wrong, which this repo does not otherwise tolerate.

If you pick (a) I will fold the wording change into the README docs issue
(#164). If you pick (b) I will file a removal issue in the 1.0.0 milestone.

Question 2 — isMetaMask: true in the injected provider

src/content/inpage.js:68:

isMetaMask: true, // compatibility — many dApps check this

This directly violates a policy stated twice: README.md:904-905 ("We don't
mention 'the other wallet' by name in code or documentation") and RULES.md:121
("No competitor mentioned by name in code or documentation").

It is also, as the comment says, load-bearing. A substantial number of dApps
gate their connect path on this exact property, and it has become a de-facto
part of the injected-provider interface. Removing it will silently break
connectivity on real sites — which is a direct hit to the project's core
purpose.

I'm not willing to decide this one either way: removing it knowingly breaks
users, and keeping it knowingly breaks a policy you wrote.

Options

  • (a) Keep it, and carve out an explicit exception. Add a code comment
    citing the policy and explaining that this is an interface-compatibility
    identifier rather than a reference to the product, and note the exception in
    the README Policies section. This mirrors how the Crypto Policy
    (README.md:658-660) already handles sanctioned exceptions.
  • (b) Remove it. Fully policy-clean; breaks an unknown but non-trivial
    number of dApps.
  • (c) Make it user-configurable. A Settings toggle, defaulting to on, so
    users who want a policy-pure provider can turn it off. Costs a setting and a
    support burden for a problem most users will never think about.

My recommendation: (a). The string here is a protocol-level flag that
dApps feature-detect, not marketing copy or a comparison — it is the same
category as a User-Agent containing "Mozilla". The policy's intent is that we
don't define ourselves against a competitor in prose; an interop shim is a
different thing, and the honest fix is to document the exception rather than
pretend it isn't there. (b) sacrifices the product's actual purpose for a
documentation principle; (c) adds a setting almost nobody can evaluate.

If you pick (a) I will file a small issue to add the exception comment and the
README note. (b) or (c) I will file accordingly.

Assigning to you for these two calls. No other work is blocked on it.

Two questions I can't resolve from the repo's own documents, because in each case the code and a stated policy contradict each other and either side could legitimately be the one that gives. Both need your call before I dispatch work. Everything else in the 1.0.0 milestone is unblocked and proceeding. --- ## Question 1 — Sepolia versus "Non-Goals for 1.0: Multi-chain support" `README.md:871` states, under **Non-Goals for 1.0**: > Multi-chain support (Ethereum mainnet only) But multi-chain is shipped and user-facing. There is a network selector in Settings (`src/popup/views/settings.js:150-153`, `:263-272`), Sepolia is a first-class network (`src/shared/networks.js:19-28`), there is a whole `src/shared/chainSwitch.js`, and the background implements the dApp-facing `wallet_switchEthereumChain` (`src/background/index.js:343-375`). Testnet support landed deliberately in #137, with a TESTNET tag and debug banner in #143, and USD display is suppressed on testnets (#142). So the feature is intentional and recent; the Non-Goal line is what looks stale. But "Ethereum mainnet only" may have meant "no *additional mainnets*" all along, with testnets always considered in scope. **Options** - **(a) Update the README.** Reword the Non-Goal to something like "Additional mainnet chains (Ethereum mainnet plus Sepolia testnet only)", making explicit that testnets are in scope and other mainnets are not. Nothing else changes. - **(b) Remove Sepolia for 1.0.** Rip out the network selector, `networks.js` testnet entries, `chainSwitch.js`, and the `wallet_switchEthereumChain` handler; ship mainnet-only and reintroduce testnets after 1.0. - **(c) Keep both as-is** and accept the documentation contradiction until after 1.0. **My recommendation: (a).** The feature is deliberate, useful for testing the wallet itself, and was built across three separate issues. The Non-Goal was about not becoming a multi-chain portfolio wallet, and a testnet does not violate that spirit. (b) throws away working, recently-landed code and would make the wallet harder to test safely; (c) leaves the authoritative document knowingly wrong, which this repo does not otherwise tolerate. If you pick (a) I will fold the wording change into the README docs issue (#164). If you pick (b) I will file a removal issue in the 1.0.0 milestone. --- ## Question 2 — `isMetaMask: true` in the injected provider `src/content/inpage.js:68`: ```js isMetaMask: true, // compatibility — many dApps check this ``` This directly violates a policy stated twice: `README.md:904-905` ("We don't mention 'the other wallet' by name in code or documentation") and `RULES.md:121` ("No competitor mentioned by name in code or documentation"). It is also, as the comment says, load-bearing. A substantial number of dApps gate their connect path on this exact property, and it has become a de-facto part of the injected-provider interface. Removing it will silently break connectivity on real sites — which is a direct hit to the project's core purpose. I'm not willing to decide this one either way: removing it knowingly breaks users, and keeping it knowingly breaks a policy you wrote. **Options** - **(a) Keep it, and carve out an explicit exception.** Add a code comment citing the policy and explaining that this is an interface-compatibility identifier rather than a reference to the product, and note the exception in the README Policies section. This mirrors how the Crypto Policy (`README.md:658-660`) already handles sanctioned exceptions. - **(b) Remove it.** Fully policy-clean; breaks an unknown but non-trivial number of dApps. - **(c) Make it user-configurable.** A Settings toggle, defaulting to on, so users who want a policy-pure provider can turn it off. Costs a setting and a support burden for a problem most users will never think about. **My recommendation: (a).** The string here is a protocol-level flag that dApps feature-detect, not marketing copy or a comparison — it is the same category as a `User-Agent` containing "Mozilla". The policy's intent is that we don't define ourselves against a competitor in prose; an interop shim is a different thing, and the honest fix is to document the exception rather than pretend it isn't there. (b) sacrifices the product's actual purpose for a documentation principle; (c) adds a setting almost nobody can evaluate. If you pick (a) I will file a small issue to add the exception comment and the README note. (b) or (c) I will file accordingly. --- Assigning to you for these two calls. No other work is blocked on it.
sneak was assigned by clawbot 2026-08-09 03:47:31 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#165
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?