Skip to content

fix: verified svm builds #997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

Reinis-FRP
Copy link
Contributor

@Reinis-FRP Reinis-FRP commented May 9, 2025

This introduces testing against verifyable SVM builds in CI. Since this is rather slow, this breaks build in parallel EVM/SVM jobs and uses caching where possible so that not to delay development unnecessarily for changes that don't affect program code.

Reinis-FRP added 19 commits May 9, 2025 11:09
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
cache: yarn
- name: Install packages
shell: bash
run: yarn install --frozen-lockfile
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still have below errors here when building optional node-hid and cpu-features. We cannot do --ignore-optional here like in other places as hardhat compile depends on other optional dependencies being installed.

warning Error running install script for optional dependency: "/home/runner/work/contracts/contracts/node_modules/node-hid: Command failed.
info This module is OPTIONAL, you can safely ignore this error

warning Error running install script for optional dependency: "/home/runner/work/contracts/contracts/node_modules/cpu-features: Command failed.
info This module is OPTIONAL, you can safely ignore this error

Comment on lines +3 to +4
env:
NODE_VERSION: 20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we had matrix strategy, but that got overly repetitive when splitting to multiple jobs. And we were not using the matrix functionality anyways as all jobs were run against a single node version.

Comment on lines -19 to -27
- name: Extract Solana versions
uses: solana-developers/github-actions/[email protected]
id: versions
- name: Setup Anchor & Solana
uses: solana-developers/github-actions/[email protected]
with:
anchor_version: ${{ steps.versions.outputs.anchor_version }}
solana_version: ${{ steps.versions.outputs.solana_version }}
node_version: 20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solana and anchor binaries were not needed for lint

# Perform installs, run tests, run a build step, etc. here, as needed.
- run: yarn
# Perform installs and run a build step.
- run: yarn install --frozen-lockfile && yarn build
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With removed prepublish script we have to build explicitly. For now this includes regular SVM build without caching, but would need to create a follow-up PR that covers verified builds.

"build-ts": "rm -rf ./dist && tsc && rsync -a --include '*/' --include '*.d.ts' --exclude '*' ./typechain ./dist/",
"copy-idl": "mkdir -p dist/src/svm/assets/idl && cp src/svm/assets/idl/*.json dist/src/svm/assets/idl/",
"build": "yarn build-evm && yarn build-svm && yarn generate-svm-assets && yarn build-ts && yarn copy-idl",
"build-svm": "bash ./scripts/svm/buildHelpers/buildSvmLocalToolchain.sh",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was unnecessarily duplicating anchor build.

Comment on lines +34 to +35
"build-ts": "rm -rf ./dist && tsc && rsync -a --include '*/' --include '*.d.ts' --exclude '*' ./typechain ./dist/ && yarn export-idl",
"export-idl": "mkdir -p dist/src/svm/assets/idl && cp src/svm/assets/idl/*.json dist/src/svm/assets/idl/",
Copy link
Contributor Author

@Reinis-FRP Reinis-FRP May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved export-idl (renamed from copy-idl before) from build to build-ts as it is more relevant for dist files.

"test:report-gas": "IS_TEST=true REPORT_GAS=true hardhat test",
"generate-evm-assets": "rm -rf typechain && TYPECHAIN=ethers yarn hardhat typechain",
"prepublish": "yarn build && hardhat export --export-all ./cache/massExport.json && ts-node ./scripts/processHardhatExport.ts && prettier --write ./deployments/deployments.json && yarn generate-evm-assets",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to delete the prepublish script as it got triggered on all yarn install commands making it impossible to split SVM/EVM builds in parallel CI jobs.

"generate-evm-assets": "rm -rf typechain && TYPECHAIN=ethers yarn hardhat typechain",
"prepublish": "yarn build && hardhat export --export-all ./cache/massExport.json && ts-node ./scripts/processHardhatExport.ts && prettier --write ./deployments/deployments.json && yarn generate-evm-assets",
"generate-evm-artifacts": "rm -rf typechain && TYPECHAIN=ethers yarn hardhat typechain",
"process-hardhat-export": "hardhat export --export-all ./cache/massExport.json && ts-node ./scripts/processHardhatExport.ts && prettier --write ./deployments/deployments.json",
Copy link
Contributor Author

@Reinis-FRP Reinis-FRP May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had deployments generation as part of prepublish script, but these commands were ran after yarn build which already had populated the dist files, so this could have potentially left local tree inconsistent with dist. Hence, these steps are moved to a dedicated script if anyone still needs them. Though I understand in practice we update deployments.json by hand.

Also removed typechain generation completely as they are already generated in hardhat compile with @typechain/hardhat plugin.

@Reinis-FRP Reinis-FRP marked this pull request as ready for review May 12, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant