Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 49 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates stress-v1 from Polygon Amoy (80002) to Base Sepolia (84532), updates asset token addresses/decimals and RPC secret, increases runtime/container resources, initializes a missing backbone field, adds/removes deployment artifacts, and enhances the deployment script to precompute and log CREATE2 library addresses. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/script/DeployChannelHub.s.sol (1)
57-69:⚠️ Potential issue | 🟠 MajorAccount for library deployment transactions before computing CREATE addresses.
Foundry's auto-linked library deployments consume deployer nonces as CREATE2 transactions. The Base Sepolia broadcast artifact confirms three CREATE2 transactions (ChannelEngine, EscrowWithdrawalEngine, EscrowDepositEngine) at nonces 0–2 before the ECDSAValidator CREATE at nonce 3. The current code ignores these library deployments and uses the initial nonce directly for
vm.computeCreateAddress()calls, resulting in incorrect predicted addresses.Proposed fix
uint64 nonce = vm.getNonce(deployer); + uint64 createNonce = nonce; + + if (channelEngineAddr.code.length == 0) { + createNonce++; + } + if (escrowDepositAddr.code.length == 0) { + createNonce++; + } + if (escrowWithdrawalAddr.code.length == 0) { + createNonce++; + } bool deployValidator = defaultValidatorAddr == address(0); if (deployValidator) { - console.log("ECDSAValidator: ", vm.computeCreateAddress(deployer, nonce)); - nonce++; + console.log("ECDSAValidator: ", vm.computeCreateAddress(deployer, createNonce)); + createNonce++; } else { console.log("DefaultValidator: ", defaultValidatorAddr); } console.log("ChannelEngine: ", channelEngineAddr); console.log("EscrowDepositEngine: ", escrowDepositAddr); console.log("EscrowWithdrawalEngine: ", escrowWithdrawalAddr); - console.log("ChannelHub: ", vm.computeCreateAddress(deployer, nonce)); + console.log("ChannelHub: ", vm.computeCreateAddress(deployer, createNonce));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/script/DeployChannelHub.s.sol` around lines 57 - 69, The predicted CREATE addresses are wrong because the code uses the initial vm.getNonce(deployer) without accounting for Foundry's auto-linked library CREATE2 deployments (ChannelEngine, EscrowWithdrawalEngine, EscrowDepositEngine) that consume nonces; update the logic in DeployChannelHub.s.sol around vm.getNonce, deployValidator, and vm.computeCreateAddress so you increment the nonce by the number of prior CREATE/CREATE2 deploys (or refresh vm.getNonce after those library deployments) before calling vm.computeCreateAddress for the ECDSAValidator and ChannelHub (ensure the same nonce variable is used consistently when computing addresses for channelEngineAddr, escrowDepositAddr, escrowWithdrawalAddr, ECDSAValidator and ChannelHub).
🧹 Nitpick comments (5)
contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.json (2)
12-12: Cross-chain decimal discrepancy: document the choice.The YUSD test token now uses 8 decimals on Base Sepolia (chain 84532), which maintains parity with the previous Polygon Amoy deployment but differs from Ethereum Sepolia (6 decimals). Based on learnings, deployment manifests should document per-chain decimal values to ensure cross-chain transfer logic and tooling handle these discrepancies gracefully.
📝 Suggested documentation in the comment field
Populate the
commentfield (line 16) with context such as:- "comment": "" + "comment": "Base Sepolia deployment using 8 decimals (matches Amoy; differs from Ethereum Sepolia's 6 decimals). Cross-chain decimal discrepancies are intentional for test coverage."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.json` at line 12, Update the deployment manifest's comment field in the PremintERC20 deployment JSON to document the per-chain decimals choice: note that on chain 84532 (Base Sepolia) the YUSD test token uses 8 decimals (matching Polygon Amoy) while Ethereum Sepolia uses 6, and include explicit guidance for cross-chain transfer tooling and contract consumers to handle this discrepancy (e.g., normalization rules, expected behavior, and any migration or compatibility notes). Reference the deployment manifest's "comment" field for PremintERC20 to add this contextual information so integrators can adjust transfer logic and tooling accordingly.
16-16: Populate the comment field for deployment context.The empty comment field misses an opportunity to document the deployment context, decimal choices, and any cross-chain considerations. Based on learnings, deployment scripts should document per-chain decimals to aid future maintainers and ensure test coverage accounts for cross-chain behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.json` at line 16, Populate the empty "comment" field in the deployment JSON for PremintERC20 (PremintERC20.sol_PremintERC20) with a concise deployment context: state the the chosen decimals, rationale for that decimals choice, any chain-specific differences (list per-chain decimals if they vary), mention cross-chain bridging/compatibility concerns and required test coverage (e.g., token arithmetic, transfer/bridge edge cases), and include references to the deployment script or environment used so future maintainers can reproduce the deployment.contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T09-59-36.json (1)
9-16: Consider documenting the decimal choice in the comment field.The deployment uses 8 decimals for the BNB test token on Base Sepolia (chain 84532). Based on learnings, cross-chain decimal differences are intentional and mirror real-world token behavior, but deployment manifests should document per-chain decimal choices.
📝 Suggested documentation improvement
- "comment": "" + "comment": "BNB test token deployed with 8 decimals on Base Sepolia (chain 84532) to mirror real-world cross-chain decimal behavior."Based on learnings: "In the nitrolite repository, this learning notes that the YUSD (PremintERC20) test token uses different decimal values across chains (6 decimals on chain 11155111 Sepolia and 8 decimals on chain 80002 Amoy). Treat cross-chain decimal discrepancies as a test scenario: ensure tests and deployment scripts document per-chain decimals and validate any cross-chain transfer logic or tooling handles inconsistent decimals gracefully."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T09-59-36.json` around lines 9 - 16, The deployment manifest for PremintERC20 includes constructorArgs with the decimals value "8" for chain 84532 but the comment field is empty; update the manifest's "comment" field to document why 8 decimals were chosen for this BNB test token on chain 84532 (reference PremintERC20 and the constructorArgs array), note that decimals differ across chains (e.g., 6 on Sepolia, 8 on Amoy) and add a short reminder to verify cross-chain tests and tooling handle inconsistent decimals when using this deployment.contracts/deployments/84532/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-48-16.json (1)
8-10: Normalize manifest fields for parser stabilityLine 8 (
./src/...) and Line 10 (emptycomment) are inconsistent with the rest of this deployment set. Consider standardizing tosrc/...and a non-empty release tag to keep manifest consumers deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/deployments/84532/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-48-16.json` around lines 8 - 10, The manifest has inconsistent fields: the "contractPath" value "./src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator" should be normalized to "src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator" (remove the leading "./") and the "comment" field should be populated with a deterministic release tag or non-empty identifier (e.g., "release: vYYYY-MM-DD" or the PR/CI tag) so parsers consume a consistent manifest; update these two keys in the JSON for the SessionKeyValidator entry to match the rest of the deployment set.contracts/deployments/11155111/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-01-49.json (1)
8-10: Consider aligning this manifest format with other deployment entriesLine 8 uses
./src/...and Line 10 keepscommentempty, unlike most records in this rollout. Standardizing these fields will make downstream parsing/filtering less brittle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/deployments/11155111/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-01-49.json` around lines 8 - 10, Manifest entry for SessionKeyValidator uses a different path format and leaves comment empty; update the "contractPath" value to match other records by removing the leading "./" so it reads "src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator" and populate the "comment" field with the same default/comment convention used by other deployment entries (e.g., a short descriptive string or null) so parsing/filtering is consistent; edit the JSON keys "contractPath" and "comment" for the SessionKeyValidator manifest entry to apply these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clearnode/chart/config/stress-v1/assets.yaml`:
- Around line 13-23: The canonical asset entry for "BNB" has decimals: 8 while
the token under blockchain_id 11155111 (the suggested_chain) uses 18 decimals;
update the BNB asset's top-level decimals to 18 to match the
suggested_blockchain_id's token (or, if you intended to make the 8-decimal chain
canonical, change suggested_blockchain_id to the 84532 token), ensuring the
fields name: "BNB", decimals, suggested_blockchain_id and the tokens array stay
consistent.
In `@clearnode/chart/config/stress-v1/clearnode.yaml`:
- Around line 45-52: The CPU request/limit are inverted: under the resources
block the requests.cpu is 1000m while limits.cpu is 500m; update the resources
section so requests.cpu is less than or equal to limits.cpu (either lower
requests.cpu to 500m or raise limits.cpu to 1000m) — edit the resources ->
requests -> cpu value (and/or resources -> limits -> cpu) in clearnode.yaml to
make millicpu(request) <= millicpu(limit).
In `@clearnode/runtime.go`:
- Around line 204-208: Backbone is constructed with ChannelMinChallengeDuration
and ChannelMaxChallengeDuration but no validation is done; add a precondition
check before returning &Backbone that fails fast (returns an error or panics per
project convention) when conf.ChannelMaxChallengeDuration <
conf.ChannelMinChallengeDuration so invalid config is rejected; update the
function that creates the Backbone (the code returning &Backbone in runtime.go)
to validate these two fields and emit a clear error message referencing
ChannelMinChallengeDuration and ChannelMaxChallengeDuration if the check fails.
In `@contracts/broadcast/DeployChannelHub.s.sol/84532/run-latest.json`:
- Around line 122-185: The broadcast artifact has misaligned
receipt-to-transaction mappings (example mismatches: transactionHash
"0x93cf089b7e57207..." appears on a CREATE2 factory receipt and
"0x3a7095c50cafe07..." appears on a CREATE receipt), so regenerate the broadcast
JSON using the same forge/hardhat broadcast command that produced it, ensuring
you re-run the full broadcast session so each receipt's transactionHash, to/null
(CREATE), and contractAddress entries are produced in the correct order and
saved; verify post-run that entries where "to": null correspond to the intended
CREATE transactionHash (e.g., "0x3a7095c50cafe07...") and that factory CREATE2
receipts hold the correct ECDSA factory tx hash (e.g., "0x93cf089b7e57207...")
before committing the artifact.
In
`@contracts/deployments/11155111/ChannelEngine.sol_ChannelEngine/2026-04-20T09-12-12.json`:
- Around line 3-10: The ChannelEngine deployment address in this artifact
(deployedTo in ChannelEngine) conflicts with the address referenced by
ChannelHub’s linked library; locate the ChannelEngine artifact (symbol
ChannelEngine and its deployedTo field) and the ChannelHub manifest that links
ChannelEngine, then regenerate or retag the release artifacts so both artifacts
use the same canonical address for this release (ensure commit and chainId
remain consistent), e.g., rebuild the deployment JSON for ChannelEngine with the
address referenced by ChannelHub or update ChannelHub’s link to the correct
ChannelEngine address and re-run artifact generation.
In
`@contracts/deployments/11155111/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-13-24.json`:
- Line 8: The contractPath value contains a typo
"src/sigValigators/ECDSAValidator.sol:ECDSAValidator" which should be
"src/sigValidators/..." — update the contractPath string in the JSON to replace
"sigValigators" with "sigValidators" so tooling that depends on the contractPath
(the contractPath key and the referenced ECDSAValidator contract) will resolve
correctly.
In
`@contracts/deployments/11155111/EscrowDepositEngine.sol_EscrowDepositEngine/2026-04-20T09-12-12.json`:
- Around line 3-10: The EscrowDepositEngine artifact's deployedTo value
(0xebfecc...EBD4) for contractPath EscrowDepositEngine on chainId 11155111
conflicts with the ChannelHub manifest which links EscrowDepositEngine at
0xb5c2...6f91; regenerate or retag the artifact so the deployedTo matches the
address used by ChannelHub for the same commit/release (commit
6c0a41d5b08eb6356fd7391536ede1db8d0b6a73 / comment clearnet-stress-v1.2.1), or
update the ChannelHub manifest to the correct library address, ensuring both
EscrowDepositEngine and ChannelHub reference the same library address and then
re-release the artifact metadata.
In
`@contracts/deployments/84532/ChannelHub.sol_ChannelHub/2026-04-20T09-46-56.json`:
- Around line 10-14: The "libraries" field currently contains an array of
strings; change it to an object mapping each library identifier to its address
to match other deployments (e.g., replace the array with an object where keys
are "src/ChannelEngine.sol:ChannelEngine",
"src/EscrowDepositEngine.sol:EscrowDepositEngine", and
"src/EscrowWithdrawalEngine.sol:EscrowWithdrawalEngine" and values are their
respective addresses 0x353A207A7bC822D8d3E58bcA2f3F9E2b90B26e78,
0xb5c2e8BAD7417E654E9087753EC1D08762a06f91,
0xaf9F833141094083B9de2cb6AA0f7E1f2d2DEee1) so consumers see a consistent
object-schema for "libraries".
In
`@contracts/deployments/84532/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-46-56.json`:
- Around line 3-4: The manifest contains a duplicate deployment: the address
0xce87...55ec with transactionHash 0x3a70...28f5c is assigned to both
ECDSAValidator and EscrowDepositEngine in the JSON deployment entry; locate the
manifest file entry that lists "deployedTo":
"0xCe87FD88F4B5Fd5475d163e2642C5c2c7dD655Ec" (the JSON for
ECDSAValidator/EscrowDepositEngine) and fix by removing or correcting the wrong
record so only the true contract retains that deployedTo/transactionHash
pair—either delete the incorrect contract entry (EscrowDepositEngine or
ECDSAValidator) or replace its deployedTo/transactionHash with the correct
values from the actual chain deployment, then re-run whatever
deployment/manifest generation to validate uniqueness.
---
Outside diff comments:
In `@contracts/script/DeployChannelHub.s.sol`:
- Around line 57-69: The predicted CREATE addresses are wrong because the code
uses the initial vm.getNonce(deployer) without accounting for Foundry's
auto-linked library CREATE2 deployments (ChannelEngine, EscrowWithdrawalEngine,
EscrowDepositEngine) that consume nonces; update the logic in
DeployChannelHub.s.sol around vm.getNonce, deployValidator, and
vm.computeCreateAddress so you increment the nonce by the number of prior
CREATE/CREATE2 deploys (or refresh vm.getNonce after those library deployments)
before calling vm.computeCreateAddress for the ECDSAValidator and ChannelHub
(ensure the same nonce variable is used consistently when computing addresses
for channelEngineAddr, escrowDepositAddr, escrowWithdrawalAddr, ECDSAValidator
and ChannelHub).
---
Nitpick comments:
In
`@contracts/deployments/11155111/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-01-49.json`:
- Around line 8-10: Manifest entry for SessionKeyValidator uses a different path
format and leaves comment empty; update the "contractPath" value to match other
records by removing the leading "./" so it reads
"src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator" and populate the
"comment" field with the same default/comment convention used by other
deployment entries (e.g., a short descriptive string or null) so
parsing/filtering is consistent; edit the JSON keys "contractPath" and "comment"
for the SessionKeyValidator manifest entry to apply these changes.
In
`@contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T09-59-36.json`:
- Around line 9-16: The deployment manifest for PremintERC20 includes
constructorArgs with the decimals value "8" for chain 84532 but the comment
field is empty; update the manifest's "comment" field to document why 8 decimals
were chosen for this BNB test token on chain 84532 (reference PremintERC20 and
the constructorArgs array), note that decimals differ across chains (e.g., 6 on
Sepolia, 8 on Amoy) and add a short reminder to verify cross-chain tests and
tooling handle inconsistent decimals when using this deployment.
In
`@contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.json`:
- Line 12: Update the deployment manifest's comment field in the PremintERC20
deployment JSON to document the per-chain decimals choice: note that on chain
84532 (Base Sepolia) the YUSD test token uses 8 decimals (matching Polygon Amoy)
while Ethereum Sepolia uses 6, and include explicit guidance for cross-chain
transfer tooling and contract consumers to handle this discrepancy (e.g.,
normalization rules, expected behavior, and any migration or compatibility
notes). Reference the deployment manifest's "comment" field for PremintERC20 to
add this contextual information so integrators can adjust transfer logic and
tooling accordingly.
- Line 16: Populate the empty "comment" field in the deployment JSON for
PremintERC20 (PremintERC20.sol_PremintERC20) with a concise deployment context:
state the the chosen decimals, rationale for that decimals choice, any
chain-specific differences (list per-chain decimals if they vary), mention
cross-chain bridging/compatibility concerns and required test coverage (e.g.,
token arithmetic, transfer/bridge edge cases), and include references to the
deployment script or environment used so future maintainers can reproduce the
deployment.
In
`@contracts/deployments/84532/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-48-16.json`:
- Around line 8-10: The manifest has inconsistent fields: the "contractPath"
value "./src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator" should
be normalized to "src/sigValidators/SessionKeyValidator.sol:SessionKeyValidator"
(remove the leading "./") and the "comment" field should be populated with a
deterministic release tag or non-empty identifier (e.g., "release: vYYYY-MM-DD"
or the PR/CI tag) so parsers consume a consistent manifest; update these two
keys in the JSON for the SessionKeyValidator entry to match the rest of the
deployment set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7a64a5a-525e-4e29-be7a-d6aaa45a81ad
📒 Files selected for processing (25)
clearnode/chart/config/stress-v1/assets.yamlclearnode/chart/config/stress-v1/blockchains.yamlclearnode/chart/config/stress-v1/clearnode.yamlclearnode/chart/config/stress-v1/secrets.yamlclearnode/runtime.gocontracts/broadcast/DeployChannelHub.s.sol/11155111/run-1776676404535.jsoncontracts/broadcast/DeployChannelHub.s.sol/11155111/run-latest.jsoncontracts/broadcast/DeployChannelHub.s.sol/80002/run-latest.jsoncontracts/broadcast/DeployChannelHub.s.sol/84532/run-1776678417339.jsoncontracts/broadcast/DeployChannelHub.s.sol/84532/run-latest.jsoncontracts/deployments/11155111/ChannelEngine.sol_ChannelEngine/2026-04-20T09-12-12.jsoncontracts/deployments/11155111/ChannelHub.sol_ChannelHub/2026-04-20T09-13-24.jsoncontracts/deployments/11155111/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-13-24.jsoncontracts/deployments/11155111/EscrowDepositEngine.sol_EscrowDepositEngine/2026-04-20T09-12-12.jsoncontracts/deployments/11155111/EscrowWithdrawalEngine.sol_EscrowWithdrawalEngine/2026-04-20T09-12-12.jsoncontracts/deployments/11155111/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-01-49.jsoncontracts/deployments/84532/ChannelEngine.sol_ChannelEngine/2026-04-20T09-46-56.jsoncontracts/deployments/84532/ChannelHub.sol_ChannelHub/2026-04-20T09-46-56.jsoncontracts/deployments/84532/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-46-56.jsoncontracts/deployments/84532/EscrowDepositEngine.sol_EscrowDepositEngine/2026-04-20T09-46-56.jsoncontracts/deployments/84532/EscrowWithdrawalEngine.sol_EscrowWithdrawalEngine/2026-04-20T09-46-56.jsoncontracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T09-59-36.jsoncontracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.jsoncontracts/deployments/84532/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-48-16.jsoncontracts/script/DeployChannelHub.s.sol
💤 Files with no reviewable changes (1)
- contracts/broadcast/DeployChannelHub.s.sol/80002/run-latest.json
5043dd2 to
6829d24
Compare
Summary by CodeRabbit
New Features
Bug Fixes
Chores