perf: keep casper-js-sdk off the wallet startup path (WALLET-1421) - #63
Open
Comp0te wants to merge 5 commits into
Open
perf: keep casper-js-sdk off the wallet startup path (WALLET-1421)#63Comp0te wants to merge 5 commits into
Comp0te wants to merge 5 commits into
Conversation
casper-js-sdk ships a single prebuilt UMD bundle (dist/lib.web.js, no module field, no import condition, no sideEffects flag), so one value import costs the whole ~900 KB parsed and nothing can be shaken back out. It was reachable from every wallet surface at render time, dominated by getAccountHashFromPublicKey, which runs synchronously while the home screen renders and so cannot be deferred behind a dynamic import. SDK-free account hash: - New utils/casperSdk/accountHash.ts derives the account hash with @noble/hashes (already in the bundle) as blake2b-256(algorithmName || 0x00 || publicKeyBytes). - Validation stops exactly where PublicKey.fromHex's does — the hex shape only. fromHex routes through fromBuffer, which calls the ED25519 and SECP256K1 constructors rather than the fromBytes factories, so it never checks that the key is a point on the curve. Adding a curve check would reject keys the SDK accepts, i.e. break parity, which is the property that keeps funds from going to the wrong address. - accountHash.test.ts uses casper-js-sdk itself as the oracle: fixed vectors, property-based parity over arbitrary keys of both algorithms, upper-case hex, and eleven malformed inputs asserting the SDK's own error messages. Network constants: - domain/constants/casperNetwork.ts imports CasperNetworkName as a type and spells the chain names out literally. The Record<CasperNetworkName, CasperNetwork> annotation still checks the keys against the SDK enum, so drift fails the build. Splittable barrel: - utils/casperSdk/index.ts becomes a pure re-export over accountHash, network, blockExplorer and cep-nft-transfer, each deep-importable. The SDK-free modules import specific domain modules instead of the domain barrel. cep-nft-transfer stays re-exported from the package root, since both clients import makeNftTransferTransaction and NFTTokenStandard from there. Tree shaking: - Declare "sideEffects": false. Audited first: no bare side-effect imports, no global or prototype mutation, no top-level expression statements anywhere in src/. Domain layer: - Four imports used only in type position were value imports and pulled the SDK in: eip712/repository.ts (PrivateKey), common/http/data-provider.ts (error-type aliases via the domain barrel), and onRamp/entities.ts plus accountInfo/repository.ts, which reach into data/repositories. With those as import type, the whole src/domain barrel is SDK-free at runtime. Regression guard: - sdk-free-modules.test.ts walks the static import graph of each SDK-free entry point and fails if any runtime import reaches casper-js-sdk, ignoring import type. Tree shaking can otherwise hide a regression here until it shows up as bundle size in another repo. data/dto/validators.ts still imports the SDK's delegation-amount constants and is unchanged. Signed-off-by: Dmytro Vynnyk <simbiatoff@gmail.com>
…LET-1421) The previous commit took casper-js-sdk off the account-hash and network constant paths, but a wallet client still linked it the moment it built its repositories: setupRepositories() constructs all ten in one call, and TxSignatureRequestRepository, EIP712Repository and ValidatorsRepository each reach the SDK. No amount of tree shaking on the client side can separate constructions that happen in the same function. Split the factory: - setupData.ts — setupDataRepositories(), the eight repositories a home screen renders from. It also returns the httpDataProvider and the resolved logger, so the signing half can share them. - setupSigning.ts — setupSigningRepositories(), txSignatureRequest and eip712, taking those shared pieces as parameters. - setup.ts — setupRepositories() now composes both. Its return shape and parameters are unchanged, so no consumer breaks; it links the SDK, as it always did. Validators: - data/dto/validators.ts imported DEFAULT_MINIMUM_DELEGATION_AMOUNT and DEFAULT_MAXIMUM_DELEGATION_AMOUNT from the SDK — ~900 KB for two numbers. Declared locally; validators.test.ts pins them against the SDK's own values, so an upstream change fails the build here. Barrels: - utils/casperSdk/index.ts and utils/eip712/index.ts no longer re-export cep-nft-transfer and eip712/sign. Both barrels are reached from the utils barrel that most of src/data imports, so those two re-exports made every DTO a transitive SDK importer whenever the host build does not tree-shake. They are re-exported from the package root instead — the public API is byte-for-byte what it was. - The SDK-free repositories and DTOs now import specific modules rather than the dto/utils/repositories barrels, and the API response types they pull from ../repositories are import type. Regression guard: - sdk-free-modules.test.ts covers src/setupData.ts, so a value import that reaches the SDK from any of the eight repositories fails the test run rather than showing up as bundle size in another repo. Signed-off-by: Dmytro Vynnyk <simbiatoff@gmail.com>
casper-js-sdk 5.0.12 -> 5.0.13. The release is dependency and packaging
hygiene only ("No source change was required"): it drops the unused glob
production dependency along with the abandoned inflight tree, raises its
own axios floor to ^1.19.0, and restricts the published tarball to dist,
resources and the docs via a files whitelist. No API surface moved, so no
source change was needed here either.
Clear the six high advisories that already failed the CI audit gate on
master, independently of the SDK bump. All six are dev-only, reached
through eslint, typescript-eslint and istanbul, and none ship to library
consumers. They are fixed with patch-level bumps, so no new entries in
npmAuditIgnoreAdvisories were needed:
brace-expansion 1.1.16 -> 1.1.18 GHSA-mh99-v99m-4gvg, GHSA-rgw5-rvv9-x895
brace-expansion 5.0.7 -> 5.0.9 GHSA-mh99-v99m-4gvg, GHSA-rgw5-rvv9-x895
js-yaml 3.15.0 -> 3.15.1 GHSA-5p4m-2wfm-xmqj
js-yaml 4.3.0 -> 4.3.1 GHSA-5p4m-2wfm-xmqj
Both packages exist in two major lines in the tree, so these resolutions
are range-scoped rather than bare. A bare "brace-expansion": "^5.0.9"
would drag minimatch@3's ^1.1.7 request onto an incompatible major.
Refresh the pre-existing resolutions while here:
axios ^1.18.1 -> ^1.19.0 latest stable, and the pin was silently
rewriting casper-js-sdk's declared ^1.19.0
back down to 1.18.1
bn.js ^5.2.3 -> ^5.2.5 latest 5.x; the lockfile was holding 5.2.3
even though the range already allowed it
elliptic 6.6.1 removed
The elliptic pin was dead: nothing in the tree depends on elliptic. It
survives only in casper-js-sdk's own "overrides" field, which is not
honored for consumers, and the SDK's runtime crypto is @noble/curves,
@noble/secp256k1 and @noble/ed25519. The pin was also counterproductive
- an exact pin to 6.6.1, which carries CVE-2025-14505 and is the newest
published version, so it would have frozen elliptic on a vulnerable
release had it ever re-entered the tree.
form-data stays at ^4.0.6: already the latest, and useful as a floor
above the ^4.0.5 that axios asks for.
bn.js remains a bare resolution, which collapses asn1.js's ^4.0.0 request
onto the 5.x line. That is left as-is deliberately: bn.js 4.12.5 and
5.2.5 are both clean, so the collapse is a dedupe decision rather than a
security one, and asn1.js sits in the SDK's key-parsing path where
introducing a second bignum implementation carries more risk than the
tidiness is worth.
yarn npm audit --all --severity high --recursive now exits 0 (was 1).
502 tests across 47 suites pass, tsc and eslint clean.
Signed-off-by: Dmytro Vynnyk <simbiatoff@gmail.com>
Run lint, type-check and tests on Node 22 only. With a single version left the matrix is redundant, so it is replaced by a pinned node-version and the now-always-true `matrix.node == '22'` guards on the audit and coverage steps are removed. The job name is unchanged so branch protection keeps matching. Signed-off-by: Dmytro Vynnyk <simbiatoff@gmail.com>
casper-js-sdk 5.0.13 -> 5.1.0. Dependency hygiene only: the release notes
state there is no runtime or API change, dist/ behaves as in 5.0.13 and
engines.node stays ">=18". Its production dependencies move within their
majors (@ethersproject/{bignumber,bytes,constants}, @noble/{curves,hashes,
ed25519,secp256k1}, bn.js, humanize-duration, ts-results, typedjson) with
no source change upstream, so none was needed here either.
The SDK also drops four unused production dependencies, which leaves our
tree smaller: node-fetch (and with it whatwg-url, tr46 and
webidl-conversions), @scure/bip32, @scure/bip39 and reflect-metadata.
reflect-metadata is the only removal with a consumer-visible edge: code
that relied on the SDK to pull it in must now declare it. Nothing under
src/, index.ts or scripts/ imports it, and typedjson's reflection path is
unused because every SDK decorator declares its type explicitly.
The existing resolutions still hold. bn.js ^5.2.5 sits above the ^5.2.4
the SDK now asks for, and axios ^1.19.0 matches its declared range.
yarn npm audit --all --severity high --recursive exits 0. 502 tests across
47 suites pass, tsc and eslint clean.
Signed-off-by: Dmytro Vynnyk <simbiatoff@gmail.com>
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
1 participant
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.
Description
Keeps
casper-js-sdkoff the code paths a wallet client touches at startup. The SDK ships a single prebuilt UMD bundle (dist/lib.web.js— nomodulefield, noimportcondition, nosideEffectsflag), so one value import costs the whole ~900 KB parsed and nothing can be shaken back out.SDK-free account hash.
getAccountHashFromPublicKeyruns synchronously while the home screen renders, so it cannot be deferred behind a dynamic import.utils/casperSdk/accountHash.tsnow derives the hash with@noble/hashes(already in the bundle) asblake2b-256(algorithmName || 0x00 || publicKeyBytes). Validation stops exactly wherePublicKey.fromHex's does — the hex shape only.fromHexroutes throughfromBuffer, which calls the ED25519 and SECP256K1 constructors rather than thefromBytesfactories, so it never checks the key is a point on the curve. Adding a curve check would reject keys the SDK accepts, breaking the parity property that keeps funds from going to the wrong address.Network constants.
domain/constants/casperNetwork.tsimportsCasperNetworkNameas a type and spells the chain names out literally. TheRecord<CasperNetworkName, CasperNetwork>annotation still checks the keys against the SDK enum, so drift fails the build.Split repository factory.
setupRepositories()constructed all ten repositories in one call, and three of them reach the SDK — no client-side tree shaking can separate constructions that happen in the same function. It is now composed fromsetupDataRepositories()(the eight a home screen renders from, which also returns the sharedhttpDataProviderand resolved logger) andsetupSigningRepositories(). Its return shape and parameters are unchanged, so no consumer breaks.Barrels and type-only imports.
utils/casperSdkandutils/eip712no longer re-export the SDK-linked modules; both are reached from the utils barrel most ofsrc/dataimports, which made every DTO a transitive SDK importer when the host build does not tree-shake. They are re-exported from the package root instead, so the public API is byte-for-byte what it was. Four domain imports used only in type position becameimport type, making the wholesrc/domainbarrel SDK-free at runtime."sideEffects": falseis declared, audited beforehand for bare side-effect imports, global or prototype mutation, and top-level expression statements.Regression guard.
sdk-free-modules.test.tswalks the static import graph of each SDK-free entry point and fails if any runtime import reachescasper-js-sdk, ignoringimport type. Tree shaking can otherwise hide a regression until it surfaces as bundle size in another repo.Dependencies.
casper-js-sdk5.0.12 → 5.0.13 — a dependency and packaging hygiene release with no API surface change, which drops the unusedglobproduction dependency along with the abandonedinflighttree. Theresolutionsblock is refreshed alongside it: patch-level bumps forbrace-expansionandjs-yamlthat clear six high advisories,axiosandbn.jsraised to latest, and a deadellipticpin removed.Motivation
The SDK was reachable from every wallet surface at render time, so both wallet clients paid for the full bundle before showing anything, and no amount of downstream tree shaking could recover it. The two entry points that forced this — a synchronous account-hash call and a single-call repository factory — are addressed directly rather than worked around.
Separately, the six high advisories were already failing
yarn npm audit --all --severity high --recursiveonmaster, so the audit gate added in #61 is red on the default branch today. They are all dev-only, reached through eslint, typescript-eslint and istanbul, and none ship to library consumers.Related issues
Refs WALLET-1421
Notes for reviewers
data/dto/validators.tsdeclaresDEFAULT_MINIMUM_DELEGATION_AMOUNTandDEFAULT_MAXIMUM_DELEGATION_AMOUNTlocally rather than importing them — ~900 KB for two numbers.validators.test.tspins them against the SDK's own values, so an upstream change fails the build here.accountHash.test.tsusescasper-js-sdkitself as the oracle: fixed vectors, property-based parity over arbitrary keys of both algorithms, upper-case hex, and eleven malformed inputs asserting the SDK's own error messages. The parity property is the part worth scrutinising.master.brace-expansionandjs-yamlresolutions are range-scoped, not bare, because both exist in two major lines in the tree. A bare"brace-expansion": "^5.0.9"would dragminimatch@3's^1.1.7request onto an incompatible major.bn.jsis left as a bare resolution, which collapsesasn1.js's^4.0.0request onto the 5.x line. This is deliberate: 4.12.5 and 5.2.5 are both clean, so the collapse is a dedupe decision rather than a security one, andasn1.jssits in the SDK's key-parsing path where a second bignum implementation carries more risk than the tidiness is worth.