-
Notifications
You must be signed in to change notification settings - Fork 1
Feedback and fixes #19
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
Coverage reportCaution Test run failed
Test suite run failedFailed tests: 4/52. Failed suites: 1/8.Report generated by 🧪jest coverage report action from cb5c320 |
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
Updates dependencies and improves functionality for the Solana SubQuery implementation by using granular imports for codegen, updating Solana-specific dependencies to newer versions, exposing decoder functionality to sandbox environments, and fixing dictionary improvement issues.
- Use granular imports in codegen to improve build efficiency
- Update Solana ecosystem dependencies (@solana/kit, @codama packages) to newer versions
- Expose decoder interface to sandbox for instruction and log decoding
- Fix dictionary condition handling to prevent usage when no filters are present
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/solana.ts | Fix typo in property name and add Decoder interface with documentation |
| packages/types/src/global.ts | Expose decoder globally for sandbox access |
| packages/types/package.json | Update @solana/kit dependency version |
| packages/node/src/solana/decoder.ts | Implement Decoder interface in SolanaDecoder class |
| packages/node/src/solana/api.solana.ts | Update import path for Solana RPC |
| packages/node/src/indexer/indexer.manager.ts | Inject decoder into sandbox environment |
| packages/node/src/indexer/dictionary/v2/solanaDictionaryV2.ts | Fix dictionary condition sanitization logic |
| packages/node/src/app.module.ts | Update SDK version tracking from ethers to @solana/kit |
| packages/node/package.json | Update multiple dependency versions |
| packages/common-solana/src/codegen/codegen.ts | Enable granular imports in codegen |
| packages/common-solana/package.json | Update @codama dependency versions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| accountKey: Address; | ||
| /** List of indices used to load addresses of readonly accounts from a lookup table. */ | ||
| readableIndexes: readonly number[]; | ||
| readonlyIndexes: readonly number[]; |
Copilot
AI
Sep 9, 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 property name has been corrected from 'readableIndexes' to 'readonlyIndexes' to match the intended meaning of read-only access indexes.
| safeApi: SolanaSafeApi, | ||
| ): IndexerSandbox { | ||
| return this.sandboxService.getDsProcessor(ds, safeApi, this.apiService.api); | ||
| const sandbox = (this as any).sandboxService as SandboxService< |
Copilot
AI
Sep 9, 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.
Using 'this as any' bypasses TypeScript's type checking. Consider updating the class property type or using a more type-safe approach to access the sandboxService.
| const sandbox = (this as any).sandboxService as SandboxService< | |
| const sandbox = this.sandboxService as SandboxService< |
| const kitPath = require.resolve('@solana/kit'); | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { version: ethersSdkVersion } = require('ethers/package.json'); | ||
| const { version: solanaSdkVersion } = require(path.resolve( | ||
| kitPath, | ||
| '../../package.json', | ||
| )); |
Copilot
AI
Sep 9, 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 path resolution logic is fragile and depends on the internal structure of @solana/kit. Consider using a more reliable method to get the package version, such as requiring '@solana/kit/package.json' directly if available.
| const kitPath = require.resolve('@solana/kit'); | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { version: ethersSdkVersion } = require('ethers/package.json'); | ||
| const { version: solanaSdkVersion } = require(path.resolve( | ||
| kitPath, | ||
| '../../package.json', | ||
| )); |
Copilot
AI
Sep 9, 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 path resolution logic is fragile and depends on the internal structure of @solana/kit. Consider using a more reliable method to get the package version, such as requiring '@solana/kit/package.json' directly if available.
Description
Fixes subquery/subql#2903
Type of change
Please delete options that are not relevant.
Checklist