YNU-905: feat: middle audit deploy#694
Merged
Merged
Conversation
nksazonov
requested review from
dimast-x,
ihsraham and
philanton
as code owners
Contributor
📝 WalkthroughWalkthroughThis PR migrates the stress-v1 test environment configuration from Polygon Amoy (chain ID 80002) to Base Sepolia (chain ID 84532), including blockchain configuration updates, asset/token remapping, contract deployments to both Ethereum Sepolia and Base Sepolia chains, and corresponding deployment metadata. It also enhances the ChannelHub deployment script with deterministic library address computation, initializes runtime configuration for channel max challenge duration, and bumps SDK package versions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
philanton
approved these changes
Apr 20, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
philanton
force-pushed
the
feat/middle-audit-deploy
branch
2 times, most recently
from
a865b12 to
5e33f78
Compare
philanton
force-pushed
the
feat/middle-audit-deploy
branch
from
5e33f78 to
c6f1448
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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)
33-40:⚠️ Potential issue | 🟡 MinorAdd NatSpec to both
runpublic APIs.
run()andrun(address,address)are public/external and should be documented with function-level NatSpec to meet repo standards.Proposed patch
contract DeployChannelHub is Script { + /// `@notice` Entry point for CLI execution. Reads env vars and delegates to overloaded run. function run() external { // Optional: reuse an existing validator or deploy a fresh ECDSAValidator address defaultValidatorAddr = vm.envOr("DEFAULT_VALIDATOR_ADDR", address(0)); address nodeAddr = vm.envAddress("NODE_ADDR"); run(defaultValidatorAddr, nodeAddr); } + /// `@notice` Deploys ChannelHub and optionally deploys an ECDSAValidator. + /// `@param` defaultValidatorAddr Existing validator address; set to zero address to deploy a new ECDSAValidator. + /// `@param` nodeAddr Node address passed into ChannelHub constructor. function run(address defaultValidatorAddr, address nodeAddr) public {As per coding guidelines,
**/*.sol: "Add NatSpec comments on all public/external functions in Solidity".🤖 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 33 - 40, Add Solidity NatSpec comments for the two public/external functions so their purpose, parameters and any side-effects are documented; specifically add a `@notice` (brief purpose) and `@param` tags for run() and run(address defaultValidatorAddr, address nodeAddr) and describe behavior (e.g., optional reuse of validator, reading NODE_ADDR from env, and that it calls the overloaded run). Target the functions named run() and run(address,uint256) — use the exact signatures run() and run(address defaultValidatorAddr, address nodeAddr) in the comments so linters and readers can map them to the implementations.
♻️ Duplicate comments (1)
clearnode/runtime.go (1)
204-208:⚠️ Potential issue | 🟠 MajorFail fast on invalid challenge duration bounds.
Line 207 fixes the zero-value bug, but
InitBackbone()still acceptsChannelMaxChallengeDuration < ChannelMinChallengeDuration. That bad config flows straight intoapi.RPCRouterConfig.MaxChallengeinclearnode/main.go:45-58, so channel validation will fail later instead of at startup. Add the precondition before constructingBackbone.🛡️ Proposed fix
blockchainRPCs := initBlockchainRPCs(logger, memoryStore) + if conf.ChannelMaxChallengeDuration < conf.ChannelMinChallengeDuration { + logger.Fatal( + "invalid challenge duration configuration", + "ChannelMinChallengeDuration", conf.ChannelMinChallengeDuration, + "ChannelMaxChallengeDuration", conf.ChannelMaxChallengeDuration, + ) + } + return &Backbone{ NodeVersion: Version, ChannelMinChallengeDuration: conf.ChannelMinChallengeDuration, ChannelMaxChallengeDuration: conf.ChannelMaxChallengeDuration,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clearnode/runtime.go` around lines 204 - 208, InitBackbone currently allows ChannelMaxChallengeDuration < ChannelMinChallengeDuration which defers a config error into api.RPCRouterConfig.MaxChallenge; add a precondition check in InitBackbone() before constructing and returning &Backbone{...} that validates conf.ChannelMaxChallengeDuration >= conf.ChannelMinChallengeDuration and fail fast (return an error or panic consistent with InitBackbone's error handling) with a clear message mentioning the two fields (ChannelMinChallengeDuration and ChannelMaxChallengeDuration) so invalid configs are rejected at startup.
🤖 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/clearnode.yaml.gotmpl`:
- Around line 49-60: Remove the Git conflict markers and the hardcoded resource
block and restore the profile-backed template: keep the {{- toYaml $p.resources
| nindent 2 }} expression (and its surrounding indentation) and delete the
=======/>>>>>>> lines and the explicit limits/requests stanza so Helm renders
using the profiles.yaml values.
---
Outside diff comments:
In `@contracts/script/DeployChannelHub.s.sol`:
- Around line 33-40: Add Solidity NatSpec comments for the two public/external
functions so their purpose, parameters and any side-effects are documented;
specifically add a `@notice` (brief purpose) and `@param` tags for run() and
run(address defaultValidatorAddr, address nodeAddr) and describe behavior (e.g.,
optional reuse of validator, reading NODE_ADDR from env, and that it calls the
overloaded run). Target the functions named run() and run(address,uint256) — use
the exact signatures run() and run(address defaultValidatorAddr, address
nodeAddr) in the comments so linters and readers can map them to the
implementations.
---
Duplicate comments:
In `@clearnode/runtime.go`:
- Around line 204-208: InitBackbone currently allows ChannelMaxChallengeDuration
< ChannelMinChallengeDuration which defers a config error into
api.RPCRouterConfig.MaxChallenge; add a precondition check in InitBackbone()
before constructing and returning &Backbone{...} that validates
conf.ChannelMaxChallengeDuration >= conf.ChannelMinChallengeDuration and fail
fast (return an error or panic consistent with InitBackbone's error handling)
with a clear message mentioning the two fields (ChannelMinChallengeDuration and
ChannelMaxChallengeDuration) so invalid configs are rejected at startup.
🪄 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: 8d3986d2-101b-4811-afd2-c63c67e9d14b
⛔ Files ignored due to path filters (2)
sdk/ts-compat/package-lock.jsonis excluded by!**/package-lock.jsonsdk/ts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
clearnode/chart/config/stress-v1/assets.yamlclearnode/chart/config/stress-v1/blockchains.yamlclearnode/chart/config/stress-v1/clearnode.yaml.gotmplclearnode/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.solsdk/ts-compat/package.jsonsdk/ts/package.json
💤 Files with no reviewable changes (1)
- contracts/broadcast/DeployChannelHub.s.sol/80002/run-latest.json
✅ Files skipped from review due to trivial changes (17)
- sdk/ts/package.json
- contracts/deployments/84532/EscrowDepositEngine.sol_EscrowDepositEngine/2026-04-20T09-46-56.json
- contracts/deployments/11155111/EscrowDepositEngine.sol_EscrowDepositEngine/2026-04-20T09-12-12.json
- contracts/deployments/84532/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-46-56.json
- contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T10-00-15.json
- contracts/deployments/84532/EscrowWithdrawalEngine.sol_EscrowWithdrawalEngine/2026-04-20T09-46-56.json
- contracts/deployments/11155111/ChannelEngine.sol_ChannelEngine/2026-04-20T09-12-12.json
- contracts/deployments/84532/ChannelEngine.sol_ChannelEngine/2026-04-20T09-46-56.json
- contracts/deployments/11155111/EscrowWithdrawalEngine.sol_EscrowWithdrawalEngine/2026-04-20T09-12-12.json
- contracts/deployments/84532/ChannelHub.sol_ChannelHub/2026-04-20T09-46-56.json
- contracts/deployments/11155111/ChannelHub.sol_ChannelHub/2026-04-20T09-13-24.json
- contracts/deployments/84532/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-48-16.json
- contracts/broadcast/DeployChannelHub.s.sol/84532/run-1776678417339.json
- contracts/broadcast/DeployChannelHub.s.sol/84532/run-latest.json
- sdk/ts-compat/package.json
- contracts/broadcast/DeployChannelHub.s.sol/11155111/run-1776676404535.json
- clearnode/chart/config/stress-v1/blockchains.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- contracts/deployments/84532/PremintERC20.sol_PremintERC20/2026-04-20T09-59-36.json
- contracts/deployments/11155111/SessionKeyValidator.sol_SessionKeyValidator/2026-04-20T09-01-49.json
- contracts/deployments/11155111/ECDSAValidator.sol_ECDSAValidator/2026-04-20T09-13-24.json
- clearnode/chart/config/stress-v1/secrets.yaml
- clearnode/chart/config/stress-v1/assets.yaml
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Network Updates
Asset Updates
Releases