chore: testnet upgrade and new Dispute Kits deployment #2058
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis update revises deployment artifacts and the README for contracts on the Arbitrum Sepolia testnet. It updates implementation addresses for several existing contracts, introduces new deployment artifacts and proxies for DisputeKitGated, DisputeKitGatedShutter, and DisputeKitShutter, and reflects ABI and function signature changes for KlerosCore, DisputeKitClassic, and SortitionModule. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Proxy
participant Implementation
User->>Proxy: Call function (e.g., createDispute)
Proxy->>Implementation: delegatecall(function, args)
Implementation-->>Proxy: Return result
Proxy-->>User: Return result
Note over Proxy,Implementation: Applies to DisputeKitGated, DisputeKitGatedShutter, DisputeKitShutter proxies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
🧰 Additional context used🧠 Learnings (1)contracts/deployments/arbitrumSepolia/SortitionModule.json (3)Learnt from: jaybuidl Learnt from: jaybuidl Learnt from: jaybuidl ⏰ 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). (2)
🔇 Additional comments (7)
✨ Finishing Touches🧪 Generate unit tests
🪧 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-university 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-neo failed. Why did it fail? →
|
![coderabbitai[bot]](https://app.smartcontext.ai/default/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
contracts/deployments/arbitrumSepolia/KlerosCore.json (1)
1839-1843: Signature ofsetStakeBySortitionModulechangedThe boolean
_alreadyTransferredparameter was removed.
Make sureSortitionModulehas been updated accordingly; otherwise the call will revert.Consider bumping interface version constants to catch mismatches at compile time.
🧹 Nitpick comments (7)
contracts/deployments/arbitrumSepolia/KlerosCore.json (2)
1857-1874: New privileged transfer hook – security review recommended
transferBySortitionModule(address _account, uint256 _amount)allows the sortition module to pull arbitrary PNK from the core.• Confirm access control (
SortitionModuleOnly) is applied in implementation.
• Double-check re-entrancy protection inside SortitionModule when it calls this.
• Unit tests covering malicious SortitionModule address replacement are desirable.
2006-2014: Deployment metadata bump – be explicit in changelog
numDeploymentsincremented to 4 andexecute.methodNameswitched toinitialize5.
Record this in release notes; consumers relying on deployment counters (e.g., CI scripts that diff artifacts) may need an update.contracts/deployments/arbitrumSepolia/DisputeKitShutter.json (1)
1058-1072: Multiple initializer variants (initializevsinitialize8) may create operational ambiguityThe ABI exposes both
initialize()andinitialize8()entry points. Having two public initializers makes it easy to invoke the wrong one during future upgrades and increases the surface area for configuration mistakes.Consider removing legacy initializers once the proxy is live, or gate the newer one behind an
onlyInitializing/ version-guarded modifier exactly as OpenZeppelin’sreinitializer(uint8)pattern does.contracts/deployments/arbitrumSepolia/DisputeKitClassic.json (1)
964-969: Initializer naming drift (initialize7) diverges from other kitsThis proxy is deployed with
initialize7()while other newly-deployed kits still use plaininitialize().
The inconsistency invites operator error when upgrading or scripting deployments.Unify the versioned-initializer strategy across all dispute-kit implementations, or clearly document the reason for the deviation.
contracts/deployments/arbitrumSepolia/DisputeKitGated.json (1)
992-999: Deployment still callsinitialize()althoughinitialize7()existsUnlike
DisputeKitClassic, this kit was deployed viainitialize(), yet it also exposesinitialize7().
Mixing version numbers across deployments complicates automated upgrade scripts and off-chain tooling.Align the initializer invoked at deployment with the intended versioned entry point, or drop the unused one.
contracts/deployments/arbitrumSepolia/SortitionModule.json (2)
70-84: Update downstream indexers & UIs for newLeftoverPNKeventThe renamed
LeftoverPNK(+LeftoverPNKWithdrawn) events add an indexed_accountparam.
Please make sure sub-graphs, analytics dashboards, and any UI components listening for the previousStakeDelayedAlreadyTransferredDeposited/…Withdrawnevents are migrated, otherwise they’ll silently stop updating.
968-1005:validateStakeis write-mutable, not view
validateStakeonly reads & returns calculations yet is markednonpayable(state-changing).
Consider declaring itviewto prevent unnecessary gas-cost suspicion and to allow static-call usage.- "stateMutability": "nonpayable", + "stateMutability": "view",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
contracts/README.md(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitClassic.json(5 hunks)contracts/deployments/arbitrumSepolia/DisputeKitGated.json(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitGatedShutter.json(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitGatedShutter_Proxy.json(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitGated_Proxy.json(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitShutter.json(1 hunks)contracts/deployments/arbitrumSepolia/DisputeKitShutter_Proxy.json(1 hunks)contracts/deployments/arbitrumSepolia/KlerosCore.json(4 hunks)contracts/deployments/arbitrumSepolia/SortitionModule.json(11 hunks)
🧰 Additional context used
🧠 Learnings (4)
contracts/README.md (1)
Learnt from: Harman-singh-waraich
PR: #1846
File: prettier-config/index.js:26-26
Timestamp: 2025-01-23T08:14:47.397Z
Learning: The prettier-plugin-solidity plugin is installed in the kleros-v2 repository, even though it's not visible in the sandbox environment's node_modules (which is expected as node_modules is not committed to the repository).
contracts/deployments/arbitrumSepolia/DisputeKitClassic.json (4)
Learnt from: Harman-singh-waraich
PR: #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: kemuru
PR: #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: kemuru
PR: #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: Harman-singh-waraich
PR: #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/deployments/arbitrumSepolia/KlerosCore.json (1)
Learnt from: jaybuidl
PR: #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/arbitrumSepolia/SortitionModule.json (3)
Learnt from: jaybuidl
PR: #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: #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: #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.
⏰ 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). (14)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- 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: contracts-testing
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
contracts/deployments/arbitrumSepolia/DisputeKitShutter_Proxy.json (1)
58-61: Proxy→implementation wiring verified
- contracts/README.md correctly references the implementation address
0x97a1DAAc5aF27383B2c2AB57d8974664A7fe4352under Arbitrum Sepolia.- Hardhat is configured to compile with Solidity 0.8.28, and the contract’s pragma (
^0.8.24) is within that range, so there are no UUPS‐upgrade incompatibilities.No further changes needed.
contracts/deployments/arbitrumSepolia/DisputeKitGatedShutter_Proxy.json (1)
58-61: ✅ Implementation address matches README – no action needed.The
args[0]value in contracts/deployments/arbitrumSepolia/DisputeKitGatedShutter_Proxy.json
(0xbd64B87c8A1276c0B83042f3F9128f7994753836) aligns with the README entry at line 40.contracts/deployments/arbitrumSepolia/DisputeKitGated_Proxy.json (1)
58-61: README implementation address is up-to-date
The implementation0x074837F8306faD19D4e6b1f3d2f98eA796c3f8a9is already referenced in contracts/README.md (line 39). No further action required.contracts/deployments/arbitrumSepolia/DisputeKitShutter.json (1)
1268-1272: Verify governor & core addresses encoded in deployment calldata
execute.methodNameshows aninitialize(...)with params
0xf1C7c037891525E360C59f708739Ac09A7670c59(governor) and
0xE8442307d36e9bf6aB27F1A009F95CE8E11C3479(core).Double-check that
- both addresses are correct for Arbitrum Sepolia,
- the governor is a timelock / multisig with upgrade authority,
- the core address is the expected KlerosCore proxy.
contracts/deployments/arbitrumSepolia/DisputeKitClassic.json (1)
1158-1166: Solc input hash & compiler bump: ensure downstream tooling is regeneratedThe artifact switched to solc 0.8.28 with a new
solcInputHash.
Make sure ABI-coded clients, typechain bindings, and subgraph models are re-generated; otherwise they will miss newly-added view functions (getLocalDisputeRoundID,hashVote, …).contracts/deployments/arbitrumSepolia/DisputeKitGated.json (1)
1185-1199: Confirm governor/core addressesThe encoded calldata passes governor
0xf1C7…c59and core0xE84423…3479. Please validate that these are the final governance & core proxies for Sepolia before the artifact is promoted to production branches.contracts/deployments/arbitrumSepolia/SortitionModule.json (2)
858-882:setStakesignature changed – confirm core linkageThe method now expects
_pnkDeposit,_pnkWithdrawal,_newStakeand dropped_alreadyTransferred.
Double-check that:
KlerosCore.setStakeBySortitionModuleencodes the new parameter list.- Any off-chain scripts that stage batched stake updates have been regenerated from the latest ABI.
Failure to update will revert with bad calldata.
725-737: penalizeStake ABI Change Verified — Internal Call Sites UpdatedAll internal Solidity callers have been updated to handle the new two‐value return. No instances of
penalizeStakebeing invoked without unpacking bothpnkBalanceandavailablePenaltywere found.• Definitions of
penalizeStakeexist in:
–contracts/src/arbitration/interfaces/ISortitionModule.sol
–contracts/src/arbitration/SortitionModuleBase.sol
–contracts/src/arbitration/university/SortitionModuleUniversity.sol• Call sites correctly unpack both return values in:
–contracts/src/arbitration/KlerosCoreBase.sol:786
–contracts/src/arbitration/university/KlerosCoreUniversity.sol:777No internal changes are required. Please ensure that any off-chain services or relay contracts using the old single-value ABI are updated to the new ABI before deployment.
contracts/deployments/arbitrumSepolia/DisputeKitGatedShutter.json (1)
1067-1072: Initializer version bump (initialize7) acknowledgedThe new proxy exposes
initialize7, continuing the monotonic versioning pattern.
No issues spotted – deployment args look correct.
| "inputs": [], | ||
| "name": "initialize4", | ||
| "name": "initialize5", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New initializer initialize5
Initializer bump means:
- Proxy execute section already calls it (good).
- Any scripts still invoking
initialize4will revert withfunction selector not found.
Sanity-scan deployment scripts for initialize4 before merging.
🏁 Script executed:
#!/bin/bash
rg -n "initialize4" scriptsLength of output: 27
🏁 Script executed:
#!/bin/bash
# Verify existence of the scripts directory
if [ -d scripts ]; then
echo "Directory 'scripts' exists."
else
echo "Directory 'scripts' does not exist."
fi
# Search for any references to initialize4 across the entire repository
rg -n "initialize4" || trueLength of output: 297801
Update deployment & upgrade scripts to call initialize5
The initializer bump to initialize5 means any hard-coded "initialize4" selectors in your deployment or upgrade scripts will now fail. I found references to initialize4 in:
contracts/deploy/upgrade-all.ts(line 103)- All JSON ABIs under
contracts/deployments/*(e.g. ARB Sepolia, Mainnet, Devnet, Testnet viem files) contracts/deployments/arbitrumSepolia.ts,arbitrum.ts,arbitrumSepoliaDevnet.ts, plus their.viem.tscounterparts
Please replace those occurrences of "initialize4" with "initialize5" (and update the corresponding ABI entries) before merging to ensure your proxies invoke the new initializer correctly.
🤖 Prompt for AI Agents
In contracts/deployments/arbitrumSepolia/KlerosCore.json around lines 1682 to
1686, the function initializer has been updated from "initialize4" to
"initialize5". You need to update all deployment and upgrade scripts that
reference "initialize4" to now use "initialize5". Specifically, replace
"initialize4" with "initialize5" in contracts/deploy/upgrade-all.ts at line 103,
all JSON ABI files under contracts/deployments/*, and the related deployment
scripts contracts/deployments/arbitrumSepolia.ts, arbitrum.ts,
arbitrumSepoliaDevnet.ts, and their .viem.ts counterparts. Also update the ABI
entries accordingly to reflect the new initializer function.
jaybuidl
force-pushed
the
chore/testnet-upgrade-jul2025
branch
from
ac946a8 to
c1ebbb1
Compare
|
PR-Codex overview
This PR focuses on updating the
README.mdfile with new contract addresses and enhancing the deployment JSON files for various proxy contracts, specifically related toDisputeKitandKlerosCore.Detailed summary
README.mdwith new implementation addresses for:DisputeKitClassicDisputeKitGatedDisputeKitGatedShutterDisputeKitShutterKlerosCoreDisputeKitGatedDisputeKitShutterDisputeKitGatedShutterDisputeKitClassic.jsonwith new functions and updated metadata.Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation