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 clawbot2026-08-09 03:47:31 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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:871states, 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 afirst-class network (
src/shared/networks.js:19-28), there is a wholesrc/shared/chainSwitch.js, and the background implements the dApp-facingwallet_switchEthereumChain(src/background/index.js:343-375). Testnetsupport 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
mainnet chains (Ethereum mainnet plus Sepolia testnet only)", making explicit
that testnets are in scope and other mainnets are not. Nothing else changes.
networks.jstestnet entries,
chainSwitch.js, and thewallet_switchEthereumChainhandler; ship mainnet-only and reintroduce testnets after 1.0.
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: truein the injected providersrc/content/inpage.js:68:This directly violates a policy stated twice:
README.md:904-905("We don'tmention '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
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.number of dApps.
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-Agentcontaining "Mozilla". The policy's intent is that wedon'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.