Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several updates across the project, primarily focusing on tournament-related features and library version upgrades. The changes include updating the Starknet library version in utility files, adding tournament-specific environment variables, creating a new tournament overview component, and modifying network configurations. The modifications aim to prepare the application for a new tournament system, with updates to UI components and configuration settings to support this new feature. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI
participant TournamentConfig
participant NetworkConfig
User->>UI: Access Application
UI->>TournamentConfig: Check Tournament Active
TournamentConfig->>NetworkConfig: Retrieve Tournament Details
NetworkConfig-->>TournamentConfig: Return Tournament Configuration
TournamentConfig-->>UI: Render Tournament Overview
UI->>User: Display Tournament Information
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
ui/src/app/components/start/DSTournamentOverview.tsx (2)
15-19: Remove debug console.log statement.Production code should not contain console.log statements. Consider using a proper logging system if debugging is needed.
- console.log(tournamentStart, startDate);
40-65: Consider making tournament prize details configurable.Prize details are currently hardcoded in the component. Consider moving these to configuration to make them easily updatable.
Consider creating a configuration file for tournament details or fetching them from an API endpoint.
ui/src/app/containers/AdventurerScreen.tsx (2)
78-79: Add type safety for environment variable.The environment variable check could benefit from type safety and a default value.
- const isDSTournamentActive = - process.env.NEXT_PUBLIC_DS_TOURNAMENT_ACTIVE === "true"; + const isDSTournamentActive = + process.env.NEXT_PUBLIC_DS_TOURNAMENT_ACTIVE?.toLowerCase() === "true" || false;
104-134: Reduce menu item duplication.The tournament menu duplicates items from the regular menu. Consider refactoring to compose menus dynamically.
- const dsTournamentMenu = [ - { - id: 1, - label: "Dark Shuffle Tournament", - value: "ds tournament", - action: () => { - setStartOption("ds tournament"); - }, - disabled: false, - }, - // ... duplicated items - ]; + const tournamentMenuItem = { + id: 1, + label: "Dark Shuffle Tournament", + value: "ds tournament", + action: () => { + setStartOption("ds tournament"); + }, + disabled: false, + }; + + const dsTournamentMenu = [tournamentMenuItem, ...menu];ui/src/app/lib/networkConfig.ts (1)
29-29: Ensure consistent tournament configuration across networks.The tournament app URL is empty for all networks except mainnet. Consider:
- Adding staging/test tournament URLs for non-production environments
- Documenting the expected behavior when URLs are empty
Also applies to: 73-73, 104-104, 137-137
indexer/src/utils/encode.ts (1)
Line range hint
44-52: Consider additional validation for tournament tokensSince this PR integrates tournament functionality, consider adding specific validation for tournament-related tokens:
- Validate token format for tournament-specific requirements
- Add explicit error handling for tournament-specific edge cases
export function computeHash( token: string | null, tokenId: string | null ): string { + // Validate tournament-specific requirements + if (!token || !tokenId) { + throw new Error("Token and tokenId are required for tournament hashing"); + } const tokenStr = formatString(token); const tokenIdStr = formatString(tokenId); const combinedString = `${tokenStr}:${tokenIdStr}`; return hash.getSelectorFromName(combinedString); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ui/public/ds-icon.svgis excluded by!**/*.svgui/public/ls-seal.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
indexer/src/utils/encode.ts(1 hunks)indexer/src/utils/events.ts(1 hunks)ui/.env(1 hunks)ui/src/app/components/navigation/Header.tsx(3 hunks)ui/src/app/components/start/CreateAdventurer.tsx(1 hunks)ui/src/app/components/start/DSTournamentOverview.tsx(1 hunks)ui/src/app/containers/AdventurerScreen.tsx(3 hunks)ui/src/app/lib/networkConfig.ts(4 hunks)ui/src/app/page.tsx(1 hunks)ui/src/app/provider.tsx(1 hunks)
🔇 Additional comments (9)
ui/src/app/provider.tsx (1)
84-84: Verify the tournament token slot name convention.The slot name "ls-tournament-tokens" has been added for tournament token management. Please confirm if this matches the expected naming convention for tournament integration.
ui/src/app/lib/networkConfig.ts (1)
70-73: Verify tournament contract deployment.The mainnet tournament configuration has been updated with new values. Please ensure:
- The tournament contract is deployed at the specified address
- The tournament ID is correctly set
- The tournament app URL is accessible
ui/src/app/components/start/CreateAdventurer.tsx (1)
169-169: LGTM! Text changes improve clarity.The updated messaging better communicates the current season status to users.
Also applies to: 172-172
ui/src/app/components/navigation/Header.tsx (2)
471-471: LGTM! Improved button styling based on authentication state.The conditional button variant provides better visual feedback about the user's authentication status.
504-504: LGTM! Improved icon positioning.The adjustment enhances the visual alignment of the CartridgeIcon.
ui/src/app/page.tsx (1)
748-748: Verify the impact of commenting out the Toaster component.Please confirm if an alternative notification system is in place or if this component is still needed.
Run this script to check for other notification mechanisms:
✅ Verification successful
Commenting out the Toaster component is safe
The app has a dedicated notification system implemented through
NotificationDisplayandNotificationHandlercomponents that remains active. The commented-out Toaster appears to be a specific seasonal implementation and its removal doesn't impact the main notification functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for notification-related components and their usage # Search for notification-related components echo "Searching for notification components..." rg -l "notification|toast|alert" --type tsx # Search for Toaster component usage echo "Checking Toaster component usage..." rg "Toaster" --type tsxLength of output: 297
Script:
#!/bin/bash # Search for notification-related components and Toaster usage echo "=== Searching for Toaster imports and usage ===" rg "import.*Toaster" "*.tsx" "*.ts" rg "Toaster" "*.tsx" "*.ts" echo -e "\n=== Searching for notification/toast/alert patterns ===" rg "notification|toast|alert" "*.tsx" "*.ts" echo -e "\n=== Finding all UI files that might contain notifications ===" fd -e tsx -e tsLength of output: 9461
Script:
#!/bin/bash echo "=== Checking toast implementation files ===" rg -A 5 "export" ui/src/app/components/season/toast.tsx ui/src/app/components/season/toaster.tsx ui/src/app/hooks/useToast.ts echo -e "\n=== Checking notification system ===" rg -A 5 "export" ui/src/app/components/notifications/NotificationComponent.tsx ui/src/app/components/notifications/NotificationDisplay.tsx ui/src/app/components/notifications/NotificationHandler.tsx echo -e "\n=== Checking Toaster usage in page.tsx ===" rg -C 5 "Toaster" ui/src/app/page.tsxLength of output: 4013
ui/.env (1)
3-6: Clarify the relationship between tournament states.Having both
NEXT_PUBLIC_TOURNAMENT_ENDED="true"andNEXT_PUBLIC_DS_TOURNAMENT_ACTIVE="true"might be confusing. Consider adding comments to clarify the difference between these states.Also, please verify if the tournament start time (Jan 24, 2025) is correct:
Consider adding comments to explain each environment variable:
NEXT_PUBLIC_NETWORK="mainnet" +# Status of the previous tournament NEXT_PUBLIC_TOURNAMENT_ENDED="true" +# Status of the current season NEXT_PUBLIC_SEASON_ACTIVE="false" +# Status of the Dark Shuffle tournament NEXT_PUBLIC_DS_TOURNAMENT_ACTIVE="true" +# Unique identifier for the Dark Shuffle tournament NEXT_PUBLIC_DS_TOURNAMENT_ID=1 +# Start time of the Dark Shuffle tournament (Jan 24, 2025) NEXT_PUBLIC_DS_TOURNAMENT_START_TIME=1737727200✅ Verification successful
No conflict in tournament states - they represent different systems
The code review shows that
NEXT_PUBLIC_TOURNAMENT_ENDEDandNEXT_PUBLIC_DS_TOURNAMENT_ACTIVEare used independently in different components, with the former referring to a previous tournament system and the latter specifically for the new Dark Shuffle tournament. The timestamp (Jan 24, 2025 14:00:00 UTC) is correctly set for a future tournament start.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Convert and verify the tournament timestamp echo "Tournament start time:" date -d @1737727200Length of output: 105
Script:
#!/bin/bash # Search for usage of tournament state environment variables echo "=== Usage of NEXT_PUBLIC_TOURNAMENT_ENDED ===" rg "NEXT_PUBLIC_TOURNAMENT_ENDED" -B 2 -A 2 echo -e "\n=== Usage of NEXT_PUBLIC_DS_TOURNAMENT_ACTIVE ===" rg "NEXT_PUBLIC_DS_TOURNAMENT_ACTIVE" -B 2 -A 2Length of output: 1039
indexer/src/utils/encode.ts (1)
1-1: Verify compatibility with Starknet v6.16.0The major version upgrade from v5.19.5 to v6.16.0 could introduce breaking changes. Ensure that
hash.getSelectorFromNamemaintains consistent behavior in the new version.Let's verify the impact:
✅ Verification successful
Version upgrade is safe for
hash.getSelectorFromNameThe function is used consistently across the codebase with a simple string parameter pattern, and being a core feature for contract selectors, its behavior is stable across versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for breaking changes in hash.getSelectorFromName between versions # and its usage across the codebase # Find all usages of getSelectorFromName echo "Searching for getSelectorFromName usage..." rg "getSelectorFromName" -A 2 # Look for any migration guides or breaking changes echo "Checking for breaking changes documentation..." curl -s https://www.npmjs.com/package/starknet | rg -A 5 "Breaking Changes|BREAKING CHANGES|Migration Guide"Length of output: 3645
Script:
#!/bin/bash # Check the specific hash function implementation and types # Look for type definitions or implementations echo "Searching for hash type definitions..." rg -t ts "hash.*getSelectorFromName.*" --type-add 'ts:*.{ts,tsx,d.ts}' -g '!node_modules' # Try to find any usage patterns that might have changed echo "Checking function call patterns..." ast-grep --pattern 'hash.getSelectorFromName($_)'Length of output: 6275
indexer/src/utils/events.ts (1)
Line range hint
249-257: Add Dark Shuffle Tournament event parsingThe PR aims to integrate Dark Shuffle Tournament, but corresponding event parsing is missing. Consider adding:
- Tournament start/end events
- Tournament progress tracking events
- Tournament ranking events
Let's check for any tournament-related changes that might need event parsing:
| const tournamentStart = process.env.NEXT_PUBLIC_DS_TOURNAMENT_START_TIME; | ||
|
|
||
| const startDate = new Date(Number(tournamentStart) * 1000); |
There was a problem hiding this comment.
Add validation for tournament start time.
The date conversion is performed without validating the environment variable or handling potential invalid dates.
- const tournamentStart = process.env.NEXT_PUBLIC_DS_TOURNAMENT_START_TIME;
- const startDate = new Date(Number(tournamentStart) * 1000);
+ const tournamentStart = process.env.NEXT_PUBLIC_DS_TOURNAMENT_START_TIME;
+ if (!tournamentStart) {
+ throw new Error('Tournament start time is not configured');
+ }
+ const startTimestamp = Number(tournamentStart);
+ if (isNaN(startTimestamp)) {
+ throw new Error('Invalid tournament start time format');
+ }
+ const startDate = new Date(startTimestamp * 1000);
+ if (startDate.toString() === 'Invalid Date') {
+ throw new Error('Invalid date conversion');
+ }📝 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 tournamentStart = process.env.NEXT_PUBLIC_DS_TOURNAMENT_START_TIME; | |
| const startDate = new Date(Number(tournamentStart) * 1000); | |
| const tournamentStart = process.env.NEXT_PUBLIC_DS_TOURNAMENT_START_TIME; | |
| if (!tournamentStart) { | |
| throw new Error('Tournament start time is not configured'); | |
| } | |
| const startTimestamp = Number(tournamentStart); | |
| if (isNaN(startTimestamp)) { | |
| throw new Error('Invalid tournament start time format'); | |
| } | |
| const startDate = new Date(startTimestamp * 1000); | |
| if (startDate.toString() === 'Invalid Date') { | |
| throw new Error('Invalid date conversion'); | |
| } |
| @@ -1,4 +1,4 @@ | |||
| import { hash } from "https://esm.sh/starknet@5.19.5"; | |||
| import { hash } from "https://esm.sh/starknet@6.16.0"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Update package.json to match Starknet import version
The Starknet version in your imports (6.16.0) is ahead of the version specified in package.json (6.11.0). While the different version in scripts/katana is acceptable as it's a separate context, you should:
- Update package.json to use Starknet version 6.16.0 to match the import statements
🔗 Analysis chain
Verify consistent Starknet version across the codebase
The version upgrade to 6.16.0 should be consistent across all Starknet imports in the project.
Let's verify version consistency:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for inconsistent Starknet versions across the codebase
echo "Searching for all Starknet import versions..."
rg "from.*starknet@[0-9]+" --type ts
# Check package.json for Starknet version if it exists
echo "Checking package.json for Starknet version..."
fd "package.json" --type f --exec jq -r '.dependencies.starknet, .devDependencies.starknet' 2>/dev/null
Length of output: 703
1 participant
Summary by CodeRabbit
New Features
Chores
UI Changes
Cleanup