feat: pnpm migration, contracts precompiles build & npm publish#1021
feat: pnpm migration, contracts precompiles build & npm publish#1021nowooj wants to merge 15 commits into
Conversation
92d6f00 to
3ca5e17
Compare
bcf6953 to
816a66c
Compare
6887e03 to
260c1dd
Compare
|
Rebase on main |
260c1dd to
40696d7
Compare
c3c1465 to
8ac47bb
Compare
|
@greptile review |
Greptile SummaryThis PR migrates the JavaScript toolchain from npm/yarn to pnpm across 115 files and introduces a new precompile build pipeline (
Confidence Score: 4/5Safe to merge after addressing the two P1 issues: CI workflow step ordering and the missing hardhat compile in prepublishOnly. Two P1 findings exist: broken pnpm caching across all CI workflows (wrong step order) and a publish pipeline that can silently produce an empty/broken package without a prior compile step. The remaining findings are P2. The bulk of the migration is correct. .github/workflows/slither.yml and all three compatibility workflows (step ordering), and contracts/package.json (prepublishOnly missing compile). Important Files Changed
|
| - name: Setup Node.js and pnpm | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| cache: 'pnpm' | ||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 9 |
There was a problem hiding this comment.
pnpm/action-setup must precede actions/setup-node
actions/setup-node@v4 with cache: 'pnpm' requires pnpm to already be installed so it can locate the cache store path. With the current order, pnpm is not available yet when setup-node runs, so the pnpm cache is silently skipped on every CI run. The same ordering bug exists in all three compatibility workflows (tests-compatibility-hardhat.yml, tests-compatibility-viem.yml, tests-compatibility-web3js.yml).
Swap the steps: run pnpm/action-setup@v4 first, then actions/setup-node@v4.
| "@types/node": "^22.0.0", | ||
| "@wagmi/cli": "^2.2.0", | ||
| "hardhat": "^3.1.10", |
There was a problem hiding this comment.
prepublishOnly skips the Hardhat compilation step
build:precompiles runs wagmi generate && tsdown. wagmi generate reads from artifacts/ (pre-built Hardhat artifacts), but hardhat compile is not called anywhere in prepublishOnly. If someone runs pnpm publish without having already compiled the contracts, wagmi generate will fail to find artifacts — either erroring out or producing an empty dist/, resulting in a broken or empty published package. Adding pnpm exec hardhat compile && before wagmi generate in build:precompiles would make the publish self-contained.
| import { defineConfig } from "tsdown"; | ||
|
|
||
| export default defineConfig({ | ||
| entry: ["./dist/abi/**/*.ts", "!./dist/abi/**/*.d.ts"], | ||
| format: ["esm", "cjs"], | ||
| outDir: "dist/abi/precompiles", | ||
| dts: true, | ||
| unbundle: true, | ||
| clean: false, | ||
| outExtensions: ({ format }) => ({ | ||
| js: format === 'cjs' ? '.cjs' : '.js', | ||
| dts: '.d.ts', | ||
| }), | ||
| platform: 'neutral', | ||
| copy: [ | ||
| { | ||
| from: [ | ||
| "solidity/precompiles/**/*.sol", | ||
| "!solidity/precompiles/**/testdata/**", | ||
| "!solidity/precompiles/**/testutil/**", | ||
| ], | ||
| to: "dist", | ||
| flatten: false, | ||
| verbose: true, | ||
| }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
Output directory overlaps with input entry glob
entry is "./dist/abi/**/*.ts" and outDir is "dist/abi/precompiles". Since wagmi generate places the generated .ts source files inside dist/abi/precompiles/, tsdown's output goes into the same tree it's compiling from. With clean: false, a second run of tsdown will also pick up .ts files produced by tsdown in a previous run (if any leaked through). Consider emitting generated TypeScript to a staging location (e.g. dist/.src/abi/) and outputting compiled artifacts to dist/abi/precompiles/, keeping source and build outputs separate.
| }, | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| "build:precompiles": "wagmi generate && tsdown", | ||
| "prepublishOnly": "pnpm run build:precompiles" | ||
| }, | ||
| "files": [ | ||
| "dist", | ||
| "README.md" | ||
| ], | ||
| "exports": { | ||
| "./*.sol": "./dist/*.sol", | ||
| "./*": { | ||
| "types": "./dist/*.d.ts", |
There was a problem hiding this comment.
Wildcard
exports patterns for .sol files are ignored by Solidity tooling
The "./*.sol" export entry maps package subpaths to dist/*.sol. However, Solidity tooling (Hardhat, Foundry) does not use the Node.js exports field for import resolution — Hardhat uses require() resolution and Foundry uses remappings.txt. The .sol export entry will be silently ignored by those tools, making the README's Hardhat import example (import "cosmos-evm-contracts/precompiles/bank/IBank.sol") work only if the resolver falls through to bare node_modules file paths. Consider verifying the Hardhat and Foundry import paths against actual resolved paths after pnpm install.
ace00fa to
0b8280a
Compare
aljo242
left a comment
There was a problem hiding this comment.
ci has not run: all workflows are action_required -- fork pr, needs maintainer approval to trigger.
wagmi.config.ts include path is malformed: join("solidity", "precompiles", module, "${contract}.sol", "*.json") treats ${contract}.sol as a directory segment. hardhat artifacts don't live there -- wagmi will silently produce no output and the published package will have empty ABIs.
exports field doesn't match output structure: the catch-all ./* maps to root dist/*.js / dist/*.d.ts, but tsdown outputs to dist/abi/precompiles/. import paths like cosmos-evm-contracts/abi/precompiles/bank/IBank (as documented in the README) won't resolve.
files field missing .generated/: wagmi writes to .generated/ during build, but only dist and precompiles are listed in files. if cleanup runs between build and publish, generated ABI files won't be included in the package.
chore: refine exports structure for TypeScript compatibility chore: enhance exports for Solidity files refactor: enhance TypeScript configuration for precompiles feat: add dual ESM/CJS build for ABI exports refactor: streamline build process for precompiles chore: make contracts-all
…nd adjust paths for generated files
0b8280a to
f2719c5
Compare
5dbb144 to
d8d837e
Compare
Thanks for the review.
|
Closes: #428
Temporary published package: cosmos-evm-contracts
If you want to keep using this package name, I will transfer npm package ownership from the account that published it.
1. contracts package – precompiles build & npm publish
.soland ABI intodist/fromsolidity/precompiles, excludingtestdataandtestutilfiles,exports, andprepublishOnlyso npm publish runs the build and exposes the right entry pathscontracts/scripts/build-precompiles.jscontracts/package.jsonbuild:precompiles,compile,prepublishOnly,files,exports; packageManager, deps, etc.contracts/README.mdcontracts/.gitignoredist/contracts/hardhat.config.jscontracts/pnpm-lock.yaml2. pnpm migration (reason)
contracts/,tests/, evm-tools-compatibility, etc.)CI, scripts, and lockfiles across the repo are updated to use pnpm accordingly.
How to verify
How to publish
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranch