feat: upgrade packages and introduce custom rpc url#49
Conversation
WalkthroughThis update introduces support for specifying a custom Ethereum RPC URL via a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI_User
participant CLI_Command
participant WalletUtil
participant Provider
CLI_User->>CLI_Command: Run command with --rpc-url option
CLI_Command->>WalletUtil: getWalletOrSigner({ ..., rpcUrl })
WalletUtil->>Provider: new JsonRpcProvider(rpcUrl)
Provider-->>WalletUtil: Provider instance
WalletUtil-->>CLI_Command: Wallet/Signer using custom provider
CLI_Command-->>CLI_User: Executes blockchain operation via custom RPC URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests / Run Tests
🔇 Additional comments (1)
package.json (1)
75-80: Switch to Node 18.x and validate end-to-end after bumping internal packagesThe bumps raise the minimum allowed versions for all internal
@tradetrust-tt/*and@trustvc/trustvcpackages, but the install failed with an EBADENGINE error (requires Node 18.x). Once you’ve switched your environment to Node 18.x, please re-run:
npm ci# install with a clean lockfilenpm ls --depth=1# check for unsatisfied peer dependenciesnpm audit --production# surface new CVEs (23 vulnerabilities found, including SSRF in axios, RCE in form-data, critical elliptic issues)npm test# execute the Jest suite (currently not found due to the failed install)After that, do a smoke-run on sample documents to confirm the CLI behaves correctly before publishing.
package.json
Outdated
| "@tradetrust-tt/document-store": "^4.1.1", | ||
| "@tradetrust-tt/token-registry": "^5.2.0", | ||
| "@tradetrust-tt/tradetrust": "^6.10.0", | ||
| "@tradetrust-tt/tradetrust-config": "^1.19.0", | ||
| "@tradetrust-tt/tt-verify": "^9.5.0", | ||
| "@trustvc/trustvc": "^1.7.0", | ||
| "@tradetrust-tt/token-registry": "^5.5.0", | ||
| "@tradetrust-tt/tradetrust": "^6.10.2", | ||
| "@tradetrust-tt/tradetrust-config": "^1.19.1", | ||
| "@tradetrust-tt/tt-verify": "^9.5.1", | ||
| "@trustvc/trustvc": "^1.7.2", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider committing the regenerated lockfile
package-lock.json (or pnpm-lock.yaml / yarn.lock) is not included in this PR. Without it, CI and downstream consumers may resolve different patch versions than you validated locally, defeating the purpose of the controlled upgrade. Commit the updated lockfile to guarantee deterministic installs.
🤖 Prompt for AI Agents
In package.json around lines 75 to 80, the updated dependencies are listed but
the corresponding lockfile (such as package-lock.json, pnpm-lock.yaml, or
yarn.lock) is missing from the commit. To fix this, regenerate the lockfile by
running the package manager install command locally, then commit the updated
lockfile alongside package.json to ensure consistent and deterministic
dependency resolution in CI and downstream environments.
nghaninn
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/__tests__/wrap.test.ts (5)
19-28: Remove debug console.log statements.The console.log statements should be removed as they will clutter test output and are not necessary for the test functionality.
- jest.spyOn(fs, "lstatSync").mockImplementation((): any => { - console.log("🚀 ~ isDirectory:"); - return { - isDirectory: (): boolean => { - console.log("🚀 ~ isDirectory:"); - return true; - }, - }; - }); + jest.spyOn(fs, "lstatSync").mockImplementation((): any => { + return { + isDirectory: (): boolean => { + return true; + }, + }; + });
30-34: Remove debug console.log statements.The console.log statement should be removed to keep test output clean.
- jest.spyOn(fs, "readdirSync").mockImplementation((): any => { - console.log("🚀 ~ fs ~ readdirSync:"); - return ["file_1.json", "file_2.json", "file_3.json"]; - }); + jest.spyOn(fs, "readdirSync").mockImplementation((): any => { + return ["file_1.json", "file_2.json", "file_3.json"]; + });
37-53: Consider simplifying the fs.readdir mock implementation.While the comprehensive callback handling is thorough, the implementation might be overly complex for test purposes. The debug console.log statements should also be removed.
- const mockReaddirImpl = jest.fn().mockImplementation((path: any, options: any, callback: any) => { - console.log("🚀 ~ DIRECT fs.readdir called with path:", path); - - // Handle both callback style and promisified style - if (typeof options === "function") { - callback = options; - options = undefined; - } - - if (callback) { - // Ensure callback is called asynchronously to better simulate real behavior - process.nextTick(() => { - callback(null, ["file_1.json", "file_2.json", "file_3.json"]); - }); - } - return undefined; - }); + const mockReaddirImpl = jest.fn().mockImplementation((path: any, options: any, callback: any) => { + // Handle both callback style and promisified style + if (typeof options === "function") { + callback = options; + options = undefined; + } + + if (callback) { + process.nextTick(() => { + callback(null, ["file_1.json", "file_2.json", "file_3.json"]); + }); + } + return undefined; + });
59-68: Remove debug console.log statements from util.promisify mock.The console.log statements should be removed to keep test output clean.
jest.spyOn(util, "promisify").mockImplementation((fn) => { - console.log("🚀 ~ promisify:"); if (fn === fs.readdir) { - console.log("🚀 ~ promisifying fs.readdir"); return jest.fn().mockResolvedValue(["file_1.json", "file_2.json", "file_3.json"]); } // Fall back to original for other functions // @ts-ignore return jest.requireActual("util").promisify(fn); });
76-76: Remove debug console.log statement.The console.log statement should be removed to keep test output clean.
jest.spyOn(fs, "readFileSync").mockImplementation((path) => { - console.log("🚀 ~ readFileSync:", path); // @ts-ignore
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src/__tests__/unwrap.test.ts(3 hunks)src/__tests__/wrap.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests / Run Tests
🔇 Additional comments (5)
src/__tests__/unwrap.test.ts (3)
9-9: LGTM! Improved test isolation with explicit disk utility import.Adding the explicit import for the disk utility module enables more targeted mocking of the
documentsInDirectorymethod, improving test reliability and isolation.
37-37: LGTM! Enhanced mocking strategy for better test control.Directly mocking
disk.documentsInDirectoryprovides more precise control over the file discovery logic, complementing the existing fs module mocks. This ensures the test focuses on the unwrapping logic rather than file system traversal details.
83-85: LGTM! Consistent multi-file mocking approach.The multi-line mock setup for
documentsInDirectoryfollows the same pattern as the single-file test, ensuring consistency in the test suite's mocking strategy.src/__tests__/wrap.test.ts (2)
4-6: LGTM! Proper imports for enhanced mocking capabilities.The addition of disk utility and util module imports enables more comprehensive mocking of both file system operations and Node.js utility functions.
71-73: LGTM! Consistent disk utility mocking approach.The direct mocking of
disk.documentsInDirectoryaligns with the approach used in the unwrap tests and provides better test isolation.
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Summary
What is the background of this pull request?
Changes
Issues
What are the related issues or stories?
Summary by CodeRabbit
New Features
Documentation
Tests
Chores