-
Notifications
You must be signed in to change notification settings - Fork 1
Api v3 refactor #2
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
- Add optional variant field to BuildInfo interface for cargo-near build variants - Implement Docker-based compilation using specified cargo-near images from contract metadata - Add intelligent routing between Docker and local compilation based on build_environment - Handle workspace path translation for Docker container output - Fix permissions for Docker container user (UID 1000) - Update cargo-near installation to use crates.io instead of building from source - Add proper handling of build_command arrays and variant flags
The verifier smart contract expects a numeric block_height field. Previously passing blockId (which could be undefined or string) was causing deserialization errors. Now we fetch the actual block height from the RPC response and pass that instead.
…essibility - Validate repository is publicly accessible before running near-cli-rs verify - Check if Docker images are available when build_environment is specified - Provide clear error messages for inaccessible repos instead of auth errors - Eliminate duplicate contract metadata fetching for better performance - Ensure temp folder cleanup in all error scenarios
- QuickNode pinning failures now log warning instead of failing verification - Fix cleanup timing from 3 minutes to 30 minutes (6000 -> 60000) - Silence ENOENT errors in cleanup for already-deleted folders
BREAKING CHANGE: removed JWT authentication and legacy endpoints - Add IPFS structure endpoint (GET /ipfs/structure) - Upgrade to Kubo v0.32.1 with auto-announce configuration - Add IPFS WebUI with basic auth protection (/ipfs-admin/) - Update Node.js to 20 LTS in CI workflow - Upgrade nginx to 1.27 - Remove unused services (auth, encryption, builder-info) - Remove unused DTOs, guards, and validators - Clean up package.json dependencies
- NestJS: 9.x → 10.4.x - Swagger: 6.x → 7.4.x - TypeScript: 4.x → 5.7.x - helmet: 7.x → 8.x - prettier: 2.x → 3.4.x - reflect-metadata, rimraf, rxjs updated - dev deps updated (jest, ts-jest, eslint plugins) Note: ipfs-http-client (v53) and near-api-js (v2) kept at stable CommonJS-compatible versions to avoid ESM migration issues.
- Daily: prune build cache older than 7 days (keep 2GB minimum) - Weekly: prune orphan volumes and dangling images - Monthly: deep clean all unused resources older than 30 days - Manual cleanup method available for programmatic use Prevents accumulation of orphan volumes and stale build cache.
…er naming - Remove deprecated ipfs-http-client in favor of axios + form-data - Remove unused dependencies: @nestjs/config, class-transformer - Add branch prefix to docker network name - Default BRANCH_NAME to 'main' if not set
- NestJS: 10.x → 11.x (requires Node 20+) - @nestjs/swagger: 7.x → 11.x - @nestjs/schedule: 4.x → 6.x - axios: 1.7 → 1.9 - typescript: 5.7 → 5.8 - Remove source-map-support (unused) Note: bs58 and near-api-js kept at current versions (ESM-only in latest)
- Update Dockerfile to Node 24 Alpine (rename development to build stage) - Update GitHub Actions workflow to Node 24.x LTS - Upgrade near-api-js to v6 and update services for new API - Upgrade Jest to v30, @types/node to v24, eslint-config-prettier to v10 - Fix tests for updated ExecService string[] return types - Fix compiler service tests for updated command format
- Replace bs58 with @scure/base for base58 encoding - Migrate eslint to v9 flat config (eslint.config.mjs) - Update tsconfig to NodeNext/ES2023 target - Silence ts-jest hybrid module warning - Remove unused test:e2e script and ts-node deps from test:debug
- Convert src/ path imports to relative imports in verify.controller.ts - Remove deprecated baseUrl from tsconfig.json - Fixes TypeScript 7.0 deprecation warning
- Remove unused compileRust/compileTypeScript methods - Remove unused scripts/compiler/rust.sh and ts.sh - Remove unused getRepoInfo method from GithubService - Remove unused GithubData interface and IpfsUploadResponse dto - Update tests for remaining functionality
- replace deprecated connect() with direct Account constructor - use KeyPairSigner instead of deprecated InMemorySigner - update IPFS kubo from v0.32.1 to v0.39.0 - add nginx /webui route for IPFS WebUI redirect
- Replace deprecated Contract class with direct provider.query() for view methods - Replace deprecated Contract.functionCall with account.signAndSendTransaction - Use JsonRpcProvider directly instead of providers.JsonRpcProvider - Add comprehensive logging to verification flow for better observability
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 is a comprehensive infrastructure and architecture refactor that modernizes the SourceScan API from v2 to v3. The PR fundamentally changes how contract verification works, moving from a multi-step Docker-based compilation process with JWT authentication to a streamlined approach using near-cli-rs for direct contract verification.
Key Changes
- Verification Architecture: Replaced Docker-based compilation with near-cli-rs verification, eliminating the need for JWT authentication, temporary folder management, and separate compilation endpoints
- Dependency Modernization: Updated to Node.js 24 LTS, NestJS 11, and migrated from ipfs-http-client SDK to direct IPFS HTTP API calls using axios
- Infrastructure Evolution: Transitioned to Docker-in-Docker for the NestJS container, enabling Rust toolchain and near-cli-rs installation directly in the runtime environment
Reviewed changes
Copilot reviewed 57 out of 59 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
nginx/scripts/entrypoint.sh |
New entrypoint script for dynamic htpasswd generation from environment variables |
nginx/enode.conf.template |
Enhanced NGINX configuration with IPFS WebUI proxy and basic authentication |
nest/Dockerfile |
Major refactor to Docker-in-Docker base with Rust and near-cli-rs installation |
nest/package.json |
Updated to v3.0.0 with modernized dependencies (NestJS 11, Node 24 support) |
nest/eslint.config.mjs |
Migrated from legacy ESLint config to modern flat config format |
nest/tsconfig.json |
Updated TypeScript configuration for NodeNext module resolution and ES2023 target |
nest/src/main.ts |
Updated CORS to allow any origin and added detailed validation error formatting |
nest/src/app.module.ts |
Removed JWT, Auth, Encryption, BuilderInfo services; added DockerCleanup service |
nest/src/controllers/verify/verify.controller.ts |
Complete rewrite to use near-cli-rs with pre-verification checks and IPFS pinning |
nest/src/controllers/ipfs/ipfs.controller.ts |
Simplified to remove upload endpoint, retained structure listing only |
nest/src/services/ipfs/ipfs.service.ts |
Migrated from ipfs-http-client to direct axios API calls; added QuickNode pinning |
nest/src/services/compiler/compiler.service.ts |
Replaced Rust/TypeScript compilation with near-cli-rs contract verification |
nest/src/services/github/github.service.ts |
Added source code snapshot parsing and repo path helper methods |
nest/src/services/exec/exec.service.ts |
Changed return type from concatenated strings to arrays of log lines |
nest/src/services/temp/temp.service.ts |
Updated WASM file reading for cargo-near build output structure |
nest/src/services/docker-cleanup/docker-cleanup.service.ts |
New service for automated Docker resource cleanup with cron jobs |
nest/src/modules/near/services/verifier.service.ts |
Refactored to use @near-js packages instead of near-api-js legacy API |
nest/src/modules/near/services/rpc.service.ts |
Added block query and function call methods with optional block height |
ipfs/001-configure.sh |
New IPFS initialization script for API CORS and announce address configuration |
docker-compose.yml |
Replaced compose.yaml with modernized configuration and dynamic container naming |
.github/workflows/main.yml |
Updated CI to Node.js 24, added linting step, and dependency caching |
| Removed files | Deleted auth, encryption, builder-info services and related controllers/guards/validators |
Comments suppressed due to low confidence (2)
nest/src/main.ts:22
- Setting
origin: '*'allows any origin to access the API, which is a security risk. This exposes the API to potential CSRF attacks and unauthorized access from any domain. Consider either:
- Maintaining the original whitelist of allowed origins
- Using environment variables to configure allowed origins based on deployment environment
- If this change is intentional for development, add a comment explaining why and ensure production uses a proper whitelist.
nest/src/services/temp/temp.service.ts:145
- The new
findRustWasmInFoldermethod lacks test coverage. This method should have tests covering:
- Successful WASM file discovery in the expected directory structure
- Error handling when no WASM files are found
- Error handling for multiple WASM files
- Proper path construction
The refactored logic changes the directory structure fromwasm32-unknown-unknown/releasetotarget/near, which is a significant change that should be tested.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| `/ip4/${this.ipfsExternalIp}/tcp/${this.swarmPort}/p2p/SourcePeerId`, | ||
| `/ip4/${this.ipfsExternalIp}/udp/${this.swarmPort}/quic/p2p/SourcePeerId`, | ||
| ]; |
Copilot
AI
Dec 2, 2025
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.
The origins array contains placeholder text SourcePeerId which will not work. This needs to be replaced with the actual IPFS peer ID. Consider fetching the peer ID dynamically via the IPFS API (/api/v0/id) or passing it as an environment variable. Without the correct peer ID, QuickNode pinning will fail or not properly attribute the content to this node.
| request_type: 'call_function', | ||
| account_id: accountId, | ||
| method_name: methodName, | ||
| args_base64: args ? Buffer.from(args).toString('base64') : '', |
Copilot
AI
Dec 2, 2025
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.
The args parameter expects any[] but is being passed to Buffer.from(args) which expects a string, Buffer, or array of bytes. If args is an object (which is typical for NEAR function calls), this will not serialize correctly. Consider using Buffer.from(JSON.stringify(args)) instead, similar to how it's done in the verifier.service.ts at line 47.
| } catch { | ||
| // Contract not found in verifier, continue |
Copilot
AI
Dec 2, 2025
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.
The error handling in the try-catch block silently catches all errors with an empty catch block. If getContract fails for reasons other than "contract not found" (e.g., network issues, RPC errors), these errors will be silently ignored. Consider logging the error or checking the specific error type to only ignore "not found" errors:
} catch (error) {
if (!error.message.includes('not found')) {
this.logger.warn(`Error fetching contract: ${error.message}`);
}
}| } catch { | |
| // Contract not found in verifier, continue | |
| } catch (error) { | |
| // Contract not found in verifier, continue | |
| if (!error?.message || !error.message.includes('not found')) { | |
| this.logger.warn(`Error fetching contract for ${accountId}: ${error?.message || error}`); | |
| } |
| # Generate htpasswd from environment variables | ||
| if [ -n "$IPFS_ADMIN_USER" ] && [ -n "$IPFS_ADMIN_PASS" ]; then | ||
| echo "Generating htpasswd for user: $IPFS_ADMIN_USER" | ||
| echo "$IPFS_ADMIN_USER:$(openssl passwd -apr1 $IPFS_ADMIN_PASS)" > /etc/nginx/.htpasswd |
Copilot
AI
Dec 2, 2025
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.
The openssl passwd command should use the -1 flag for MD5 or -apr1 flag should be properly quoted. The current usage of $IPFS_ADMIN_PASS is not quoted, which could cause issues if the password contains special characters. Consider using: echo "$IPFS_ADMIN_USER:$(openssl passwd -apr1 "$IPFS_ADMIN_PASS")" to properly quote the password variable.
| .json({ message: 'Contract verification failed' }); | ||
| } |
Copilot
AI
Dec 2, 2025
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.
The error message "Contract verification failed" is too generic. Consider including more details about why the verification failed, such as:
- What was expected vs. what was found
- Whether it was a code mismatch, compilation error, or repository issue
This will help users understand and resolve verification failures more easily.
| .json({ message: 'Contract verification failed' }); | |
| } | |
| .json({ | |
| message: 'Contract verification failed: code mismatch', | |
| expectedCodeHash: contractData ? contractData.code_hash : null, | |
| actualCodeHash: checksum, | |
| }); |
| // 60000ms = 1 minute, so cleanup after JWT_EXPIRATION minutes | ||
| if ( | ||
| new Date().getTime() - creationTime.getTime() > | ||
| 6000 * parseInt(process.env.JWT_EXPIRATION) | ||
| 60000 * parseInt(process.env.JWT_EXPIRATION) |
Copilot
AI
Dec 2, 2025
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.
The comment states "60000ms = 1 minute" which is correct, but this timing should be verified. The calculation 60000 * parseInt(process.env.JWT_EXPIRATION) means if JWT_EXPIRATION is 30 (minutes), folders are cleaned up after 30 minutes. Ensure this aligns with the intended behavior, especially since JWT-based authentication was removed from the codebase in this refactor.
| 'cannot', // Cannot perform an operation | ||
| "can't", // Short form of cannot | ||
| 'unable', // Unable to perform an operation | ||
| 'panic:', // Rust panic messages |
Copilot
AI
Dec 2, 2025
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.
The removal of 'unable' from the error indicators list should be documented or reconsidered. The comment "// Unable to perform an operation" was removed along with the indicator. If this was intentional (perhaps 'unable' was causing false positives), it should be documented why. Otherwise, this could miss legitimate error messages containing "unable".
| # Start application | ||
| CMD [ "node", "dist/main.js" ] | ||
| # Start the application when dockerd is ready | ||
| CMD sh -c "dockerd & while (! docker stats --no-stream ); do sleep 1; done && node dist/main.js" |
Copilot
AI
Dec 2, 2025
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.
The startup command waits for dockerd to be ready before starting the Node.js application, but there's no timeout or error handling. If dockerd fails to start, this will loop indefinitely. Consider:
- Adding a timeout with a maximum retry count
- Adding error logging if docker fails to start
- Using a healthcheck instead of a polling loop
Example:timeout 60 sh -c 'while ! docker stats --no-stream; do sleep 1; done' && node dist/main.js || (echo 'Docker failed to start' && exit 1)
| CMD sh -c "dockerd & while (! docker stats --no-stream ); do sleep 1; done && node dist/main.js" | |
| CMD sh -c "dockerd & timeout 60 sh -c 'while ! docker stats --no-stream; do sleep 1; done' && node dist/main.js || (echo 'Docker failed to start' >&2 && exit 1)" |
| async block(blockId?: number): Promise<BlockResult> { | ||
| try { | ||
| const response = await this.provider.query({ | ||
| const response = await this.provider.block( | ||
| blockId ? { blockId: blockId.toString() } : { finality: 'final' }, | ||
| ); | ||
|
|
||
| this.logger.log('Latest block details retrieved'); | ||
|
|
||
| return response; | ||
| } catch (error) { | ||
| this.logger.error('Error retrieving latest block details', error.stack); | ||
|
|
||
| throw new HttpException(error.message, 500); | ||
| } | ||
| } | ||
|
|
||
| async viewCode( | ||
| accountId: string, | ||
| blockId?: number, | ||
| ): Promise<QueryResponseKind> { | ||
| try { | ||
| const queryParams: any = { | ||
| request_type: 'view_code', | ||
| account_id: accountId, | ||
| finality: 'final', | ||
| }); | ||
| }; | ||
|
|
||
| if (blockId) { | ||
| queryParams.block_id = blockId; | ||
| } else { | ||
| queryParams.finality = 'final'; | ||
| } | ||
|
|
||
| const response = await this.provider.query(queryParams); | ||
|
|
||
| this.logger.log(`Code viewed for account ID: ${accountId}`); | ||
|
|
||
| return response; | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `Error viewing code for account ID: ${accountId}`, | ||
| error.stack, | ||
| ); | ||
| throw error; | ||
|
|
||
| throw new HttpException(error.message, 500); | ||
| } | ||
| } | ||
|
|
||
| async callFunction( | ||
| accountId: string, | ||
| methodName: string, | ||
| blockId?: number, | ||
| args?: any[], | ||
| ): Promise<any> { | ||
| try { | ||
| const queryParams: any = { | ||
| request_type: 'call_function', | ||
| account_id: accountId, | ||
| method_name: methodName, | ||
| args_base64: args ? Buffer.from(args).toString('base64') : '', | ||
| }; | ||
|
|
||
| if (blockId) { | ||
| queryParams.block_id = blockId; | ||
| } else { | ||
| queryParams.finality = 'final'; | ||
| } | ||
|
|
||
| const response: any = await this.provider.query(queryParams); | ||
|
|
||
| this.logger.log( | ||
| `Function ${methodName} called for account ID: ${accountId}`, | ||
| ); | ||
|
|
||
| if (response && response.result) { | ||
| const jsonString = this.byteArrayToString(response.result); | ||
| response.result = JSON.parse(jsonString); | ||
| } | ||
|
|
||
| return response; | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `Error calling function ${methodName} for account ID: ${accountId}`, | ||
| error.stack, | ||
| ); | ||
|
|
||
| throw new HttpException(error.message, 500); | ||
| } | ||
| } |
Copilot
AI
Dec 2, 2025
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.
The new block and callFunction methods lack test coverage. These are significant additions to the RPC service that should have comprehensive tests covering:
- Successful block retrieval (with and without blockId)
- Function call with different argument types
- Error handling scenarios
- Response parsing and transformation logic
Consider adding tests following the existing pattern in the spec file.
| # Generate htpasswd from environment variables | ||
| if [ -n "$IPFS_ADMIN_USER" ] && [ -n "$IPFS_ADMIN_PASS" ]; then | ||
| echo "Generating htpasswd for user: $IPFS_ADMIN_USER" | ||
| echo "$IPFS_ADMIN_USER:$(openssl passwd -apr1 $IPFS_ADMIN_PASS)" > /etc/nginx/.htpasswd |
Copilot
AI
Dec 2, 2025
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.
The use of openssl passwd -apr1 generates an Apache MD5 (apr1) hash, which is a weak, fast algorithm vulnerable to rapid offline brute-force if the .htpasswd file is exposed—allowing attackers to recover IPFS_ADMIN_PASS and gain access to the protected /ipfs-admin/ endpoints. Replace -apr1 with a stronger algorithm such as bcrypt (e.g., openssl passwd -bcrypt "$IPFS_ADMIN_PASS") or migrate to a more secure auth mechanism (e.g., reverse proxy with OAuth/JWT) and ensure file permissions restrict read access.
This pull request introduces major updates to the project’s infrastructure, dependencies, and configuration, modernizing the stack and improving maintainability. Key changes include upgrading Node.js and NestJS versions, refactoring Docker and Compose setups, enhancing CI workflows, updating environment variables, and improving documentation and linting.
Infrastructure & Docker Modernization
compose.yamlto a newdocker-compose.ymlwith updated service definitions, explicit platform targeting, dynamic container naming based on branch, healthchecks, and improved IPFS/NGINX configuration. Added IPFS startup script for automatic announce address configuration. [1] [2]nest/Dockerfileto use Node.js 24 for build, switch runtime to Docker-in-Docker, install Rust and near-cli-rs for contract verification, and ensure proper startup sequencing. [1] [2]Dependency & Configuration Upgrades
3.0.0. [1] [2].eslintrc.js) with moderneslint.config.mjsusing typescript-eslint and Prettier, and updated CI workflow to run linting and use Node.js 24. [1] [2] [3]Environment & Documentation Improvements
.env.examplewith new variables for branch, host, IPFS external IP, and clarified port usage.README.mdwith clearer installation steps, detailed firewall/port configuration, and IPFS WebUI access instructions.Codebase Cleanup
nest/scripts/compiler/rust.sh,nest/scripts/compiler/ts.sh) and eliminated obsolete controllers/services fromnest/src/app.module.ts. [1] [2] [3]nest/src/controllers/compile/compile.controller.ts.Testing & Miscellaneous
nest/package.jsonfor improved diagnostics.These changes collectively update the project to modern standards, streamline development workflows, and improve reliability across environments.