Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/dispute dates and hashes link #1816

Merged
merged 9 commits into from
Jan 7, 2025
Merged

Conversation

Harman-singh-waraich
Copy link
Contributor

@Harman-singh-waraich Harman-singh-waraich commented Dec 23, 2024

  • Adds txn link for dispute creation
  • Adds timestamp and txn link for Vote / justification
  • Adds txn link for dispute Ruling transaction

PR-Codex overview

This PR primarily focuses on enhancing the dispute and voting history functionalities by adding transaction-related fields and improving the user interface for displaying transaction information.

Detailed summary

  • Added rulingTimestamp and rulingTransactionHash to Dispute and ClassicJustification.
  • Introduced transactionHash and timestamp to voting history and justification.
  • Updated the getTxnExplorerLink utility for transaction links.
  • Modified components to display transaction details and links.
  • Updated subgraph version in package.json to 0.10.1.
  • Adjusted various components to utilize new transaction data and improve UI interactions.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced dispute and justification tracking with new fields for transaction hashes and timestamps.
    • Improved transaction explorer link generation across various components.
  • Bug Fixes

    • Updated Ethereum addresses and start blocks for several modules to ensure accurate data sourcing.
  • Documentation

    • Updated GraphQL queries to return additional fields for disputes and voting history.
  • Chores

    • Updated package version for @kleros/kleros-v2-subgraph.

Copy link
Contributor

coderabbitai bot commented Dec 23, 2024

Walkthrough

This pull request introduces comprehensive updates across the Kleros subgraph and web application, focusing on enhancing transaction and timestamp tracking for disputes, evidence, and voting processes. The changes span multiple files in the subgraph and web components, adding new fields like transactionHash and rulingTimestamp to various entities. These modifications improve data granularity by capturing more detailed information about blockchain transactions and events related to dispute resolution.

Changes

File Change Summary
subgraph/core-neo/subgraph.yaml Updated Ethereum data sources with new addresses and start blocks for KlerosCore, PolicyRegistry, DisputeKitClassic, EvidenceModule, and SortitionModule.
subgraph/core/schema.graphql Added transactionHash and rulingTimestamp fields to Dispute and ClassicJustification types.
subgraph/core/src/DisputeKitClassic.ts, subgraph/core/src/KlerosCore.ts, subgraph/core/src/entities/Dispute.ts Introduced transactionHash and timestamp/rulingTimestamp properties to respective entities.
subgraph/package.json Version bumped from 0.9.1 to 0.10.1.
web/src/components/EvidenceCard.tsx, web/src/components/TxnHash.tsx, web/src/components/Verdict/DisputeTimeline.tsx, web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx Updated various components to support transaction hash links and display.
web/src/hooks/queries/useDisputeDetailsQuery.ts, web/src/hooks/queries/useVotingHistory.ts Modified GraphQL queries to include new transaction and timestamp fields.
web/src/utils/getDrawnJurorsWithCount.ts, web/src/utils/index.ts Added getTxnExplorerLink utility function and updated type definitions.

Possibly related PRs

Suggested Labels

Type: Feature🗿, Package: Web, Type: Enhancement :sparkles:

Suggested Reviewers

  • alcercu
  • kemuru

Poem

🐰 Hop, hop, through the blockchain's maze,
Transactions dance in digital rays,
Timestamps whisper their secret tale,
Each hash a story without fail,
Code rabbits track with joyful glee! 🚀


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b01f6f0 and 65e2ec1.

📒 Files selected for processing (2)
  • web/src/components/EvidenceCard.tsx (2 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/src/components/EvidenceCard.tsx
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Dec 23, 2024

Deploy Preview for kleros-v2-neo ready!

Name Link
🔨 Latest commit 65e2ec1
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-neo/deploys/676ab2a916c1550008cb0c04
😎 Deploy Preview https://deploy-preview-1816--kleros-v2-neo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 23, 2024

Deploy Preview for kleros-v2-testnet ready!

Name Link
🔨 Latest commit 65e2ec1
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet/deploys/676ab2a944e2bf0008ff7449
😎 Deploy Preview https://deploy-preview-1816--kleros-v2-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 23, 2024

Deploy Preview for kleros-v2-testnet-devtools ready!

Name Link
🔨 Latest commit 65e2ec1
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet-devtools/deploys/676ab2a9c4bd760008bd3164
😎 Deploy Preview https://deploy-preview-1816--kleros-v2-testnet-devtools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 23, 2024

Deploy Preview for kleros-v2-university ready!

Name Link
🔨 Latest commit 65e2ec1
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-university/deploys/676ab2a93ef7b40008395b9e
😎 Deploy Preview https://deploy-preview-1816--kleros-v2-university.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (8)
web/src/utils/index.ts (1)

1-1: Consider providing a fallback if no chain is found
If getChain(DEFAULT_CHAIN) returns undefined or null, the generated link could be invalid. A fallback or error check can prevent potential runtime errors.

web/src/utils/getDrawnJurorsWithCount.ts (1)

22-23: Validate types for vote justification data
Storing timestamps as strings might complicate date/time operations. Consider converting them to numeric timestamps or Date objects to avoid parsing overhead later.

web/src/hooks/queries/useVotingHistory.ts (1)

33-34: Excellent addition of 'transactionHash' and 'timestamp' for justifications
This will enable precise tracking and referencing of votes. Ensure UI components handle null or undefined values for these properties gracefully.

subgraph/core/src/entities/Dispute.ts (1)

24-24: Transaction hash assignment is correct.
Storing the transaction hash in the Dispute entity helps trace dispute creation to a specific transaction. Consider also storing the block number if further debugging or auditing is needed down the line.

web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

81-81: Line-height adjustment to 1.25
Minor styling improvement for better readability with longer justifications.

web/src/utils/shortenAddress.ts (1)

12-18: New shortenTxnHash function.

The logic parallels shortenAddress. Consider customizing the error message to clarify that it’s specific to invalid transaction hashes, not addresses. Otherwise, this is a neat utility.

-throw new TypeError("Invalid input, address can't be parsed");
+throw new TypeError("Invalid input, transaction hash can't be parsed");
subgraph/core/schema.graphql (2)

163-163: Consider adding a description comment for the new transaction hash field

The transactionHash field is correctly marked as non-nullable, but adding a description comment would improve schema documentation.

+ # Hash of the transaction that created this dispute
  transactionHash: String!

184-185: Consider making rulingTransactionHash non-nullable

Since rulingTimestamp and rulingTransactionHash are related fields that would be set together when a ruling occurs, consider making rulingTransactionHash non-nullable (String!) to maintain data consistency.

  rulingTimestamp: BigInt
- rulingTransactionHash: String
+ rulingTransactionHash: String!
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c106865 and 35f2581.

📒 Files selected for processing (15)
  • subgraph/core-neo/subgraph.yaml (5 hunks)
  • subgraph/core/schema.graphql (3 hunks)
  • subgraph/core/src/DisputeKitClassic.ts (1 hunks)
  • subgraph/core/src/KlerosCore.ts (1 hunks)
  • subgraph/core/src/entities/Dispute.ts (1 hunks)
  • subgraph/package.json (1 hunks)
  • web/src/components/EvidenceCard.tsx (2 hunks)
  • web/src/components/TxnHash.tsx (2 hunks)
  • web/src/components/Verdict/DisputeTimeline.tsx (7 hunks)
  • web/src/hooks/queries/useDisputeDetailsQuery.ts (1 hunks)
  • web/src/hooks/queries/useVotingHistory.ts (2 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (4 hunks)
  • web/src/utils/getDrawnJurorsWithCount.ts (1 hunks)
  • web/src/utils/index.ts (1 hunks)
  • web/src/utils/shortenAddress.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • subgraph/package.json
🔇 Additional comments (34)
web/src/utils/getDrawnJurorsWithCount.ts (1)

4-8: Confirm optional fields usage
Marking both transactionHash and timestamp as optional is appropriate if these fields might sometimes be absent. Ensure upstream logic, such as UI rendering or date parsing, can handle undefined values gracefully.

web/src/components/TxnHash.tsx (2)

6-6: Replaced inline link construction with getTxnExplorerLink
This change enhances code maintainability by centralizing the transaction link generation logic.


26-26: Safeguard against empty transaction hash
If the hash is somehow empty or undefined, getTxnExplorerLink might return an invalid URL. Consider a check or fallback to inform the user.

web/src/hooks/queries/useDisputeDetailsQuery.ts (1)

37-38: Verify usage of new dispute fields
The newly added rulingTimestamp and rulingTransactionHash lines up with the updated subgraph schema. Ensure these fields are displayed or consumed in the UI where needed.

✅ Verification successful

Based on the search results, I can generate the final response as we have clear evidence of how these fields are being used in the codebase:

New dispute fields are properly utilized in the UI
The rulingTimestamp and rulingTransactionHash fields are correctly consumed in the DisputeTimeline component where they are used to:

  • Display the ruling date using formatDate(disputeDetails.dispute.rulingTimestamp)
  • Create a transaction explorer link using rulingTransactionHash
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Script to search for all references to the new fields in the codebase
# ensuring they're being properly used in logic/UI.

rg "rulingTimestamp|rulingTransactionHash" -A 5

Length of output: 2627

web/src/hooks/queries/useVotingHistory.ts (1)

15-15: Looks good adding 'transactionHash'!
No issues detected; it aligns with the schema where the dispute entity now includes a transaction hash.

web/src/components/EvidenceCard.tsx (3)

17-18: Appropriate utility import for transaction links.
Centralizing link generation in “getTxnExplorerLink” simplifies maintenance. Continue using it consistently throughout the codebase.


228-228: Memoized transaction explorer link usage.
Memoizing the link is a good practice for performance, preventing creation of new strings whenever the component re-renders.


19-21: Import statement reordering.
The shift in import positions looks fine but keep an eye on any potential circular dependencies that can surface from reordering.

✅ Verification successful

Let me verify if there are any potential circular dependencies by analyzing the imports.


Let me try searching with corrected paths since the files might be in the web/src directory.


Based on the search results, I can see that the style files (commonStyles, landscapeStyle, and responsiveSize) are widely used throughout the codebase as utility styles, and there are no circular dependencies between these files. The imports in EvidenceCard.tsx are simply consuming these utility styles, and their order doesn't affect the functionality.

No circular dependencies found in style imports.
The reordering of these style imports is safe as:

  • These are utility style files that are consumed by many components
  • No circular dependencies exist between these style files
  • The files are independent style utilities serving different purposes (hover transitions, landscape layouts, and responsive sizing)
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Check for any imports of EvidenceCard in the imported files
rg -l "EvidenceCard" "styles/commonStyles" "styles/landscapeStyle" "styles/responsiveSize" || true

# Check the content of these style files to see if they import from each other
rg -l "import.*commonStyles" "styles/landscapeStyle" "styles/responsiveSize" || true
rg -l "import.*landscapeStyle" "styles/commonStyles" "styles/responsiveSize" || true
rg -l "import.*responsiveSize" "styles/commonStyles" "styles/landscapeStyle" || true

# Find the actual paths of these style files
fd -g "*commonStyles*" -g "*landscapeStyle*" -g "*responsiveSize*"

Length of output: 1125


Script:

#!/bin/bash
# Find the actual style files first
fd -g "*commonStyles*" -g "*landscapeStyle*" -g "*responsiveSize*" web/src/

# Once we find the files, let's check their content for imports
rg "import.*from.*styles" web/src/ -A 2 -B 2

Length of output: 52841

web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (7)

7-7: Formatting utility used is consistent.
Using the same formatDate function ensures a uniform date representation across the application.


10-10: Utility import 'getTxnExplorerLink' highlights good abstractions.
Centralizing link logic keeps code DRY and maintainable.


15-15: 'ExternalLink' import for user navigation.
No issues; consistent with how external links are handled in the rest of the codebase.


102-104: Extending component props with optional 'timestamp' and 'transactionHash'.
Great for rendering additional context. Make sure any code path that consumes these props handles them if undefined or null.


105-108: Memoized transaction link
Good approach for performance. Confirms the link is only recomputed when the transaction hash changes.


109-132: Conditional rendering of vote details
Well-structured approach:

  • If choice is defined, show the “Voted” info.
  • If timestamp is defined, show the date link.
  • If justification is set, display it; otherwise, show a fallback text.
    No functional issues found.

165-166: Passing transactionHash and timestamp to AccordionContent
Keeps voting details consistent with newly retrieved data from the query. Ensures they are displayed properly in the UI.

subgraph/core/src/DisputeKitClassic.ts (1)

69-70: Looks good!

Capturing the transaction hash and block timestamp allows for enhanced traceability of votes. Implementation is straightforward and aligns well with typical The Graph event processing patterns.

web/src/components/Verdict/DisputeTimeline.tsx (14)

4-4: Dependency on react-loading-skeleton is consistent with usage.

The Skeleton component is properly imported. No issues found here.


16-16: shortenTxnHash import is correct.

No functional or security concerns observed; this is a straightforward import for better user readability of transaction hashes.


22-23: Implicit usage of getTxnExplorerLink.

This import is consistent with providing a link to an external block explorer. Good job referencing a reusable utility.


24-24: Import of responsiveSize is standard practice.

No concerns. Maintains consistent styling across different screen sizes.


28-29: Use of ExternalLink is appropriate.

This helps maintain consistent link styling and behavior throughout the application.


58-66: LinkContainer styling is suitable for the timeline.

Organizing link display with proper flex alignment improves readability.


67-67: Empty line ensures clarity.

No actionable feedback needed.


140-151: "Dispute created" timeline item.

LinkContainer usage and fallback skeleton for transaction hash are well-handled. This improves UX by showing partial data while loading.


160-160: Empty line.

No further action required.


170-170: Retrieving votingHistory.

Query structure is consistent with previous logic. No issues found.


173-176: Ruling transaction link.

Memoizing the link based on rulingTransactionHash is consistent with the approach used for the dispute creation link. Good pattern reuse.


180-180: Conditional rendering of enforcement data.

Good use of optional chaining. Ensures the UI remains stable if enforcement data is not available.


183-193: Displaying the enforcement date with transaction link.

This approach fosters transparency by linking back to the original on-chain data. Skeleton usage for loading states is also consistent.


85-87: Memoizing txnExplorerLink.

Generating the explorer link within a useMemo is a good approach to avoid unnecessary re-computations. Confirm that the logic gracefully handles empty or undefined transaction hashes.

✅ Verification successful

Let me verify how the code handles empty or undefined transaction hashes by examining the implementation of getTxnExplorerLink.


From the search results, I can see the implementation of getTxnExplorerLink in web/src/utils/index.ts. Let me examine how it's implemented and what chain explorer URL it uses.


Based on the code analysis, I can now provide the final response:

The memoization and null handling in txnExplorerLink is implemented correctly

The implementation gracefully handles empty or undefined transaction hashes through:

  • Nullish coalescing operator (??) in the component: votingHistory?.dispute?.transactionHash ?? ""
  • Optional chaining in the getTxnExplorerLink implementation: getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url

These safeguards ensure the link generation won't throw errors even with missing data.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for getTxnExplorerLink implementation
ast-grep --pattern 'getTxnExplorerLink = $_'

# Also search with function declaration pattern in case it's defined differently
ast-grep --pattern 'function getTxnExplorerLink($_) { $$$ }'

# Backup search using ripgrep
rg "getTxnExplorerLink" -A 5

Length of output: 4427


Script:

#!/bin/bash
# Get the implementation of getChain and DEFAULT_CHAIN
rg "getChain = " -A 3
rg "DEFAULT_CHAIN = " -A 1

# Also check if there are any null checks in the chain explorer URL construction
rg "blockExplorers.*url" -A 2

Length of output: 2239

subgraph/core/src/KlerosCore.ts (1)

187-188: Capturing ruling transaction details.

Storing both the transaction hash and timestamp is a solid improvement for dispute traceability and auditing. Implementation appears correct.

subgraph/core-neo/subgraph.yaml (2)

11-13: Verify contract deployment addresses and start blocks

The KlerosCore contract address and start block have been updated. Let's verify these values match the deployment records.

✅ Verification successful

Let me try another approach to verify the deployment blocks using the deployment artifacts.


Contract addresses and start blocks are correctly configured

The verification confirms that:

  • The contract addresses in subgraph.yaml match exactly with the deployment artifacts
  • The start blocks in subgraph.yaml correspond precisely to the actual deployment block numbers:
    • KlerosCore: 272063254
    • PolicyRegistry: 272063037
    • DisputeKitClassic: 272063168
    • EvidenceModule: 272063086
    • SortitionModule: 272063201
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify contract addresses and deployment blocks
# Expected: Contract addresses should match deployment records and start blocks should be after deployment blocks

# Check contract addresses in deployment artifacts
for contract in KlerosCoreNeo PolicyRegistry DisputeKitClassicNeo EvidenceModule SortitionModuleNeo; do
  echo "Checking $contract..."
  if [ -f "contracts/deployments/arbitrum/$contract.json" ]; then
    jq -r '.address' "contracts/deployments/arbitrum/$contract.json"
  fi
done

# Verify start blocks are after contract deployments
for contract in "0x991d2df165670b9cac3B022f4B68D65b664222ea" "0x553dcbF6aB3aE06a1064b5200Df1B5A9fB403d3c" "0x70B464be85A547144C72485eBa2577E5D3A45421" "0x48e052B4A6dC4F30e90930F1CeaAFd83b3981EB3" "0x21A9402aDb818744B296e1d1BE58C804118DC03D"; do
  echo "Checking deployment block for $contract..."
  gh api graphql -f query='{
    search(query: "org:kleros '$contract'", type: ISSUE, first: 1) {
      nodes {
        ... on Issue {
          body
        }
      }
    }
  }'
done

Length of output: 4968


Script:

#!/bin/bash
# Check deployment blocks from deployment artifacts
for contract in KlerosCoreNeo PolicyRegistry DisputeKitClassicNeo EvidenceModule SortitionModuleNeo; do
  echo "Checking $contract deployment block..."
  if [ -f "contracts/deployments/arbitrum/$contract.json" ]; then
    jq -r '.receipt.blockNumber' "contracts/deployments/arbitrum/$contract.json"
  fi
done

# Also check the start block in subgraph.yaml for comparison
echo "Checking start blocks in subgraph.yaml..."
grep -A 1 "startBlock:" subgraph/core-neo/subgraph.yaml

Length of output: 2166


67-69: Ensure consistent start blocks across modules

The start blocks for different modules vary slightly:

  • PolicyRegistry: 272063037
  • DisputeKitClassic: 272063168
  • EvidenceModule: 272063086
  • SortitionModule: 272063201

While some variation is expected due to deployment order, verify that all modules were deployed and initialized properly before their respective start blocks.

Also applies to: 87-89, 122-124, 143-145

✅ Verification successful

Let me gather information about the deployment blocks and related configurations in the codebase.

These scripts will help us:

  1. Find all start block configurations across subgraph files
  2. Look for deployment-related files that might contain block information
  3. Search for configuration files specific to these modules

This will help us verify if the start blocks are properly aligned with deployment sequences and initialization.


Let me try a more specific search for the subgraph yaml file mentioned in the review comment.

These scripts will help us:

  1. View the complete subgraph configuration
  2. Find deployment order and block information in initialization scripts
  3. Verify the deployment address matches initialization records

Based on the subgraph.yaml content, I can now generate the final response:

Start block sequence appears correct and properly ordered

The deployment sequence shows a logical order with small block number gaps:

  1. PolicyRegistry: 272063037 (first)
  2. EvidenceModule: 272063086 (+49 blocks)
  3. DisputeKitClassic: 272063168 (+82 blocks)
  4. SortitionModule: 272063201 (+33 blocks)
  5. KlerosCore: 272063254 (+53 blocks)

The block gaps between deployments (33-82 blocks) are reasonable for contract deployment and initialization sequences. The modules are indexed from their respective deployment blocks, and KlerosCore starts indexing after all its dependent modules are deployed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for start block configurations in subgraph yaml files
rg -A 3 "startBlock:" "subgraph.*yaml"

Length of output: 93


Script:

#!/bin/bash
# Look for any deployment or initialization related files
fd -e ts -e js -e sol "deploy|init" --exec rg -l "address|block"

Length of output: 185


Script:

#!/bin/bash
# Check for any deployment scripts or configuration files
fd "deploy|config" --type f --exec rg -l "PolicyRegistry|DisputeKitClassic|EvidenceModule|SortitionModule"

Length of output: 191


Script:

#!/bin/bash
# Search for the specific subgraph yaml file
cat subgraph/core-neo/subgraph.yaml

Length of output: 6265


Script:

#!/bin/bash
# Look for deployment timestamps or block numbers in the initialization scripts
rg -A 5 "PolicyRegistry|DisputeKitClassic|EvidenceModule|SortitionModule" ./contracts/scripts/console-init*.ts

Length of output: 1006


Script:

#!/bin/bash
# Check deployment addresses in the initialization scripts
rg "0x553dcbF6aB3aE06a1064b5200Df1B5A9fB403d3c" ./contracts/scripts/console-init*.ts

Length of output: 178

subgraph/core/schema.graphql (1)

309-310: LGTM! Proper field constraints for ClassicJustification

The new fields transactionHash and timestamp are correctly marked as non-nullable, which ensures data integrity for vote justifications.

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 23, 2024
@kemuru kemuru self-requested a review December 23, 2024 13:08
Copy link
Contributor

@kemuru kemuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In the justification part, I would place the date field after the justification. The user is first interested in seeing the vote, then the justification, and then the date. (or at least that's how I see it as a juror)

  2. font-size of date should be 16px to match the other font sizes around it

image
  1. if in one place we put the txHash and in other place we put the date, we are not being consistent. maybe we could show a "Dispute created (ArrowICON)" or maybe "Date ArrowICON"

CURRENT
image

possible other way
image

other possible way
image

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

102-136: Optimize transaction link generation.

The transaction explorer link is currently generated even when transactionHash is undefined, and the link will be rendered with an empty hash. Consider optimizing this by:

  1. Moving the useMemo inside the conditional render.
  2. Adding a null check for the transaction hash.
-  const transactionExplorerLink = useMemo(() => {
-    return getTxnExplorerLink(transactionHash ?? "");
-  }, [transactionHash]);

   return (
     <AccordionContentContainer>
       {!isUndefined(choice) && (
         <div>
           <StyledLabel>Voted:&nbsp;</StyledLabel>
           <SecondaryTextLabel>{getVoteChoice(parseInt(choice), answers)}</SecondaryTextLabel>
         </div>
       )}
-      {!isUndefined(timestamp) && (
+      {!isUndefined(timestamp) && transactionHash && (
         <div>
           <StyledLabel>Date:&nbsp;</StyledLabel>
-          <StyledExternalLink to={transactionExplorerLink} rel="noopener noreferrer" target="_blank">
+          <StyledExternalLink 
+            to={useMemo(() => getTxnExplorerLink(transactionHash), [transactionHash])}
+            rel="noopener noreferrer" 
+            target="_blank"
+          >
             {formatDate(Number(timestamp), true)}
           </StyledExternalLink>
         </div>
       )}
web/src/components/Verdict/DisputeTimeline.tsx (1)

87-89: Consider centralizing transaction link generation.

The transaction explorer link generation logic is duplicated in two useMemo hooks. Consider extracting this into a custom hook to maintain DRY principles:

const useTransactionLink = (hash?: string) => {
  return useMemo(() => hash ? getTxnExplorerLink(hash) : '', [hash]);
};

This would simplify the component and make it easier to maintain.

Also applies to: 168-170

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35f2581 and d3642da.

📒 Files selected for processing (2)
  • web/src/components/Verdict/DisputeTimeline.tsx (7 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (4 hunks)
🔇 Additional comments (6)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (3)

7-7: LGTM: Import statements are properly organized.

The new imports align well with the added functionality for displaying timestamps and transaction links.

Also applies to: 10-10, 15-15


81-81: LGTM: Styled components are well-defined.

The line height adjustment improves readability, and the new StyledExternalLink component maintains consistent styling with a 16px font size.

Also applies to: 98-100


169-170: LGTM: Props are correctly passed to AccordionContent.

The transaction hash and timestamp are properly passed from the drawn juror data to the AccordionContent component.

web/src/components/Verdict/DisputeTimeline.tsx (3)

4-4: LGTM: New imports and styled components are well-structured.

The added imports and styled component implementation align well with the PR objectives. The StyledNewTabIcon follows proper styling patterns with theme integration and hover states.

Also applies to: 11-11, 28-28, 58-68


180-186: LGTM: Well-implemented loading state.

The loading state implementation using Skeleton is clean and provides a good user experience while waiting for the timestamp.


87-89: Handle empty transaction hash case.

The txnExplorerLink could be an empty string if transactionHash is undefined. Consider adding a conditional to only show the link when the hash exists.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

102-130: Enhance accessibility and user experience of the transaction link.

While the implementation is functionally correct, consider the following improvements:

  1. Add an aria-label to the ExternalLink for better accessibility
  2. Add a tooltip showing the transaction hash
  3. Consider adding a loading state while the transaction details are being fetched
-        <ExternalLink to={transactionExplorerLink} rel="noopener noreferrer" target="_blank">
+        <ExternalLink
+          to={transactionExplorerLink}
+          rel="noopener noreferrer"
+          target="_blank"
+          aria-label={`View transaction from ${formatDate(Number(timestamp), true)}`}
+          title={`Transaction: ${transactionHash}`}
+        >
           {formatDate(Number(timestamp), true)}
         </ExternalLink>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3642da and 213bc80.

📒 Files selected for processing (1)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (4 hunks)
🔇 Additional comments (3)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (3)

7-7: LGTM: Import statements are properly organized.

The new imports align with the PR objectives of adding transaction links and timestamps to the voting details.

Also applies to: 10-10, 15-15


81-81: LGTM: Minor styling improvement.

The line-height adjustment from 1.2 to 1.25 enhances text readability.


163-164: LGTM: Props are correctly passed to AccordionContent.

The implementation properly integrates the new transaction and timestamp information from the drawnJuror object.

@kemuru kemuru self-requested a review December 23, 2024 14:49
kemuru
kemuru previously approved these changes Dec 23, 2024
kemuru
kemuru previously approved these changes Dec 23, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (2)

98-100: Consider using theme tokens for spacing

The hardcoded margin value could be replaced with a theme token for better consistency and maintainability.

const StyledInfoCard = styled(InfoCard)`
-  margin-top: 18.5px;
+  margin-top: ${({ theme }) => theme.spacing.lg};
`;

102-134: LGTM! Consider extracting timestamp display logic

The component changes look good with proper handling of optional props and memoization. However, the timestamp display logic could be extracted into a separate component for better reusability.

Consider creating a reusable component:

const TransactionTimestamp: React.FC<{ timestamp?: string; transactionHash?: string }> = ({
  timestamp,
  transactionHash,
}) => {
  const transactionExplorerLink = useMemo(() => {
    return getTxnExplorerLink(transactionHash ?? "");
  }, [transactionHash]);

  if (isUndefined(timestamp)) return null;

  return (
    <ExternalLink to={transactionExplorerLink} rel="noopener noreferrer" target="_blank">
      {formatDate(Number(timestamp), true)}
    </ExternalLink>
  );
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 213bc80 and 5d12cd0.

📒 Files selected for processing (1)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (5 hunks)
🔇 Additional comments (1)
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

167-168: Verify the data contract for new fields

The component now expects transactionHash and timestamp fields on the drawnJuror object. Let's verify that these fields are consistently provided by the GraphQL schema.

Also applies to: 179-179

✅ Verification successful

Optional fields are correctly typed and sourced from vote justification

The verification reveals that transactionHash and timestamp are:

  1. Correctly typed as optional fields in the DrawnJuror type
  2. Properly sourced from the vote justification data in getDrawnJurorsWithCount utility:
    transactionHash: current.vote?.justification?.transactionHash,
    timestamp: current.vote?.justification?.timestamp,
  3. Safely handled in the component with null checks before usage
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the GraphQL schema includes the new fields

# Check if the DrawnJuror type in the schema includes the new fields
ast-grep --pattern 'type DrawnJuror {
  $$$
  transactionHash: String
  timestamp: String
  $$$
}'

# Check for any queries that might need updating
rg -A 5 'query.*DrawnJuror'

Length of output: 130


Script:

#!/bin/bash
# Let's try a different approach to find GraphQL schema and query definitions

# Search for GraphQL schema files
echo "=== Looking for GraphQL schema files ==="
fd -e graphql -e gql

# Search for DrawnJuror type definition in any file
echo -e "\n=== Searching for DrawnJuror type definition ==="
rg -p "type\s+DrawnJuror\s*\{" -A 10

# Search for queries using DrawnJuror in TypeScript/JavaScript files
echo -e "\n=== Searching for GraphQL queries using DrawnJuror ==="
rg -p "query.*\{.*DrawnJuror.*\}" -A 10 -g "*.{ts,tsx,js,jsx}"

# Search for interfaces or types related to DrawnJuror in TypeScript files
echo -e "\n=== Searching for TypeScript interfaces/types for DrawnJuror ==="
rg -p "type\s+DrawnJuror|interface\s+DrawnJuror" -A 10 -g "*.{ts,tsx}"

Length of output: 1272


Script:

#!/bin/bash
# Let's check the schema files we found and look for the base type definition

# Check schema files for DrawnJuror or related types
echo "=== Checking schema files for DrawnJuror related types ==="
for schema in subgraph/core/schema.graphql subgraph/core-neo/schema.graphql subgraph/core-university/schema.graphql; do
  echo -e "\nChecking $schema:"
  cat "$schema" | grep -A 10 "type.*VotingHistoryRound\|type.*Juror"
done

# Let's also check the IVotingHistoryRounds type since it's the base type
echo -e "\n=== Checking IVotingHistoryRounds interface/type ==="
rg "interface IVotingHistoryRounds|type IVotingHistoryRounds" -A 10 -g "*.{ts,tsx}"

# Check the actual usage of these fields in the component
echo -e "\n=== Checking usage of transactionHash and timestamp in the component ==="
rg "transactionHash|timestamp" web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx -B 2 -A 2

Length of output: 3881


Script:

#!/bin/bash
# Let's check the GraphQL query that fetches the voting history data

# Find the VotingHistory query definition
echo "=== Checking VotingHistory Query ==="
rg -p "VotingHistoryQuery|query.*rounds.*drawnJurors" -A 20 -g "*.{ts,tsx,graphql}"

# Also check for any fragments that might be used
echo -e "\n=== Checking for related GraphQL fragments ==="
rg -p "fragment.*on.*Round|fragment.*on.*DrawnJuror" -A 10 -g "*.{ts,tsx,graphql}"

# Check the actual query execution in the component
echo -e "\n=== Checking query usage in VotesDetails component ==="
rg "useVotingHistoryQuery|useQuery.*VotingHistory" web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx -B 2 -A 2

Length of output: 2530

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 23, 2024
alcercu
alcercu previously approved these changes Dec 23, 2024
Copy link
Contributor

@alcercu alcercu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 24, 2024
Copy link

codeclimate bot commented Dec 24, 2024

Code Climate has analyzed commit 65e2ec1 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 1

View more on Code Climate.

Copy link
Contributor

@alcercu alcercu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@alcercu alcercu added this pull request to the merge queue Jan 7, 2025
Merged via the queue into dev with commit e7aab2a Jan 7, 2025
24 of 28 checks passed
This was referenced Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants