Release to Master #114
Release to Master #114
Conversation
fix: toastify fix
chore(web): update-subgraph-status-lib
…ss-by-deployment fix: pull updated kleroscore address by deployment and update arbitra…
Arbitrator contracts update and subgraph redeploy
fix(web): contract arbitrators in wagmi hook generation
WalkthroughThis set of changes refactors how arbitrator contracts are retrieved and managed across deployment scripts, utilities, and configuration files. It introduces a new centralized function for fetching arbitrator contracts, updates deployment and script logic to use this function, and removes legacy chain-based logic. Dependency versions are updated in multiple Changes
Sequence Diagram(s)sequenceDiagram
participant DeploymentScript
participant getArbitratorContracts
participant getContractsEthers
DeploymentScript->>getArbitratorContracts: Request arbitrator contracts (hre)
getArbitratorContracts->>getContractsEthers: Fetch contracts for network
getContractsEthers-->>getArbitratorContracts: Return contract instances
getArbitratorContracts-->>DeploymentScript: Return {klerosCore, disputeTemplateRegistry}
sequenceDiagram
participant ReactComponent
participant useScrollTop
participant OverlayScrollContext
ReactComponent->>useScrollTop: Call hook
useScrollTop->>OverlayScrollContext: Access scroll instance
ReactComponent->>useScrollTop: Call scrollTop()
useScrollTop->>OverlayScrollContext: Scroll to top
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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-escrow-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
web/src/styles/global-style.ts (1)
125-128: Consider avoiding !important in CSS rulesWhile this positioning fix for toast containers works, using
!importantis generally considered a last resort in CSS as it breaks the natural cascade and can make future style changes more difficult to maintain.Consider investigating if this can be achieved with more specific selectors or by adjusting the toast library configuration instead.
[class*="Toastify__toast-container"] { top: unset; - padding-top: 20px !important; + padding-top: 20px; }If the
!importantis truly necessary due to the toast library's styles having higher specificity, please add a comment explaining this requirement.web/src/components/ScrollTop.tsx (1)
12-28: Consider adding cleanup for hasScrolled refThe component uses a ref to track if scrolling has occurred, but doesn't reset it when the component unmounts or when dependencies change.
Consider adding a cleanup function to the useEffect:
useEffect(() => { if (hasScrolled.current) return; const params = new URLSearchParams(search); const section = params.get("section"); if (section) { const targetElement = document.getElementById(section); if (targetElement) { targetElement.scrollIntoView({ behavior: "smooth" }); hasScrolled.current = true; navigate(pathname, { replace: true }); return; } } scrollTop(); + + return () => { + hasScrolled.current = false; + }; }, [navigate, pathname, scrollTop]);This ensures the component will work correctly if it's unmounted and remounted.
contracts/deploy/utils/getContracts.ts (1)
11-20: Good implementation of centralized arbitrator contract retrievalThe new helper function effectively centralizes the retrieval of arbitrator contracts with proper error handling for unsupported networks.
Consider adding a fallback deployment name for local development environments:
const NETWORK_TO_DEPLOYMENT: Record<string, DeploymentName> = { arbitrumSepoliaDevnet: "devnet", arbitrumSepolia: "testnet", arbitrum: "mainnetNeo", + hardhat: "devnet", // Fallback for local development } as const;This would make local development and testing more straightforward.
web/wagmi.config.ts (3)
15-24: Consider using more specific typing for ABI.The
ArbitratorContractstype definition is well-structured, but usesany[]for the ABI type. Consider using a more specific type for better type safety.- abi: any[]; + abi: readonly unknown[];
77-80: New logic to include KlerosCore contract.The code now extracts the KlerosCore contract from arbitrator contracts and adds it to the results. This approach centralizes contract management and avoids hardcoding addresses.
Consider adding error handling in case KlerosCore is undefined in the imported contracts.
- const { KlerosCore } = arbitratorContracts.default.contracts; + const { KlerosCore } = arbitratorContracts.default.contracts; + if (!KlerosCore) { + throw new Error('KlerosCore contract not found in arbitrator contracts'); + }
78-78: Consider making the contract list more flexible.The arbitrator contract name is hardcoded. For better maintainability and future-proofing, consider defining contract names as constants or making this approach more flexible.
- const arbitratorContractConfigs = [{ name: "KlerosCore", contract: KlerosCore }]; + const ARBITRATOR_CONTRACTS = ["KlerosCore"]; + const arbitratorContractConfigs = ARBITRATOR_CONTRACTS.map(name => ({ + name, + contract: arbitratorContracts.default.contracts[name] + }));
📜 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 (15)
contracts/deploy/00-escrow.ts(2 hunks)contracts/deploy/utils/getContracts.ts(1 hunks)contracts/hardhat.config.ts(0 hunks)contracts/package.json(2 hunks)contracts/scripts/setDisputeTemplate.ts(3 hunks)subgraph/package.json(2 hunks)subgraph/subgraph.yaml(2 hunks)web/package.json(3 hunks)web/src/components/ScrollTop.tsx(1 hunks)web/src/hooks/queries/useArbitrationCostFromKlerosCore.ts(0 hunks)web/src/hooks/useScrollTop.ts(1 hunks)web/src/pages/AttachmentDisplay/index.tsx(3 hunks)web/src/pages/MyTransactions/TransactionDetails/index.tsx(3 hunks)web/src/styles/global-style.ts(1 hunks)web/wagmi.config.ts(3 hunks)
💤 Files with no reviewable changes (2)
- contracts/hardhat.config.ts
- web/src/hooks/queries/useArbitrationCostFromKlerosCore.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
web/src/hooks/useScrollTop.ts (1)
web/src/context/OverlayScrollContext.tsx (1)
OverlayScrollContext(3-3)
contracts/deploy/00-escrow.ts (1)
contracts/deploy/utils/getContracts.ts (1)
getArbitratorContracts(11-20)
web/src/pages/AttachmentDisplay/index.tsx (3)
web/src/styles/landscapeStyle.ts (2)
MAX_WIDTH_LANDSCAPE(3-3)landscapeStyle(7-11)web/src/styles/responsiveSize.ts (1)
responsiveSize(9-12)web/src/components/ExternalLink.tsx (1)
ExternalLink(5-9)
web/wagmi.config.ts (1)
contracts/scripts/wagmiHelpers.ts (1)
readArtifacts(15-42)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (31)
web/src/hooks/useScrollTop.ts (1)
1-15: Well-structured hook for scroll managementThis new hook provides a clean, reusable way to scroll to the top of the page. The implementation is well thought out with:
- Good use of optional chaining to safely handle possible undefined values
- Clean API with the optional smooth scrolling parameter
- Proper context usage to access the scroll instance
The hook follows React best practices and provides a focused, single-responsibility utility.
subgraph/package.json (2)
3-3: Version bump follows semantic versioningAppropriate minor version increase from 2.0.8 to 2.1.0, indicating non-breaking changes with new functionality, which aligns with the contract management refactoring in this PR.
33-33: Dependency update to latest versionThe gluegun dependency update from ^5.1.2 to ^5.2.0 is a minor version bump, which should maintain compatibility while providing new features or fixes.
contracts/deploy/00-escrow.ts (2)
5-5: Import update aligns with centralized contract retrievalThe import change from
getContractstogetArbitratorContractsfollows the refactoring pattern described in the PR objectives.
31-31: Implementation successfully updated to use centralized functionThe code now uses
getArbitratorContracts(hre)to retrieve the arbitrator contracts, which is consistent with the refactoring to centralize contract retrieval logic. This change improves maintainability by removing duplicated network detection logic across deployment scripts.subgraph/subgraph.yaml (3)
7-7: Network change to arbitrum-sepolia implemented correctly.The network change from arbitrum-one to arbitrum-sepolia aligns with the broader changes in contract deployment strategy. This properly targets the testnet environment.
9-11: Contract address and starting block updated appropriately.The contract address and starting block have been updated to match the arbitrum-sepolia deployment. This ensures the subgraph will index events from the correct contract and starting point.
25-25: ABI file path correctly updated.The ABI file path now correctly points to the arbitrumSepoliaDevnet deployment directory, which aligns with the network change.
contracts/package.json (2)
63-77: Dev dependency updates look good.The dev dependencies have been updated to newer versions, which should provide bug fixes and new features. The most significant update is hardhat from 2.22.18 to 2.23.0, which includes improvements and bug fixes.
86-87: Key dependency updates align with contract retrieval changes.The upgrade of
@kleros/kleros-v2-contractsfrom ^0.7.0 to ^0.9.3 is particularly important as it supports the centralized contract retrieval refactoring described in the PR. The OpenZeppelin contracts update to 5.3.0 brings in the latest security improvements.web/package.json (2)
48-48: Correctly added kleros-v2-contracts dependency.Adding
@kleros/kleros-v2-contractsas a dev dependency is essential for the centralized contract retrieval pattern being implemented in this PR.
70-70: Frontend dependency updates align with architectural changes.The updates to
@kleros/kleros-app,subgraph-status,viem, andwagmisupport the shift from ethers.js to viem/wagmi for contract interactions. This is a positive architectural improvement that brings more type safety and better performance.Also applies to: 101-103
contracts/scripts/setDisputeTemplate.ts (3)
9-9: Frontend URLs correctly configured for devnet and mainnet.The frontend URLs for arbitrumSepoliaDevnet and arbitrum are properly configured with the appropriate placeholder for external dispute IDs.
Also applies to: 18-18
22-22: Dispute template function correctly updated to use dynamic frontend URL.The function signature now includes the frontendUrl parameter, and the template correctly uses this dynamic value instead of a hardcoded URL. This improves flexibility and maintainability.
Also applies to: 49-49
101-101: Implementation correctly uses the new frontendUrl parameter.The code properly extracts the frontendUrl alongside the subgraphEndpoint and passes it to the disputeTemplateFn. This ensures the dispute template is correctly configured for each network.
Also applies to: 108-108
web/src/pages/MyTransactions/TransactionDetails/index.tsx (3)
13-13: Implementation of generated contract hookThe import change from
useArbitrationCosttouseReadKlerosCoreArbitrationCostaligns with the broader refactoring to use generated contract hooks instead of custom ones.
35-37: Improved arbitration cost retrieval implementationThe code now uses the generated hook with proper argument structure, passing escrow parameters' arbitrator extra data as an argument array.
93-93: Updated prop passing to match new data structureThe arbitration cost is now properly passed as
arbitrationCost?.datato account for the new hook's return structure.web/src/pages/AttachmentDisplay/index.tsx (4)
18-32: Improved container styling with responsive designThe updated container styles now include background color, proper padding, and responsive adjustments for landscape orientation. This enhances the visual consistency and user experience.
40-46: Refined link styling with proper component usageThe switch from a styled
<a>tag to using theExternalLinkcomponent improves code consistency and maintainability.
63-65: Security improvement in external linkThe
relattribute now includes both "noopener" and "noreferrer", which enhances security when opening links in new tabs by preventing the new page from accessing thewindow.openerproperty.
77-77: ScrollTop component integrationThe addition of the ScrollTop component enables scrolling to top functionality, improving user experience.
contracts/deploy/utils/getContracts.ts (3)
2-2: Improved contract import with proper namingImporting the arbitrator contracts from the Kleros V2 package and renaming the function for clarity enhances code readability.
5-9: Well-structured network to deployment mappingThe constant mapping of network names to deployment names provides a clear and maintainable way to handle different environments.
22-28: Simplified contract retrieval with improved maintainabilityThe refactored
getContractsfunction now uses the centralized helper, making the code more maintainable and reducing duplication.web/wagmi.config.ts (6)
7-11: Import structure is well organized.The imports from
@kleros/kleros-v2-contractsare clearly labeled to match their respective environments (devnet, testnet, mainnet), making the code more maintainable and easier to understand.
26-44: Good abstraction of contract addition logic.Creating a separate helper function for adding arbitrator contracts improves code organization and reusability. The function signature is also well typed.
46-50: Function signature updated to support arbitrator contracts.The updated parameter list properly supports the new requirements, making the function's purpose clearer.
88-88: Environment-specific contract selection is clear.The code correctly selects the appropriate arbitrator contracts based on the deployment environment, improving the configuration process.
Also applies to: 93-93, 98-98, 103-103
109-109: Updated function call with new parameter.The
readArtifactsfunction call is correctly updated to include thearbitratorContractsparameter.
57-57:❓ Verification inconclusive
Path construction has changed.
The directory path now uses a relative path
../contracts/deployments/${hardhatChainName}instead of the previous./deployments/${hardhatChainName ?? viemChainName}.This change assumes a specific directory structure and removes the fallback to
viemChainName. Ensure this works correctly in all environments.
🏁 Script executed:
#!/bin/bash # Check if the directory structure exists as expected ls -la ../contracts/deployments/ 2>/dev/null || echo "Directory does not exist as expected"Length of output: 117
🏁 Script executed:
#!/bin/bash # List directories named 'contracts' or 'deployments' find . -type d | grep -E "contracts|deployments" || echo "No matching directories found" # Search for references to 'deployments' in code files rg -l "deployments" || echo "No references to 'deployments' found"Length of output: 1141
🏁 Script executed:
#!/bin/bash # Display the lines around directoryPath in web/wagmi.config.ts to confirm fallback logic rg -n 'directoryPath' -A3 -B3 web/wagmi.config.tsLength of output: 596
🏁 Script executed:
#!/bin/bash # Show imports and chain name mappings in web/wagmi.config.ts head -n 50 web/wagmi.config.ts rg -n "hardhatChainName" web/wagmi.config.ts rg -n "viemChainName" web/wagmi.config.tsLength of output: 1473
🏁 Script executed:
#!/bin/bash # Find where readArtifacts is invoked to inspect the passed chain names and fallback logic rg -n 'readArtifacts' -A5 -B5 web/wagmi.config.ts || echo "No invocations found"Length of output: 673
Ensure deployments path and chain‐name fallback are correct
The snippet at web/wagmi.config.ts:57 now reads:
const directoryPath = `../contracts/deployments/${hardhatChainName}`;This removes the previous
…/${hardhatChainName ?? viemChainName}fallback and relies on a relative lookup from wherever the CLI is invoked. Please verify:
- That the wagmi CLI always runs with CWD set to
web/, so../contracts/deployments/...points at the repo’scontracts/deploymentsfolder.- Every value of
hardhatChainNameyou pass (e.g.arbitrum,arbitrumSepoliaDevnet) matches an existing subfolder undercontracts/deployments.- If you need a fallback to the viem chain name or more robust resolution, consider either re-adding
?? viemChainNameor using an absolute path helper:const directoryPath = join(__dirname, "../contracts/deployments", hardhatChainName);
PR-Codex overview
This PR focuses on updating the Kleros escrow application with various improvements, including new hooks, style adjustments, and dependency updates.
Detailed summary
useArbitrationCostFromKlerosCore.tshook.useScrollTophook for scrolling to the top.Toastifycomponent.subgraph/package.jsonversion to2.1.0.subgraph.yamlfromarbitrum-onetoarbitrum-sepolia.TransactionDetailsto use new arbitration cost hook.AttachmentDisplaywith new styles andScrollTopcomponent.package.jsonfiles across multiple directories.Summary by CodeRabbit
New Features
Enhancements
Dependency Updates
Bug Fixes
Refactor
Chores
Style