Stop shipping a Trezor path that cannot work - #280
Merged
Conversation
Trezor signing has been broken end to end in this fork. `TrezorConnect` was called in `initHW` and `signTxHW`, but there is no Trezor package in package.json or node_modules, no webpack external and no script tag, so choosing Trezor raised `ReferenceError: TrezorConnect is not defined`. `globals.d.ts` declared it and four Cardano* enums as ambient `any`, which hid the missing bindings from `npm run typecheck` as well as from readers. ESLint reported all fifteen as no-undef errors, but this repo has no lint gate and `.eslintignore` excludes `extension/index.js` anyway. Trezor is not a supported device yet, so remove the surface rather than wire it up: the trezorTx tab and its HTML page, the Trezor widget whose iframe pointed at a popup file that is not in the repo, the content script whose port had no listener in the background worker, the two webpack entries, and `txToTrezor`/`outputsToTrezor` in util.js (427 lines that only that path called). The manifest drops the Trezor content script and the `frame-src https://connect.trezor.io/` allowance with it, which narrows the extension CSP. An account already stored as `trezor-*` must not be stranded, so keep `HW.trezor`, the vault prefix check that makes it a hardware account, and its logo. Every signing entry point now refuses it with one shared message naming Keystone, Ledger and software accounts, instead of a ReferenceError. The hardware dialog also stops calling anything that is not a Ledger a Trezor.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of two PRs for approved pitch #7. The owner's decision on the pitch's open question was do not support Trezor yet, so this removes the dead surface rather than wiring it up. PR 2 then turns ESLint into a CI gate, which is only possible once these errors are gone.
Trezor has never worked in this fork
TrezorConnectwas called in two places:src/api/extension/index.js:1345—initHWsrc/api/extension/signing.js:579—signTxHWThere is no Trezor package in
package.jsonornode_modules, no webpackexternals, and no<script>tag. Choosing Trezor raisedReferenceError: TrezorConnect is not defined.Three things kept that invisible:
src/api/globals.d.tsdeclaredTrezorConnectand fourCardano*enums as ambientany, sonpm run typecheckpassed over bindings that do not exist at runtime.no-undeferrors — but there is no lint stage inJenkinsfileorci.yml..eslintignoreexcludessrc/api/extension/index.jsoutright.The surrounding code was equally dead:
trezorWidget.jsxopened an iframe ontoTrezor/popup.html, which is not in the repo, andtrezorContentScript.jsconnected atrezor-connectport that no background listener answers. The hardware picker inhw.jsxhas only ever offered Keystone and Ledger, so no user can create a Trezor account in this build.Removed
txToTrezor+outputsToTrezor(util.js, 427 lines, called only from the Trezor branch); thesignTxHWTrezor branch; theinitHWTrezor branchglobals.d.tstrezorTxandtrezorContentScriptwebpack entries, thetrezorTx.htmlpagetabs/trezorTx.jsx,components/trezorWidget.jsx,pages/Content/trezorContentScript.js,pages/Tab/trezorTx.htmlframe-src https://connect.trezor.io/— which narrows the extension CSPTAB.trezorTx, the unusedtrezorRef/<TrezorWidget>inhw.jsx, now-unusedTAB/createTabimports in four filesNet: 847 deletions, 160 insertions.
Kept, deliberately
An account already stored as
trezor-*must not be stranded — if it stopped being recognised as hardware it would have no password path either. SoHW.trezor, theisHardwareAccountIndexprefix check invault.js,HW_LOGO_DEVICES, and the Trezor logo all stay. Such an account still loads and still shows its own device name.What changes for it is the failure: every entry point (
signTxHW,initHW, the hardware confirm dialog, and the send / staking / governance / transaction-builder paths) now throws one sharedTREZOR_UNSUPPORTEDmessage naming Keystone, Ledger and software accounts, instead of aReferenceError.confirmModalalso stops labelling anything that is not a Ledger as a "Trezor".Verification
trezor-unsupported.test.js(6 tests): the message names alternatives;trezor-*is still a hardware account and keeps its logo; all seven entry points refuse with the message; noTrezorConnectorCardano*enum remains; and no Trezor bundle, page, or iframe permission ships.mobile-layoutlayout contracts and thestake-unified-walletassertions that guarded files which no longer exist.e2e/send-csp.spec.jsCSP mirror updated to match the manifest.npm run typecheckclean, and ESLint errors in hand-written code go 15 → 0 (the 63 remaining are all in generatedsrc/wasm/, which PR 2 ignores).README.md,AGENTS.md,docs/TX_ARCHITECTURE_PLAN.md,MOBILE.md, and the CSL-bridge rule.