Conversation
📝 WalkthroughWalkthroughThis pull request updates contract deployment configurations for Ethereum Sepolia testnet, introduces new deployment artifacts and broadcast records for ChannelEngine, ChannelHub, and escrow engine contracts, and adds a Forge script for registering node validators with ChannelHub. The configuration changes update contract address references to reflect new deployments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @dimast-x, 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, despite its title "New Contract Integration," primarily involves a minor update to the 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
|
dimast-x
changed the title
![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
The pull request introduces a minor change to the README.md file, adding an HTML comment. Since this change doesn't introduce any functional changes or potential issues, the review focuses on ensuring the change doesn't negatively impact the document's readability or maintainability.
![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
🧹 Nitpick comments (3)
contracts/script/RegisterNodeValidator.s.sol (2)
6-6: Remove the unusedMessageHashUtilsimport andusingdeclaration.The import on line 6 and the
using MessageHashUtils for bytesdeclaration on line 33 are unused in this contract — the EIP-191 hashing is performed insideTestUtils.signEip191. Remove both to improve clarity.Diff
-import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; - import {TestUtils} from "../test/TestUtils.sol";contract RegisterNodeValidator is Script { - using MessageHashUtils for bytes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/script/RegisterNodeValidator.s.sol` at line 6, Remove the unused OpenZeppelin import and using declaration: delete the line importing MessageHashUtils and the `using MessageHashUtils for bytes` declaration in RegisterNodeValidator.s.sol, since EIP-191 hashing is already handled by TestUtils.signEip191 and those symbols (MessageHashUtils / using MessageHashUtils for bytes) are never referenced in this contract.
8-8: Reconsider necessity of TestUtils import in deployment script.RegisterNodeValidator.s.sol is the only non-test file that imports TestUtils. While TestUtils lives in the test directory, the actual risk of tight coupling is minimal—only
signEip191andbuildAndSignValidatorRegistrationare used by this script, and any removal of TestUtils would fail at compile time, not silently. Consider whether extracting these signing utilities to a sharedsrc/library is worth the refactoring effort, but this is not a critical architectural concern given the current scope of usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/script/RegisterNodeValidator.s.sol` at line 8, RegisterNodeValidator.s.sol currently imports TestUtils only to use signEip191 and buildAndSignValidatorRegistration; extract those two functions into a shared source library (e.g., SigningUtils or ValidatorSigning in src/) and update RegisterNodeValidator.s.sol to import that new src library instead of the test-only TestUtils, then remove the TestUtils import; ensure the new library exposes signEip191 and buildAndSignValidatorRegistration with the same signatures so RegisterNodeValidator.s.sol compiles unchanged.contracts/broadcast/RegisterNodeValidator.s.sol/11155111/run-1772029057181.json (1)
67-69: Consider reducing timestamped artifact churn in VCS.If historical replay artifacts are not strictly required, consider committing only
run-latest.json(or documenting a retention policy) to reduce merge conflicts and repository noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/broadcast/RegisterNodeValidator.s.sol/11155111/run-1772029057181.json` around lines 67 - 69, The repository currently commits timestamped replay artifacts (e.g., RegisterNodeValidator.s.sol/.../run-1772029057181.json), which causes churn; update the process to commit only a stable artifact like run-latest.json (or add a documented retention policy) and remove or gitignore historical timestamped run-*.json files, ensuring build/replay tooling writes timestamped files to a local artifacts/ or tmp/ directory instead of the VCS; update any references in CI or docs to read run-latest.json or follow the retention policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/script/RegisterNodeValidator.s.sol`:
- Around line 35-42: The call in run() that does
uint8(vm.envUint("VALIDATOR_ID")) can silently wrap values >255; before casting,
read the uint256 from vm.envUint("VALIDATOR_ID"), validate it is within 1..255
(or your allowed range) and only then cast to uint8 and assign to validatorId;
update the overloaded run call to use the validated uint8 and handle/report
out-of-range values (e.g., revert or vm.stop/emit) so invalid env inputs cannot
bypass the validatorId != 0 guard.
In `@README.md`:
- Around line 146-147: Delete the stray HTML comment "<!-- Branch for updatinc
sc and further fixes on clearnode and sdk -->" from the README (it's a dev
reminder with a typo) so it isn't merged into main; simply remove that comment
line and ensure no other development-only HTML comments remain in README.md.
---
Nitpick comments:
In
`@contracts/broadcast/RegisterNodeValidator.s.sol/11155111/run-1772029057181.json`:
- Around line 67-69: The repository currently commits timestamped replay
artifacts (e.g., RegisterNodeValidator.s.sol/.../run-1772029057181.json), which
causes churn; update the process to commit only a stable artifact like
run-latest.json (or add a documented retention policy) and remove or gitignore
historical timestamped run-*.json files, ensuring build/replay tooling writes
timestamped files to a local artifacts/ or tmp/ directory instead of the VCS;
update any references in CI or docs to read run-latest.json or follow the
retention policy.
In `@contracts/script/RegisterNodeValidator.s.sol`:
- Line 6: Remove the unused OpenZeppelin import and using declaration: delete
the line importing MessageHashUtils and the `using MessageHashUtils for bytes`
declaration in RegisterNodeValidator.s.sol, since EIP-191 hashing is already
handled by TestUtils.signEip191 and those symbols (MessageHashUtils / using
MessageHashUtils for bytes) are never referenced in this contract.
- Line 8: RegisterNodeValidator.s.sol currently imports TestUtils only to use
signEip191 and buildAndSignValidatorRegistration; extract those two functions
into a shared source library (e.g., SigningUtils or ValidatorSigning in src/)
and update RegisterNodeValidator.s.sol to import that new src library instead of
the test-only TestUtils, then remove the TestUtils import; ensure the new
library exposes signEip191 and buildAndSignValidatorRegistration with the same
signatures so RegisterNodeValidator.s.sol compiles unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.mdclearnode/chart/config/v1-rc/blockchains.yamlcontracts/broadcast/RegisterNodeValidator.s.sol/11155111/run-1772029057181.jsoncontracts/broadcast/RegisterNodeValidator.s.sol/11155111/run-latest.jsoncontracts/deployments/11155111/ChannelEngine.sol_ChannelEngine/2026-02-25T13-52-01.jsoncontracts/deployments/11155111/ChannelHub.sol_ChannelHub/2026-02-25T13-59-25.jsoncontracts/deployments/11155111/EscrowDepositEngine.sol_EscrowDepositEngine/2026-02-25T13-52-50.jsoncontracts/deployments/11155111/EscrowWithdrawalEngine.sol_EscrowWithdrawalEngine/2026-02-25T13-53-36.jsoncontracts/script/RegisterNodeValidator.s.sol
dimast-x
force-pushed
the
feat/new-contract-support
branch
from
2e44234 to
f5ca30d
Compare
dimast-x
force-pushed
the
feat/new-contract-support
branch
from
f5ca30d to
d55c2c8
Compare
dimast-x
changed the title
dimast-x
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Summary by CodeRabbit
New Features
Configuration