ci: publish ABIs, types, compiled tasks and scripts #42
Conversation
📝 WalkthroughWalkthroughThis pull request makes several modifications across configuration files and scripts. The Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Build as Build Script (build.sh)
participant TS as TypeScript Compiler (tsc)
participant Contract as Hardhat Compiler
participant Artifacts as Artifact Handler
Dev->>Build: Initiate build process
Build->>Build: Create directories (abi, types)
Build->>TS: Run tsc for TS compilation
TS-->>Build: Output compiled files in dist
Build->>Contract: Compile contracts
Contract-->>Build: Return compiled artifacts
Build->>Artifacts: Copy artifacts to designated locations
Build->>Dev: Build process completed
sequenceDiagram
participant User as User
participant Script as localnet.sh
User->>Script: Run script with "start" argument
Script->>Script: Start local network process
Script->>Script: Sleep for 25 seconds (wait period)
Script->>User: Local network ready
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Code from this PR has been published on npm for testing purposes: https://www.npmjs.com/package/@zetachain/standard-contracts/v/1.0.0-rc4?activeTab=code Seems to be working fine. I've also used v1.0.0-rc4 in zeta-chain/example-contracts#229 |
|
@zeta-chain/smart-contracts please, review. |
package.json
Outdated
| "contracts/token/scripts" | ||
| ], | ||
| "packageManager": "[email protected]+sha1.1959a18351b811cdeedbd484a8f86c3cc3bbaf72", |
package.json
Outdated
| "packageManager": "[email protected]+sha1.1959a18351b811cdeedbd484a8f86c3cc3bbaf72", | ||
| "dependencies": { | ||
| "validator": "^13.12.0" |
There was a problem hiding this comment.
why is only this package included on this top level?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
contracts/nft/tasks/deploy.ts (1)
1-2:⚠️ Potential issueFix TypeScript module resolution error.
The pipeline is failing due to missing type declarations for hardhat modules.
Add
@types/hardhatto devDependencies:{ "devDependencies": { + "@types/hardhat": "^2.0.0" } }🧰 Tools
🪛 GitHub Check: slither (contracts/token, token.sarif)
[failure] 1-1:
Cannot find module 'hardhat/config' or its corresponding type declarations.
[failure] 2-2:
Cannot find module 'hardhat/types' or its corresponding type declarations.🪛 GitHub Check: slither (contracts/nft, nft.sarif)
[failure] 1-1:
Cannot find module 'hardhat/config' or its corresponding type declarations.
[failure] 2-2:
Cannot find module 'hardhat/types' or its corresponding type declarations.🪛 GitHub Actions: Slither
[error] 1-1: error TS2307: Cannot find module 'hardhat/config' or its corresponding type declarations.
🧹 Nitpick comments (2)
contracts/token/scripts/localnet.sh (1)
7-7: Extended Sleep Duration for Network Readiness
The sleep duration has been increased from 10 seconds to 25 seconds when starting the local network. This change helps ensure that the network has enough time to initialize before subsequent commands are executed. As an enhancement, consider implementing a dynamic readiness check (e.g., polling a health endpoint or waiting for a specific output) rather than a fixed sleep duration for improved robustness in various environments.contracts/nft/scripts/localnet.sh (1)
7-7: Updated Sleep Period for NFT Local Network Initialization
The sleep time on the local network start command has been updated to 25 seconds to allow more time for contract deployment and network setup. This adjustment mirrors the change made in the token script, ensuring consistency across both modules. As a follow-up, you might consider using a readiness check instead of a fixed delay to optimize startup performance on more capable systems.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.gitignore(1 hunks)contracts/nft/package.json(2 hunks)contracts/nft/scripts/localnet.sh(1 hunks)contracts/nft/tasks/deploy.ts(1 hunks)contracts/token/package.json(1 hunks)contracts/token/scripts/localnet.sh(1 hunks)package.json(1 hunks)scripts/build.sh(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- contracts/nft/package.json
- contracts/token/package.json
- tsconfig.json
🧰 Additional context used
🪛 GitHub Actions: Slither
contracts/nft/tasks/deploy.ts
[error] 1-1: error TS2307: Cannot find module 'hardhat/config' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (contracts/nft)
🔇 Additional comments (2)
contracts/nft/tasks/deploy.ts (1)
43-43: LGTM! Contract address property is now correctly referenced.The change from
contract.targettocontract.addressis correct as it uses the standard property for accessing contract addresses.package.json (1)
31-31: LGTM! Package manager version is now simplified.The packageManager field has been simplified to just the version number as requested.
| @@ -3,6 +3,8 @@ | |||
| rm -rf abi && mkdir -p abi/contracts | |||
| rm -rf typescript-types && mkdir typescript-types | |||
|
|
|||
| tsc | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for TypeScript compilation.
The script should exit if TypeScript compilation fails to prevent generating incomplete artifacts.
Add error handling:
-tsc
+if ! tsc; then
+ echo "TypeScript compilation failed"
+ exit 1
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tsc | |
| if ! tsc; then | |
| echo "TypeScript compilation failed" | |
| exit 1 | |
| fi |
| ], | ||
| "packageManager": "[email protected]", | ||
| "devDependencies": { | ||
| "typescript": "^5.7.3" |
There was a problem hiding this comment.
Fix invalid TypeScript version.
Version 5.7.3 doesn't exist. Use the latest stable version.
- "typescript": "^5.7.3"
+ "typescript": "^5.3.3"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "typescript": "^5.7.3" | |
| "typescript": "^5.3.3" |
Expected package contents:
Summary by CodeRabbit
New Features
Bug Fixes
Chores