-
Notifications
You must be signed in to change notification settings - Fork 10
fix/dataProtector sharing fix ci #469
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
Conversation
…or smart contract deployment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes CI issues and adds support for Arbitrum mainnet deployment in the sharing smart contract package. The changes standardize deployment configuration, update environment variable names, and modernize the CI setup.
Key Changes:
- Refactored deployment system to use Hardhat Ignition instead of custom scripts
- Updated Node.js version from 18 to 20 throughout the codebase
- Standardized environment variable naming and added support for flexible contract verification
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sharing-smart-contract/package.json | Updates deploy script to use Hardhat Ignition with create2 strategy |
| packages/sharing-smart-contract/hardhat.config.cjs | Adds Arbitrum mainnet config and standardizes environment variables |
| packages/sharing-smart-contract/config/env.cjs | Renames PRIVATE_KEY to DEPLOYER_PRIVATE_KEY and adds verification config |
| packages/sharing-smart-contract/CHANGELOG.md | Documents the CI fixes and Arbitrum mainnet support |
| packages/sharing-smart-contract/.nvmrc | Updates Node.js version from 18 to 20 |
| packages/sharing-smart-contract/.env.template | Updates template with new environment variable names |
| .github/workflows/sharing-smart-contracts-ci.yml | Consolidates CI workflow and updates to Node.js 20 |
| .github/workflows/sharing-smart-contract-deploy.yml | Major refactor of deployment workflow with improved validation |
| .github/workflows/sharing-smart-contract-ci.yml | Removes redundant CI workflow file |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deployment ID Issue:
Could you rename the existing committed deployment-id folder chain-421614, or just remove it?
Code Modification :
Could you modify this line in the deployment script: const proxyAdminOwner = m.getAccount(0); to use the ADMIN_PRIVATE_KEY environment variable instead? According to our workflow, the deployer should not be the admin.
…lback strategy for CREATE2
…aring-smart-contract deployment workflow
…r CREATE2 strategy
7405d62 to
fc617a9
Compare
…smart-contract deployment workflow
No description provided.