Devnet redeployment #2046
Devnet redeployment #2046
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (4)
⛔ Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThis update introduces redeployment and upgrade of multiple smart contracts on the Arbitrum Sepolia Devnet. It adds new deployment artifacts, updates contract addresses, ABIs, and proxy metadata, and revises deployment scripts and documentation. Notably, new dispute kits and proxies are deployed, initialization methods and ABI signatures are updated, and deployment instructions are clarified. Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer
participant Proxy
participant Implementation
participant Registry
participant DisputeKit
participant CourtConfig
Deployer->>Proxy: Deploy new Proxy with Implementation address
Deployer->>Implementation: Deploy new Implementation contract
Deployer->>Proxy: Initialize Proxy (calls initializeX)
Deployer->>Registry: Deploy/Upgrade Registry contracts
Deployer->>DisputeKit: Deploy/Upgrade DisputeKit contracts
Deployer->>CourtConfig: Populate courts and policy registry
Deployer->>Proxy: (If upgrade) Call upgradeToAndCall(newImplementation)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json (1)
1061-1072: Having bothinitializeand legacyinitialize8invites mis-usageLeaving the zero-arg
initialize8()callable post-deployment means:
- Anyone can later re-initialise the implementation contract,
- A mistaken proxy upgrade could delegatecall it and brick state.
Unless you rely on it for backward compatibility, remove or make it
onlyInitializing.contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreSnapshotProxy.json (1)
196-213: Watch out for storage-layout drift in future upgrades
storageLayoutshows slot 0 =core, slot 1 =governor. Any implementation upgrade that reorders/adds state vars before these two will brick the proxy. Keep this ordering immutable or useStorageGap.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic.json (1)
959-969: Two initializers may confuse upgrade pluginsBoth
initializeandinitialize7arenonpayablepublic methods.
OpenZeppelin Upgrade Safe-checks consider any external function callable only once with theinitializermodifier. Ifinitialize7is meant as the post-upgrade initializer, mark it withreinitializer(x)so the proxy admin plugin prevents double-runs.- function initialize7(...) public nonpayable { ... } + function initialize7(...) public reinitializer(7) { ... }
🧹 Nitpick comments (18)
contracts/DEVNET_REDEPLOY.md (2)
10-44: Deletion script is brittle – add a safety net before wiping artifactsBlindly
rm-ing dozens of JSON files invites accidents (e.g. wrong$PWD).
Prefer a guarded one-liner:- rm deployments/arbitrumSepoliaDevnet/*.json +read -p "About to delete old devnet artifacts – continue? [y/N] " ans +[[ $ans == y || $ans == Y ]] && \ + fd --extension json deployments/arbitrumSepoliaDevnet \ + --exec rm {} +– single glob instead of 27 commands
– interactive confirmation
–fdkeeps the list explicit in git but safer in the shell.
66-67:yarn cleanalso wipescache/– remind to clear Hardhat’s cache between dry-runsIf you only remove
deployments/**but leave.hh_cacheyou can get “contract already verified” noise.
Add:yarn hardhat cache:cleanafter each dry-run.
contracts/deployments/arbitrumSepoliaDevnet/DisputeResolverRuler.json (1)
397-435: Storage layout checks out, butdisputesdynamic array sits after two contract refsAny future upgrade adding a new simple
uint256beforedisputeswill shift storage and corrupt data.
Reserve a gap slot now (e.g.uint256[50] private __gap;) to keep layout upgrade-safe.contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule_Implementation.json (1)
229-233: Compiler bump without version string updateThe contract is now compiled with
0.8.28, yet the constantstring public constant version = "0.8.0"remains unchanged. If you use that field operationally to detect upgrades, bump it (e.g.,"0.8.1") to avoid misleading downstream tooling.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGatedShutter_Proxy.json (2)
58-61: Initialization calldata is opaque—decode once for audit trails
_data(0x485cc955…) is passed to the implementation during construction. Decode it to a human-readable function call (e.g., usingcast calldata-decodeorabigen --decode) and archive the result in repo/docs. This greatly eases future debugging of initial state.
65-66: No admin/upgrade mechanism on the proxy itself
UUPSProxybytecode is minimal (delegate-only). Ensure governance has a path to upgrade by restrictingupgradeToAndCallin the implementation and that monitoring alerts are in place; otherwise a compromised governor could silently hot-swap logic.contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Implementation.json (2)
235-243: Gas spike: >780 k used for deploymentDeployment gas jumped compared with earlier artefact (~580 k previously). Re-check optimizer settings or inadvertent code growth; unnecessary bytecode inflates future upgrades.
381-395: Mapping remains at slot 1—good; still cross-check for new variables
governor(slot 0) andpoliciesmapping (slot 1) are unchanged. Any future variable must be appended after slot 1 to keep proxies safe. Consider adding a__gaparray now to future-proof.contracts/deployments/arbitrumSepoliaDevnet/KlerosCore_Proxy.json (1)
29-38: Gas usage looks high for a plain proxy deploy (~580k)UUPS proxy deployments are usually ~200k. The extra ~380k suggests an expensive initializer in
_data. If that logic can be deferred to a separate tx, redeploy costs and failure blast-radius shrink.contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler.json (1)
1513-1524: Expose a constantversion()instead of storage stringReturning a dynamically-allocated string per call costs unnecessary gas. If the implementation already has
string public constant version = "…", the auto-generated getter is free; otherwise consider:-string public version; +string public constant version = "1.0.0";contracts/deployments/arbitrumSepoliaDevnet/SortitionModule_Proxy.json (1)
64-66: Trim duplicate proxy byte-code artifacts
All generated proxy JSON files now embed identical byte-code. Consider keeping a single canonicalUUPSProxy.jsonand referring to it in deployment scripts to cut repo bloat.contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler_Proxy.json (1)
78-84: Proxy zoo getting wide – centralise?
KlerosProxies.solnow defines >20 one-line “name wrappers”. Compilation time and artifact noise grow quickly. A factory pattern or a single parametrised deploy script would keep things leaner.contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule.json (1)
252-254: Pin compiler for deterministic buildsArtifacts are now compiled with
0.8.28.
For reproducible deployments it’s safer to pin the compiler version infoundry.toml/ hardhat.config.tsrather than using^0.8.24pragmas, which could silently move to 0.8.29+ later.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic_Proxy.json (1)
64-66: Same note on compiler pinningThe proxy bytecode is also built with
0.8.28. Consider pinning to avoid drift across future builds.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGatedShutter.json (1)
726-736:drawexposes address of drawn juror – privacy leak?Returning the juror address directly may break commit-reveal anonymity for hidden-vote courts.
Re-evaluate if exposure before commit phase end is intended.contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Proxy.json (1)
64-66: SharedsolcInputHashacross unrelated artifacts looks suspicious
solcInputHash3f11dda26f…appears in several distinct proxy artefacts.
If you compiled them separately the hash should differ (it’s the keccak of the flattenedStandardJsonInput).
Double-check your compile-and-export pipeline; re-using the same hash can mis-lead Etherscan’s “verify & publish” step.contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry.json (1)
260-305: Governance address hard-coding
execute.args[0]hard-codes governor0xf1C7c0…. If you ever rotate governance you must redeploy or callchangeGovernor.
Safer pattern: inject via env/CLI variable in deploy script rather than baking into JSON.contracts/deployments/arbitrumSepoliaDevnet/SortitionModule.json (1)
968-1005:validateStakemarkednonpayablebut appears pure/viewThe function only computes and returns deltas; if no state write happens inside implementation consider marking it
viewto save 21 000 gas intrinsic & make static-callable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
contracts/DEVNET_REDEPLOY.md(1 hunks)contracts/README.md(3 hunks)contracts/README.md.template(2 hunks)contracts/deploy/00-home-chain-arbitration.ts(1 hunks)contracts/deploy/00-home-chain-resolver.ts(2 hunks)contracts/deploy/upgrade-all.ts(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic.json(7 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated.json(1 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGatedShutter.json(1 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGatedShutter_Proxy.json(1 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated_Proxy.json(1 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json(3 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeResolver.json(6 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeResolverRuler.json(6 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Implementation.json(3 hunks)contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule_Implementation.json(3 hunks)contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/KlerosCore.json(5 hunks)contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler.json(5 hunks)contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreSnapshotProxy.json(4 hunks)contracts/deployments/arbitrumSepoliaDevnet/KlerosCore_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Implementation.json(3 hunks)contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Proxy.json(2 hunks)contracts/deployments/arbitrumSepoliaDevnet/SortitionModule.json(12 hunks)contracts/deployments/arbitrumSepoliaDevnet/SortitionModule_Proxy.json(2 hunks)
🧰 Additional context used
🧠 Learnings (15)
contracts/deploy/upgrade-all.ts (2)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1744
File: web/src/hooks/useGenesisBlock.ts:9-31
Timestamp: 2024-11-19T05:31:48.701Z
Learning: In `useGenesisBlock.ts`, within the `useEffect` hook, the conditions (`isKlerosUniversity`, `isKlerosNeo`, `isTestnetDeployment`) are mutually exclusive, so multiple imports won't execute simultaneously, and race conditions are not a concern.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
contracts/README.md (2)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
contracts/README.md.template (2)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json (2)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
contracts/deploy/00-home-chain-resolver.ts (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
contracts/deploy/00-home-chain-arbitration.ts (3)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler.json (1)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1805
File: contracts/src/proxy/UUPSProxiable.sol:127-129
Timestamp: 2025-01-22T00:55:35.842Z
Learning: In Solidity, a public state variable automatically implements a corresponding external view function with the same name. For example, `string public constant override version = "0.8.0"` implements the abstract `function version() external view returns (string memory)`.
contracts/deployments/arbitrumSepoliaDevnet/DisputeResolver.json (7)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: kemuru
PR: kleros/kleros-v2#1702
File: web/src/pages/Home/TopJurors/JurorCard/index.tsx:10-11
Timestamp: 2024-10-08T16:23:56.291Z
Learning: In the `kleros-v2` codebase, the property `totalResolvedDisputes` should remain and should not be renamed to `totalResolvedVotes`.
Learnt from: kemuru
PR: kleros/kleros-v2#1702
File: web/src/pages/Home/TopJurors/JurorCard/index.tsx:10-11
Timestamp: 2024-10-07T06:18:23.427Z
Learning: In the `kleros-v2` codebase, the property `totalResolvedDisputes` should remain and should not be renamed to `totalResolvedVotes`.
Learnt from: tractorss
PR: kleros/kleros-v2#1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreSnapshotProxy.json (4)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:167-170
Timestamp: 2024-11-19T16:31:08.965Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the `minStake` parameter is denominated in PNK, not ETH.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1620
File: contracts/test/arbitration/draw.ts:84-84
Timestamp: 2024-11-05T11:32:11.238Z
Learning: In TypeScript code using ethers.js version 6, `contract.target` should be used instead of `contract.address` to access a contract's address.
contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Implementation.json (3)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1805
File: contracts/src/proxy/UUPSProxiable.sol:127-129
Timestamp: 2025-01-22T00:55:35.842Z
Learning: In Solidity, a public state variable automatically implements a corresponding external view function with the same name. For example, `string public constant override version = "0.8.0"` implements the abstract `function version() external view returns (string memory)`.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
contracts/deployments/arbitrumSepoliaDevnet/DisputeResolverRuler.json (5)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: tractorss
PR: kleros/kleros-v2#1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Implementation.json (2)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic.json (2)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
contracts/deployments/arbitrumSepoliaDevnet/KlerosCore.json (1)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:167-170
Timestamp: 2024-11-19T16:31:08.965Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the `minStake` parameter is denominated in PNK, not ETH.
contracts/deployments/arbitrumSepoliaDevnet/SortitionModule.json (3)
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:167-170
Timestamp: 2024-11-19T16:31:08.965Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the `minStake` parameter is denominated in PNK, not ETH.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-17
Timestamp: 2024-11-19T16:09:41.467Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (ID: 1) can have its `parent` ID set to itself (`"parent": 1`), as there is no parent court with ID 0 currently.
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:3-5
Timestamp: 2024-11-19T17:18:39.007Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the General Court (id: 1) intentionally references itself as its parent (`"parent": 1`). This self-reference is acceptable and should not be flagged as an issue in future reviews.
🧬 Code Graph Analysis (1)
contracts/deploy/upgrade-all.ts (1)
contracts/scripts/utils/contracts.ts (1)
getContractNamesFromNetwork(174-183)
⏰ 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). (15)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: contracts-testing
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
🔇 Additional comments (41)
contracts/deploy/00-home-chain-arbitration.ts (1)
32-36: LGTM: DisputeTemplateRegistry deployment correctly added.The addition of DisputeTemplateRegistry as an upgradable proxy deployment is consistent with the existing pattern and aligns with the restructuring effort described in the PR summary. The placement between EvidenceModule and DisputeKitClassic deployments is logical.
contracts/deploy/00-home-chain-resolver.ts (2)
16-16: LGTM: Correctly retrieves existing DisputeTemplateRegistry deployment.The change from deploying to retrieving the existing DisputeTemplateRegistry deployment aligns with the restructuring where DisputeTemplateRegistry is now deployed in the arbitration script.
25-25: LGTM: Deployment tag updated to reflect separation of concerns.The tag change from "HomeArbitrable" to "Resolver" correctly reflects the script's focused responsibility and aligns with the updated deployment strategy.
contracts/deploy/upgrade-all.ts (4)
18-28: LGTM: Contract names destructuring updated for new dispute kits.The addition of
disputeKitGatedanddisputeKitGatedShutterto the destructuring assignment correctly supports the new dispute kit contracts being introduced in this redeployment.
30-33: LGTM: Console logging added for new dispute kits.The console.log statements for the new dispute kit contracts provide useful debugging information during the upgrade process.
97-98: LGTM: Upgrade calls added for new dispute kits.The upgrade calls for
disputeKitGatedanddisputeKitGatedShutterusinginitialize7are consistent with the pattern used fordisputeKitClassicand correctly initialize the new contracts.
103-103: LGTM: Sortition module initialization version updated.The update from
initialize3toinitialize4for the sortition module indicates a version bump that aligns with the broader contract upgrade effort.contracts/README.md.template (2)
92-92: LGTM: Deployment tag updated to match script changes.The change from
HomeArbitrabletoResolveraligns with the updated deployment tag in the resolver deployment script.
154-159: LGTM: Import instructions updated with new Hardhat tasks.The updated commands using
populate:courtsandpopulate:policy-registrytasks with specific parameters provide better control and align with the modernized deployment process.contracts/README.md (3)
77-98: LGTM: Contract addresses updated for Arbitrum Sepolia Devnet redeployment.The updated addresses reflect the successful redeployment of multiple contracts including the new dispute kit variants (DisputeKitGated, DisputeKitGatedShutter) and other core contracts. The proxy/implementation address pairs are correctly documented.
200-200: LGTM: Deployment tag updated consistently with template and scripts.The change from
HomeArbitrabletoResolvermaintains consistency across the documentation and deployment scripts.
262-267: LGTM: Import instructions updated with modern Hardhat tasks.The updated commands using
populate:courtsandpopulate:policy-registrytasks with specific parameters align with the improved deployment workflow and provide better control over the import process.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json (1)
589-607: New read-onlycoreDisputeIDToActive()looks good – double-check indexationThe mapping is exposed as a view.
Confirm that the underlying storage key usescoreDisputeID(not local ID) everywhere or the value will always befalse.No code change required if this is already consistent.
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Proxy.json (1)
58-61: Confirm the constructor-encoded initializer matches the new signature
0x485cc955decodes toinitialize(address,address).
The payload embeds governor0xf1c7…c59and core0x1bd4…0f9.If those were place-holders during dry-run, redeploying without updating will lock governance forever.
contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule_Implementation.json (2)
2-2: Proxy must point to the new implementation address
addresschanged to0x450Aa35d…. Double-check that everyEvidenceModuleProxyon dev-net has been upgraded to this implementation; otherwise calls will keep hitting the outdated logic.
336-342: Validate storage-layout compatibility with the previous deployment
astIdforgovernordiffers (24479). While slot/offset are unchanged, re-confirm no other variables were re-ordered or added; a silent storage mismatch will brick existing proxies.contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Implementation.json (1)
2-2: New implementation address requires proxy upgrade
PolicyRegistryProxymust be upgraded to0x472846F8…to take effect. Confirm that the migration script executed the upgrade transaction and emitted theUpgradedevent.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated_Proxy.json (1)
58-61: Verify_datapayload really triggers the implementation initializerThe constructor arg at index 1 contains an encoded call (
0x485c…0f9). Please double-check that:
- The selector/parameters match the expected
initializesignature of the implementation at0x89BF27….- No state-changing initializer is still outstanding (empty
_datawould leave the proxy uninitialized and upgrade-lockable by anyone).A quick decode with
cast calldata-decodeorethers.jshelps ensure safety.contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreSnapshotProxy.json (1)
148-150: Constructor wiring looks correct – core address matches freshly-deployed proxy
args[1]points to0x1Bd44c4a4511DbFa7DC1d5BC201635596E7200f9, which matches the newKlerosCore_Proxyaddress in this PR, so the proxy should read balances from the right core. LGTM.contracts/deployments/arbitrumSepoliaDevnet/KlerosCore_Proxy.json (1)
102-104: Sanity-check the encoded_datainitializerThe blob passed into the proxy constructor is non-trivial. Before shipping to prod, decode and ensure it exactly calls
initialize(address,address,uint256[4])with the intended constants. A mismatch will lock the system with wrong parameters.contracts/deployments/arbitrumSepoliaDevnet/DisputeResolver.json (2)
295-318: Confirm compiler bump & byte-for-byte storage layout match before promoting to prod
The deployment switches the compiler from0.8.24to0.8.28(seesolcInputHashand metadata) while preserving the same source files. Even very small compiler changes can reshuffle struct packing/offsets.
Please re-runsolc --storage-layout(orforge inspect <contract>:storage) against the previous 0.8.24 build and compare the JSON to this artifact to guarantee 100 % identical layout; otherwise any future upgrade/proxy that relies on this implementation is unsafe.
397-435: Quick sanity check for live config constants
Slots 0-4 still map togovernor,arbitrator,templateRegistry,disputes, andarbitratorDisputeIDToLocalIDrespectively, which matches the V1 layout. Good.
Just make sure the new constructor args (line 312 ff.) are the intended governor/registry addresses for dev-net; mis-wiring here bricks governance.contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry.json (1)
254-263: Version string & initialisation check
The implementation address moved to0x4728…5deDand gas used dropped ~15 %. Verify that:
version()in the implementation now returns an incremented semantic version (helps off-chain tooling).initialize2()is not required anymore; front-end & scripts should stop calling it if so.contracts/deployments/arbitrumSepoliaDevnet/SortitionModule_Proxy.json (1)
58-61: Double-check encodedargspayload
The init calldata (0x54812d17…) bundles several params. Make sure the new court IDs / RNG address (0x7e40f5ac…) align with the freshly deployed core on dev-net; a mismatch soft-bricks sortition.contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler_Proxy.json (1)
72-75: Large numeric constants – please sanity-check
Init payload contains0x016345785d8a0000(≈ 100 ETH) and0x2710(10 000). Ensure these match the expected stake/min-juror params for the Ruler flavour; values were different in the previous deployment.contracts/deployments/arbitrumSepoliaDevnet/PolicyRegistry_Proxy.json (1)
59-61: Governor address hard-coded – verify against multisig rotation
The init calldata still wires governor to0xf1C7…c59. Confirm this is the current dev-net multisig; if you rotated keys after the last redeploy, the contract becomes immutable-without-control.contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Implementation.json (2)
2-2: Confirm registry address propagationNew implementation address
0xBc9B5643C9B1C478DAe1b950e886CC50D8d868b1looks fine syntactically.
Please double-check that:
- All deployment docs / scripts (README,
.env, front-end constants, etc.) reference the new address.- No other artifact on the same devnet re-uses this address (copy/paste accidents happen).
241-276: Verify storage layout consistency after compiler bumpWe’ve moved from
0.8.24→0.8.28and widened the pragma (^0.8.24), so even though the ABI hasn’t changed, you must confirm the UUPS storage layout is identical to the last deployed implementation.Run this to diff against the previous artifact in Git history:
git show HEAD~1:contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Implementation.json \ | jq '.storageLayout.storage' > /tmp/old jq '.storageLayout.storage' contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Implementation.json \ > /tmp/new diff -u /tmp/old /tmp/new• No output ⇒ storage layout is unchanged (safe to upgrade)
• Any diff ⇒ halt rollout and reconcile storage changescontracts/deployments/arbitrumSepoliaDevnet/EvidenceModule.json (2)
2-2: New proxy address — update consumer configsProxy moved to
0xA1F72e0445fc395A393247F5B8c958Ec9b7C0B49.
Ensure every service that callsEvidenceModule(subgraph, front-end, test scripts) is updated, otherwise evidence submissions will silently fail.
247-248: Constructor args sanity-check
implementation = 0x450Aa35da0ad8B282C5d910254055651417C2200and calldata encode aninitialize(address)call for governor0xf1C7…0c59.Double-check that this governor is correct for the devnet; the wrong governor bricks future upgrades.
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic_Proxy.json (2)
2-2: Proxy redeployed — propagate new addressNew proxy address
0xeEEbbbff8f377dCFc7d4F7876C531db0d22720e1must be reflected in:• Court configuration
• Dispute kit registry in KlerosCore
• Any off-chain resolvers / front-ends
59-60: Verify implementation & initialization calldataImplementation set to
0xD76E74049003faCe2EBA0B6FB61DFC9941da959Fwithinitialize(address)targeting governor0xf1C7…0c59.Please confirm that:
- Implementation bytecode corresponds to the Classic kit built with the same compiler version.
- Governor address is correct (same check as EvidenceModule).
contracts/deployments/arbitrumSepoliaDevnet/KlerosCore.json (1)
1330-1336: Return value added todraw– audit downstream integrations
drawnow returnsuint256 nbDrawnJurors.
Verify every consumer (DKs, scripts, UIs, subgraphs) expects / handles the extra return value to avoid stack-underflow or silent logic errors.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGatedShutter.json (1)
1067-1071: Dual public initializers (initializevsinitialize7) – potential mis-initTwo independent initializers are present. Confirm:
- Only one can be called once (use
initializerguards).- Upgrade scripts consistently target
initialize7.- Off-chain tooling doesn’t mistakenly call the wrong one.
Consider deprecating the older initializer with an explicit revert.
contracts/deployments/arbitrumSepoliaDevnet/EvidenceModule_Proxy.json (1)
59-60: Constructor arg points to new implementation – confirm byte-to-address castThe encoded
_dataparameter (0xc4d66de8…) seems to beinitialize()call data.
Double-check it was generated against the NEW implementation’s ABI; mismatched layout bricks the proxy.contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry_Proxy.json (1)
2-3: Confirm proxy ↔ implementation linkage
args[0](0xBc9B5643…) is assumed to be the new implementation behind proxy0xc852F94f….
Please run a quick on-chain check (or Hardhat task) to verify that:ERC1967Proxy(0xc852…).implementation() == 0xBc9B5643…to ensure the deployment script didn’t silently overwrite the slot after deploy/upgrade.
Also applies to: 59-60
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitClassic.json (2)
324-352: ABI grew – make sure off-chain consumers keep upNew getters (
alreadyDrawn,coreDisputeIDToActive,getLocalDisputeRoundID,getNumberOfRounds,hashVote) will break type-safe clients (TypeChain, wagmi, ethers-v6 auto-gen).
Regenerate typings and update any front-end caches / subgraphs that rely on static ABIs.Also applies to: 486-504, 757-804, 916-944
1126-1166: SamesolcInputHashcomment as proxy artefacts
solcInputHashidentical to other contracts – unlikely for an implementation with a totally different compilation target. Verify.contracts/deployments/arbitrumSepoliaDevnet/SortitionModule.json (2)
68-116: Breaking ABI changes – coordinate downstream modules
Events renamed (
LeftoverPNK*,StakeDelayed, etc.).
• Update all log parsers / subgraphs – otherwise indexers will silently miss emissions.
penalizeStakenow returns two values.
• Core contract & governor scripts must handle the tuple.
setStakesignature re-ordered & no longer returns.
• If any off-chain service expects return values (oldbool), they will revert.New
validateStake,withdrawLeftoverPNK,getJurorLeftoverPNK.
• Ensure they are wired in web dashboard before enabling on devnet.
initialize4replaces previous initializers; yourupgrade-all.tsscript already calls it – good.Also applies to: 491-509, 725-738, 862-881, 968-1005, 1019-1031
1082-1089:numDeployments= 2 – duplicate proxy instances?Unlike other artefacts (
1), this proxy recordsnumDeployments: 2.
Confirm you didn’t accidentally create an orphan proxy at an earlier tx that still points to the first implementation.contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated.json (1)
1185-1198: Mismatch between constructor_implementationargument and theimplementationfieldThe constructor argument (
0x89BF27…inargs[0]) differs from theimplementationkey (0x4D5Eb5…).
If this is an intentional post-deployment upgrade, please add a comment in the artifact (or README) that clarifies the upgrade path; otherwise consumers may read a stale implementation address.
| anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc | ||
| ``` | ||
|
|
||
| Shell 2: deployer | ||
|
|
||
| ```shell | ||
| export ARBITRUM_SEPOLIA_RPC=http://127.0.0.1:8545 | ||
|
|
||
| yarn clean | ||
| yarn deploy --network arbitrumSepoliaDevnet --tags Resolver | ||
| yarn deploy --network arbitrumSepoliaDevnet --tags ArbitrationRuler | ||
|
|
||
| unset ARBITRUM_SEPOLIA_RPC | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make the fork network id explicit to avoid Anvil/Arbitrum mismatch
Arbitrum Sepolia uses chain-id 421614.
Anvil occasionally defaults to 31337, breaking signature replay & EIP-1559 gas math.
and propagate the id in hardhat.config.ts’s networks.arbitrumSepoliaDevnet.
🏁 Script executed:
anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc \
--chain-id 421614Length of output: 171
To confirm whether the fork chain ID is already set in your Hardhat configuration, let’s locate and inspect your hardhat.config.ts:
🏁 Script executed:
#!/bin/bash
# Locate the Hardhat config file
echo "Locating hardhat.config.ts…"
fd --hidden --max-depth 2 hardhat.config.ts
# If found, search for the Arbitrum Sepolia Devnet network and any chainId entries
if [ -f hardhat.config.ts ]; then
echo -e "\nLooking for 'arbitrumSepoliaDevnet' configuration…"
rg -n "arbitrumSepoliaDevnet" -A5 -B2 hardhat.config.ts || true
echo -e "\nChecking for 'chainId' settings in config…"
rg -n "chainId" -A2 -B2 hardhat.config.ts || true
else
echo "hardhat.config.ts not found at repo root."
fiLength of output: 262
Let’s inspect the contracts/hardhat.config.ts to see if chainId is already specified for the arbitrumSepoliaDevnet network:
🏁 Script executed:
#!/bin/bash
# Show the networks section and look for chainId in contracts/hardhat.config.ts
echo "Extracting networks configuration…"
rg -n "networks" -A10 -B2 contracts/hardhat.config.ts || true
echo -e "\nLooking specifically for 'arbitrumSepoliaDevnet'…"
rg -n "arbitrumSepoliaDevnet" -A5 -B2 contracts/hardhat.config.ts || true
echo -e "\nSearching for any 'chainId' entries…"
rg -n "chainId" -A2 -B2 contracts/hardhat.config.ts || trueLength of output: 5268
Explicitly set the fork chain ID in DEVNET_REDEPLOY.md
The Hardhat config already defines chainId: 421614 for arbitrumSepoliaDevnet, so no updates are needed there.
Please update contracts/DEVNET_REDEPLOY.md to pin the fork’s chain ID:
@@ File: contracts/DEVNET_REDEPLOY.md
- anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc
+ anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc \
+ --chain-id 421614📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc | |
| ``` | |
| Shell 2: deployer | |
| ```shell | |
| export ARBITRUM_SEPOLIA_RPC=http://127.0.0.1:8545 | |
| yarn clean | |
| yarn deploy --network arbitrumSepoliaDevnet --tags Resolver | |
| yarn deploy --network arbitrumSepoliaDevnet --tags ArbitrationRuler | |
| unset ARBITRUM_SEPOLIA_RPC | |
| ``` | |
| anvil --fork-url https://sepolia-rollup.arbitrum.io/rpc \ | |
| --chain-id 421614 |
🤖 Prompt for AI Agents
In contracts/DEVNET_REDEPLOY.md around lines 51 to 64, the anvil fork command
does not explicitly set the chain ID, which can cause inconsistencies. Update
the anvil command to include the --fork-chain-id 421614 flag to pin the fork's
chain ID to match the Hardhat config. This ensures the forked network uses the
correct chain ID for arbitrumSepoliaDevnet.
| "inputs": [], | ||
| "name": "initialize2", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, |
There was a problem hiding this comment.
initialize2() can be abused unless guarded by reinitializer
A second initializer without onlyInitializing/reinitializer modifiers exposes the proxy to grief-reinitialization after upgrade. Confirm that the implementation marks initialize2() with OpenZeppelin’s reinitializer(version) or equivalent.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/KlerosCoreRuler.json around lines
1380 to 1385, the initialize2() function lacks protection with OpenZeppelin's
reinitializer or onlyInitializing modifiers, which risks unauthorized
reinitialization after upgrades. Modify the implementation to include the
reinitializer(version) modifier or an equivalent guard to ensure initialize2()
can only be called once per version and prevent misuse.
| "inputs": [], | ||
| "name": "initialize4", | ||
| "name": "initialize5", | ||
| "outputs": [], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
initialize5() still exposed – remove or make it internal
The contract migrated to initialize (multi-arg) yet legacy initialize5 remains public.
Keeping multiple public initializers is dangerous (grief upgrade attacks, accidental misuse).
Either:
- function initialize5() external { ... }
+ // remove entirely or mark as `internal` and call it from the new initializer if truly needed[strictly speaking on-chain ABI removal isn’t possible post-deployment, but implementation can ignore the selector].
At minimum add a revert guard inside initialize5 returning “Deprecated”.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/KlerosCore.json around lines 1682
to 1684, the legacy public function initialize5() remains exposed, which risks
misuse or attacks. To fix this, modify the implementation of initialize5() to
immediately revert with a "Deprecated" error message, effectively disabling it
while keeping the ABI unchanged. This prevents any calls to initialize5() from
succeeding and ensures only the new initialize() function is used.
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "address", | ||
| "name": "_account", | ||
| "type": "address" | ||
| }, | ||
| { | ||
| "internalType": "uint256", | ||
| "name": "_amount", | ||
| "type": "uint256" | ||
| } | ||
| ], | ||
| "name": "transferBySortitionModule", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, |
There was a problem hiding this comment.
Access control on transferBySortitionModule must be airtight
New method allows arbitrary transferBySortitionModule(address,uint256).
Confirm onlySortitionModule (or equivalent) modifier is enforced in the implementation; otherwise funds can be siphoned by anyone via the proxy.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/KlerosCore.json around lines 1857
to 1874, ensure the function transferBySortitionModule(address,uint256) has
strict access control by enforcing the onlySortitionModule modifier or an
equivalent check in its implementation. This prevents unauthorized users from
calling this function and transferring funds. Review the Solidity contract code
implementing this ABI entry and add the modifier to the function definition if
missing.
| "address": "0xc852F94f90E3B06Da6eCfB61d76561ECfb94613f", | ||
| "abi": [ |
There was a problem hiding this comment.
File naming collision: implementation vs proxy
This artefact points to the same proxy address (0xc852F94f…) as the previous _Proxy.json.
Tooling (Hardhat‐deploy, Truffle DB, Tenderly, etc.) will treat the first file it finds – the second one is silently ignored.
Consider renaming to something deterministic, e.g.
DisputeTemplateRegistryProxy.json ← proxy
DisputeTemplateRegistryImpl_v2.json ← implementation
to avoid accidental overwrites during future migrations.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/DisputeTemplateRegistry.json at
lines 2-3, the file currently points to the proxy contract address but is named
ambiguously, causing tooling conflicts with the proxy artifact file. Rename this
file to a deterministic name indicating it is the proxy, such as
DisputeTemplateRegistryProxy.json, and ensure the implementation artifact uses a
distinct name like DisputeTemplateRegistryImpl_v2.json to prevent overwriting
and confusion during deployments.
| "inputs": [], | ||
| "name": "initialize7", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, |
There was a problem hiding this comment.
Multiple public initializers exposed – potential re-initialisation vector
Both initialize() and initialize7() are externally callable and return void. Unless the contract uses OpenZeppelin’s initializer / reinitializer modifiers with version guards, the proxy can be re-initialised, breaking UUPS invariants.
Confirm that each function is protected by the AlreadyInitialized/ NotInitializing guards on‐chain. If not, gate the older initializer or remove it from the ABI supplied to front-ends.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated.json around lines
993 to 998, the function initialize7 is publicly accessible without protection,
risking re-initialization of the contract. Verify if the on-chain implementation
uses OpenZeppelin's initializer or reinitializer modifiers to prevent multiple
initializations. If these guards are missing, either add appropriate
initialization guards to initialize7 or remove this function from the ABI
exposed to front-ends to prevent external calls.
| "address": "0x677dA30B4b27D129354DdA1e219Bcc86802132d1", | ||
| "abi": [ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that the recorded proxy address is unique and registered in the deployment map
0x677dA30B4b27D129354DdA1e219Bcc86802132d1 must not collide with any other Devnet deployments. A duplicated address will break tooling that relies on a 1-to-1 address-artifact mapping.
🏁 Script executed:
#!/bin/bash
# Grep every dev-net deployment artifact for this address
fd --extension json deployments | xargs rg -n '0x677dA30B4b27D129354DdA1e219Bcc86802132d1'Length of output: 2599
Duplicate proxy address in deployment artifacts
The proxy address 0x677dA30B4b27D129354DdA1e219Bcc86802132d1 appears in more than one JSON artifact under arbitrumSepoliaDevnet, which breaks the 1-to-1 address→artifact mapping:
- contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated.json (line 2)
- contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated_Proxy.json (line 2)
Please update or remove the unintended duplicate in DisputeKitGated.json so that each deployed address is recorded in exactly one artifact.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepoliaDevnet/DisputeKitGated.json at lines 2
to 3, the proxy address 0x677dA30B4b27D129354DdA1e219Bcc86802132d1 is duplicated
from DisputeKitGated_Proxy.json. To fix this, remove or update the address field
in DisputeKitGated.json so that this proxy address only appears in one artifact,
ensuring a unique 1-to-1 mapping between deployed addresses and JSON artifacts.
|
PR-Codex overview
This PR primarily focuses on updating the smart contracts and their deployment scripts, including version upgrades, new contract deployments, and adjustments to configuration files for the Kleros arbitration system.
Detailed summary
deploy.tsanddotenv.shscripts.@shutter-network/shutter-sdkversion from0.0.2to0.0.1.DisputeKitClassicBase(from0.10.0to0.11.0),SortitionModuleNeo(from0.8.0to0.9.0), etc.DisputeKitGatedandDisputeKitGatedShutter.hardhat.config.tsto use Etherscan.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Style/Refactor
Chores