Support for Reality v3.2 #71
Conversation
…fy the version and get the deployment block number
… on the Reality contracts lib instead
WalkthroughThis pull request introduces several documentation and configuration updates for the Realitio contracts and related SDK modules. A new entry for "Chiado default" is added with deployment and ABI details, and associated JSON files and IPFS CIDs are updated. Additionally, new utilities and type definitions are incorporated into the SDK, along with test and script enhancements. Formatting configurations and new files for mock data, policies, and biome settings are also included, while legacy policy functions have been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as RealitioDisplayInterface
participant M as Mock Data
U->>C: Access URL with ?example=true
C->>C: Check URL for "example" flag
C->>M: Load mockQuestionData
M-->>C: Return mock data
C->>U: Render UI with example data (including description)
sequenceDiagram
participant U as User
participant S as reality-contracts.cjs
participant C as RealityETH Contract
U->>S: Execute script
S->>S: Retrieve chain info & config details
S->>C: Create contract client and query contract
C-->>S: Return question result
S->>U: Log chain and query information
Possibly related PRs
Poem
🪧 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: 0
🧹 Nitpick comments (2)
contracts/tasks/generate-metaevidence.js (1)
7-7: Refined Parameter Definition Format
The optional parametertermsOfServiceis now defined on a single line with a default value. This improves readability without altering functionality.sdk/scripts/reality-contracts.cjs (1)
1-85: Well-structured example script for Reality.eth contracts integration.This script demonstrates the proper usage of the @reality.eth/contracts package with viem, showing:
- Chain configuration and token selection
- Contract version feature detection
- Contract initialization and querying
- Using configForAddress to work with specific contract instances
A few suggestions:
- Consider adding error handling for the contract calls
- The hardcoded question_id may become outdated - consider adding a comment explaining its significance or making it configurable
- contractClient.read.resultFor([question_id]).then((result) => { - console.log("The result for question", question_id, "is", result); - }); + // Example of error handling with async/await + try { + const result = await contractClient.read.resultFor([question_id]); + console.log("The result for question", question_id, "is", result); + } catch (error) { + console.error("Error querying question result:", error); + }
📜 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 (12)
contracts/README.md(3 hunks)contracts/deploy/home/gnosis.js(1 hunks)contracts/deployments/chiado/RealitioProxy-v1.3.0.json(1 hunks)contracts/metaevidence-cids.json(1 hunks)contracts/metaevidences/metaevidence-chiado-default.json(1 hunks)contracts/tasks/generate-metaevidence.js(2 hunks)sdk/package.json(1 hunks)sdk/scripts/reality-contracts.cjs(1 hunks)sdk/src/__tests__/lib.test.ts(2 hunks)sdk/src/contracts.ts(2 hunks)sdk/src/lib.ts(7 hunks)sdk/src/types/reality-eth-lib.d.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
contracts/README.md
[typographical] ~51-~51: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0x8FeAB3..EC44b6](https://uniscan.xyz/address/0x8F...
(DOUBLE_PUNCTUATION)
[typographical] ~51-~51: Two consecutive dots
Context: ...40b1593A38a13607d122BEC44b6) | [0x3FB831..48Ac17](https://etherscan.io/address/0x3...
(DOUBLE_PUNCTUATION)
[typographical] ~58-~58: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0xcB4B48..b045a6](https://basescan.org/address/0xc...
(DOUBLE_PUNCTUATION)
[typographical] ~58-~58: Two consecutive dots
Context: ...247A00048963F169d2b4Ab045a6) | [0x87f58F..e2EAf9](https://etherscan.io/address/0x8...
(DOUBLE_PUNCTUATION)
[typographical] ~59-~59: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0xBeeB21..FBe96B](https://basescan.org/address/0xB...
(DOUBLE_PUNCTUATION)
[typographical] ~59-~59: Two consecutive dots
Context: ...2E75992488A66F65b0477FBe96B) | [0x20E1D4..aAe373](https://etherscan.io/address/0x2...
(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...
(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...
(DOUBLE_PUNCTUATION)
[typographical] ~85-~85: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0x87f58F..e2EAf9](https://sepolia.uniscan.xyz/addr...
(DOUBLE_PUNCTUATION)
[typographical] ~85-~85: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0xA42986..fFDF15](https://sepolia.etherscan.io/add...
(DOUBLE_PUNCTUATION)
🔇 Additional comments (25)
sdk/package.json (1)
21-22: Dependency Updates for Reality Integration
The new dependency@reality.eth/contractsat version^3.2.2and the updated version of@reality.eth/reality-eth-libto^3.4.2are in line with supporting Reality v3.2. Please double-check that any SDK modules interacting with these packages are updated to accommodate any API changes.contracts/tasks/generate-metaevidence.js (1)
40-41: Updated IPFS URIs for Metaevidence Assets
TheevidenceDisplayInterfaceURIanddynamicScriptURIvalues have been updated to new IPFS links. Please ensure these new links point to the correct and intended assets, and that they are consistent with the corresponding metaevidence JSON file updates.contracts/deploy/home/gnosis.js (1)
5-7: Updated Chiado Deployment Parameters
Therealitioaddress for the Chiado network has been updated to"0x012fb3aDce7D60672cF634e730927Fa5822b3cAb", and the accompanying comment now references the RealityETH-3.2 deployment JSON. This aligns with the Reality v3.2 support objective. Verify that any contracts or scripts dependent on the previous address are updated accordingly.contracts/metaevidences/metaevidence-chiado-default.json (1)
7-8: Updated Metaevidence Asset URIs
The IPFS links forevidenceDisplayInterfaceURIanddynamicScriptURIhave been modified to the new endpoints. Ensure that these URIs correctly resolve to the intended metaevidence content.contracts/metaevidence-cids.json (1)
7-7: Updated Metaevidence CID
The CID for"metaevidence-chiado-default.json"has been updated to"QmbhQkaQkKDFCejtPYA6AgV1j8NUCPGp2T43bkR7AD33QH". Please confirm that this new CID accurately reflects the content of the updated metaevidence file.sdk/src/types/reality-eth-lib.d.ts (1)
7-24: Good addition of the Reality.eth contracts module type definitions.This properly defines the types for the Reality.eth contracts v3.2 module, including the RealityConfig interface and configForAddress function. The types are well-structured and include all necessary properties.
sdk/src/contracts.ts (2)
186-192: Good addition of the template_hashes function to the ABI.This ABI addition aligns with the Reality v3.2 contract interface, which includes the template_hashes function.
195-208: Well-typed contract exports using viem's GetContractReturnType.These type exports improve type safety when interacting with the contracts, leveraging viem's typing system.
sdk/src/__tests__/lib.test.ts (2)
51-51: Updated mock address to match a real Reality.eth contract address.The new address appears to be a real Sepolia testnet Reality.eth contract address, which makes the tests more realistic.
97-110: Enhanced mock client implementation for more accurate testing.The expanded mock client now includes all required properties of a viem PublicClient, particularly:
- Chain ID (11155111n for Sepolia)
- Transport configuration
- Client methods
This will make the tests more reliable by better simulating the actual client behavior used in production.
sdk/src/lib.ts (9)
2-4: Good addition of configuration imports.The imports from
@reality.eth/contractsprovide a better way to handle configurations and templates, making the code more maintainable and less prone to hardcoded values.
8-9: Well-structured template constants.Creating constants for the different template versions improves code readability and maintainability.
11-21: Good implementation of version-based template selection.The
getDefaultTemplatesfunction provides a clean way to select templates based on contract version. The logging helps with debugging and transparency.
36-42: Appropriate type definition for question types.Creating the
QuestionTypetype improves type safety and centralizes the definition of allowed question types, making future additions easier.
49-49: Good refactoring to use the new type.Updating the
RealityQuestionDatainterface to use the newQuestionTypetype improves type safety throughout the codebase.
106-108: Good implementation of dynamic configuration.Using
configForAddressto get the configuration based on chain ID and address is more flexible than hardcoded values. The logging helps with debugging.
179-182: Well-updated template selection logic.The template selection logic now properly uses the new
getDefaultTemplatesfunction, making it more dynamic and version-aware.
232-232: Good type update in RealityMetaEvidence.The
RealityMetaEvidenceinterface now properly uses the newQuestionTypetype, ensuring type consistency.
286-290: Good addition of hash type support.The addition of the "hash" case in the ruling options switch statement ensures all question types are properly handled.
contracts/README.md (3)
76-76: Well-documented new Chiado default deployment.The addition of the Chiado default entry provides proper documentation for the new deployment. All necessary information is included and properly linked.
🧰 Tools
🪛 LanguageTool
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
51-51: Improved policy URL formatting.The formatting of policy URLs now consistently uses square brackets and quotes, improving readability and maintainability.
Also applies to: 59-59, 85-85
🧰 Tools
🪛 LanguageTool
[typographical] ~51-~51: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0x8FeAB3..EC44b6](https://uniscan.xyz/address/0x8F...(DOUBLE_PUNCTUATION)
[typographical] ~51-~51: Two consecutive dots
Context: ...40b1593A38a13607d122BEC44b6) | [0x3FB831..48Ac17](https://etherscan.io/address/0x3...(DOUBLE_PUNCTUATION)
51-85: Note on address formatting in tables.The static analysis flags consecutive dots in the truncated addresses, but this is the standard format used throughout the file for address abbreviation.
🧰 Tools
🪛 LanguageTool
[typographical] ~51-~51: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0x8FeAB3..EC44b6](https://uniscan.xyz/address/0x8F...(DOUBLE_PUNCTUATION)
[typographical] ~51-~51: Two consecutive dots
Context: ...40b1593A38a13607d122BEC44b6) | [0x3FB831..48Ac17](https://etherscan.io/address/0x3...(DOUBLE_PUNCTUATION)
[typographical] ~58-~58: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0xcB4B48..b045a6](https://basescan.org/address/0xc...(DOUBLE_PUNCTUATION)
[typographical] ~58-~58: Two consecutive dots
Context: ...247A00048963F169d2b4Ab045a6) | [0x87f58F..e2EAf9](https://etherscan.io/address/0x8...(DOUBLE_PUNCTUATION)
[typographical] ~59-~59: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0xBeeB21..FBe96B](https://basescan.org/address/0xB...(DOUBLE_PUNCTUATION)
[typographical] ~59-~59: Two consecutive dots
Context: ...2E75992488A66F65b0477FBe96B) | [0x20E1D4..aAe373](https://etherscan.io/address/0x2...(DOUBLE_PUNCTUATION)
[typographical] ~66-~66: Two consecutive dots
Context: ...ealitioProxy-v1.1.0.json#L6) | [0x5AFa42..86F90e](https://polygonscan.com/address/...(DOUBLE_PUNCTUATION)
[typographical] ~66-~66: Two consecutive dots
Context: ...137e10f89dfb5EF1542a186F90e) | [0x776e58..231e52](https://etherscan.io/address/0x7...(DOUBLE_PUNCTUATION)
[typographical] ~67-~67: Two consecutive dots
Context: ...alitioProxy-v1.1.0.json#L29) | [0x68154E..7491Dc](https://polygonscan.com/address/...(DOUBLE_PUNCTUATION)
[typographical] ~67-~67: Two consecutive dots
Context: ...BF582b80Dd6453FA401737491Dc) | [0x68c4cc..bAFE28](https://etherscan.io/address/0x6...(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
[typographical] ~77-~77: Two consecutive dots
Context: ...ealitioProxy-v1.1.0.json#L6) | [0xE62094..FffeD7](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~77-~77: Two consecutive dots
Context: ...02aa625BBB4669fE317c9FffeD7) | [0x5d7cB7..969D42](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
[typographical] ~84-~84: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x122D6B..b7954D](https://sepolia.uniscan.xyz/addr...(DOUBLE_PUNCTUATION)
[typographical] ~84-~84: Two consecutive dots
Context: ...bF4e9314fD00259b1dc1Db7954D) | [0x4C10F0..26191D](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
[typographical] ~85-~85: Two consecutive dots
Context: ...alitioProxy-v1.3.0.json#L29) | [0x87f58F..e2EAf9](https://sepolia.uniscan.xyz/addr...(DOUBLE_PUNCTUATION)
[typographical] ~85-~85: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0xA42986..fFDF15](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
contracts/deployments/chiado/RealitioProxy-v1.3.0.json (3)
1-4: Appropriate version information.Version information and notes clearly indicate the purpose of this deployment (Reality v3.2.0) which aligns with the PR objectives.
5-28: Well-structured deployment configuration.The deployment configuration for "Chiado default" is well-structured with:
- Clear Reality contract specifications
- Complete home and foreign proxy details
- All necessary network addresses and transaction hashes
- Proper references to TOS and meta evidence
This information enables easy verification and interaction with the deployed contracts.
29-1539: Complete ABI definitions.The home and foreign proxy ABIs are thoroughly documented, providing all necessary interface details for contract interactions, including:
- Event definitions
- Function signatures
- Parameter and return types
- Visibility modifiers
This comprehensive ABI documentation ensures proper integration with the SDK.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
.vscode/settings.json (1)
7-10: Trailing Comma in "editor.codeActionsOnSave" BlockThe addition of the trailing comma (as indicated on line 10) helps maintain consistency and makes the block more resilient to future modifications. This style is acceptable in VS Code’s JSONC files.
evidence-display/src/index.css (1)
17-26: Consider avoiding!importantin CSS rulesWhile the new Reality.eth link styles look good, using
!importantflags (lines 19 and 25) can lead to specificity wars and make future CSS maintenance more difficult. Consider increasing the selector specificity instead.Also, ensure the link colors (#2093ff and #1a75cc) have sufficient contrast against your background colors for accessibility.
.reality-link { - color: #2093ff !important; + color: #2093ff; text-decoration: underline; transition: color 0.2s ease; } .reality-link:hover { - color: #1a75cc !important; + color: #1a75cc; }sdk/src/__tests__/lib.v3.2.test.ts (1)
32-34: Consider removing or relocating the module reset logic.Calling
vi.resetModules()andvi.clearAllMocks()directly here might cause unexpected side effects for other tests or mocks. If your intention is to isolate the environment for these tests only, consider using a dedicated test setup function or a global setup/teardown mechanism to avoid accidental interference with other suites.sdk/src/lib.ts (2)
8-9: Consider consolidating template definitions for future versions.Having separate arrays for
DEFAULT_TEMPLATES_V3_0andDEFAULT_TEMPLATES_V3_2works for now. However, if more versions emerge, you might consider a single unified map keyed by version for easier maintainability.
190-191: Handle JSON parsing failures gracefully.The code unconditionally parses
populatedJSONwithout a try/catch. If the underlying data is somehow corrupted or contains unexpected values, it will throw an uncaught error. Consider wrapping in a try/catch to provide clearer error handling and logging.sdk/src/__tests__/test-helpers.ts (3)
4-18: Consider improving type safety in mock transport implementation.The mock transport implementation uses a type assertion (
as unknown as Transport) which bypasses TypeScript's type checking. While this is a common pattern for mocks, it could hide potential type incompatibilities.Consider implementing the Transport interface more explicitly or using a library like
jest-mock-extendedthat provides type-safe mocks:- export function createMockTransport(): Transport { - return { - type: "mock", - request: async () => ({}), - config: {}, - // Add required Transport methods - /* eslint-disable @typescript-eslint/no-unused-vars */ - retryCount: 0, - retryDelay: 0, - timeout: 0, - key: "mock", - name: "Mock Transport", - /* eslint-enable @typescript-eslint/no-unused-vars */ - } as unknown as Transport; - } + /** + * Creates a mock transport object for testing + * @returns A mock Transport implementation + */ + export function createMockTransport(): Transport { + const mockTransport: Partial<Transport> = { + type: "mock", + request: async () => ({}), + config: {}, + retryCount: 0, + retryDelay: 0, + timeout: 0, + key: "mock", + name: "Mock Transport", + }; + return mockTransport as Transport; + }
20-35: Add test scenarios for request failure handling.The mock client implementation doesn't include scenarios for failed requests, which could be important for testing error handling paths.
Consider enhancing the mock client to support simulating errors:
+ /** + * Creates a mock public client for testing + * @param mockTransport The mock transport to use + * @returns A mock PublicClient implementation + */ export function createMockClient(mockTransport: Transport): PublicClient { return { account: undefined, batch: undefined, cacheTime: 0, chain: null, key: "mock", name: "Mock Client", pollingInterval: 4000, - request: vi.fn(), + request: vi.fn().mockImplementation(({ method }) => { + // Allow tests to simulate failures for specific methods + if (method === 'eth_simulateFailure') { + return Promise.reject(new Error('Simulated failure')); + } + return Promise.resolve({}); + }), transport: mockTransport, type: "publicClient", uid: "mock", getChainId: vi.fn().mockResolvedValue(11155111n), // Sepolia chain ID } as unknown as PublicClient; }
37-85: Add JSDoc documentation for the contract mocking function.The
createMockContractsfunction lacks documentation explaining its purpose and parameter usage, which would be helpful for other developers.+ /** + * Creates mock contract objects for testing Reality.eth integration + * + * @param mockQuestionID The question ID to use in mock responses + * @param mockHomeProxyAddress The home proxy address to return + * @param mockRealitioAddress The Realitio contract address to return + * @param mockTemplateID The template ID to use in mock responses + * @param mockQuestion The question content to use in mock responses + * @returns Object containing mock contracts for foreignProxy, homeProxy, and realitio + */ export function createMockContracts( mockQuestionID: string, mockHomeProxyAddress: string, mockRealitioAddress: string, mockTemplateID: bigint, mockQuestion: string ) {Additionally, consider making the mock event data more configurable to support different test scenarios:
const mockForeignProxy = { read: { homeProxy: vi.fn().mockResolvedValue(mockHomeProxyAddress), arbitrationCreatedBlock: vi.fn().mockResolvedValue(12345n), }, getEvents: { - ArbitrationCreated: vi.fn().mockResolvedValue([ - { - args: { - _disputeID: 114n, - _questionID: mockQuestionID, - }, - }, - ]), + ArbitrationCreated: vi.fn().mockImplementation((options = {}) => { + return Promise.resolve([ + { + args: { + _disputeID: options.disputeID || 114n, + _questionID: mockQuestionID, + }, + }, + ]); + }), }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (10)
.vscode/settings.json(1 hunks)biome.json(1 hunks)evidence-display/package.json(2 hunks)evidence-display/src/containers/realitio.tsx(3 hunks)evidence-display/src/index.css(1 hunks)evidence-display/src/mocks/question.ts(1 hunks)sdk/src/__tests__/lib.v3.0.test.ts(3 hunks)sdk/src/__tests__/lib.v3.2.test.ts(1 hunks)sdk/src/__tests__/test-helpers.ts(1 hunks)sdk/src/lib.ts(9 hunks)
✅ Files skipped from review due to trivial changes (2)
- biome.json
- evidence-display/src/mocks/question.ts
🧰 Additional context used
🧬 Code Definitions (4)
evidence-display/src/containers/realitio.tsx (1)
evidence-display/src/mocks/question.ts (1)
mockQuestionData(3-13)
sdk/src/__tests__/lib.v3.2.test.ts (2)
sdk/src/__tests__/test-helpers.ts (3)
createMockTransport(4-18)createMockClient(20-35)createMockContracts(37-85)sdk/src/lib.ts (1)
fetchRealityQuestionData(52-201)
sdk/src/lib.ts (1)
contracts/tasks/update-deployments.js (1)
realitioAddress(85-85)
sdk/src/__tests__/lib.v3.0.test.ts (1)
sdk/src/__tests__/test-helpers.ts (3)
createMockTransport(4-18)createMockClient(20-35)createMockContracts(37-85)
🔇 Additional comments (10)
.vscode/settings.json (1)
11-13: TypeScript Formatter Configuration AddedA new configuration block for TypeScript (lines 11–13) ensures that TypeScript files explicitly use
"biomejs.biome"as the default formatter. This aligns well with the overall project updates and reinforces consistency in formatting across the codebase.evidence-display/package.json (1)
17-18: LGTM: Good addition for developer experienceThe new "example" script provides a convenient way to launch the application in example mode with a test chain ID. This aligns well with the mock data implementation in the realitio.tsx file.
evidence-display/src/containers/realitio.tsx (3)
14-20: LGTM: Well-implemented example modeThe example mode implementation is clean and straightforward. It checks for the "example" URL parameter early in the fetch process and short-circuits appropriately when enabled.
81-81: Verify markdown rendering for descriptionThe description field in
mockQuestionDatacontains markdown formatting ([here](https://www.google.com)), but there's no indication that markdown is being parsed. The current implementation will likely display raw markdown syntax to users.Consider implementing a markdown parser for the description field. A simple solution would be to use a library like
react-markdown:import { fetchRealityQuestionData } from "@kleros/cross-chain-realitio-sdk"; import type { RealityQuestionData } from "@kleros/cross-chain-realitio-sdk"; import { useEffect, useState } from "react"; +import ReactMarkdown from 'react-markdown' import RealityLogo from "../assets/images/reality_eth_logo.png"; import { mockQuestionData } from "../mocks/question"; // ... <div className="my-4 text-xl leading-relaxed break-words">{questionData.title}</div> -<div className="my-4 text-lg leading-relaxed break-words">{questionData.description}</div> +<div className="my-4 text-lg leading-relaxed break-words"> + <ReactMarkdown>{questionData.description}</ReactMarkdown> +</div>
83-83: LGTM: Consistent link stylingGood use of the new
reality-linkclass, which ensures consistent styling for Reality.eth links.sdk/src/__tests__/lib.v3.2.test.ts (1)
57-122: Looks good; consider adding additional negative-path tests.This test case ensures correct contract interactions and data formatting. However, you might add negative-path coverage for scenarios where the arbitration logs are found but corrupted, or where the question event fails to match the expected template. Extending negative-path testing further strengthens the overall test reliability.
sdk/src/__tests__/lib.v3.0.test.ts (2)
15-18: Mocking RealitioQuestion for v3.0 is consistent with the 3.2 approach.Defining a separate mock for version 3.0 supports a clear distinction between protocol versions. This improves maintainability for future upgrades or additional test scenarios.
31-54: Unified mock creation pattern improves readability.Adopting the
createMockTransport,createMockClient, andcreateMockContractshelpers significantly reduces boilerplate. The test configuration is clearer, and updating mock behaviors in one place benefits all related test files.sdk/src/lib.ts (1)
262-266: Congrats on extending support for "hash" question type.Providing a dedicated branch for
"hash"ensures that new question types are cleanly integrated into the existing logic. The added coverage is clear and aligns well with the other standard question types.sdk/src/__tests__/test-helpers.ts (1)
1-3: LGTM! Appropriate imports for test mocking.The imports provide the necessary types from "viem" and mocking functionality from "vitest" needed for the helper functions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/README.md (1)
76-76: Verify Abbreviation Formatting for Contract AddressesThe contract addresses are abbreviated using two consecutive dots (e.g., "0x87f58F..e2EAf9" and "0x781Bfb..904aE0"). If this is an intentional stylistic choice consistent with your other entries, it is acceptable. Otherwise, consider standardizing the format (for example, using an ellipsis character "…" or a single pair of dots) to improve readability and avoid potential typographical concerns.
🧰 Tools
🪛 LanguageTool
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
📜 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 (7)
.vscode/settings.json(1 hunks)contracts/README.md(1 hunks)contracts/deploy/shared/consts.js(1 hunks)contracts/deploy/shared/utils.js(2 hunks)contracts/policies.json(1 hunks)contracts/scripts/generateDeploymentsMarkdown.sh(1 hunks)evidence-display/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- contracts/policies.json
🚧 Files skipped from review as they are similar to previous changes (2)
- evidence-display/package.json
- .vscode/settings.json
🧰 Additional context used
🧬 Code Definitions (1)
contracts/deploy/shared/utils.js (4)
contracts/deploy/shared/consts.js (1)
require(2-2)contracts/deploy/home/optimism.js (2)
require(1-1)require(2-2)contracts/deploy/home/arbitrum.js (2)
require(1-1)require(2-2)contracts/deploy/home/polygon.js (3)
require(1-1)require(2-2)require(3-3)
🪛 LanguageTool
contracts/README.md
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...
(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...
(DOUBLE_PUNCTUATION)
🔇 Additional comments (8)
contracts/README.md (1)
72-77: New Chiado Deployment Entry ValidityThe new "Chiado default" entry under the Testnets section appears to be correctly added with version v1.3.0. Please double-check that all addresses and links (e.g., the home proxy, foreign proxy, and policy) are accurate and consistent with your deployment configurations for Reality v3.2.
🧰 Tools
🪛 LanguageTool
[typographical] ~76-~76: Two consecutive dots
Context: ...ealitioProxy-v1.3.0.json#L6) | [0x87f58F..e2EAf9](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~76-~76: Two consecutive dots
Context: ...9BA2F3eB0604e5c335893e2EAf9) | [0x781Bfb..904aE0](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
[typographical] ~77-~77: Two consecutive dots
Context: ...ealitioProxy-v1.1.0.json#L6) | [0xE62094..FffeD7](https://gnosis-chiado.blockscout...(DOUBLE_PUNCTUATION)
[typographical] ~77-~77: Two consecutive dots
Context: ...02aa625BBB4669fE317c9FffeD7) | [0x5d7cB7..969D42](https://sepolia.etherscan.io/add...(DOUBLE_PUNCTUATION)
contracts/scripts/generateDeploymentsMarkdown.sh (1)
18-19: Improved policy name retrieval logicThe previous implementation has been replaced with a more structured approach using
jqto parse the policies JSON file. This is a good improvement that makes the code more maintainable and less error-prone.contracts/deploy/shared/utils.js (4)
9-17: Well-designed policy retrieval with helpful error handlingThe
getPolicyfunction provides a robust way to retrieve policy paths with descriptive error messages that include all valid policy options. This makes the API more user-friendly.
19-22: Clear URI generation with informative commentThe function creates standardized URIs for policies with the appropriate multiaddr format. The comment about Kleros' use of multiformats is helpful for maintainers.
24-28: Well-implemented metadata generationThis function creates properly formatted metadata with the correct URI format for Reality. The distinction between URI formats for different systems (Kleros vs Reality) is well-documented in the comments.
53-55: Appropriate module exportsAll new functions are properly exported, making them available for use throughout the codebase.
contracts/deploy/shared/consts.js (2)
2-2: Good refactoring: Import generateMetadata from utilsImporting the function from utils instead of implementing it inline is a good refactoring that improves code organization and follows the DRY principle.
27-27: Consistent metadata generationThe metadata is still generated with the "default" policy, maintaining the same behavior as before the refactoring.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
dynamic-script/src/test.html (5)
4-4: Nit:typeattribute is redundant
In HTML5,<script>defaults to JavaScript. You can simplify the tag to<script>to reduce verbosity.
5-5: Optional: simplify URL param parsing
URLSearchParamswill decode percent-encoded values automatically, so wrapping the query string indecodeURIComponentis unnecessary and could lead to double-decoding edge cases. Consider:- const message = Object.fromEntries(new URLSearchParams(decodeURIComponent(window.location.search.substring(1)))); + const message = Object.fromEntries(new URLSearchParams(window.location.search.substring(1)));
7-16: Consider validating and defaulting destructured params
Right now, every property onmessagemay beundefined, leading to runtime errors later. You might:
- Supply defaults:
const { disputeID = null, jsonRpcUrl = '' } = message;- Narrow the destructuring to only the parameters you actually need.
18-19: Renamecidto avoid ambiguity
The variablecidcurrently holds a chain identifier, which can be confusing since “CID” often refers to IPFS content IDs. Consider renaming tochainIdortargetChainId.
21-24: Enhance error handling for missing params
Throwing anErrorstops script execution, but the user sees only a console error. For a more user-friendly test harness, consider writing an error message into the DOM (e.g., into the#containerelement) before throwing.
📜 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 (6)
dynamic-script/package.json(2 hunks)dynamic-script/src/test.html(1 hunks)dynamic-script/tsconfig.json(2 hunks)dynamic-script/vite.config.ts(1 hunks)evidence-display/package.json(2 hunks)sdk/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- dynamic-script/package.json
- dynamic-script/vite.config.ts
- sdk/tsconfig.json
- dynamic-script/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- evidence-display/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
dynamic-script/src/test.html (1)
3-3: External script inclusion looks good
The relative path to../dist/index.iife.jsis correct for a local test harness. Ensure that the bundle is up-to-date after every build.
| const scriptParameters = { | ||
| arbitrableContractAddress, | ||
| disputeID, | ||
| arbitratorChainID, | ||
| arbitrableChainID, | ||
| chainID, | ||
| arbitratorJsonRpcUrl, | ||
| arbitrableJsonRpcUrl, | ||
| jsonRpcUrl, | ||
| } | ||
|
|
||
| let resolveScript | ||
| let rejectScript | ||
| const returnPromise = new Promise((resolve, reject) => { | ||
| resolveScript = resolve | ||
| rejectScript = reject | ||
| let resolveScript | ||
| let rejectScript | ||
| const returnPromise = new Promise((resolve, reject) => { | ||
| resolveScript = resolve | ||
| rejectScript = reject | ||
| }) | ||
|
|
||
| const evaluator = () => { | ||
| getMetaEvidence() | ||
| } | ||
| evaluator() | ||
|
|
There was a problem hiding this comment.
Critical: scriptParameters and promise callbacks are never passed to the evaluator
You build scriptParameters and capture resolveScript/rejectScript, but getMetaEvidence() is invoked with no arguments. Unless the dynamic script reads globals by side effect, nothing will ever resolve or reject returnPromise.
Please update the evaluator call to pass in both the parameters and the callbacks. For example:
- const evaluator = () => {
- getMetaEvidence()
- }
+ const evaluator = () => {
+ getMetaEvidence(scriptParameters, resolveScript, rejectScript)
+ }This ensures the dynamic script has the context and can signal back via resolveScript/rejectScript.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const scriptParameters = { | |
| arbitrableContractAddress, | |
| disputeID, | |
| arbitratorChainID, | |
| arbitrableChainID, | |
| chainID, | |
| arbitratorJsonRpcUrl, | |
| arbitrableJsonRpcUrl, | |
| jsonRpcUrl, | |
| } | |
| let resolveScript | |
| let rejectScript | |
| const returnPromise = new Promise((resolve, reject) => { | |
| resolveScript = resolve | |
| rejectScript = reject | |
| let resolveScript | |
| let rejectScript | |
| const returnPromise = new Promise((resolve, reject) => { | |
| resolveScript = resolve | |
| rejectScript = reject | |
| }) | |
| const evaluator = () => { | |
| getMetaEvidence() | |
| } | |
| evaluator() | |
| const scriptParameters = { | |
| arbitrableContractAddress, | |
| disputeID, | |
| arbitratorChainID, | |
| arbitrableChainID, | |
| chainID, | |
| arbitratorJsonRpcUrl, | |
| arbitrableJsonRpcUrl, | |
| jsonRpcUrl, | |
| } | |
| let resolveScript | |
| let rejectScript | |
| const returnPromise = new Promise((resolve, reject) => { | |
| resolveScript = resolve | |
| rejectScript = reject | |
| }) | |
| const evaluator = () => { | |
| getMetaEvidence(scriptParameters, resolveScript, rejectScript) | |
| } | |
| evaluator() |
| <p id="container" style="white-space: pre;" /> | ||
| </div> |
There was a problem hiding this comment.
Fix invalid self-closing <p> tag
HTML does not support self-closing <p> elements. Change:
- <p id="container" style="white-space: pre;" />
+ <p id="container" style="white-space: pre;"></p>Without this fix, browsers may auto-close the tag in unexpected ways and your appended text won’t appear.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p id="container" style="white-space: pre;" /> | |
| </div> | |
| <p id="container" style="white-space: pre;"></p> | |
| </div> |
| returnPromise | ||
| .then(result => { | ||
| console.log(result); | ||
| const element = document.getElementById("container"); | ||
| element.appendChild( | ||
| document.createTextNode(JSON.stringify(result, null, 2)) | ||
| ); | ||
| }) | ||
| .catch(error => { console.error('Error:', error) }); | ||
| </script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against missing container and display errors in UI
If document.getElementById("container") returns null, appendChild will throw. Also, errors caught in the catch won’t surface in the page. Consider:
returnPromise
.then(result => {
- const element = document.getElementById("container");
- element.appendChild(
- document.createTextNode(JSON.stringify(result, null, 2))
- );
+ const element = document.getElementById("container");
+ if (element) {
+ element.textContent = JSON.stringify(result, null, 2)
+ } else {
+ console.warn("Container element not found; result:", result)
+ }
})
.catch(error => {
- console.error('Error:', error)
+ console.error('Error:', error)
+ const element = document.getElementById("container");
+ if (element) {
+ element.textContent = `Error: ${error.message || error}`
+ }
});This both prevents uncaught exceptions on missing elements and surfaces errors in the test UI.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| returnPromise | |
| .then(result => { | |
| console.log(result); | |
| const element = document.getElementById("container"); | |
| element.appendChild( | |
| document.createTextNode(JSON.stringify(result, null, 2)) | |
| ); | |
| }) | |
| .catch(error => { console.error('Error:', error) }); | |
| </script> | |
| returnPromise | |
| .then(result => { | |
| console.log(result); | |
| const element = document.getElementById("container"); | |
| if (element) { | |
| element.textContent = JSON.stringify(result, null, 2) | |
| } else { | |
| console.warn("Container element not found; result:", result) | |
| } | |
| }) | |
| .catch(error => { | |
| console.error('Error:', error) | |
| const element = document.getElementById("container"); | |
| if (element) { | |
| element.textContent = `Error: ${error.message || error}` | |
| } | |
| }); | |
| </script> |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
sdk/src/lib.ts (1)
163-170:⚠️ Potential issue
realitio.read.templates()does NOT return the block number – resulting query is wrongThe public getter
templates(uint256)returns the template string stored at that index, not the block at which it was created.
You then treat the returned string as a block number and use it infromBlock/toBlock, producing an invalid range and sporadic failures.-const templateCreationBlock = await realitio.read.templates([template_id]); -const templateEventLog = await realitio.getEvents.LogNewTemplate( - { - template_id: template_id, - }, - { fromBlock: templateCreationBlock, toBlock: templateCreationBlock } -); +const templateEventLog = await realitio.getEvents.LogNewTemplate( + { template_id }, + { fromBlock: BigInt(realitioConfig.block), toBlock: "latest" } +);If you want an exact
fromBlock, you can fetchLogNewTemplateonce, extract the block number from the returned log, and cache it instead.
As written, the current logic is functionally incorrect.
🧹 Nitpick comments (1)
sdk/src/lib.ts (1)
63-94: Reduce noisyconsole.logstatements or wrap them in a proper loggerThe function emits ~20 console lines per call, which quickly becomes unusable in production or test output.
Consider:
- Injecting a
loggerinstance and using levels (debug/info/warn) instead of bareconsole.log.- Replacing long JSON dumps with
logger.debuggated behind an environment flag.This keeps the helpful traces without flooding stdout in normal operation.
| let templateText: string; | ||
| if (template_id < 5n) { | ||
| // first 5 templates are part of reality.eth spec, hardcode for faster loading | ||
| templateText = [ | ||
| '{"title": "%s", "type": "bool", "category": "%s", "lang": "%s"}', | ||
| '{"title": "%s", "type": "uint", "decimals": 18, "category": "%s", "lang": "%s"}', | ||
| '{"title": "%s", "type": "single-select", "outcomes": [%s], "category": "%s", "lang": "%s"}', | ||
| '{"title": "%s", "type": "multiple-select", "outcomes": [%s], "category": "%s", "lang": "%s"}', | ||
| '{"title": "%s", "type": "datetime", "category": "%s", "lang": "%s"}', | ||
| ][Number(template_id)]; | ||
| const defaultTemplates = await getDefaultTemplates(realitioConfig); | ||
| if (template_id < defaultTemplates.length) { | ||
| templateText = defaultTemplates[Number(template_id)]; | ||
| } else { |
There was a problem hiding this comment.
BigInt vs number comparison breaks under --strict and may overflow
template_id comes from an on-chain event and is typed/returned as bigint, whereas defaultTemplates.length is a number.
Direct comparison (<) between the two is a type error and will not compile in strict mode.
Casting with Number(template_id) later also silently loses precision for ids larger than Number.MAX_SAFE_INTEGER.
- if (template_id < defaultTemplates.length) {
- templateText = defaultTemplates[Number(template_id)];
+ // Safe comparison by converting the array length to BigInt.
+ if (template_id < BigInt(defaultTemplates.length)) {
+ templateText = defaultTemplates[Number(template_id)];
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let templateText: string; | |
| if (template_id < 5n) { | |
| // first 5 templates are part of reality.eth spec, hardcode for faster loading | |
| templateText = [ | |
| '{"title": "%s", "type": "bool", "category": "%s", "lang": "%s"}', | |
| '{"title": "%s", "type": "uint", "decimals": 18, "category": "%s", "lang": "%s"}', | |
| '{"title": "%s", "type": "single-select", "outcomes": [%s], "category": "%s", "lang": "%s"}', | |
| '{"title": "%s", "type": "multiple-select", "outcomes": [%s], "category": "%s", "lang": "%s"}', | |
| '{"title": "%s", "type": "datetime", "category": "%s", "lang": "%s"}', | |
| ][Number(template_id)]; | |
| const defaultTemplates = await getDefaultTemplates(realitioConfig); | |
| if (template_id < defaultTemplates.length) { | |
| templateText = defaultTemplates[Number(template_id)]; | |
| } else { | |
| let templateText: string; | |
| const defaultTemplates = await getDefaultTemplates(realitioConfig); | |
| // Safe comparison by converting the array length to BigInt. | |
| if (template_id < BigInt(defaultTemplates.length)) { | |
| templateText = defaultTemplates[Number(template_id)]; | |
| } else { |
| const realitioChainId = await homeClient.getChainId(); | ||
| const realitioConfig = configForAddress(realitioAddress, realitioChainId); | ||
| console.log("Realitio config:", realitioConfig); | ||
|
|
||
| console.log("Getting realitio contract..."); |
There was a problem hiding this comment.
Guard against undefined Reality config before dereferencing
configForAddress can legitimately return undefined when the address/chain-id pair is not present in the embedded database.
realitioConfig is dereferenced two lines below (realitioConfig.block) and later passed to getDefaultTemplates, so the function will crash with a cryptic Cannot read properties of undefined instead of an actionable error.
-const realitioConfig = configForAddress(realitioAddress, realitioChainId);
-console.log("Realitio config:", realitioConfig);
+const realitioConfig = configForAddress(realitioAddress, realitioChainId);
+if (!realitioConfig) {
+ throw new Error(
+ `No Reality config found for address ${realitioAddress} on chain ${realitioChainId}. ` +
+ "Either the contract is unsupported or the config package is out of date."
+ );
+}
+console.log("Realitio config:", realitioConfig);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const realitioChainId = await homeClient.getChainId(); | |
| const realitioConfig = configForAddress(realitioAddress, realitioChainId); | |
| console.log("Realitio config:", realitioConfig); | |
| console.log("Getting realitio contract..."); | |
| const realitioChainId = await homeClient.getChainId(); | |
| const realitioConfig = configForAddress(realitioAddress, realitioChainId); | |
| if (!realitioConfig) { | |
| throw new Error( | |
| `No Reality config found for address ${realitioAddress} on chain ${realitioChainId}. ` + | |
| "Either the contract is unsupported or the config package is out of date." | |
| ); | |
| } | |
| console.log("Realitio config:", realitioConfig); | |
| console.log("Getting realitio contract..."); |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
sdk/package.json(1 hunks)sdk/src/__tests__/lib.integration.test.ts(1 hunks)sdk/src/__tests__/lib.v3.0.test.ts(4 hunks)sdk/src/__tests__/lib.v3.2.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/package.json
- sdk/src/tests/lib.v3.2.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
sdk/src/__tests__/lib.v3.0.test.ts (1)
sdk/src/__tests__/test-helpers.ts (3)
createMockTransport(4-18)createMockClient(20-35)createMockContracts(37-85)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
sdk/src/__tests__/lib.integration.test.ts (1)
1-5: Good security practice using environment variablesUsing dotenv to load and manage environment variables for API keys is a good security practice. This replaces hardcoded keys with environment-based configuration, reducing security risks.
sdk/src/__tests__/lib.v3.0.test.ts (6)
2-9: Good modularization of test utilitiesThe change to use
dotenvfor environment variables and importing helper functions fromtest-helpers.tsimproves code organization and maintainability. The explicit type imports fromviemandvitestenhance type safety.
18-22: Improved mock implementation with clear parameter namingThe update to use underscore-prefixed parameters in the mock implementation follows good naming conventions for unused or ignored parameters, making the code more readable.
34-34: Clear test suite naming with version specificationAdding the version identifier ("v3.0") to the test suite name improves clarity and helps distinguish between different version-specific test suites. This is particularly valuable as the PR is adding support for Reality v3.2.
37-37: Enhanced type safety with explicit type castingThe explicit type casting of
http as unknown as Mockimproves type safety and makes the intent clearer.
49-58: Improved test setup with mock helper functionsReplacing manual mock creation with helper functions from
test-helpers.tsreduces code duplication and improves maintainability. The use of these functions makes the test setup more consistent and easier to understand.
59-65: Improved contract mocking logicThe updated implementation for mocking
getContractusing the ABI property is more robust than before. This approach correctly differentiates between contract types based on their ABI.
| 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 () => { | ||
| try { | ||
| const result = await fetchRealityQuestionData({ | ||
| disputeID: "114", | ||
| arbitrableContractAddress: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", | ||
| arbitratorJsonRpcUrl: "https://1rpc.io/sepolia", | ||
| arbitratorJsonRpcUrl: `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add environment variable validation
The code directly uses the environment variable without checking if it exists. If ALCHEMY_API_KEY is undefined, this will result in an invalid URL.
- arbitratorJsonRpcUrl: `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`,
+ arbitratorJsonRpcUrl: process.env.ALCHEMY_API_KEY
+ ? `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`
+ : "https://eth-sepolia.g.alchemy.com/v2/demo", // Fallback to demo key or throw a clear error| @@ -96,7 +68,7 @@ describe("fetchRealityQuestionData", () => { | |||
| const result = await fetchRealityQuestionData({ | |||
| disputeID: "114", | |||
| arbitrableContractAddress: "0x807f4D900E0c5B63Ed87a5C97f2B3482d82649eE", | |||
| arbitratorJsonRpcUrl: "https://1rpc.io/sepolia", | |||
| arbitratorJsonRpcUrl: `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add environment variable validation
Similar to the integration test, this code directly uses the environment variable without checking if it exists. If ALCHEMY_API_KEY is undefined, this would result in an invalid URL.
- arbitratorJsonRpcUrl: `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`,
+ arbitratorJsonRpcUrl: process.env.ALCHEMY_API_KEY
+ ? `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`
+ : "https://eth-sepolia.g.alchemy.com/v2/demo", // Fallback to demo key or throw a clear errorAdditionally, since this is a unit test using mocks, consider if a real API key is actually needed here at all - the transport is mocked and no real requests should be made during this test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| arbitratorJsonRpcUrl: `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}`, | |
| arbitratorJsonRpcUrl: process.env.ALCHEMY_API_KEY | |
| ? `https://eth-sepolia.g.alchemy.com/v2/${process.env.ALCHEMY_API_KEY}` | |
| : "https://eth-sepolia.g.alchemy.com/v2/demo", // Fallback to demo key or throw a clear error |
Summary by CodeRabbit