Conversation
* [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts * [AUTOMATED] Update: proto/clients/indexer*.gen.ts --------- Co-authored-by: marino39 <722509+marino39@users.noreply.github.com> Co-authored-by: pkieltyka <18831+pkieltyka@users.noreply.github.com> Co-authored-by: xiam <385670+xiam@users.noreply.github.com>
…e#966) * fix(relayer): wrap feeOptions calldata for simulation * fix(relayer): request FeeOptions as wallet (match wallet-webapp-v3) * fix(relayer): restore AbiFunction import
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Dargon789, your pull request is larger than the review limit of 150000 diff characters
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello @Dargon789, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the project's blockchain compatibility by integrating new networks and addresses a critical bug in the relayer's transaction fee estimation logic. It also enhances development workflow and project transparency through the addition of CI automation, standardized issue reporting, and clear project governance documents. These changes collectively improve the platform's robustness, security, and maintainability. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on updating package versions, adding new chain configurations, and making a security improvement in ID generation. It also includes new issue templates and a basic CircleCI configuration. A security audit found no vulnerabilities meeting the required severity threshold (Medium, High, or Critical) in the implemented changes, and the changes appear to be well-contained and address the stated goals.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/wallet/dapp-client/src/DappTransport.ts (563-567)
The change to use window.crypto.getRandomValues() instead of Math.random() for generating random parts of the ID is a significant security improvement. Math.random() is not cryptographically secure and should not be used for generating unique identifiers that might have security implications, such as session or request IDs. This change correctly addresses that concern.
.circleci/config.yml (15)
The job name web3-defi-game-project- ends with a hyphen. While functional, it's an unconventional naming practice and could be a typo. It's generally recommended to use descriptive names without trailing punctuation for clarity and consistency in CI/CD configurations.
There was a problem hiding this comment.
Code Review
This pull request is an automated release, bumping versions to v3.0.0-beta.17, updating changelogs, adding new chains (HYPEREVM, SONIC, BERACHAIN), and fixing the relayer RPC. A security audit found no critical vulnerabilities, and noted that the DApp transport improvements using cryptographically secure randomness and adherence to security best practices are consistent with the implementation. Regarding code quality, feedback includes addressing a hardcoded repository path and an unusual job name in .circleci/config.yml, along with a suggestion to add a placeholder step to an empty job. Additionally, in packages/wallet/dapp-client/src/DappTransport.ts, while the switch to window.crypto is a good security enhancement, it introduces a regression for non-browser environments; a fix using globalThis.crypto with a fallback is suggested for broader compatibility. The other changes, including the relayer fix and new network additions, look good.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/wallet/dapp-client/src/DappTransport.ts (563-567)
Using window.crypto is a great security improvement over Math.random(). However, this will break in non-browser environments like Node.js where window is not defined. To ensure compatibility, you can use globalThis.crypto, which is available in modern browsers and Node.js versions. It's also good practice to include a fallback to the previous Math.random() implementation for older environments that might not support the Web Crypto API.
// Use crypto.getRandomValues for cryptographically secure randomness
const array = new Uint32Array(2);
// Use globalThis.crypto for broader environment support (browsers, Node.js >= 19)
if (typeof globalThis.crypto?.getRandomValues === 'function') {
globalThis.crypto.getRandomValues(array);
const randStr = (array[0].toString(36) + array[1].toString(36)).slice(0, 9);
return `${Date.now().toString(36)}-${randStr}`;
} else {
// Fallback for older environments that don't support Web Crypto API
return `${Date.now().toString(36)}-${Math.random().toString(36).substring(2, 9)}`;
}.circleci/config.yml (11)
This comment seems to contain a hardcoded repository path from another project (Dargon789/hardhat-project). It would be better to make this more generic or remove it to avoid confusion for future contributors.
# visit app.circleci.com/settings/project/github/<your-org>/<your-repo>/environment-variables.circleci/config.yml (15)
The job name web3-defi-game-project- ends with a dash, which looks like a typo. Consider renaming it to something more descriptive of its purpose, for example build-and-test. If you do, remember to also update the workflow on line 24.
build-and-test:.circleci/config.yml (19)
This job only performs a checkout and has no other steps. If this is a placeholder for future CI steps, consider adding a comment or a placeholder run step to clarify its purpose and prevent it from being overlooked.
- checkout
- run:
name: Placeholder for future steps
command: echo "Build and test steps will be added here."3 participants
No description provided.