Repository navigation
chore(deps): bump script SDKs (arbitrum, cow, dotenv, chalk, bs58, merkletreejs) - #2606
Closed
philippzentner wants to merge 24 commits into
Closed
philippzentner wants to merge 24 commits into
philippzentner wants to merge 24 commits into
Conversation
Node 22 reaches end-of-life on 2027-04-30. Pin CI and .nvmrc to Node 26, raise engines.node to >=24 so developers on the 24 LTS line keep working, and move @types/node to 24.19.2 to match the lowest supported runtime.
tj-actions/changed-files was compromised in March 2025. The Olympix job only needs the added/copied/modified/renamed src/**/*.sol list and a boolean, which dorny/paths-filter (already used elsewhere) provides with identical semantics.
The immutable-registry job used bun-version: latest; use bun-version-file: package.json like every other setup-bun step.
Bump the overrides still needed to the latest safe version within their current major (protobufjs 7.6.6, axios 1.21.0, smol-toml 1.9.1, lodash 4.18.1, katex 0.18.11, ws 8.22.1, ip-address 10.7.5, fast-uri 3.1.9; markdown-it is already at the latest 14.x) and move the direct axios and smol-toml pins with them. Drop socks, validator and handlebars: without the override the tree already resolves a non-vulnerable version. bun audit: 83 -> 60.
Node 24+ matches engines.node; .nvmrc now selects 26. Bun follows the packageManager pin, which CI also uses.
Unify actions/checkout on v7.0.1 (four different v4 SHAs were in use) and move every other external action to its latest release, SHA-pinned with a version comment.
Minor and patch bumps for bun.lock dependencies arrive as one grouped PR per month, with at most 5 open Dependabot PRs. The github-actions entry stays weekly and ungrouped.
Moves the submodule to the v1.17.0 tag commit (f3dae6e) and updates foundry.lock. src artifacts compile to identical bytecode, and forge test gives the same pass/fail set as before.
TypeScript 7 (the native Go port) only ships the tsc binary and an
unstable API under typescript/unstable/*; require('typescript') returns
just the version. The node-runtime and funnel fences, validate-scripts
and several placement tests use the classic compiler API, so 6.0.3 is the
newest release that works.
husky 9: prepare runs `husky` (no `husky install`), and hooks no longer source husky.sh. husky 9's wrapper still runs hooks under `sh -e`, so pre-commit and pre-push keep re-execing themselves under `bash -e`, now behind their own guard variable instead of husky v8's husky_skip_init. lint-staged 17: the pre-commit output filter also drops the new progress lines so failures still show only the tool output.
solhint 6 adds use-natspec, function-max-lines and seven gas-* rules to solhint:recommended, which would raise ~900 warnings on src/. They are turned off to keep the v5 rule set; the other new recommended rules (duplicated-imports, no-immutable-before-declaration, no-unchecked-calls) report nothing on src/ and stay on. no-unused-vars now also checks modifier parameters (solhint 6.0.1) and flags the unused _polymerData parameter of PolymerCCTPFacet's validatePolymerData modifier. Fixing it changes src bytecode, so the rule drops to warn and lint:sol reports 0 errors, 1 warning. .solhint.json is plain JSON and solhint rejects extra keys, so the reasons live here.
.oxfmtrc.json is generated by `oxfmt --migrate=prettier` from .prettierrc.cjs (same options, .prettierignore entries as ignorePatterns). Prettier stays on 2.8 + prettier-plugin-solidity 1.4 for *.sol only: Prettier 3 + plugin 2 would reformat 113 .sol files (49 under src/), and any src/*.sol change alters bytecode metadata. solhint-plugin-prettier stays on 0.0.5 because 0.1.0 needs Prettier 3. Updates the format scripts, lint-staged, the agent format hook, the factory format check, the docs that name the formatter and the VS Code settings template.
Mechanical output of `bun format:fix` after the switch to oxfmt. No .sol file changes. Listed in .git-blame-ignore-revs. Committed with --no-verify: lint-staged's typecheck-files step fails on script/demoScripts/demoChainflip.ts with the same 4 type errors it has on main. oxlint, both fences and typecheck of the other 89 files pass.
- node-gyp 10 -> 13 - plop 3 -> 4 (bun codegen --help and a non-interactive facet run work) - @typechain/ethers-v5 10 -> 11: same 369 files; inputs lose the PromiseOrValue<T> wrapper and overloaded event keys use full tuple signatures. No script uses either; tsc reports the same errors. - markdownlint-cli2 0.23.3 -> 0.24.0 (lint:md clean) - @types/bun 1.2 -> 1.4 - patch-package removed together with its postinstall step: the repo has no patches/ directory, so it never applied anything.
Its header says only humans edit the factory rules, so the switch of its format check to oxfmt is left to a maintainer.
The file has 4 type errors on main, including an undefined lifiDiamondAddress, so they can't be fixed with type-only changes. Reformatting it made validate-scripts type-check it and fail. It is restored byte-for-byte to main and listed in the oxfmt ignorePatterns. lint-staged's oxfmt entry gets --no-error-on-unmatched-pattern so staging an ignored file doesn't fail the commit. Committed with --no-verify: staging the file makes lint-staged's typecheck-files step report its 4 pre-existing errors, which happens for any commit that touches it on main too.
v11 drops PromiseOrValue<T> from common.ts and every input type and renames overloaded event keys. types.yaml publishes this output to lifinance/lifi-contract-types as a patch/minor release, so v11 would break downstream TS consumers. It needs a coordinated major release of lifi-contract-types plus a downstream migration. The typechain output is identical to the lifinance#2604 baseline again.
v4 renamed L1ToL2MessageGasEstimator to ParentToChildMessageGasEstimator and exports it from the package root. estimateAll keeps the same arguments and the l2CallValue field, so the demo script only needs the import and constructor rename.
bs58 6 ships its own types, so the @ts-expect-error on the default import is no longer needed. decode() now returns a Uint8Array instead of a Buffer; the only consumer passes it to viem's toHex, which accepts both.
Output is unchanged: demoGenerateMerkleTree.ts writes a byte-identical outputMerkleProofs.json under 0.3.11 and 0.6.0 (sha256 0ab8f479…), and a comparison over 72 trees (both gasRebates networks plus synthetic trees of 1-70 leaves, including odd counts and a duplicate leaf) gives the same roots and proofs.
chalk 6 only raises the Node floor to 22 (the repo runs Node 26) and changes numeric FORCE_COLOR / ansi256 downsampling, neither of which the scripts use. Kept as an exact pin, like before.
dotenv 17+ logs an "injected env" line on every config() call (to
stderr since 18). Pass { quiet: true } at all 34 explicit config() call
sites, including the facet demo template, so script output stays the
same as on dotenv 16. The 'dotenv/config' side-effect import already
defaults to quiet since 18.0.4. No script uses the removed .env.vault
support or preloading.
Touches signing-path files under script/deploy/safe/ (one-line
config() -> config({ quiet: true }) each).
cow-sdk 7+ takes a provider adapter instead of an ethers signer. The demo now passes a ViemAdapter (@cowprotocol/sdk-viem-adapter) built from the same Arbitrum RPC and private key; the ethers v5 adapter was not used because it peers on @typechain/ethers-v5 11. postSwapOrder now returns an object, so the order id is read from .orderId. The CowShed constants moved to @cowprotocol/sdk-cow-shed and are keyed by version; the helper keeps the 1.0.0 factory and implementation, which are the same addresses cow-sdk 5 exported.
Contributor
|
Important Review skippedToo many files! This PR contains 178 files, which is 78 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
2 of 8 tasks
Contributor
|
Thanks @philippzentner, especially for the merkletreejs equivalence check and the citty notes. CI and secrets don't run on fork PRs, so I've taken this over in #2630, which keeps your 6 commits as they were and is stacked on #2609. Changes on top:
Closing in favour of #2630. |
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.
Builds on #2604 and #2605. Review only the commits listed below; this PR will be rebased once the earlier ones merge.
278c18d225f815d768c0a50a98401a3227f0b7dac11bb8d63decfdWhich Linear task belongs to this PR?
None. This is script dependency maintenance proposed from a fork, so there is no Linear ticket.
Why did I implement it this way?
Only TypeScript scripts, one Handlebars template,
package.jsonandbun.lockchange. No.solfile, nolib/submodule, no Foundry config. Compiledsrcoutput is byte-identical (see Verification).Packages
@arbitrum/sdkL1ToL2MessageGasEstimator→ParentToChildMessageGasEstimator, now imported from the package root instead ofdist/lib/....estimateAllkeeps its arguments and thel2CallValuefield. 1 file (demoArbitrumBridge.ts).bs58@ts-expect-erroris gone.decode()returnsUint8Arrayinstead ofBuffer; the only consumer passes it to viem'stoHex, which accepts both. Round trip of the USDC mint checked.merkletreejschalkFORCE_COLOR/ansi256downsampling, which no script uses. Still an exact pin.dotenvconfig()call (stderr since 18). All 34 explicit calls now pass{ quiet: true }, includingtemplates/facetDemoScript.template.hbs, so script output matches dotenv 16.import 'dotenv/config'is quiet by default since 18.0.4. No script uses the removed.env.vaultsupport or-r dotenv/configpreloading.@cowprotocol/cow-sdkTradingSdktakes a provider adapter.demoPatcher.tsnow passes aViemAdapter(new dependency@cowprotocol/sdk-viem-adapter), built from the same Arbitrum RPC and private key the ethers signer used. I didn't use the ethers v5 adapter because it peers on@typechain/ethers-v511, which #2605 deliberately keeps on 10.postSwapOrdernow returns an object, so the code reads.orderId. TheappCodeas anycast is gone (AppCodeisstringnow). The CowShed constants moved to@cowprotocol/sdk-cow-shed(new dependency) and are keyed by version. The helper uses1.0.0, which resolves to the same factory0x00E989b8…bEF6and implementation0x2CFFA8cf…9F73that cow-sdk 5 exported.@solana/web3.js@solana/spl-token0.4.15 (the latest) declarespeerDependencies: { "@solana/web3.js": "^1.95.5" }, and both demo files usegetAssociatedTokenAddressSyncfrom spl-token with web3.js 1PublicKeys. Both stay at the compatible pair web3.js 1.98.4 + spl-token 0.4.x. Moving to web3.js 3 means replacing spl-token with@solana-program/token.cittynode:util.parseArgs, coerces parsed values to their declared types, made optionals and enums stricter, and is ESM-only. It's used in 96 files, including 25 underscript/deploy/safe/**plus the Tron Safe scripts. A trial install made 11script/deploy/safetests fail. 10 of them are the signing-path flag contracts incli-flags.test.ts(flagIsOnwith repeated, numeric or defaulted flags, value args in kebab and camel spelling) and 1 isnormalizeProposeCalls > repeated string flags parse to arrays.tscalso reported 6 new errors (repair-deployment-records.ts,manageWalletFunds.ts,unpauseAllDiamonds.ts,troncast/send.ts). Those tests pin how Safe/timelock scripts read--dry-run/--execute-style flags, so this needs its own reviewed PR.Merkle output equivalence (
merkletreejs)bunx tsx script/demoScripts/demoGenerateMerkleTree.tswrites a byte-identicalscript/output/outputMerkleProofs.jsonunder 0.3.11 and 0.6.0 (sha2560ab8f479effcfc6a112e3c9f174346b1f7f56386333bde0d85933f3e54a3116eboth times).{ sortPairs: true }. It covered 72 trees: bothgasRebates.jsonnetworks, plus deterministic synthetic trees of 1–70 leaves, odd counts included, with a duplicated leaf from 4 leaves up. It dumped every root and every proof. Both dumps are byte-identical (sha256e159bdb5…af1), and every proof verifies.Signing path
Only
b7dac11bb(dotenv) touches signing-path files. It changes one line in each of these 7 files underscript/deploy/safe/:config()/dotenv.config()→…({ quiet: true }).add-safe-owners-and-threshold.tsconfirm-safe-tx.tsdeploy-safe.tsenqueue-parked-task.tslist-parked-tasks.tslist-pending-proposals.tssafe-utils.tsThat line is the only change the dotenv 18 bump strictly needs: it keeps stderr free of the new "injected env" line. Env loading is otherwise unchanged. Nothing else under
script/deploy/safe/**, ledger/signing helpers or the paths inprotectSecurityRelevantCode.ymlchanges. citty was skipped precisely because it would change signing-path argument parsing.Known pre-existing type errors in touched files (
validate-scripts/ pre-commit)validate-scripts(CIvalidateScripts.ymland the pre-push hook) and the pre-committypecheck-files.shtype-check every changed script. Six of the demo scripts this PR touches already have type errors on the base branch. That's 20 errors, all unrelated to these bumps, and the text is identical on the base apart from line shifts indemoArbitrumBridge.ts:demoArbitrumBridge.ts: 5 errors.config['goerli'].l2GatewayRouterisn't in the config type, andpopulateTransaction.*is possibly undefined. A Goerli-era script.demoChainflip.ts: 4 errors.demoGasZip.ts: 4 errors.demoLidoWrapper.ts: 1 error (missingdiamond.json).demoMayanSolana.ts: 5 errors (Mayan SDK API drift).demoSquid.ts: 1 error.Fixing them would change script behaviour or need SDK migrations, so it's out of scope here. Because of this, the
validateScriptscheck is expected to fail on this PR for exactly these 20 errors. All six commits were made with--no-verify, and the branch was pushed with--no-verify. The four commits that don't touch these demo scripts had passed the full hooks before a rebase onto #2605's latest commit, and were recommitted unchanged. The hooks that tripped (typecheck-files.sh, pre-pushvalidate-scripts) fail the same way onmainfor any change that touches these files. Every other hook step (oxfmt, oxlint, funnel fence, node-runtime fence) passed on all commits. I ran them manually for the--no-verifyones.Verification
All runs are local: bun 1.4.4, Node 26.13.0, Foundry 1.8.7 (
.foundry-versionsays 1.7.1; the hook warns but proceeds). Base =origin/chore/ci-tooling-upgrades@1f34498d3(#2605).git clean -xfd, nonode_modules/out/cache/typechain)bun install --frozen-lockfilebun lintbun lint:mdbun run format(oxfmt + prettier sol)bun typechaintsc --noEmit -p .bun test:tsbun validate-scriptsforge buildforge testforge build srcbytecode + deployedBytecode, 196 artifactsbun attestations:checkThe single
bun test:tsfailure iscreatePinnedTargetStateReader > reads the remote-tracking ref, not a same-named tag in the clone. It fails on the base too, because it depends on the local clone's refs.Bytecode diff: the base build ran in the same clone path as the branch build. A build from a separate worktree path only differs in the CBOR metadata hash, so that comparison isn't meaningful.
Offline script checks:
demoPatcher.ts --helpandbalances.ts --help(chalk) render.@arbitrum/sdk4 root exportsParentToChildMessageGasEstimatorwithestimateAll, andRetryableDataTools.ErrorTriggeringParamsis unchanged.TradingSdkbuilds with aViemAdapterand resolves the signer address.bs58round tripsolanaAddressToBytes32↔bytes32ToSolanaAddressworks.{ quiet: true }or viadotenv/config, and prints◇ injected envwithout it.Not runnable on a fork / not run
forge testfork suites), coverage and any secret-backed CI jobs: fork PRs get no repo secrets. The 71 failing forge tests fail identically on the base.Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)