fix resource bridging #2253
fix resource bridging #2253
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several modifications across various files, primarily focusing on enhancing the clarity and structure of documentation, updating environment variables, and refining component logic related to realm management. Key changes include the addition of a new JSON file for resource addresses, updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
You are out of MentatBot reviews. Your usage will refresh December 9 at 08:00 AM. |
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (15)
season_resources/scripts/deployment/.env.sample (1)
Line range hint
1-5: Security: Ensure sensitive credentials are properly documentedWhile these are sample values for local development, it's important to:
- Document that these are example values only
- Indicate the expected format and requirements for production values
Add a header comment to clarify the purpose:
+# Sample environment configuration for local development +# DO NOT use these values in production + export RPC_API_KEY=http://127.0.0.1:5050 export STARKNET_NETWORK=local🧰 Tools
🪛 Gitleaks (8.21.2)
5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
season_resources/scripts/deployment/.env.sepolia (2)
Line range hint
9-12: Critical: Additional sensitive information exposureSimilar to the earlier issue, these environment variables contain sensitive information that should not be committed to the repository.
Apply the same solution as suggested above:
-export VITE_PUBLIC_MASTER_ADDRESS="0x01BFC84464f990C09Cc0e5D64D18F54c3469fD5c467398BF31293051bAde1C39" -export VITE_PUBLIC_MASTER_PRIVATE_KEY="0x075362a844768f31c8058ce31aec3dd7751686440b4f220f410ae0c9bf042e60" +export VITE_PUBLIC_MASTER_ADDRESS=${SEPOLIA_MASTER_ADDRESS} +export VITE_PUBLIC_MASTER_PRIVATE_KEY=${SEPOLIA_MASTER_PRIVATE_KEY}🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Line range hint
1-12: Security: Implement comprehensive secrets managementThe current implementation exposes multiple sensitive credentials directly in the configuration file. A more secure approach is needed:
- Create a comprehensive secrets management strategy
- Use a secrets manager service for production environments
- Document the required environment variables
- Add the following to your
.gitignore:**/.env.* !**/.env.example !**/.env.sampleWould you like assistance in creating a secure configuration template and documentation?
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
landing/src/hooks/query/entities.tsx (1)
3-22: LGTM! Consider adding error handling documentation.The GraphQL query is well-structured and properly typed. However, it would be beneficial to add documentation about potential error cases and how they should be handled by consumers of this query.
Consider adding a comment block above the query explaining:
- Expected behavior when no realms are found
- How to handle network errors
- Any rate limiting considerations
+/** + * Fetches realm IDs owned by a specific account address. + * @throws {Error} When the GraphQL request fails + * @returns Empty edges array when no realms are found + */ export const GET_ETERNUM_OWNER_REALM_IDS = graphql(`landing/src/hooks/helpers/useRealms.tsx (1)
3-3: Consider caching the realms data.Loading the JSON file on every function call could be inefficient if called frequently.
Consider implementing a memoization strategy:
import { useMemo } from 'react'; // Inside the hook const realmsData = useMemo(() => realmsJson.features, []);landing/src/hooks/helpers/useEntities.tsx (1)
34-36: Enhance return type with loading and error statesConsider returning additional states to improve error handling in consuming components.
return { playerRealms, + isLoading, + error, };season_resources/scripts/deployment/config/index.ts (1)
Line range hint
28-40: Add error handling for resource configurationThe resource bridge whitelist configuration lacks error handling for failed transactions or invalid configurations.
export const setResourceBridgeWhitlelistConfig = async (config: Config) => { let resourceWhitelistConfigs: ResourceWhitelistConfig[] = []; + try { for (const [resourceName, [resourceId, tokenAddress]] of Object.entries(resourceAddresses)) { + if (!resourceId || !tokenAddress) { + throw new Error(`Invalid configuration for resource ${resourceName}`); + } resourceWhitelistConfigs.push({ token: tokenAddress, resource_type: resourceId, }); console.log(`Configuring whitelist for ${resourceName} (${resourceId}) (${tokenAddress}) for in-game asset bridge`); } const tx = await config.provider.set_resource_bridge_whitlelist_config({ signer: config.account, resource_whitelist_configs: resourceWhitelistConfigs, }); console.log(`Finished configuring whitelist for in-game asset bridge ${tx.statusReceipt}`); + } catch (error) { + console.error('Failed to configure resource bridge whitelist:', error); + throw error; + } };landing/src/components/modules/bridge-out-step-2.tsx (1)
Line range hint
87-97: Consider enhancing error handling in the bridge operation.The
onFinishWithdrawFromBankfunction could benefit from more robust error handling to provide better user feedback.const onFinishWithdrawFromBank = async () => { if (selectedResourceId) { const resourceAddresses = await getSeasonAddresses(); const selectedResourceName = ResourcesIds[selectedResourceId]; let tokenAddress = resourceAddresses[selectedResourceName.toUpperCase() as keyof typeof resourceAddresses][1]; try { setIsLoading(true); await bridgeFinishWithdrawFromRealm(tokenAddress as string, ADMIN_BANK_ENTITY_ID, donkeyEntityId); + } catch (error) { + console.error('Bridge withdrawal failed:', error); + // Consider adding a toast notification or error message for the user } finally { setIsLoading(false); } } };landing/src/components/modules/bridge-in.tsx (2)
130-130: Remove debug console.log statement.Debug logging should be removed before merging to production.
- console.log({ resourceAddresses });
161-167: Consider adding type safety for realm mapping.The realm mapping could benefit from explicit type definitions to ensure type safety.
+type PlayerRealm = { + realmId: number; + entityId: number; + name: string; +}; const playerRealmsIdAndName = useMemo(() => { - return playerRealms.map((realm) => ({ + return playerRealms.map((realm): PlayerRealm => ({ realmId: realm!.realm_id, entityId: realm!.entity_id, name: getRealmNameById(realm!.realm_id), })); }, [playerRealms]);landing/src/components/modules/bridge-out-step-1.tsx (3)
Line range hint
91-102: Optimize useMemo dependenciesThe useMemo dependency array includes
selectedResourceIdandselectedResourceAmountbut these values aren't used in the computation.Consider updating the dependency array:
}, [selectedResourceId, realmEntityId, selectedResourceAmount]); + }, [realmEntityId, computeTravelTime]);
159-170: Add aria-label for better accessibilityThe Select component should have an aria-label for screen readers.
Consider adding an aria-label:
- <Select onValueChange={(value) => setRealmEntityId(value)}> + <Select + onValueChange={(value) => setRealmEntityId(value)} + aria-label="Select a realm" + >
221-221: Simplify button disabled state logicThe current disabled state check uses multiple negations which can be hard to read.
Consider simplifying the logic:
- disabled={(!selectedResourceAmount && !selectedResourceId && !realmEntityId) || isLoading} + disabled={isLoading || !(selectedResourceAmount && selectedResourceId && realmEntityId)}season_resources/scripts/deployment/libs/commands.js (2)
59-59: Consider centralizing theLORDS_RESOURCE_IDconstantFor better maintainability and to avoid magic numbers, consider defining
LORDS_RESOURCE_IDin a configuration or constants file. This ensures consistency if the value needs to be updated in the future.
151-153: Redundant array type checkSince
resourceAddressesArrayis the result of amapoperation, it will always be an array. The checkif (!Array.isArray(resourceAddressesArray))may be unnecessary and can be removed to simplify the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (24)
eternum-docs/docs/pages/mechanics/realm/realm.mdx(1 hunks)landing/.env.preview(1 hunks)landing/.env.production(1 hunks)landing/public/resource_addresses/sepolia/resource_addresses.json(1 hunks)landing/src/components/modules/bridge-in.tsx(7 hunks)landing/src/components/modules/bridge-out-step-1.tsx(6 hunks)landing/src/components/modules/bridge-out-step-2.tsx(3 hunks)landing/src/components/ui/SelectResources.tsx(0 hunks)landing/src/dojo/setup.ts(1 hunks)landing/src/hooks/gql/gql.ts(1 hunks)landing/src/hooks/helpers/useEntities.tsx(1 hunks)landing/src/hooks/helpers/useRealms.tsx(2 hunks)landing/src/hooks/query/entities.tsx(1 hunks)landing/src/hooks/useBridge.tsx(1 hunks)season_resources/contracts/src/contract.cairo(0 hunks)season_resources/scripts/deployment/.env.example(1 hunks)season_resources/scripts/deployment/.env.production(1 hunks)season_resources/scripts/deployment/.env.sample(1 hunks)season_resources/scripts/deployment/.env.sepolia(1 hunks)season_resources/scripts/deployment/config/index.ts(1 hunks)season_resources/scripts/deployment/deploy.js(2 hunks)season_resources/scripts/deployment/libs/commands.js(4 hunks)season_resources/scripts/deployment/libs/common.js(1 hunks)season_resources/scripts/deployment/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- landing/src/components/ui/SelectResources.tsx
- season_resources/contracts/src/contract.cairo
✅ Files skipped from review due to trivial changes (5)
- landing/src/hooks/useBridge.tsx
- landing/.env.production
- landing/.env.preview
- landing/public/resource_addresses/sepolia/resource_addresses.json
- eternum-docs/docs/pages/mechanics/realm/realm.mdx
🧰 Additional context used
🪛 Gitleaks (8.21.2)
season_resources/scripts/deployment/.env.sample
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
season_resources/scripts/deployment/.env.sepolia
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (17)
season_resources/scripts/deployment/.env.example (1)
5-5: LGTM! Path standardization looks good.
The change from addresses/dev to addresses/local aligns well with the environment standardization across other configuration files (.env.production, .env.sepolia, .env.sample).
season_resources/scripts/deployment/.env.production (1)
5-5: Verify mainnet addresses folder existence
Ensure that the addresses/mainnet directory exists and contains the necessary address configuration files.
season_resources/scripts/deployment/.env.sample (2)
12-12: Verify the impact of VITE_PUBLIC_DEV flag
The addition of VITE_PUBLIC_DEV="true" might affect application behavior.
✅ Verification successful
VITE_PUBLIC_DEV flag is used for development environment configuration
The VITE_PUBLIC_DEV flag is used throughout the codebase to:
- Toggle between development and production manifests
- Switch between local (127.0.0.1:5050) and production node URLs
- Show/hide development-specific UI components (like test minting buttons)
- Enable development features (debug menus, GUI controls)
- Configure deployment safety checks
Adding this flag to .env.sample is correct as it serves as a template for setting up development environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of VITE_PUBLIC_DEV to understand its impact
rg "VITE_PUBLIC_DEV" --type ts --type tsx -A 3
Length of output: 74
Script:
#!/bin/bash
# Search for usage of VITE_PUBLIC_DEV in all files
rg "VITE_PUBLIC_DEV" -A 3
Length of output: 10557
9-12: Security: Review hardcoded development credentials
The master address and private key are hardcoded. While these are for local development, we should:
- Ensure these are documented as development-only credentials
- Verify they are not used in any other environment
✅ Verification successful
Let's verify the content of these files to understand the context better.
Security: Development credentials found in multiple example/preview files
The development credentials are present in multiple configuration files:
landing/.env.preview(only as a fee recipient address)season_pass/scripts/deployment/.env.example(full credentials)season_resources/scripts/deployment/.env.example(full credentials)
All instances are properly marked as example/preview configurations with clear local development context:
- Files use
.exampleor.previewextensions - URLs point to localhost or test environments
- Contains explicit comments about changing values for production
- Development mode flags are set to true
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any usage of these development credentials in non-local environments
MASTER_ADDRESS="0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec"
MASTER_KEY="0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912"
# Check .env files for these values (excluding .env.sample)
find . -name ".env*" ! -name ".env.sample" -type f -exec grep -l "$MASTER_ADDRESS\|$MASTER_KEY" {} \;
Length of output: 490
Script:
#!/bin/bash
# Check content of the files that contain these credentials
echo "=== landing/.env.preview ==="
cat ./landing/.env.preview
echo -e "\n=== season_pass/scripts/deployment/.env.example ==="
cat ./season_pass/scripts/deployment/.env.example
echo -e "\n=== season_resources/scripts/deployment/.env.example ==="
cat ./season_resources/scripts/deployment/.env.example
Length of output: 3147
🧰 Tools
🪛 Gitleaks (8.21.2)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
season_resources/scripts/deployment/.env.sepolia (2)
1-2: Verify the network configuration consistency
The network name has been changed from uppercase SEPOLIA to lowercase sepolia. While this works, let's verify this is consistent with other configuration files.
✅ Verification successful
Network configuration is consistent across the codebase
The lowercase sepolia value for STARKNET_NETWORK is consistent with other environment files in the codebase. The pattern shows:
- Network values are consistently lowercase across all env files (
sepolia,local,mainnet) - Both
season_resourcesandseason_passprojects follow the same convention
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check network naming consistency across configuration files
# Expected: All network names should follow the same case convention
# Search for STARKNET_NETWORK in all env files
fd -e env -e env.example -e env.sample -e env.sepolia -e env.production --exec grep -l "STARKNET_NETWORK" {} \; | \
while read -r file; do
echo "=== $file ==="
grep "STARKNET_NETWORK" "$file"
done
Length of output: 129
Script:
#!/bin/bash
# Let's try a broader search for any env files and network configurations
# First, find all env-like files
echo "=== Finding env files ==="
fd -e env -e env.example -e env.sample -e env.sepolia -e env.production
echo -e "\n=== Searching for network configurations in these files ==="
# Use rg for better pattern matching
rg -i "starknet.*network|network.*starknet" --type-add 'env:*.{env,env.*}' --type env
Length of output: 901
7-7: Verify admin address configuration
The SEASON_RESOURCE_ADMIN is now set to match STARKNET_ACCOUNT_ADDRESS. This change implies a significant shift in admin privileges. Please confirm this is intentional.
✅ Verification successful
Admin address configuration alignment confirmed
The verification shows that the new SEASON_RESOURCE_ADMIN value matches the account_address in dojo_prod.toml, and it's being properly used in the deployment scripts for both admin and upgrader roles. This alignment between the account address and admin privileges appears to be an intentional design choice for consistent access control.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for references to the old admin address to ensure all occurrences are updated
# Expected: No references to the old admin address should remain
# Search for the old admin address
rg "0x1a3e37c77be7de91a9177c6b57956faa6da25607e567b10a25cf64fea5e533b" --type-not env
Length of output: 113
Script:
#!/bin/bash
# Search for any references to admin addresses in .env files
fd -e env -x cat {} \; | grep -i "SEASON_RESOURCE_ADMIN"
# Search for admin address references in other files
rg "SEASON_RESOURCE_ADMIN" --type yaml --type ts --type js --type json
# Search for the new admin address in other files
rg "0x01BFC84464f990C09Cc0e5D64D18F54c3469fD5c467398BF31293051bAde1C39"
Length of output: 586
season_resources/scripts/deployment/package.json (1)
8-9: Verify environment file existence and add error handling.
While the scripts are well-structured, they might fail silently if environment files are missing.
Let's verify the environment files exist and check their content structure:
Consider adding error handling to the scripts:
- "deploy": "source .env && bun --env-file=.env ./deploy && bun --env-file=.env ./config/index.ts",
- "deploy::sepolia": "source .env.sepolia && bun --env-file=.env.sepolia ./deploy && bun --env-file=.env.sepolia ./config/index.ts"
+ "deploy": "test -f .env || (echo '.env file missing' && exit 1) && source .env && bun --env-file=.env ./deploy && bun --env-file=.env ./config/index.ts",
+ "deploy::sepolia": "test -f .env.sepolia || (echo '.env.sepolia file missing' && exit 1) && source .env.sepolia && bun --env-file=.env.sepolia ./deploy && bun --env-file=.env.sepolia ./config/index.ts"✅ Verification successful
Let me verify if there are any example environment files or templates in the repository that could indicate the expected structure.
Let me check if there are any deployment-related files or documentation in the deployment directory that might provide insights about the environment setup.
Let me check the content of the environment example files to understand the required structure and variables.
Environment files are properly configured but need to be created from templates
The review comment's concern about environment files is valid. The repository has proper environment file templates (.env.example and .env.sample) with all required variables, but the actual .env and .env.sepolia files need to be created by the developer. The .gitignore correctly excludes these files for security.
Required actions:
- Create
.envfrom.env.examplefor default deployment - Create
.env.sepoliafrom.env.examplefor Sepolia network deployment - Update the network-specific values in each file:
- RPC_API_KEY
- STARKNET_NETWORK
- STARKNET_ACCOUNT_ADDRESS
- STARKNET_ACCOUNT_PRIVATE_KEY
- DEPLOYMENT_ADDRESSES_FOLDER
- SEASON_RESOURCE_ADMIN
The suggested error handling in the original review is still valuable and should be implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify environment files and their structure
# Check if environment files exist
echo "Checking environment files..."
for env_file in .env .env.sepolia; do
if [ -f "$env_file" ]; then
echo "✓ $env_file exists"
# Check for required variables without displaying values
grep -E "^(STARKNET_NETWORK|DEPLOYMENT_ADDRESSES_FOLDER)=" "$env_file" | cut -d'=' -f1
else
echo "✗ $env_file missing"
fi
done
Length of output: 278
Script:
#!/bin/bash
# Search for environment file templates or examples
echo "Searching for environment file templates..."
fd -t f -e example -e template -e sample | grep -i env
# Search for documentation mentioning environment setup
echo -e "\nSearching for environment setup documentation..."
rg -i "environment|env file" "README.md" || true
Length of output: 364
Script:
#!/bin/bash
# Check deployment directory structure and documentation
echo "Checking deployment directory contents..."
ls -la season_resources/scripts/deployment/
echo -e "\nSearching for environment setup in deployment files..."
rg -i "env" season_resources/scripts/deployment/ || true
echo -e "\nChecking for any README or documentation files in deployment directory..."
fd -t f "README|readme|\.md$" season_resources/scripts/deployment/
Length of output: 5092
Script:
#!/bin/bash
echo "Checking environment example files..."
for env_file in season_resources/scripts/deployment/.env.*; do
if [[ "$env_file" != *".example" && "$env_file" != *".sample" ]]; then
continue
fi
echo -e "\nContents of $env_file:"
cat "$env_file"
done
echo -e "\nChecking .gitignore for env file patterns..."
cat season_resources/scripts/deployment/.gitignore
Length of output: 2967
landing/src/dojo/setup.ts (1)
45-45: Verify resource bridging component dependencies
The "Resource" component has been added for resource bridging. Please verify if any additional components are required for complete resource bridging functionality.
✅ Verification successful
Let me try another search strategy to verify the resource bridging components.
Let me try one more search to specifically look for the component filtering logic.
Based on the search results showing the component filtering logic in setup.ts, I can now provide the final response:
Resource component is properly integrated with other bridging components
The "Resource" component has been added to the filtered components array alongside other related components like "Owner", "Realm", and "HyperstructureResourceConfig" that are necessary for resource bridging functionality. The component dependencies are properly configured for complete resource bridging operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Resource component usage and related components
echo "Searching for Resource component usage..."
rg -l "Resource" --type ts --type tsx
echo "\nSearching for resource bridging related components..."
ast-grep --pattern 'interface $_ {
$$$
Resource
$$$
}'
Length of output: 342
Script:
#!/bin/bash
# Search for Resource component usage with proper context
echo "Searching for Resource component references..."
rg "Resource" --type ts -B 2 -A 2
echo "\nSearching for component filtering logic..."
rg "filteredComponents" --type ts -B 3 -A 3
Length of output: 65825
Script:
#!/bin/bash
# Search for filteredComponents array and related setup code
echo "Searching for filteredComponents in setup.ts..."
rg -A 10 -B 10 'filteredComponents\s*=' landing/src/dojo/setup.ts
echo "\nSearching for component filtering related code..."
ast-grep --pattern 'const filteredComponents = [
$$$
]'
Length of output: 1027
landing/src/hooks/gql/gql.ts (1)
16-17: LGTM! GraphQL query implementation looks good.
The new query getEternumOwnerRealmIds is well-structured and properly typed with the corresponding document type.
Also applies to: 27-29
landing/src/components/modules/bridge-out-step-2.tsx (1)
76-79: LGTM! Clean implementation of realm fetching.
The switch to useEntities hook simplifies the realm data management.
landing/src/components/modules/bridge-in.tsx (1)
75-82: LGTM! Well-structured realm data management.
The memoized playerRealmsIdAndName provides a clean interface for realm selection.
landing/src/components/modules/bridge-out-step-1.tsx (2)
Line range hint 44-89: Verify precision handling in fee calculations
The fee calculations involve floating-point arithmetic which could lead to precision issues. Additionally, ensure that the RESOURCE_PRECISION constant is properly synchronized with the smart contract implementation.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Fee calculations are correctly implemented with proper precision handling
The fee calculations in the component are secure because:
- The component uses
BRIDGE_FEE_DENOMINATORfor fee calculations which aligns with the smart contract's precision handling - All fee calculations are performed using the same consistent formula pattern:
(percent * amount) / denominator - The final amounts are properly formatted using the
formatFeeutility before display - The component correctly synchronizes with the smart contract's
RESOURCE_PRECISION(1000) constant for resource amount handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent RESOURCE_PRECISION usage
rg -A 2 "RESOURCE_PRECISION"
# Check for potential floating-point precision issues in calculations
ast-grep --pattern 'const $_ = $$$_amount * $$$_percent / $$$;'
Length of output: 26215
2-2: Verify error handling in useEntities hook
The switch to useEntities hook for realm management is a good architectural choice. However, we should ensure proper error handling is implemented.
✅ Verification successful
Let's examine the implementation of useEntities hook to verify its error handling:
Error handling in useEntities hook is properly implemented
After reviewing the implementation, the useEntities hook is well-designed and handles errors appropriately through:
- Null coalescing for account address:
account?.address || "" - Safe component value access using
getComponentValue - Proper type guards and filtering of undefined values
- Defensive programming with optional chaining and nullish coalescing
- Type safety through TypeScript
The hook is correctly implemented and follows React best practices with proper memoization and error prevention. No additional error handling is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if useEntities hook implements proper error handling
ast-grep --pattern 'export function useEntities() {
$$$
try {
$$$
} catch {
$$$
}
$$$
}'
Length of output: 108
Script:
#!/bin/bash
# Let's first locate the useEntities hook implementation
fd "useEntities" --type f
# Then examine its content
rg "useEntities" -A 10 -B 2
Length of output: 77312
Script:
#!/bin/bash
# Check the implementation of useEntities hook in both client and landing
cat landing/src/hooks/helpers/useEntities.tsx
cat client/src/hooks/helpers/useEntities.tsx
Length of output: 12993
season_resources/scripts/deployment/libs/commands.js (2)
8-14: New imports from common.js are appropriate
The additional functions imported enhance the module's functionality and are necessary for resource management.
140-177: New function grantMinterRoleToAllSeasonResourceContracts is well-implemented
The function correctly retrieves resource addresses, filters out the LORDS_RESOURCE_ID, and constructs the execution array to grant the minter role. Error handling and logging are appropriately used.
season_resources/scripts/deployment/deploy.js (2)
1-1: Importing grantMinterRoleToAllSeasonResourceContracts
The new function is correctly imported from ./libs/commands.js, ensuring it is available for use in the deployment script.
10-10: Calling grantMinterRoleToAllSeasonResourceContracts after deployment
The invocation of await grantMinterRoleToAllSeasonResourceContracts(); ensures that the minter role is granted to all season resource contracts immediately after deployment, enhancing the automation of the deployment process.
| export STARKNET_ACCOUNT_ADDRESS=0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec | ||
| export STARKNET_ACCOUNT_PRIVATE_KEY=0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912 |
There was a problem hiding this comment.
Security: Remove sensitive data from example file.
The example environment file contains actual private keys and addresses. Even if these are for local development, it's recommended to:
- Use placeholder values (e.g.,
<YOUR_PRIVATE_KEY>) - Add comments explaining the expected format
Apply this diff to improve security:
-export STARKNET_ACCOUNT_ADDRESS=0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec
-export STARKNET_ACCOUNT_PRIVATE_KEY=0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912
+# Your StarkNet account address (0x followed by 64 hex characters)
+export STARKNET_ACCOUNT_ADDRESS=<YOUR_ACCOUNT_ADDRESS>
+# Your StarkNet private key (0x followed by 64 hex characters)
+export STARKNET_ACCOUNT_PRIVATE_KEY=<YOUR_PRIVATE_KEY>
-export VITE_PUBLIC_MASTER_ADDRESS="0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec"
-export VITE_PUBLIC_MASTER_PRIVATE_KEY="0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912"
+# Master account address and private key for local development
+export VITE_PUBLIC_MASTER_ADDRESS=<MASTER_ADDRESS>
+export VITE_PUBLIC_MASTER_PRIVATE_KEY=<MASTER_PRIVATE_KEY>Also applies to: 9-10
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| export RPC_API_KEY=https://api.cartridge.gg/x/starknet/mainnet | ||
| export STARKNET_NETWORK=mainnet |
There was a problem hiding this comment.
💡 Codebase verification
Mixed environment configurations detected in network configuration
There appears to be a potential environment inconsistency. While .env.production sets mainnet configuration, the libs/network.js file contains hardcoded Sepolia RPC URL (https://free-rpc.nethermind.io/sepolia-juno) in the NETWORKS configuration object. This could lead to mixed environments if not properly handled.
season_resources/scripts/deployment/libs/network.js: Review and update the hardcoded Sepolia RPC URL in theNETWORKSconfiguration to ensure it's not accidentally used in production.
🔗 Analysis chain
Verify URL consistency across configuration
The RPC endpoint and node URL are both correctly set to mainnet, but let's verify there are no mixed environments in other configuration files.
Also applies to: 10-10
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any potential mixed environment configurations
echo "Checking for any sepolia references in mainnet config:"
rg -i "sepolia" season_resources/scripts/deployment/.env.production
echo "Checking URL consistency across all env files:"
rg "api.cartridge.gg/x/starknet/" season_resources/scripts/deployment/
Length of output: 360
Script:
#!/bin/bash
# Let's check all environment-related files in the deployment directory
echo "Listing all environment files:"
fd -t f "\.env" season_resources/scripts/deployment/
echo "Checking contents of all env files for network configurations:"
fd -t f "\.env" season_resources/scripts/deployment/ -x cat {}
echo "Checking for any network-related configurations in other files:"
rg -i "network|rpc|api.cartridge" season_resources/scripts/deployment/
Length of output: 3668
| export STARKNET_ACCOUNT_ADDRESS=0x0 | ||
| export STARKNET_ACCOUNT_PRIVATE_KEY=0x0 |
There was a problem hiding this comment.
💡 Codebase verification
Critical: Production credentials must not be committed with placeholder values
The verification confirms that these credentials are directly used in deployment scripts:
STARKNET_ACCOUNT_ADDRESSandSTARKNET_ACCOUNT_PRIVATE_KEYare used to create the StarkNet account innetwork.jsSEASON_RESOURCE_ADMINis used for admin configuration incommands.jsVITE_PUBLIC_MASTER_ADDRESSandVITE_PUBLIC_MASTER_PRIVATE_KEYare required for the application setup inconfig/index.ts
Setting these to 0x0 in .env.production will cause deployment failures. These credentials should be:
- Removed from version control
- Injected during deployment through a secure secrets management system
- Documented in a
.env.examplefile
🔗 Analysis chain
Critical: Production credentials are set to placeholder values
Several critical credentials are set to 0x0, which would prevent the application from functioning in production:
- StarkNet account credentials
- Season resource admin address
- Master address and private key
These should be securely configured with valid production values before deployment.
Let's verify if there are any deployment scripts that might populate these values:
Also applies to: 7-9
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for deployment scripts that might handle credential injection
rg -l "STARKNET_ACCOUNT|SEASON_RESOURCE_ADMIN|MASTER" season_resources/scripts/deployment/
Length of output: 249
Script:
#!/bin/bash
# Let's examine how these credentials are handled in the deployment scripts
echo "=== Checking config/index.ts ==="
rg -A 5 "STARKNET_ACCOUNT|SEASON_RESOURCE_ADMIN|MASTER" season_resources/scripts/deployment/config/index.ts
echo -e "\n=== Checking commands.js ==="
rg -A 5 "STARKNET_ACCOUNT|SEASON_RESOURCE_ADMIN|MASTER" season_resources/scripts/deployment/libs/commands.js
echo -e "\n=== Checking network.js ==="
rg -A 5 "STARKNET_ACCOUNT|SEASON_RESOURCE_ADMIN|MASTER" season_resources/scripts/deployment/libs/network.js
Length of output: 1779
| export DEPLOYMENT_ADDRESSES_FOLDER=addresses/local # change to addresses/prod for prod | ||
| export SEASON_RESOURCE_ADMIN=0x1a3e37c77be7de91a9177c6b57956faa6da25607e567b10a25cf64fea5e533b # change to prod admin address |
There was a problem hiding this comment.
💡 Codebase verification
The path change from 'addresses/dev' to 'addresses/local' requires attention
The codebase search reveals inconsistencies in address path handling:
- Scripts in
scripts/indexer.shstill referenceaddresses/dev/ - The project uses multiple address directories:
common/addresses/,landing/public/resource_addresses/ - Both 'local' and 'prod' environments are referenced in different scripts
The change should be coordinated with:
- Update needed in
scripts/indexer.shto use consistent paths - Ensure alignment with the address files in
common/addresses/andlanding/public/resource_addresses/
🔗 Analysis chain
Verify the deployment folder path change impact
The change from 'addresses/dev' to 'addresses/local' should be verified across the codebase to ensure consistency.
Let me gather more information to verify the impact of this path change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to 'addresses/dev'
rg "addresses/dev" --type-not env
# Check if 'addresses/local' directory exists and contains expected files
if [ -d "addresses/local" ]; then
find addresses/local -type f -name "*.json"
else
echo "Warning: addresses/local directory not found"
fi
Length of output: 189
Script:
#!/bin/bash
# Search for any references to 'addresses/' to understand the directory structure and usage
rg "addresses/"
# List all directories starting with 'addresses' to see available environments
fd "addresses"
# Check the content of deployment scripts that might use these paths
fd -t f "deployment" -x cat {}
Length of output: 1537
| export STARKNET_ACCOUNT_ADDRESS=0x01BFC84464f990C09Cc0e5D64D18F54c3469fD5c467398BF31293051bAde1C39 | ||
| export STARKNET_ACCOUNT_PRIVATE_KEY=0x075362a844768f31c8058ce31aec3dd7751686440b4f220f410ae0c9bf042e60 |
There was a problem hiding this comment.
Critical: Sensitive information exposure
The file contains sensitive information (private key and account address). These should be loaded from secure environment variables or a secrets management system, not committed to the repository.
Consider:
- Moving sensitive values to a local
.env.sepolia.localthat is git-ignored - Using placeholder values in the committed file
- Adding documentation for required environment variables
-export STARKNET_ACCOUNT_ADDRESS=0x01BFC84464f990C09Cc0e5D64D18F54c3469fD5c467398BF31293051bAde1C39
-export STARKNET_ACCOUNT_PRIVATE_KEY=0x075362a844768f31c8058ce31aec3dd7751686440b4f220f410ae0c9bf042e60
+export STARKNET_ACCOUNT_ADDRESS=${SEPOLIA_ACCOUNT_ADDRESS}
+export STARKNET_ACCOUNT_PRIVATE_KEY=${SEPOLIA_PRIVATE_KEY}📝 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.
| export STARKNET_ACCOUNT_ADDRESS=0x01BFC84464f990C09Cc0e5D64D18F54c3469fD5c467398BF31293051bAde1C39 | |
| export STARKNET_ACCOUNT_PRIVATE_KEY=0x075362a844768f31c8058ce31aec3dd7751686440b4f220f410ae0c9bf042e60 | |
| export STARKNET_ACCOUNT_ADDRESS=${SEPOLIA_ACCOUNT_ADDRESS} | |
| export STARKNET_ACCOUNT_PRIVATE_KEY=${SEPOLIA_PRIVATE_KEY} |
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| const address = ContractAddress(account?.address || ""); | ||
|
|
||
| // Get all realms | ||
| const allRealms = useEntityQuery([Has(Realm)]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add loading and error states
The useEntityQuery hook might be in a loading state or encounter errors. Consider returning these states to handle them in the UI.
+ const [isLoading, setIsLoading] = useState(true);
+ const [error, setError] = useState<Error | null>(null);
const allRealms = useEntityQuery([Has(Realm)]);
+
+ useEffect(() => {
+ if (allRealms) {
+ setIsLoading(false);
+ }
+ }, [allRealms]);Committable suggestion skipped: line range outside the PR's diff.
| const network = process.env.STARKNET_NETWORK; | ||
| const filePath = join(__dirname, `../addresses/${network}/resource_addresses.json`); | ||
| return JSON.parse(readFileSync(filePath, "utf-8")); | ||
| }; |
There was a problem hiding this comment.
Add network validation
The STARKNET_NETWORK environment variable is used without validation. Consider adding checks for supported networks.
- const network = process.env.STARKNET_NETWORK;
+ const network = process.env.STARKNET_NETWORK;
+ const supportedNetworks = ['mainnet', 'sepolia', 'dev'];
+ if (!supportedNetworks.includes(network)) {
+ throw new Error(`Unsupported network: ${network}. Must be one of: ${supportedNetworks.join(', ')}`);
+ }📝 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 network = process.env.STARKNET_NETWORK; | |
| const filePath = join(__dirname, `../addresses/${network}/resource_addresses.json`); | |
| return JSON.parse(readFileSync(filePath, "utf-8")); | |
| }; | |
| const network = process.env.STARKNET_NETWORK; | |
| const supportedNetworks = ['mainnet', 'sepolia', 'dev']; | |
| if (!supportedNetworks.includes(network)) { | |
| throw new Error(`Unsupported network: ${network}. Must be one of: ${supportedNetworks.join(', ')}`); | |
| } | |
| const filePath = join(__dirname, `../addresses/${network}/resource_addresses.json`); | |
| return JSON.parse(readFileSync(filePath, "utf-8")); | |
| }; |
| const playerRealmsIdAndName = useMemo(() => { | ||
| return playerRealms.map((realm) => ({ | ||
| realmId: realm!.realm_id, | ||
| entityId: realm!.entity_id, | ||
| name: getRealmNameById(realm!.realm_id), | ||
| })); | ||
| }, [playerRealms]); |
There was a problem hiding this comment.
Add null checks in realm mapping
The mapping function assumes realm properties are always non-null with the ! operator. This could lead to runtime errors if the API returns incomplete data.
Consider adding proper null checks:
const playerRealmsIdAndName = useMemo(() => {
- return playerRealms.map((realm) => ({
- realmId: realm!.realm_id,
- entityId: realm!.entity_id,
- name: getRealmNameById(realm!.realm_id),
- }));
+ return playerRealms
+ .filter((realm): realm is NonNullable<typeof realm> => realm != null)
+ .map((realm) => ({
+ realmId: realm.realm_id,
+ entityId: realm.entity_id,
+ name: getRealmNameById(realm.realm_id),
+ }));
}, [playerRealms]);📝 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 playerRealmsIdAndName = useMemo(() => { | |
| return playerRealms.map((realm) => ({ | |
| realmId: realm!.realm_id, | |
| entityId: realm!.entity_id, | |
| name: getRealmNameById(realm!.realm_id), | |
| })); | |
| }, [playerRealms]); | |
| const playerRealmsIdAndName = useMemo(() => { | |
| return playerRealms | |
| .filter((realm): realm is NonNullable<typeof realm> => realm != null) | |
| .map((realm) => ({ | |
| realmId: realm.realm_id, | |
| entityId: realm.entity_id, | |
| name: getRealmNameById(realm.realm_id), | |
| })); | |
| }, [playerRealms]); |
| const resourceNamePrefix = "Eternum S0 "; | ||
| const resourceSymbolPrefix = "ES0_"; | ||
| for (const resource of Object.values(RESOURCE_NAMES)) { | ||
| const resourceName = `${resourceNamePrefix}${resource.name}`; | ||
| if (resourceName.length > 31) { | ||
| throw new Error("Resource name must be less than or equal to 32 characters"); | ||
| } | ||
| const resourceSymbol = `${resourceSymbolPrefix}${resource.symbol}`; | ||
| if (resourceSymbol.length > 31) { | ||
| throw new Error("Resource symbol must be less than or equal to 32 characters"); | ||
| } | ||
| console.log({ resourceName, resourceSymbol }); | ||
|
|
There was a problem hiding this comment.
Fix inconsistency between length checks and error messages
The condition checks if (resourceName.length > 31), but the error message states that the resource name must be less than or equal to 32 characters. Since resourceName.length > 31 includes lengths of 32 and above, the maximum length is actually 31 characters. The same issue exists for resourceSymbol.
Apply this diff to correct the error messages:
- throw new Error("Resource name must be less than or equal to 32 characters");
+ throw new Error("Resource name must be less than or equal to 31 characters");- throw new Error("Resource symbol must be less than or equal to 32 characters");
+ throw new Error("Resource symbol must be less than or equal to 31 characters");📝 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 resourceNamePrefix = "Eternum S0 "; | |
| const resourceSymbolPrefix = "ES0_"; | |
| for (const resource of Object.values(RESOURCE_NAMES)) { | |
| const resourceName = `${resourceNamePrefix}${resource.name}`; | |
| if (resourceName.length > 31) { | |
| throw new Error("Resource name must be less than or equal to 32 characters"); | |
| } | |
| const resourceSymbol = `${resourceSymbolPrefix}${resource.symbol}`; | |
| if (resourceSymbol.length > 31) { | |
| throw new Error("Resource symbol must be less than or equal to 32 characters"); | |
| } | |
| console.log({ resourceName, resourceSymbol }); | |
| const resourceNamePrefix = "Eternum S0 "; | |
| const resourceSymbolPrefix = "ES0_"; | |
| for (const resource of Object.values(RESOURCE_NAMES)) { | |
| const resourceName = `${resourceNamePrefix}${resource.name}`; | |
| if (resourceName.length > 31) { | |
| throw new Error("Resource name must be less than or equal to 31 characters"); | |
| } | |
| const resourceSymbol = `${resourceSymbolPrefix}${resource.symbol}`; | |
| if (resourceSymbol.length > 31) { | |
| throw new Error("Resource symbol must be less than or equal to 31 characters"); | |
| } | |
| console.log({ resourceName, resourceSymbol }); |
| export const getResourceAddressesFromFile = async () => { | ||
| const folderPath = process.env.DEPLOYMENT_ADDRESSES_FOLDER; | ||
| const fileName = path.join(folderPath, `resource_addresses.json`); | ||
| const data = await readFileAsync(fileName, "utf8"); | ||
| return JSON.parse(data); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to getResourceAddressesFromFile
The function does not currently handle potential errors when reading the file or parsing JSON data. To enhance robustness, consider adding error handling similar to other functions in the module.
Apply this diff to add error handling:
export const getResourceAddressesFromFile = async () => {
const folderPath = process.env.DEPLOYMENT_ADDRESSES_FOLDER;
const fileName = path.join(folderPath, `resource_addresses.json`);
+ try {
const data = await readFileAsync(fileName, "utf8");
return JSON.parse(data);
+ } catch (err) {
+ if (err.code === "ENOENT") {
+ console.error(`File not found: ${fileName}`);
+ } else if (err instanceof SyntaxError) {
+ console.error("Error parsing JSON:", err);
+ } else {
+ console.error("Error reading file:", err);
+ }
+ throw err; // Re-throw the error to notify the caller of the failure
+ }
};📝 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.
| export const getResourceAddressesFromFile = async () => { | |
| const folderPath = process.env.DEPLOYMENT_ADDRESSES_FOLDER; | |
| const fileName = path.join(folderPath, `resource_addresses.json`); | |
| const data = await readFileAsync(fileName, "utf8"); | |
| return JSON.parse(data); | |
| }; | |
| export const getResourceAddressesFromFile = async () => { | |
| const folderPath = process.env.DEPLOYMENT_ADDRESSES_FOLDER; | |
| const fileName = path.join(folderPath, `resource_addresses.json`); | |
| try { | |
| const data = await readFileAsync(fileName, "utf8"); | |
| return JSON.parse(data); | |
| } catch (err) { | |
| if (err.code === "ENOENT") { | |
| console.error(`File not found: ${fileName}`); | |
| } else if (err instanceof SyntaxError) { | |
| console.error("Error parsing JSON:", err); | |
| } else { | |
| console.error("Error reading file:", err); | |
| } | |
| throw err; // Re-throw the error to notify the caller of the failure | |
| } | |
| }; |
Summary by CodeRabbit
Release Notes
New Features
useEntities, to streamline data fetching for realms owned by users.GET_ETERNUM_OWNER_REALM_IDSto retrieve realm IDs associated with specific accounts.Improvements
Bug Fixes
Chores