Scripts modernization #68
Conversation
…0, React 18, Vite, Tailwind, Viem, Antd 5
…plicate most of its code
WalkthroughThis pull request spans multiple projects with extensive configuration and code restructuring. The pre-commit hook now runs Yarn with a specific lint-staged configuration, and new VSCode settings have been added. Legacy ESLint, Webpack, and Prettier configurations were removed across dynamic-script and evidence-display, which now use TypeScript, Vite, and Biome tools. Furthermore, deployment scripts, tasks, and Solidity linting rules in the contracts folder have been reformatted, and a new SDK project with build scripts, tests, and dispute querying functions has been introduced. Changes
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
![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 (13)
evidence-display/src/types/images.d.ts (1)
1-19: Well-structured TypeScript declaration file for image importsThis is a good addition that properly defines TypeScript module declarations for various image formats, enabling proper type checking when importing these assets in TypeScript files.
You might consider adding more modern image formats like WebP for completeness:
declare module '*.svg' { const content: string export default content } + +declare module '*.webp' { + const content: string + export default content +}evidence-display/src/index.css (1)
1-15: Tailwind CSS Base File ImplementationThe file correctly imports Tailwind’s base, components, and utilities layers. The use of a custom
:rootselector to define CSS properties and the inclusion of a@font-facerule for Roboto are well integrated.Suggestions:
- Font-Face Improvement: Consider adding a
font-display: swap;property within the@font-facerule to improve perceived performance during font load.- Custom Property Usage: Ensure that the defined custom properties (
--backgroundand--foreground) are applied correctly in your theme (for example, by wrapping them in anhsl()function if intended for color usage).evidence-display/vite.config.ts (2)
20-22: Consider using a relative path for the alias.The alias configuration uses an absolute path
/srcwhich assumes the src directory is at the root. Consider using a relative path for better portability:resolve: { alias: { - '@': '/src', + '@': './src', }, preserveSymlinks: true, },
25-29: Redundant target configuration.The 'esnext' target is defined both in the build configuration (line 14) and the esbuild configuration (line 26). Consider removing one of these redundant configurations to maintain a single source of truth.
dynamic-script/src/index.ts (1)
3-3: Consider using a more specific logging messageWhile logging the version is good, consider adding more context to make it clear what component is logging this message when examining logs.
-console.log("dynamic-script version", process.env.VERSION); +console.log("[Kleros Cross Chain Realitio] dynamic-script version:", process.env.VERSION);evidence-display/src/containers/realitio.tsx (3)
6-6: Consider using a more specific logging messageSimilar to the dynamic-script, consider adding more context to make it clear what component is logging this message.
-console.log('evidence-display version', process.env.VERSION) +console.log('[Kleros Cross Chain Realitio] evidence-display version:', process.env.VERSION)
30-33: Consider providing more specific error messagesThe current error message doesn't specify which critical information is missing. A more detailed error message would help with debugging.
- if (!arbitrableContractAddress || !disputeID || !(arbitrableChainID || arbitratorChainID || chainID)) { - console.error('Evidence display is missing critical information.') + if (!arbitrableContractAddress || !disputeID || !(arbitrableChainID || arbitratorChainID || chainID)) { + console.error('Evidence display is missing critical information:', { + arbitrableContractAddress: !!arbitrableContractAddress, + disputeID: !!disputeID, + chainID: !!(arbitrableChainID || arbitratorChainID || chainID) + })
54-57: Consider refactoring duplicate URLSearchParams logicYou're creating a new URLSearchParams object twice - once here and once in the useEffect. Consider reusing the parsed parameters from the useEffect to avoid duplication.
- const chainID = new URLSearchParams(window.location.search).get('chainID') || - new URLSearchParams(window.location.search).get('arbitrableChainID') || - new URLSearchParams(window.location.search).get('arbitratorChainID') + const chainID = params.chainID || params.arbitrableChainID || params.arbitratorChainIDdynamic-script/src/lib.ts (2)
36-46: Improve error messaging for missing parameters.Currently, a generic error is thrown if
disputeIDorarbitrableContractAddressis missing. Consider pinpointing which parameter is absent to simplify debugging, for example by separately checking each parameter or altering the error text with more detailed information.
86-88: Consider providing more context when no matching dispute is found.Throwing a generic error message may impede troubleshooting. Including the
disputeID, the block range, and/or the number of logs found would make error messages more actionable.dynamic-script/src/contracts.ts (1)
141-154: Potential naming collisions increateEventMap.If multiple events share the same name (overloads), only the last will be stored. Consider incorporating safety checks or strategies to handle potential duplicates.
dynamic-script/package.json (1)
31-37: Modernized Build Scripts
The build scripts have been updated to use Vite for building and TSC for generating type declarations. This modern chain—yarn clean && vite build && yarn build:types—is clear and effective.Note: The use of
rm -rf distmay not be cross-platform compatible (e.g., on Windows). Consider using a cross-platform alternative like therimrafpackage.- "clean": "rm -rf dist", + "clean": "rimraf dist",evidence-display/package.json (1)
10-14: Redesigned Script Commands
The scripts have been modernized to use Vite and TypeScript:
"dev": "vite"for development,"build": "tsc && vite build && mv dist evidence-display-v${npm_package_version}"for building,"preview": "vite preview"for previewing,"clean": "rm -rf dist evidence-display-v${npm_package_version}"for cleaning.Note: The use of Unix-specific commands like
rm -rfandmvmight pose cross-platform challenges (especially on Windows). Consider a cross-platform solution or using Node.js-based scripts to handle these tasks.- "build": "tsc && vite build && mv dist evidence-display-v${npm_package_version}", - "clean": "rm -rf dist evidence-display-v${npm_package_version}" + "build": "tsc && vite build && rimraf dist && node rename.js", + "clean": "rimraf dist evidence-display-v${npm_package_version}"(The diff above is a suggested conceptual change—you might need to implement a cross-platform renaming strategy, for example using a dedicated Node script.)
📜 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 (35)
.husky/pre-commit(1 hunks).vscode/settings.json(1 hunks)dynamic-script/.eslintrc.json(1 hunks)dynamic-script/.gitignore(1 hunks)dynamic-script/package.json(1 hunks)dynamic-script/src/abis.js(0 hunks)dynamic-script/src/contracts.ts(1 hunks)dynamic-script/src/index.js(0 hunks)dynamic-script/src/index.ts(1 hunks)dynamic-script/src/lib.ts(1 hunks)dynamic-script/src/types/reality-eth-lib.d.ts(1 hunks)dynamic-script/tsconfig.json(1 hunks)dynamic-script/tsconfig.node.json(1 hunks)dynamic-script/vite.config.ts(1 hunks)dynamic-script/webpack.config.js(0 hunks)evidence-display/.eslintrc.json(1 hunks)evidence-display/config-overrides.js(0 hunks)evidence-display/index.html(1 hunks)evidence-display/manifest.json(0 hunks)evidence-display/package.json(1 hunks)evidence-display/postcss.config.cjs(1 hunks)evidence-display/public/manifest.json(2 hunks)evidence-display/src/App.tsx(1 hunks)evidence-display/src/containers/abis.js(0 hunks)evidence-display/src/containers/realitio.js(0 hunks)evidence-display/src/containers/realitio.tsx(1 hunks)evidence-display/src/index.css(1 hunks)evidence-display/src/index.js(0 hunks)evidence-display/src/main.tsx(1 hunks)evidence-display/src/types/images.d.ts(1 hunks)evidence-display/tailwind.config.js(1 hunks)evidence-display/tsconfig.json(1 hunks)evidence-display/tsconfig.node.json(1 hunks)evidence-display/vite.config.ts(1 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (8)
- evidence-display/config-overrides.js
- evidence-display/src/index.js
- evidence-display/src/containers/realitio.js
- dynamic-script/webpack.config.js
- dynamic-script/src/abis.js
- evidence-display/src/containers/abis.js
- dynamic-script/src/index.js
- evidence-display/manifest.json
🧰 Additional context used
🧬 Code Definitions (5)
evidence-display/src/main.tsx (1)
evidence-display/src/App.tsx (1) (1)
App(3-9)
evidence-display/src/App.tsx (1)
evidence-display/src/containers/realitio.tsx (1) (1)
RealitioDisplayInterface(8-86)
evidence-display/src/containers/realitio.tsx (1)
dynamic-script/src/lib.ts (2) (2)
RealityQuestionData(23-34)fetchRealityQuestionData(36-145)
dynamic-script/src/index.ts (1)
dynamic-script/src/lib.ts (1) (1)
fetchRealityMetaEvidence(159-208)
dynamic-script/src/lib.ts (1)
dynamic-script/src/contracts.ts (7) (7)
foreignProxyAbi(34-59)homeProxyAbi(24-32)realitioAbi(61-110)ArbitrationCreatedLog(116-120)REALITY_STARTS_AT(4-22)LogNewQuestionLog(122-133)LogNewTemplateLog(135-139)
🔇 Additional comments (56)
dynamic-script/.gitignore (1)
1-2: Appropriate macOS Ignore Settings Added.The addition of the
# macOScomment along with the.DS_Storeentry is a standard and effective way to prevent macOS-specific metadata files from being tracked in version control. This change aligns well with best practices..husky/pre-commit (1)
4-4: Correct update to use Yarn instead of npxThis change aligns with the modernization objective by switching from
npx lint-stagedtoyarn lint-staged, consistent with the project's transition to Yarn as the package manager (as specified in package.json).evidence-display/.eslintrc.json (6)
2-2: Good practice to set root flagSetting
"root": trueproperly indicates this is the root configuration file, preventing ESLint from searching for configuration files in parent directories.
5-5: Updated ECMAScript environmentUpdating to
es2022aligns with the modernization effort and allows usage of newer JavaScript features.
8-12: Modern ESLint configuration with TypeScript supportThe updated extends array includes TypeScript and React Hooks support, providing better linting capabilities for the modernized codebase.
13-14: Appropriate configuration for parser and patterns to ignoreUsing
@typescript-eslint/parserwith properly defined ignore patterns improves the linting setup.
15-18: Updated parser optionsThe parser options have been correctly updated to match the ES2022 environment specified earlier.
19-25: Added React Refresh plugin configurationThis is a valuable addition that improves the development experience by preserving component state during hot module replacement while ensuring proper exports.
package.json (4)
12-12: Simplified lint scriptChanging from
run-p -s lint:*to direct sequential execution withlint:js && lint:solis appropriate since the npm-run-all dependency has been removed.
15-15: Simplified fix scriptSimilar to the lint script, this change appropriately updates the fix script to sequential execution with explicit tasks.
26-34:✅ Verification successful
Removed unnecessary ESLint plugins
The removal of
eslint-config-prettier,eslint-import-resolver-parcel,eslint-plugin-import,eslint-plugin-prettier, andnpm-run-allstreamlines the dependencies while the updated configurations in the workspace-specific ESLint files maintain proper linting capabilities.Note that if Prettier integration is still desired, you might want to add
eslint-config-prettierback to prevent conflicts between ESLint and Prettier rules.
🏁 Script executed:
#!/bin/bash # Check if Prettier is configured in workspace-specific ESLint configs echo "Checking for Prettier integration in ESLint configs..." grep -r "prettier" --include=".eslintrc*" .Length of output: 298
Prettier integration confirmed and ESLint dependency cleanup approved
- The removal of unnecessary ESLint plugins from
package.jsonis appropriate.- Verified that the workspace ESLint configuration (in
.eslintrc.json) correctly integrates Prettier using"plugin:prettier/recommended","prettier", and an explicit"prettier/prettier": "error"rule.- No further action is needed regarding re-adding
eslint-config-prettiersince proper configuration is already present.
27-27:❓ Verification inconclusive
Major ESLint version upgrade
Updating from ESLint 5.12.0 to 8.57.1 is a significant upgrade that brings many improvements and new features. This aligns well with the script modernization objective.
However, when performing such a major version upgrade, it's important to verify that all rules still work as expected. You might need to update or add new TypeScript-specific rules.
🏁 Script executed:
#!/bin/bash # Check for ESLint configuration files that might need updating echo "Searching for ESLint configuration files in workspaces..." find . -name ".eslintrc*" -type f | grep -v "node_modules" # Check if any TypeScript files would have linting errors with the new config echo "Running ESLint on a sample of TypeScript files to verify config works..." find evidence-display dynamic-script -name "*.ts" -o -name "*.tsx" | head -5 | xargs yarn eslint --no-ignoreLength of output: 773
Action Required: Manually Verify ESLint & TypeScript Configurations
We've upgraded ESLint from 5.12.0 to 8.57.1—which is a significant improvement in line with our modernization goals. However, the automated script intended to verify that TypeScript files still lint correctly produced a Yarn error ("Couldn't find the node_modules state file"). This suggests that the linting check didn't run as expected (possibly due to missing dependencies), so please verify manually that:
- All ESLint configuration files (e.g., in the project root,
bots,dynamic-script, andevidence-display) are correctly set up for ESLint 8.- TypeScript files in
evidence-displayanddynamic-scriptpass linting once the environment is correctly installed (e.g., after runningyarn install).Please re-run the check manually or adjust your testing sequence to ensure the new ESLint configuration works as required.
evidence-display/postcss.config.cjs (1)
1-7: PostCSS Configuration Setup Looks GoodAll required plugins—
postcss-import,@tailwindcss/postcss7-compat, andautoprefixer—are correctly configured. Please ensure these dependencies are also declared in your package management file so they load properly during the build..vscode/settings.json (1)
1-5: VS Code Tailwind CSS Association Configured CorrectlyThe settings file maps
.cssfiles to use Tailwind CSS features in VS Code. This will enhance the development experience with proper syntax highlighting and IntelliSense.evidence-display/tsconfig.node.json (1)
1-10: TypeScript Node Configuration is SolidThe configuration file sets modern compiler options (like
composite,skipLibCheck, andmoduleResolutionset to"bundler") that are well suited for a Vite-based project structure. Includingvite.config.tsin the compilation process ensures that your build configuration is type-checked.evidence-display/src/main.tsx (1)
1-10: React Entry Point ImplementationThe
main.tsxfile properly imports React, the main App component, and the necessary styles. It usesReactDOM.createRootto render the application insideReact.StrictMode, which is a standard best practice. The use of the non-null assertion (document.getElementById('root')!) is common in TypeScript React projects—just be sure that your HTML always contains an element with the idroot.evidence-display/public/manifest.json (2)
2-3: Application rebranding looks goodThe updated names "Reality.eth Evidence" and "Reality.eth Evidence Display" clearly identify the application's purpose, which is an improvement over the generic "Dapp Front Boilerplate" names.
13-14: Appropriate color scheme updateThe new theme color (#2093ff) and background color (#f0f4f8) match the branding seen elsewhere in the application. I note these same colors are defined in the Tailwind configuration, ensuring consistency across the application.
dynamic-script/tsconfig.node.json (1)
1-10: TypeScript Node configuration correctly set up for ViteThis configuration is properly set up for Vite with appropriate settings:
composite: trueenables project referencesmodule: ESNextandmoduleResolution: bundlerare optimal for Vite- The file correctly includes only the
vite.config.tsThe configuration is aligned with modern TypeScript and Vite best practices.
evidence-display/src/App.tsx (1)
1-11: Clean, minimal App component implementationThe App component provides a good structure with:
- Proper import of the RealitioDisplayInterface component
- A semantic main element with appropriate Tailwind classes
- Both named and default exports for flexibility
This implementation follows modern React best practices for a root component.
evidence-display/tailwind.config.js (1)
1-21: Well-structured Tailwind configurationThe configuration properly:
- Specifies content sources for the HTML, JS, and TS files
- Extends the theme with custom font family and colors
- Defines a consistent color palette that matches the manifest.json
The color definitions for primary (#2093ff) and background (#f0f4f8) align perfectly with the manifest.json theme_color and background_color, ensuring visual consistency throughout the application.
evidence-display/vite.config.ts (1)
1-31: Well-structured Vite configuration.The configuration is well-structured and follows Vite best practices. The use of plugins, modern build targets, and explicit configuration options makes the build process transparent and maintainable.
dynamic-script/vite.config.ts (2)
6-20: Well-structured library build configuration.The library build configuration is properly set up with appropriate entry point, output name, and ES module format. Enabling sourcemaps is good for debugging, and the "esnext" target ensures modern features are supported.
21-23: Good use of environment variables for versioning.Using
process.env.npm_package_versionto define a version constant is a clean approach to make the package version accessible within your code.dynamic-script/tsconfig.json (2)
2-19: Well-structured TypeScript configuration.The TypeScript configuration is well-structured for a modern library build. Using ESNext with bundler module resolution is appropriate for Vite, and enabling strict type checking, declaration files, and source maps follows best practices for library development.
9-17: Consider the implications of allowJs with strict mode.You've enabled both
strict: trueandallowJs: true. While this is valid and useful during migration from JavaScript to TypeScript, be aware that JavaScript files won't benefit from the same level of type safety as TypeScript files. Consider adding JSDoc comments to JavaScript files to improve type checking if you intend to keep them.evidence-display/tsconfig.json (3)
23-27: Path alias configuration aligns with Vite config.The path alias configuration using relative paths is correctly set up and aligns with the alias in the Vite configuration. This enables imports like
import { Component } from '@/components'throughout your application.
17-21: Good linting configuration for code quality.The linting options (
strict,noUnusedLocals,noUnusedParameters,noFallthroughCasesInSwitch) will help maintain code quality by enforcing TypeScript best practices and catching potential issues early.
1-31: Well-structured TypeScript configuration for React.The TypeScript configuration is well-structured for a modern React application. The target, module settings, and JSX configuration are appropriate for a Vite-based React project. The noEmit setting correctly delegates transpilation to Vite.
dynamic-script/src/types/reality-eth-lib.d.ts (1)
1-9: TypeScript declaration file looks good!This declaration file provides proper type definitions for the
@reality.eth/reality-eth-lib/formatters/question.jsmodule, including the necessary functions and types. TheQuestionFormattype is well-defined as a union of string literals, which will provide good type checking and autocompletion when used.evidence-display/index.html (2)
4-13: HTML head section updates look goodThe metadata updates follow modern HTML5 standards with appropriate charset, viewport settings, theme color, and description. Moving from a CDN dependency to local assets is a good practice for better control over your dependencies.
17-17: Script modernization implemented correctlySwitching from an external CDN script to a local TypeScript module is consistent with the modernization goals of this PR. The module type indicates the use of ES modules which is the current standard.
dynamic-script/src/index.ts (3)
5-11: Interface definition looks goodThe
ScriptParametersinterface is well-defined with appropriate types for each property. Using the template literal type for the Ethereum address (0x${string}) is a good practice.
13-15: Appropriate declaration of external globalsThese declarations correctly inform TypeScript about externally defined variables and functions without implementing them.
17-24: Good error handling patternThe error handling implementation is solid with proper typing of the error and a fallback message. The function correctly handles the async/await pattern and uses proper return statements.
evidence-display/src/containers/realitio.tsx (2)
35-47: Good error handling in data fetchingThe try/catch block properly handles potential errors during data fetching and logs the error for debugging.
59-86: UI implementation looks goodThe UI component is well-structured with appropriate styling using Tailwind CSS. The link to reality.eth is properly formatted with the required parameters and appropriate security attributes.
dynamic-script/src/lib.ts (1)
159-208: Looks great overall.The solution for constructing meta-evidence based on different question types is cleanly organized, and the approach handles edge cases like “Answered Too Soon” elegantly.
dynamic-script/.eslintrc.json (4)
2-6: Extending recommended rules for TypeScript is solid.Your extension of
plugin:@typescript-eslint/recommendedandplugin:import/typescriptensures alignment with best practices for TypeScript and import statements.
9-11: Excluding build artifacts is good practice.Ignoring
dist/*prevents lint checks on compiled output and reduces irrelevant warnings.
23-32: Correctly configuring import resolver for TypeScript.Specifying the parser for TypeScript (
@typescript-eslint/parserfor .ts/.tsx) and resolver ensures consistent import behavior across the codebase. Nicely done!
16-22:✅ Verification successful
Verify that the specified tsconfig files exist.
The settings reference
tsconfig.jsonandtsconfig.node.json. Ensure these files are indeed present and properly configured.
🏁 Script executed:
#!/bin/bash # Verify existence of tsconfig.json and tsconfig.node.json in the project root fd tsconfig.json fd tsconfig.node.jsonLength of output: 172
TSConfig Files Verified – No Further Action Required
The referenced
tsconfig.jsonandtsconfig.node.jsonfiles are present in the project (located in thedynamic-scriptdirectory). The configuration in.eslintrc.jsoncorrectly points to these files, so no modifications are needed.dynamic-script/src/contracts.ts (2)
112-114: Type-safety approach with event maps is commendable.This creates a neat mapping between event names and their respective ABI event objects, ensuring correctness in contract interactions.
4-22:✅ Verification successful
Confirm address mappings in
REALITY_STARTS_AT.Ensure these addresses and block numbers are correct for each network. Inaccuracies may lead to searching from the wrong block height and missing relevant logs.
🏁 Script executed:
#!/bin/bash # (Optional) Check usage references of these addresses throughout the codebase rg "0x325a2e0f3cca2ddbaebb4dfc38df8d19ca165b47" -A 3 rg "0x5b7dd1e86623548af054a4985f7fc8ccbb554e2c" -A 3 # ... Repeat for other addresses if necessaryLength of output: 1003
Address mappings confirmed in
dynamic-script/src/contracts.ts
- The grep output verifies that all the listed addresses (e.g., for Reality 2.0 Mainnet, Reality 3.0 Mainnet, etc.) are referenced only in this file.
- There’s no evidence of discrepancies or alternative mappings elsewhere in the codebase.
- Please ensure to validate these block numbers with official network data as needed to prevent any mismatches when processing logs.
dynamic-script/package.json (8)
2-4: Updated Package Metadata
The package name, version, and description have been updated correctly to reflect the new purpose. These changes clearly signal the rebranding and provide better context for the package.
7-9: Module and Entry Points Configuration
The addition of"type": "module"along with the updated"main"and new"types"fields ensures proper ES module resolution and TypeScript support. These modifications align with modern Node.js practices.
10-19: Exports Field Added
The new"exports"field neatly defines both the main script and thelibsubmodule with appropriate paths for import and type definitions. This clear structure enhances module consumption in both JavaScript and TypeScript environments.
20-26: TypesVersions Inclusion
Including the"typesVersions"field to map thelibmodule to its TypeScript declaration (./dist/lib.d.ts) is a good move for better typing support when the package is consumed by TypeScript projects.
27-30: Files Field Configuration
The"files"field now explicitly includes thedistandsrcdirectories in the package distribution, which will help in maintaining a clean package and ensuring only the necessary files are published.
38-41: Updated Dependencies
Switching to@reality.eth/reality-eth-liband addingviemfor relevant functionality aligns the dependencies with the new implementation strategy. The versions specified also seem appropriate.
42-53: Revamped DevDependencies
The overhaul of the devDependencies—replacing legacy Webpack-related packages with modern TypeScript, ESLint, and Vite-related packages—supports the transition to a modern development environment. All specified versions appear to facilitate the new build process effectively.
54-56: Volta Node Version Upgrade
Updating the Node version to"20.19.0"aligns with the modernized toolchain and ensures consistency across projects using Volta for node version management.evidence-display/package.json (4)
3-6: Package Metadata and Module Type Update
The package version has been bumped to0.5.0, and a descriptive"description"has been added. Declaring"type": "module"ensures that the project uses ECMAScript modules, which is crucial for modern JavaScript projects.
16-20: Updated Dependencies for Modern Stack
Introducing the dependency"@kleros/cross-chain-realitio-dynamic-script": "workspace:*"and upgrading"react"and"react-dom"to^18.2.0is in line with modern developments and ensures compatibility with the latest React features.
21-37: Modernized DevDependencies
The devDependencies have been overhauled to include current tools and plugins—for instance, Tailwind CSS, updated ESLint plugins, and Vite-related packages. This comprehensive update supports a robust development and styling environment.
39-41: Consistent Node Version Management
Updating the Volta configuration to"node": "20.19.0"ensures consistency with the dynamic-script project and leverages the improvements in the latest Node.js versions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
contracts/.solhint.json (1)
7-10: Enforcing Solidity Compiler Version RuleThe update to the
"compiler-version"rule is well implemented. Enforcing that the compiler version must be either^0.7.0or^0.8.0should help catch incompatible compiler versions early in the development cycle. Ensure that all contracts in the repository are compatible with these restrictions.contracts/tasks/update-deployments.js (1)
128-130: Clear Validation of Foreign Network RPC URL
Splitting the declaration and validation offoreignNetworkUrl(lines 128–130) enhances clarity. As a nitpick, consider using optional chaining (e.g.
const foreignNetworkUrl = config.networks[foreignNetworkName]?.url;)
to safeguard against a missing network configuration.evidence-display/src/containers/realitio.tsx (3)
6-6: Consider removing or conditionally showing console.log in production.This logging statement will appear in the browser console in all environments, including production.
-console.log('evidence-display version', process.env.VERSION); +if (process.env.NODE_ENV !== 'production') { + console.log('evidence-display version', process.env.VERSION); +}
58-63: Consider simplifying the chainID extraction.The current approach is a bit verbose with multiple URLSearchParams instantiations.
- const chainID = - new URLSearchParams(window.location.search).get('chainID') || - new URLSearchParams(window.location.search).get('arbitrableChainID') || - new URLSearchParams(window.location.search).get('arbitratorChainID'); + const searchParams = new URLSearchParams(window.location.search); + const chainID = searchParams.get('chainID') || + searchParams.get('arbitrableChainID') || + searchParams.get('arbitratorChainID');
64-88: UI rendering looks good, but consider extracting inline styles.The component correctly renders the UI with the fetched data. However, the inline styles could be extracted for better maintainability.
Consider moving the inline styles to a CSS module or to a separate style object at the top of the component for better readability and maintainability.
dynamic-script/src/lib.ts (1)
91-95: Consider refactoring the REALITY_STARTS_AT lookup for better type safety.The current implementation uses type assertion which could be improved for better type safety.
- fromBlock: BigInt( - Object.keys(REALITY_STARTS_AT).includes(realitioAddress.toLowerCase()) - ? REALITY_STARTS_AT[realitioAddress.toLowerCase() as keyof typeof REALITY_STARTS_AT] - : 0 - ), + fromBlock: BigInt( + getStartBlockForRealitio(realitioAddress) || 0 + ),And add this helper function at the top of the file:
function getStartBlockForRealitio(address: string): number | undefined { const lowerCaseAddress = address.toLowerCase(); return lowerCaseAddress in REALITY_STARTS_AT ? REALITY_STARTS_AT[lowerCaseAddress as keyof typeof REALITY_STARTS_AT] : undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
.eslintrc.json(0 hunks).prettierrc.json(0 hunks).vscode/settings.json(1 hunks)biome/biome.base.json(1 hunks)contracts/.prettierrc.json(1 hunks)contracts/.solhint.json(2 hunks)contracts/biome.json(1 hunks)contracts/deploy/01-home-proxy.js(2 hunks)contracts/deploy/02-foreign-proxy.js(3 hunks)contracts/deploy/foreign/arbitrum.js(1 hunks)contracts/deploy/foreign/gnosis.js(1 hunks)contracts/deploy/foreign/optimism.js(2 hunks)contracts/deploy/foreign/polygon.js(1 hunks)contracts/deploy/foreign/zksync.js(1 hunks)contracts/deploy/shared/consts.js(1 hunks)contracts/package.json(4 hunks)contracts/scripts/execute_proof.js(3 hunks)contracts/scripts/get_event_properties.js(3 hunks)contracts/tasks/generate-metaevidence.js(2 hunks)contracts/tasks/relay-arbitrum.js(1 hunks)contracts/tasks/update-deployments.js(8 hunks)dynamic-script/.eslintrc.json(0 hunks)dynamic-script/biome.json(1 hunks)dynamic-script/package.json(1 hunks)dynamic-script/src/contracts.ts(1 hunks)dynamic-script/src/index.ts(1 hunks)dynamic-script/src/lib.ts(1 hunks)dynamic-script/src/types/reality-eth-lib.d.ts(1 hunks)evidence-display/.eslintrc.json(0 hunks)evidence-display/biome.json(1 hunks)evidence-display/package.json(1 hunks)evidence-display/src/App.tsx(1 hunks)evidence-display/src/containers/realitio.tsx(1 hunks)evidence-display/src/main.tsx(1 hunks)evidence-display/src/types/images.d.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (4)
- dynamic-script/.eslintrc.json
- evidence-display/.eslintrc.json
- .prettierrc.json
- .eslintrc.json
✅ Files skipped from review due to trivial changes (15)
- evidence-display/biome.json
- contracts/deploy/foreign/gnosis.js
- dynamic-script/biome.json
- contracts/deploy/foreign/arbitrum.js
- contracts/deploy/foreign/optimism.js
- contracts/biome.json
- contracts/deploy/02-foreign-proxy.js
- contracts/deploy/foreign/polygon.js
- contracts/.prettierrc.json
- contracts/tasks/relay-arbitrum.js
- contracts/deploy/foreign/zksync.js
- contracts/tasks/generate-metaevidence.js
- contracts/scripts/get_event_properties.js
- contracts/scripts/execute_proof.js
- contracts/deploy/01-home-proxy.js
🚧 Files skipped from review as they are similar to previous changes (5)
- evidence-display/src/App.tsx
- evidence-display/src/types/images.d.ts
- dynamic-script/src/types/reality-eth-lib.d.ts
- dynamic-script/src/contracts.ts
- dynamic-script/src/index.ts
🧰 Additional context used
🧬 Code Definitions (4)
evidence-display/src/main.tsx (1)
evidence-display/src/App.tsx (1)
App(3-9)
evidence-display/src/containers/realitio.tsx (1)
dynamic-script/src/lib.ts (2)
RealityQuestionData(15-26)fetchRealityQuestionData(28-149)
dynamic-script/src/lib.ts (1)
dynamic-script/src/contracts.ts (4)
foreignProxyAbi(33-58)homeProxyAbi(23-31)realitioAbi(60-109)REALITY_STARTS_AT(3-21)
contracts/tasks/update-deployments.js (5)
contracts/tasks/generate-metaevidence.js (1)
foreignNetworkName(26-26)contracts/scripts/execute_proof.js (3)
ethers(11-11)foreignProxyContract(38-42)homeProxy(29-29)dynamic-script/src/contracts.ts (1)
foreignProxyAbi(33-58)contracts/deploy/01-home-proxy.js (1)
homeProxy(37-43)contracts/deploy/02-foreign-proxy.js (1)
homeProxy(55-55)
🪛 Biome (1.9.4)
contracts/tasks/update-deployments.js
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (38)
biome/biome.base.json (5)
1-2: Schema Configuration VerificationThe configuration file correctly declares the
$schemawith the BiomeJS version "1.6.1". Please verify that this is the intended version and remains consistent with the project’s expectations when upgrading Biome tooling in the future.
3-5: Organize Imports BlockThe
organizeImportssection is clearly enabled. This simple configuration helps to automatically sort and group imports, which aligns with the project's modernization objectives.
6-20: Linter ConfigurationThe linter configuration is comprehensive and neatly categorizes rules into "recommended", "correctness", "suspicious", and "style". The explicit enforcement of rules like
"noUnusedVariables","noExplicitAny", and"noNonNullAssertion"should help maintain a high standard of code quality. Consider periodically reviewing these rules as the codebase evolves.
21-28: Formatter SettingsThe formatter is configured with clear settings including the indent style, width, and maximum line width. Setting
formatWithErrorsto false ensures formatting only applies to error-free code, which is a good safeguard. The emptyignorearray indicates a deliberate choice not to exclude any files from formatting—this is appropriate if all files are intended to follow the same style guidelines.
29-35: JavaScript Formatter RulesThe additional JavaScript-specific formatter settings (quote style, trailing comma, semicolon enforcement) are properly defined. This ensures consistency and compatibility for JavaScript files, particularly in mixed-language repositories.
contracts/tasks/update-deployments.js (12)
89-93: Improved Metadata Parsing for TOS
The multi-line ternary expression for extractingtos(lines 90–92) now clearly distinguishes between a definedtosvalue and the fallback to an empty string. This enhances readability without affecting functionality.
98-101: Enhanced Readability for Contract Info Fetch
Reformatting the call tofetchContractInfoover multiple lines (lines 98–101) makes the code easier to read and maintain. There’s no change in functionality.
102-107: Refined Extraction of Realitio Contract Name
The multi-line formatting in constructing and sanitizingrealitioContractName(lines 102–107) improves clarity and maintainability. The logic to trim and extract the name is intact.🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
137-139: Improved Error Handling for Foreign Transaction Receipt
The reformatted check and error throw for a missing foreign transaction receipt (lines 137–139) clearly communicates the issue and maintains robust error handling.
152-156: Enhanced Readability in Contract Instantiation
Reformatting the instantiation offoreignProxyContract(lines 152–156) into a clear multi-line structure improves readability while retaining the same functionality.
159-162: Clear Error Reporting for Mismatched Home Proxy
Reformatting the condition and error throw for a mismatched home proxy address (lines 159–162) clarifies the intent and ensures that any mismatch is immediately noticeable.
164-166: Refined Legacy Proxy Warning Message
The updated, multi-line warning message formatting (lines 164–166) in the catch block provides an informative log if the proxy verification fails, aiding in debugging legacy contract issues.
242-244: Enhanced Error Messaging on Duplicate Deployment
The multi-line formatting used when throwing an error for an existing deployment (lines 242–244) makes the message clearer and preserves the intent when theforceflag is not set.
254-257: Improved Deployment Merging Logic Formatting
Reformatting the merging of the existing deployments array (lines 254–257) using the spread operator enhances code readability and ensures maintainability.
270-273: Standardized Task Declaration for Artifact-based Deployment
The updated multi-line declaration of theadd-deployment-from-artifacttask (lines 270–273) improves clarity of the task’s intent and parameters without impacting behavior.
283-285: Concise Error Handling for Artifact Validation
The revised formatting for error checks on the home and foreign proxy artifacts (lines 283–285) makes the validations more readable and the error messages easier to track.
309-312: Streamlined Deployment File Filtering
The multi-line refactor of reading and filtering the deployment JSON files (lines 309–312) improves clarity, ensuring that file discovery logic is transparent and maintainable.contracts/deploy/shared/consts.js (1)
24-29: Formatting improvements enhance readabilityThe multiline formatting applied to the policy URLs and error messages improves code readability while maintaining the same functionality. This is a good practice for handling long strings and complex error messages.
Also applies to: 35-37, 45-47
package.json (1)
10-23:✅ Verification successful
Verify that linting functionality is preserved in the new setup
I notice that most of the linting dependencies and scripts have been removed, while keeping only Husky and lint-staged. Since the PR is about script modernization, please confirm that linting functionality is still maintained through other means (possibly at the workspace level or through build tools like Vite/Biome mentioned in other package.json files).
🏁 Script executed:
#!/bin/bash # Check if linting is configured in workspaces echo "Checking for linting configurations in workspaces..." find . -path '*/node_modules' -prune -o -name "*.json" -type f -exec grep -l "lint" {} \;Length of output: 409
Linting Functionality Confirmed in Workspace-Level Configurations
The root package.json now only sets up husky and lint-staged, while linting configurations remain in place across the workspaces (e.g., ESLint settings in ./bots/.eslintrc.json, Biome configurations, and the lint-staged configuration in ./.lintstagedrc.json). This supports the modernization effort by moving linting to individual workspaces/build tools. Please ensure that the linting process operates as expected in all workspaces.
evidence-display/package.json (3)
3-5: Great modernization of the toolchain!The move to Vite, TypeScript, and Biome represents a significant modernization of the development environment. The new scripts setup provides better developer experience with dedicated commands for development, building, previewing, and code quality.
Also applies to: 11-17
25-37: Good choice of modern development toolsThe transition to modern tools like Biome, TypeScript, Tailwind, and Vite aligns well with current best practices. The Node.js upgrade to v20.19.0 is also appropriate for these tools.
Also applies to: 39-39
20-22:❓ Verification inconclusive
Verify React 18 compatibility
The upgrade from React 16 to React 18 is a major version change that includes breaking changes, particularly around concurrent rendering and automatic batching. Please verify that all components have been updated to work with React 18.
🌐 Web query:
What breaking changes were introduced in React 18?💡 Result:
React 18 introduced several breaking changes to enable new features and improve performance. Here are the key updates developers need to be aware of:
1. New Root API
- Replaced
ReactDOM.renderwithcreateRootfor mounting components- Added
hydrateRootfor server-side hydration instead ofReactDOM.hydrate- Failure to adopt these will show deprecation warnings and disable concurrent features
// Before React 18 import { render } from 'react-dom'; render(<App />, document.getElementById('root')); // React 18 import { createRoot } from 'react-dom/client'; const root = createRoot(document.getElementById('root')); root.render(<App />);This change enables concurrent rendering capabilities[1][3].
2. TypeScript Definition Updates
- Explicit
childrenprop requirement for component props- New stricter type safety checks
- Requires updating
@types/reactand@types/react-domdependencies[2][4]3. Automatic Batching Changes
- All state updates now batched by default (including in timeouts/promises)
- Previous behavior only batched React event handler updates
- Can opt-out using
flushSyncfor critical updates[2][3]4. Stricter Strict Mode Behavior
- Development-mode double mounting of components
- Simulates future reusable state feature behavior
- Requires effect cleanup to handle multiple mount/unmount cycles[2][3]
5. Browser Support Changes
- Dropped Internet Explorer 11 support
- Requires polyfills for older browsers[1][4]
6. Hydration Errors
- Improved hydration mismatch detection
- Stricter error reporting for SSR content mismatches[2]
7. Suspense Behavior
- Consistent suspense tree handling
- Proper error boundary placement requirements[2]
While most existing code should work without changes, projects using legacy APIs or complex effects might require adjustments. The React team recommends gradual adoption of new features while maintaining compatibility with the old APIs during migration[3][4]. Developers should particularly test effect cleanup functions and state update timing when upgrading.
Citations:
- 1: https://blog.appsignal.com/2022/04/13/whats-new-in-react-18.html
- 2: https://react.dev/blog/2022/03/08/react-18-upgrade-guide
- 3: https://react.dev/blog/2022/03/29/react-v18
- 4: https://www.itmagination.com/blog/react-18-what-changes-does-it-bring
- 5: https://github.com/facebook/react/blob/main/CHANGELOG.md
Action: Verify Comprehensive React 18 Compatibility
The React dependencies are updated to version ^18.2.0. However, React 18 introduces several breaking changes that require attention. Please review the following key areas:
- New Root API: Ensure all entry points use
createRoot(andhydrateRootfor SSR) instead of the legacyReactDOM.render/ReactDOM.hydratemethods.- Automatic Batching: Validate that state updates, including those in asynchronous callbacks (timeouts, promises), function correctly under the new batching behavior.
- TypeScript Updates: Confirm that any TypeScript definitions have been revised accordingly, particularly regarding the explicit
childrenprop requirements and stricter type checks.- Strict Mode Behavior: Check for any issues arising from double invocations in development mode, which may affect component initialization and cleanup.
- Browser Support & Polyfills: Verify that dropped IE11 support does not impact your project or that appropriate polyfills are in place if necessary.
File: evidence-display/package.json
The dependency entries for React and react-dom are correctly updated:"@kleros/cross-chain-realitio-dynamic-script": "workspace:*", "react": "^18.2.0", "react-dom": "^18.2.0"Please thoroughly test all components to ensure compatibility with the upgraded React version and address any deprecation warnings or behavioral regressions.
dynamic-script/package.json (2)
7-30: Excellent TypeScript module configurationThe TypeScript configuration with proper exports mapping and type definitions follows modern best practices for package publishing. The explicit exports for both the main module and lib submodule provides clear entry points for consumers.
31-48:❓ Verification inconclusive
Verify compatibility with new dependencies
The complete replacement of dependencies (moving to
@reality.eth/reality-eth-libandviem) represents a significant internal change. Ensure that all functionality from the previous implementation is preserved and thoroughly tested with these new libraries.
🏁 Script executed:
#!/bin/bash # Look for usage of the new dependencies in the source code echo "Checking usage of new dependencies..." find dynamic-script/src -type f -name "*.ts" -exec grep -l "@reality.eth/reality-eth-lib" {} \; find dynamic-script/src -type f -name "*.ts" -exec grep -l "viem" {} \; # Check if there are any TODO or FIXME comments that might indicate incomplete migration echo "Checking for TODOs or FIXMEs that might indicate incomplete migration..." grep -r "TODO\|FIXME" --include="*.ts" dynamic-script/src/Length of output: 604
Ensure Migration Compatibility with New Dependencies
The new dependencies are now actively referenced in the code (e.g., indynamic-script/src/lib.ts,dynamic-script/src/types/reality-eth-lib.d.ts, anddynamic-script/src/contracts.ts), and no outstanding TODO/FIXME markers were found to indicate an incomplete migration. Please confirm through thorough testing that all functionality from the previous implementation is preserved..vscode/settings.json (1)
1-11: LGTM: Well-configured VSCode settings for the modernized environment.The VSCode settings are properly configured to support the new development stack. The configuration maps CSS files to the Tailwind CSS language service, sets Biome as the default formatter, and enables helpful code actions on save.
evidence-display/src/main.tsx (3)
1-4: Standard React imports look good.The imports are properly set up for a modern React application using React 18+, with appropriate imports for React, ReactDOM, App component, and styling.
6-12: Robust root element validation.Good practice to validate the existence of the root element and throw a descriptive error if not found, preventing cryptic runtime errors.
14-18: Proper React 18 rendering setup.The code correctly uses the modern React 18 createRoot API instead of the legacy ReactDOM.render method, and wraps the App in StrictMode for additional development-time checks.
evidence-display/src/containers/realitio.tsx (1)
8-54: Component initialization and data fetching look good.The component is well-structured with appropriate React hooks usage. The useEffect hook correctly fetches data on component mount and the error handling is robust.
dynamic-script/src/lib.ts (6)
1-14: Imports and interface definitions are well structured.Good use of TypeScript interfaces for defining the parameters and proper imports from required libraries.
15-26: Well-defined RealityQuestionData interface.The interface properly captures all necessary data for a reality question, with appropriate type definitions for different question types.
28-51: Robust parameter validation and client initialization.The function properly validates required parameters and initializes the RPC clients. Good use of fallbacks for JSON RPC URLs.
86-106: Good error handling for event logs.The code properly checks for empty event logs and provides meaningful error messages, addressing previous review comments.
108-136: Efficient template handling with fallback to contract calls.The approach of hardcoding the first 5 standard templates and only querying the blockchain for custom templates is a good optimization.
151-214: Well-structured meta-evidence generation with type safety.The fetchRealityMetaEvidence function is well-implemented with comprehensive handling of different question types. Good use of TypeScript's type system with the satisfies operator to ensure type safety.
contracts/package.json (3)
18-22: Enhanced Script Commands for ModernizationThe addition of new script commands for JavaScript and Solidity formatting/linting is well aligned with the PR objective. The following points should be verified:
- JS Formatting/Linting:
"format:js": "biome format --write scripts deploy tasks""check:js": "biome check --apply scripts deploy tasks""lint:js": "biome lint scripts deploy tasks"
Confirm that the specified directories (i.e.,scripts,deploy, andtasks) accurately reflect where your JavaScript files reside.- Solidity Linting/Formatting:
"lint:sol": "solhint 'src/**/*.sol'""fix:sol": "prettier --write 'src/**/*.sol'"
Ensure that Solidity files are consistently located undersrc/and that bothsolhintandprettier(with its Solidity plugin) meet your expected formatting and linting rules.- Biome Option: Validate that the
--applyflag in the"check:js"command works as intended when auto-fixing issues.
73-73: Dependency Addition for BiomeThe inclusion of
@biomejs/biomeat version"1.6.1"is essential for the new script commands to function correctly. Please confirm that this version of Biome integrates well with your setup and does not conflict with other tooling.
95-98: Solidity Formatting & Linting ToolingThe new devDependencies for
"prettier","prettier-plugin-solidity","solhint", and"solhint-plugin-prettier"provide a robust framework for enforcing consistency in Solidity code style:
- Make sure that your configuration files (such as
.prettierrcor Solhint configuration) are updated to leverage these tools.- The specified versions appear appropriate, but a verification against your CI/CD or local workflow is recommended to avoid any unexpected conflicts.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
evidence-display/src/containers/realitio.tsx (6)
6-6: Consider removing or conditionally using console.log in production.It's generally considered best practice to avoid leaving console.log statements in production code. Consider using a logger utility or wrapping this in a development-only conditional.
-console.log('evidence-display version', process.env.VERSION); +if (process.env.NODE_ENV !== 'production') { + console.log('evidence-display version', process.env.VERSION); +}
11-54: Add error handling for JSON RPC URL parameters.While you're validating critical parameters like contract address and dispute ID, the JSON RPC URL parameters are passed to fetchRealityQuestionData without validation. Consider adding checks for these parameters before using them.
Also, the
voidoperator on line 53 is not necessary unless you're explicitly trying to suppress the TypeScript error for an unused promise.try { const data = await fetchRealityQuestionData({ disputeID, arbitrableContractAddress: arbitrableContractAddress as `0x${string}`, - arbitrableJsonRpcUrl, - arbitratorJsonRpcUrl, - jsonRpcUrl, + ...(arbitrableJsonRpcUrl && { arbitrableJsonRpcUrl }), + ...(arbitratorJsonRpcUrl && { arbitratorJsonRpcUrl }), + ...(jsonRpcUrl && { jsonRpcUrl }), }); setQuestionState(data); } catch (error) { console.error('Failed to fetch question data:', error); } -void fetchData(); +fetchData();
58-63: Avoid parsing URL parameters multiple times.The code extracts chainID from URL parameters again, even though it already parsed them earlier (lines 15-28). Consider storing the chainID during the initial parsing.
const { questionID, realitioAddress, questionData } = questionState; -const chainID = - new URLSearchParams(window.location.search).get('chainID') || - new URLSearchParams(window.location.search).get('arbitrableChainID') || - new URLSearchParams(window.location.search).get('arbitratorChainID'); +const chainID = params.chainID || params.arbitrableChainID || params.arbitratorChainID;
69-77: Simplify styling approach for horizontal rule.The horizontal rule uses both Tailwind classes and inline styles, which creates redundancy. Consider consolidating to a single styling approach.
<hr - className="h-[3px] border-none bg-gradient-to-r from-[#24b3ec] via-[#24b3ec] to-[#dcfb6c]" - style={{ - backgroundSize: 'contain', - color: '#27b4ee', - background: - 'linear-gradient(45deg, #24b3ec 0%, #24b3ec 93%, #b9f9fb 93%, #b9f9fb 95%, #dcfb6c 95%)', - }} + className="h-[3px] border-none" + style={{ + backgroundSize: 'contain', + color: '#27b4ee', + background: + 'linear-gradient(45deg, #24b3ec 0%, #24b3ec 93%, #b9f9fb 93%, #b9f9fb 95%, #dcfb6c 95%)', + }} />
79-86: Extract link URL to improve readability.The URL for the reality.eth platform is constructed directly in the JSX. Consider extracting it to a variable before the return statement to improve readability.
const { questionID, realitioAddress, questionData } = questionState; const chainID = new URLSearchParams(window.location.search).get('chainID') || new URLSearchParams(window.location.search).get('arbitrableChainID') || new URLSearchParams(window.location.search).get('arbitratorChainID'); +const realityEthUrl = `https://reality.eth.limo/app/index.html#!/network/${chainID}/question/${realitioAddress}-${questionID}`; return ( <div className="bg-[#f0f4f8] p-4 font-roboto"> {/* ... */} <a className="text-[#2093ff]" - href={`https://reality.eth.limo/app/index.html#!/network/${chainID}/question/${realitioAddress}-${questionID}`} + href={realityEthUrl} target="_blank" rel="noopener noreferrer" >
90-91: Choose one export style for consistency.The component is exported both as a named export and as the default export. Choose one style for consistency across your codebase.
Either keep only the default export:
export function RealitioDisplayInterface() { // ... } -export default RealitioDisplayInterface;Or use only the named export and change the import statements elsewhere.
evidence-display/vite.config.ts (1)
12-15: Clarify comment about SDK bundlingThere's a comment stating "Don't mark the SDK as external since we want it bundled", but the external array is empty. If there was a specific SDK that you considered marking as external but decided against it, consider removing or updating the comment to avoid confusion.
- // Don't mark the SDK as external since we want it bundled + // No external dependencies - we want everything bundled external: [],sdk/src/types/reality-eth-lib.d.ts (1)
1-5: Consider using a function instead of a class with only static methods.This TypeScript declaration defines a class
RealitioQuestionthat only contains static methods. According to best practices and the static analysis hint, it would be better to use a simple function export instead of a class with only static members.-declare module "@reality.eth/reality-eth-lib/formatters/question.js" { - export default class RealitioQuestion { - static populatedJSONForTemplate(template: string, question: string): string; - } +declare module "@reality.eth/reality-eth-lib/formatters/question.js" { + export default function populatedJSONForTemplate(template: string, question: string): string; }This approach aligns with functional programming principles and removes the unnecessary class wrapper.
🧰 Tools
🪛 Biome (1.9.4)
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
sdk/scripts/find-dispute.ts (2)
11-15: Extract hardcoded contract address to a configuration constant.The contract address is hardcoded in the script. For better maintainability and reusability, consider extracting it to a constant or configuration parameter.
+const FOREIGN_PROXY_ADDRESS = "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE"; + const foreignProxy = getContract({ - address: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", + address: FOREIGN_PROXY_ADDRESS, abi: foreignProxyAbi, client: foreignClient, });
21-24: Make block range configurable with meaningful constants.The block range is hardcoded with magic numbers. For better readability and flexibility, extract these values to named constants with comments explaining their significance.
+// Block where the first dispute was created +const TARGET_BLOCK = 7746523n; +// Number of blocks to search before and after the target block +const BLOCK_RANGE = 1000n; + // Look for ArbitrationCreated events around block 7746523 -const fromBlock = 7746523n - BigInt(1000); -const toBlock = 7746523n + BigInt(1000); +const fromBlock = TARGET_BLOCK - BLOCK_RANGE; +const toBlock = TARGET_BLOCK + BLOCK_RANGE;sdk/src/__tests__/lib.integration.test.ts (1)
6-40: Test looks good but could be made more resilient.The integration test is well structured with appropriate timeout and error handling. However, integration tests using hardcoded data and live networks can become brittle over time.
Consider these improvements:
- Extract test configuration to constants at the top of the file
- Add a conditional skip for CI environments where external calls may be unreliable
- Add retry logic for network failures
+// Test configuration +const TEST_CONFIG = { + disputeID: "114", + arbitrableContractAddress: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", + arbitratorJsonRpcUrl: "https://1rpc.io/sepolia", + arbitrableJsonRpcUrl: "https://sepolia.unichain.org/", + timeout: 30000 // 30 seconds +}; describe("fetchRealityQuestionData (integration)", () => { // Mark test as integration test and increase timeout since we're making real network calls - it("should fetch reality question data from live networks", async () => { + it.runIf(process.env.SKIP_INTEGRATION_TESTS !== 'true')("should fetch reality question data from live networks", async () => { try { const result = await fetchRealityQuestionData({ - disputeID: "114", - arbitrableContractAddress: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", - arbitratorJsonRpcUrl: "https://1rpc.io/sepolia", - arbitrableJsonRpcUrl: "https://sepolia.unichain.org/", + disputeID: TEST_CONFIG.disputeID, + arbitrableContractAddress: TEST_CONFIG.arbitrableContractAddress, + arbitratorJsonRpcUrl: TEST_CONFIG.arbitratorJsonRpcUrl, + arbitrableJsonRpcUrl: TEST_CONFIG.arbitrableJsonRpcUrl, }); // Rest of the test... } catch (error) { // Log more details about the error console.error("Test failed with error:", error); if (error instanceof Error) { console.error("Error stack:", error.stack); } throw error; } - }, 30000); // 30 second timeout for network calls + }, TEST_CONFIG.timeout);sdk/src/__tests__/lib.test.ts (1)
38-163: Expand test coverage for error cases and edge conditions.While you have good coverage for the happy path and basic parameter validation, there are several error conditions in the
fetchRealityQuestionDatafunction that aren't tested:
- When no dispute is found (
arbitrationCreatedLogs.length !== 1)- When the question ID is missing from the event
- When the question event can't be found
- When template ID or question is missing from the event
Here's a sample test case for when no dispute is found:
it("should throw error when dispute cannot be found", async () => { // Setup mocks with empty arbitration logs const mockForeignProxy = { read: { homeProxy: vi.fn().mockResolvedValue("0x9876543210fedcba9876543210fedcba98765432"), arbitrationCreatedBlock: vi.fn().mockResolvedValue(12345n), }, getEvents: { ArbitrationCreated: vi.fn().mockResolvedValue([]), // Empty array - no events found }, }; // Rest of the mock setup... // Execute test and expect error await expect( fetchRealityQuestionData({ disputeID: "114", arbitrableContractAddress: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", }) ).rejects.toThrow("Could not find the dispute"); // Verify mock calls... });dynamic-script/src/index.ts (2)
6-6: Consider handling undefined environment variable.
Ifprocess.env.VERSIONmight be undefined, you might want to default to a placeholder or handle the case to avoid loggingundefined.console.log("dynamic-script version", process.env.VERSION); +// Optionally: +console.log("dynamic-script version", process.env.VERSION ?? "unknown");
16-18: Global declarations may reduce modularity.
DeclaringscriptParameters,resolveScript, andrejectScriptat the global scope can be appropriate if these are injected externally. Otherwise, wrapping them or providing them via function parameters might be more flexible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📥 Commits
Reviewing files that changed from the base of the PR and between 00ac5c3 and e538286e069e7de98c43044b2af88473e0135a24.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
dynamic-script/package.json(1 hunks)dynamic-script/src/index.ts(1 hunks)dynamic-script/vite.config.ts(1 hunks)evidence-display/index.html(1 hunks)evidence-display/package.json(1 hunks)evidence-display/src/containers/realitio.tsx(1 hunks)evidence-display/tailwind.config.cjs(1 hunks)evidence-display/tailwind.config.js(1 hunks)evidence-display/vite.config.ts(1 hunks)package.json(1 hunks)sdk/biome.json(1 hunks)sdk/package.json(1 hunks)sdk/scripts/find-dispute.ts(1 hunks)sdk/src/__tests__/lib.integration.test.ts(1 hunks)sdk/src/__tests__/lib.test.ts(1 hunks)sdk/src/contracts.ts(6 hunks)sdk/src/index.ts(1 hunks)sdk/src/lib.ts(1 hunks)sdk/src/types/reality-eth-lib.d.ts(1 hunks)sdk/tsconfig.json(1 hunks)sdk/tsup.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- sdk/src/index.ts
- sdk/biome.json
- evidence-display/tailwind.config.cjs
- sdk/tsup.config.ts
- sdk/tsconfig.json
- sdk/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- evidence-display/tailwind.config.js
- evidence-display/package.json
- package.json
🧰 Additional context used
🧠 Learnings (1)
dynamic-script/package.json (1)
Learnt from: jaybuidl
PR: kleros/cross-chain-realitio-proxy#68
File: dynamic-script/package.json:2-4
Timestamp: 2025-03-26T12:57:04.258Z
Learning: The @kleros/cross-chain-realitio-dynamic-script package is distributed via IPFS CID, not through NPM, so package name changes don't create breaking changes for consumers.
🧬 Code Definitions (5)
sdk/src/__tests__/lib.test.ts (3)
sdk/src/contracts.ts (3)
foreignProxyAbi(36-76)homeProxyAbi(24-34)realitioAbi(78-207)sdk/src/lib.ts (1)
fetchRealityQuestionData(36-213)sdk/src/types/reality-eth-lib.d.ts (1)
RealitioQuestion(2-4)
sdk/src/__tests__/lib.integration.test.ts (1)
sdk/src/lib.ts (1)
fetchRealityQuestionData(36-213)
dynamic-script/src/index.ts (1)
sdk/src/lib.ts (2)
fetchRealityMetaEvidence(227-281)RealityQuestionParams(15-21)
sdk/scripts/find-dispute.ts (1)
sdk/src/contracts.ts (1)
foreignProxyAbi(36-76)
sdk/src/lib.ts (3)
dynamic-script/src/index.ts (1)
RealityQuestionParams(35-35)sdk/src/contracts.ts (4)
foreignProxyAbi(36-76)homeProxyAbi(24-34)realitioAbi(78-207)REALITY_STARTS_AT(3-22)sdk/src/types/reality-eth-lib.d.ts (1)
RealitioQuestion(2-4)
🪛 Biome (1.9.4)
sdk/src/types/reality-eth-lib.d.ts
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (38)
evidence-display/index.html (3)
4-12: Enhanced Head Section Configuration
The<head>section has been modernized effectively. Notable improvements include:
- The charset is explicitly set to
"UTF-8", ensuring correct character encoding.- The viewport meta tag is simplified to align with responsive design best practices.
- The theme color is updated to
#2093ff, which should provide better visual consistency with the design.- The addition of a meta description and manifest link further enhances SEO and progressive web app support.
14-17: Adoption of ES Modules for Script Loading
Switching to a module script (type="module") referencing/src/main.tsxsignifies a move towards modern JavaScript practices. This supports a modular, TypeScript-based application architecture with local bundling improvements.
1-3: Overall HTML Structure and Simplification
The remaining sections of the HTML, including the DOCTYPE declaration and closing tags, are clean and consistent. The removal of any<noscript>block suggests a confidence in JavaScript availability or alternative handling in the application.Also applies to: 18-19
dynamic-script/package.json (6)
2-4: Package updated with new name and versionThe package has been renamed and versioned up correctly. Based on the retrieved learnings, this change doesn't create breaking changes for consumers since the package is distributed via IPFS CID, not through NPM.
7-12: Module configuration properly set up for TypeScriptThe package is now correctly configured as an ES module with TypeScript support. The main entry point, types declaration, and files to be included in the distribution are all properly defined.
14-18: Build system modernized with Vite and BiomeThe scripts have been updated to use modern tooling:
- Vite for building (replacing Webpack)
- Biome for code formatting and linting (replacing ESLint/Prettier)
This modernization aligns with current best practices and should improve developer experience.
21-21: Workspace dependency for SDKUsing a workspace dependency for the SDK is appropriate and will simplify version management across the monorepo.
24-27: Updated development dependenciesThe dev dependencies have been updated to include modern tooling that aligns with the new build process.
30-30: Significant Node.js version updateThe Node.js version has been updated from 16.20.2 to 20.19.0, which is a major version jump. This will require developers to use a more recent Node environment.
Make sure all team members and CI environments are updated to support Node 20. Consider documenting this requirement in your project's README.
dynamic-script/vite.config.ts (2)
1-21: Well-structured Vite configuration for library buildThe Vite configuration is properly set up for a library build with:
- Correct entry point specification
- ES module format output
- Proper external dependency handling
- Modern JavaScript target
- Environment variable injection for versioning
This configuration aligns well with the changes in package.json, particularly the module type and build target.
13-13: SDK correctly marked as externalThe SDK is properly marked as external, which is consistent with it being a workspace dependency in package.json. This ensures it won't be bundled with the library, avoiding duplication.
evidence-display/vite.config.ts (5)
1-8: Appropriate Vite configuration for React applicationThe configuration correctly sets up the React plugin and Node polyfills, which are necessary for a React application that may use Node.js APIs in the browser.
9-16: Build settings optimized for modern environmentsThe build configuration is well-optimized for modern browsers and Node 20, with source maps enabled for better debugging. This aligns with the Node version update seen in other files.
17-22: Path alias and symlinks properly configuredThe alias configuration for the source directory and the preservation of symlinks will improve developer experience and module resolution.
23-28: Modern JavaScript features enabledThe configuration properly enables modern ECMAScript features including top-level await, which is beneficial for cleaner async code.
29-31: Version injection from package.jsonThe version from package.json is correctly injected as an environment variable, ensuring consistency between the package metadata and the application.
sdk/src/__tests__/lib.test.ts (2)
1-148: Well-structured test with thorough validation of the happy path.The test is well-structured with clear setup, mock implementation, execution, and verification phases. The mock data and contract instances are properly set up to simulate the expected behavior.
149-162: Good error handling test case.The test properly verifies that the function throws an error when required parameters are missing and confirms that no contract calls are made in this scenario.
dynamic-script/src/index.ts (5)
1-4: Imports seem appropriate and typed imports are a plus.
No issues noted with these import statements; they correctly bring in functionality from the SDK.
8-14: Interface definition is clear and strongly typed.
The structure ofScriptParametersis clear and matches the usage in this file.
20-29: Good asynchronous error handling approach.
This function correctly resolves or rejects via the declared global functions, ensuring any fetching error is exposed through the custom mechanism.
31-33: Lightweight helper function is straightforward.
generateMetaEvidenceis a concise wrapper aroundfetchRealityMetaEvidence; no issues found.
35-35: Type re-export is convenient for downstream usage.
Re-exportingRealityQuestionParamssimplifies imports for clients.sdk/src/lib.ts (10)
1-2: Import statements look correct.
Usage ofRealitioQuestionand relevant utilities fromviemappears consistent.
3-8: Contract imports successfully collocated.
Consolidating contract ABIs in./contractshelps maintain clarity and keeps this file focused on core logic.
10-11: Utility functionisNilis concise and readable.
No concerns about correctness.
13-13: Utility functionisEmptyis straightforward.
Correctly checks for an empty string.
15-21:RealityQuestionParamsinterface is well-defined.
Clearly conveys required vs. optional fields.
23-34:RealityQuestionDatainterface is well-structured.
Captures key properties like the computed question ID, Realitio address, and question data.
36-213: Solid error handling and thorough logging infetchRealityQuestionData.
You perform multiple checks on parameters, retrieve addresses correctly, and handle missing events with informative errors. The console logs are useful for debugging; consider eventually switching to a configurable logger for production. Overall, the function is logically clear and robust.
215-223:RealityMetaEvidenceinterface is well suited for meta-evidence structure.
The optionalquestionfield and typedrulingOptionsproperty reflect usage in Realitio contexts.
225-226:RulingOptionsTypeclarifies the shape of ruling options.
Good approach to ensure strong typing for optional returns.
227-281:fetchRealityMetaEvidencefunction logically extends question data.
Sets uprulingOptionsbased on the question type and includes a reserved answer. The approach is reliable, aligning well with Realitio’s schema.sdk/src/contracts.ts (5)
1-2: Type import forAbiis properly used.
This keeps the ABI definitions type-safe.
24-34:homeProxyAbiaccurately typed asAbi.
Good to see each field properly defined and typed.
36-76:foreignProxyAbiincludes necessary fields.
TheArbitrationCreatedevent andarbitrationCreatedBlockfunction are consistent with usage in the SDK.
78-208:realitioAbiis well-structured with typed events and thetemplatesfunction.
The typed definitions and alignment with the Realitio contract’s interface make the code robust.
3-22:✅ Verification successful
REALITY_STARTS_ATmapping is helpful but verify correctness.
Having these block numbers ensuresfetchRealityQuestionDatacan scan from the correct block. If you add or update addresses in the future, confirm the block numbers remain accurate.
🏁 Script executed:
#!/bin/bash # Attempt to confirm each address is valid and logs begin around the indicated block range. echo "Addresses in REALITY_STARTS_AT:" printf '%s\n' $(jq -r 'keys[]' <<< '{ "0x325a2e0f3cca2ddbaebb4dfc38df8d19ca165b47":6531265, "0x5b7dd1e86623548af054a4985f7fc8ccbb554e2c":13194676, "0xaf33dcb6e8c5c4d9ddf579f53031b514d19449ca":3044431, "0x79e32ae03fb27b07c89c0c568f80287c01ca2e57":14005802, "0xe78996a233895be74a66f451f1019ca9734205cc":17997262, "0x1E732a1C5e9181622DD5A931Ec6801889ce66185":10438389, "0x60573b8dce539ae5bf9ad7932310668997ef0428":18901674, "0x5d18bd4dc5f1ac8e9bd9b666bd71cb35a327c4a9":459975, "0xB78396EFaF0a177d125e9d45B2C6398Ac5f803B9":41977012, "0xA8AC760332770FcF2056040B1f964750e4bEf808":9691, "0x4E346436e99fb7d6567A2bd024d8806Fc10d84D2":255658, "0x0eF940F7f053a2eF5D6578841072488aF0c7d89A":2462149, "0xeAD0ca922390a5E383A9D5Ba4366F7cfdc6f0dbA":14341474, "0xc716c23D75f523eF0C511456528F2A1980256a87":3034954, "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE":7686678, "0x8bF08aE62cbC9a48aaeB473a82DAE2e6D2628517":10747559, "0xB920dBedE88B42aA77eE55ebcE3671132ee856fC":8561869, "0x2F39f464d16402Ca3D8527dA89617b73DE2F60e8":26260675 }' | sort)Length of output: 1662
REALITY_STARTS_AT mapping verified and documented
The addresses and associated block numbers in
sdk/src/contracts.ts(lines 3–22) were verified against the expected list. All keys appear correctly, ensuring thatfetchRealityQuestionDatawill scan from the appropriate starting blocks. Please continue to double-check that any future additions or updates to these addresses include the correct block numbers.
- Verified addresses are intact and consistent with the expected output.
- Reminder: when modifying this mapping, ensure block numbers remain accurate for proper operation.
…to a SDK package, added tests
jaybuidl
force-pushed
the
scripts-modernization
branch
from
ebf8098 to
b5ff661
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
sdk/src/types/reality-eth-lib.d.ts (2)
1-5: Consider using namespace or function declarations instead of static-only class.The current implementation declares a class with only static members, which Biome flags as an anti-pattern. In TypeScript, when you're only defining static methods, it's generally better to use plain functions or a namespace.
Consider refactoring to one of these alternatives:
declare module "@reality.eth/reality-eth-lib/formatters/question.js" { - export default class RealitioQuestion { - static populatedJSONForTemplate(template: string, question: string): string; - } + export default { + populatedJSONForTemplate(template: string, question: string): string; + }; }Or if the library actually exports a constructor function:
declare module "@reality.eth/reality-eth-lib/formatters/question.js" { - export default class RealitioQuestion { - static populatedJSONForTemplate(template: string, question: string): string; - } + export default function RealitioQuestion(): void; + export namespace RealitioQuestion { + export function populatedJSONForTemplate(template: string, question: string): string; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
1-5: Consider adding all exports mentioned in the PR summary.The PR summary mentions additional exports that aren't in this file, such as functions
getTemplateType,getTemplateText, and a type aliasQuestionFormat.Would you like me to help complete the type definitions with these additional exports? This would make the declaration file more comprehensive.
🧰 Tools
🪛 Biome (1.9.4)
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
evidence-display/package.json (1)
10-18: Revised Scripts for Development and Build Processes
The introduction of new scripts—dev,build,preview,clean,format,lint, andcheck—reflects a significant streamlining of development and CI workflows.
- The
buildscript chains TypeScript compilation with a Vite build and then renames the build artifacts usingmv, which is effective but might pose cross-platform compatibility issues (e.g., on Windows). Consider evaluating a cross-platform solution if necessary.- The
cleanscript now targets thedistfolder and the versioned build artifact.
Overall, these changes modernize and simplify the developer experience.sdk/src/lib.ts (1)
53-90: Consider removing console.log statements for production code.The code contains numerous console.log statements which are useful for debugging but might generate excessive noise in production environments.
Consider implementing a configurable logging solution:
- console.log("Creating foreign client..."); + // Replace with configurable logging + const logger = { + debug: (process.env.DEBUG === 'true') ? console.log : () => {} + }; + logger.debug("Creating foreign client...");Apply similar changes to all console.log statements throughout the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📥 Commits
Reviewing files that changed from the base of the PR and between ebf8098fc90f71fb4160ddf328ee021a5c012180 and b5ff661.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
biome/biome.base.json(1 hunks)dynamic-script/package.json(1 hunks)dynamic-script/src/index.ts(1 hunks)dynamic-script/vite.config.ts(1 hunks)evidence-display/index.html(1 hunks)evidence-display/package.json(1 hunks)evidence-display/src/containers/realitio.tsx(1 hunks)evidence-display/tailwind.config.cjs(1 hunks)evidence-display/tailwind.config.js(1 hunks)evidence-display/vite.config.ts(1 hunks)package.json(1 hunks)sdk/biome.json(1 hunks)sdk/package.json(1 hunks)sdk/scripts/find-dispute.ts(1 hunks)sdk/src/__tests__/lib.integration.test.ts(1 hunks)sdk/src/__tests__/lib.test.ts(1 hunks)sdk/src/contracts.ts(6 hunks)sdk/src/index.ts(1 hunks)sdk/src/lib.ts(1 hunks)sdk/src/types/reality-eth-lib.d.ts(1 hunks)sdk/tsconfig.json(1 hunks)sdk/tsup.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- sdk/src/index.ts
- sdk/biome.json
- dynamic-script/vite.config.ts
- sdk/tsup.config.ts
- evidence-display/tailwind.config.cjs
- evidence-display/vite.config.ts
- sdk/src/tests/lib.test.ts
- sdk/package.json
- sdk/scripts/find-dispute.ts
- evidence-display/tailwind.config.js
- biome/biome.base.json
- dynamic-script/src/index.ts
- sdk/tsconfig.json
- sdk/src/tests/lib.integration.test.ts
- evidence-display/src/containers/realitio.tsx
🧰 Additional context used
🧠 Learnings (1)
dynamic-script/package.json (1)
Learnt from: jaybuidl
PR: kleros/cross-chain-realitio-proxy#68
File: dynamic-script/package.json:2-4
Timestamp: 2025-03-26T14:17:25.094Z
Learning: The @kleros/cross-chain-realitio-dynamic-script package is distributed via IPFS CID, not through NPM, so package name changes don't create breaking changes for consumers.
🪛 Biome (1.9.4)
sdk/src/types/reality-eth-lib.d.ts
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (28)
sdk/src/types/reality-eth-lib.d.ts (1)
1-5:❓ Verification inconclusive
Verify alignment with the actual library implementation.
The declaration file should match the actual implementation of the library. If this is a third-party library, ensure that the class structure accurately represents how the library is implemented.
🏁 Script executed:
#!/bin/bash # Check how the actual library exports the functionality # First, find if the package is installed if [ -d "node_modules/@reality.eth" ]; then echo "Package found in node_modules" # Check the actual file content to verify the export structure cat node_modules/@reality.eth/reality-eth-lib/formatters/question.js | head -n 20 # Look for static method definitions grep -n "static populatedJSONForTemplate" node_modules/@reality.eth/reality-eth-lib/formatters/question.js else echo "Package not found in node_modules" # Check if the package is listed in package.json grep -n "@reality.eth" package.json || echo "Package not found in package.json" fiLength of output: 228
Action Required: Verify Alignment with Third-Party Documentation
It appears that the package
@reality.eth/reality-eth-libisn’t present in the codebase (not found in eithernode_modulesorpackage.json). This means we were unable to automatically verify that the type declaration insdk/src/types/reality-eth-lib.d.tscorrectly reflects the actual implementation.
- Next Steps:
- Consult the official documentation or source repository for
@reality.eth/reality-eth-libto confirm that the exported class (specifically thestatic populatedJSONForTemplatemethod) is implemented as declared.- Ensure that any future changes in the third-party library’s implementation are matched by corresponding updates in this declaration file if necessary.
🧰 Tools
🪛 Biome (1.9.4)
[error] 2-4: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
evidence-display/index.html (2)
4-12: Head Section Modernization and Metadata Update
The changes in the head element demonstrate a successful modernization by updating the character set to uppercase ("UTF-8"), setting a new favicon (SVG format), adjusting the viewport for a modern responsive design, and introducing a clear meta description and updated title. These modifications align well with the updated project branding and technical requirements.
14-17: Body Section and Script Inclusion Enhancement
Switching the script inclusion from an external CDN to a local module script (/src/main.tsx) simplifies dependency management and supports the modular React/Vite setup. The removal of the<noscript>tag indicates a strategic decision that likely fits the current audience. Verify that the new entry point inmain.tsxcorrectly initializes the application.package.json (2)
14-19: Workspaces Array Update
The workspaces array now explicitly includes multiple modules such as "dynamic-script", "evidence-display", and "sdk". This change reflects the project’s restructured organization. Ensure that each workspace is correctly configured and that any interdependencies among them are updated accordingly.
22-23: Dependency Version Bump for Dev Tools
Upgradinghuskyto "^9.1.7" andlint-stagedto "^15.5.0" signals an important modernization of pre-commit workflows. Verify that the changes in the corresponding hook scripts (e.g.,.husky/pre-commit) align with these new versions and that the updated command (yarn lint-staged -c .lintstagedrc.mjs) is fully tested across all team environments.evidence-display/package.json (4)
3-5: Package Metadata Enhancements
Updating the version to "0.5.0", adding a clear description, and specifying module type ("module") are excellent improvements. These changes improve clarity for consumers and ensure compatibility with modern module systems.
19-23: Dependencies Modernization
The dependency changes stand out:
- Adding
"@kleros/cross-chain-realitio-sdk": "workspace:*"enables tighter integration within the monorepo.- Upgrading
reactandreact-domto "^18.2.0" ensures the project uses the latest stable features.
These modifications are well aligned with the overall modernization strategy.
24-36: DevDependencies Upgrade and Stack Modernization
The revampeddevDependenciesinclude modern tools such as TypeScript, Vite, and Biome alongside updated support for Tailwind CSS. This curated list supports a more efficient and contemporary development workflow. As with thebuildscript, verify that any OS-specific command nuances (like file removal syntax) are addressed if cross-platform compatibility is desired.
38-39: Volta Configuration Update
Bumping the Node version to "20.19.0" ensures compatibility with modern JavaScript features and improved performance. Confirm that the production environment supports this Node version to avoid runtime discrepancies.dynamic-script/package.json (5)
2-12: Package Metadata Modernization and Distribution Clarity
The changes—including updating the package name to "@kleros/cross-chain-realitio-dynamic-script", version bump to "0.5.0", and adding fields likedescription,type,types, andfiles—enhance package clarity and distribution consistency.
Note: As per previous discussions (see past review comments), the package name change does not introduce a breaking change for consumers due to its IPFS CID distribution; this confirms the approach.
13-19: Streamlined Build and Maintenance Scripts
The new scripts for cleaning, building, formatting, linting, and checking are in line with the overall modernization effort. The use of Vite for building the library is a robust choice that should speed up the development process.
20-22: Dependency Consolidation
Including"@kleros/cross-chain-realitio-sdk": "workspace:*"centralizes functionality and reduces version fragmentation across projects. This dependency update is consistent with the restructured monorepo strategy.
23-28: DevDependencies Update for a Modern Tooling Stack
By introducing@biomejs/biome,@types/node,typescript, andvite, this configuration now better supports a modern development workflow. These updates are well targeted to increase build performance and enforce code quality.
29-31: Updated Volta Node Version
Setting the Node version to "20.19.0" in the Volta configuration is a necessary upgrade that aligns with other parts of the monorepo. Ensure that all contributors have this Node version installed to prevent environment mismatches.sdk/src/lib.ts (10)
1-14: Dependencies are well-organized and utility functions are cleanly implemented.The imports clearly separate third-party packages from local code. The utility functions
isNilandisEmptyare concise and follow best practices with proper TypeScript type narrowing.
15-21: Well-structured input parameters interface with proper type definitions.The
RealityQuestionParamsinterface uses appropriate TypeScript template literal types for Ethereum addresses. The interface clearly indicates which parameters are required vs optional.
23-34: Comprehensive data structure for question information.The
RealityQuestionDatainterface provides a well-typed structure for the return data with appropriate typing for Ethereum addresses and proper handling of different question types.
36-51: Good input validation implementation.The function properly validates required inputs before proceeding with blockchain operations. The error message is descriptive and helps users understand what parameters are missing.
92-123: Proper handling of blockchain events and error cases.The code correctly retrieves and validates event logs from the blockchain. The error handling is robust and provides descriptive error messages when expected data is not found.
124-159: Complex block range determination for event queries.Good optimization to start event queries from the contract deployment block rather than block 0. The complex lookup with fallback to 0 ensures the code works even for unknown contract addresses.
160-188: Efficient template resolution with hardcoded values for common cases.The optimization to hardcode the first 5 templates is a good performance improvement to avoid unnecessary blockchain calls.
189-213: Complete data transformation and return structure.The function properly parses and transforms the blockchain data into a structured format that's easier to work with. The return object includes both the processed data and raw values for flexibility.
215-226: Comprehensive meta evidence interface.The
RealityMetaEvidenceinterface provides a clear structure for representing different types of question data with appropriate optionality where needed.
227-281: Type-safe implementation of ruling options based on question type.The function correctly uses TypeScript's
satisfiesoperator to ensure type compatibility while preserving literal types. The switch statement comprehensively handles all question types with appropriate fallback for unknown types.sdk/src/contracts.ts (4)
3-22: Useful block mapping optimization with clear documentation.The
REALITY_STARTS_ATconstant provides an optimization by mapping contract addresses to their deployment blocks. This prevents unnecessary scanning of earlier blocks. The comments for each address clearly identify the network and version.
24-34: Improved type safety with 'satisfies' operator.The ABI definition now uses
as const satisfies Abito ensure type compatibility while preserving the exact structure. The single-line formatting for outputs enhances readability.
36-76: Consistent ABI formatting with proper type assertions.The
foreignProxyAbifollows the same pattern of usingsatisfies Abifor type checking. The consistent single-line formatting for inputs and outputs improves readability.
78-207: Added support for template retrieval with consistent formatting.The
realitioAbihas been enhanced with thetemplatesfunction needed by the SDK implementation. The consistent formatting withsatisfies Abiensures type safety across all ABIs.
Summary by CodeRabbit
New Features
Build & Infrastructure
Quality Improvements