update contracts folder structure #2626
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Hi @credence0x! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page. You are receiving this comment because I am set to review all PRs. That is configurable here. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a significant update to resource precision across the entire project. The primary change involves modifying the Changes
Sequence DiagramsequenceDiagram
participant GlobalConfig as EternumGlobalConfig
participant ResourceHandler as Resource Management
participant UIComponent as UI Components
participant ContractSystem as Contract Systems
GlobalConfig->>ResourceHandler: Set resourcePrecision to 1,000,000,000
ResourceHandler-->>UIComponent: Update resource calculations
ResourceHandler-->>ContractSystem: Update resource precision
UIComponent->>ContractSystem: Use new precision for transactions
Possibly related PRs
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 (
|
|
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: 0
🔭 Outside diff range comments (1)
.github/workflows/discord-bot.yml (1)
Update paths-ignore to match the current project structure
The paths-ignore section needs the following updates to match the current folder structure:
- Remove
config/**as this directory no longer exists- Remove
.github/**as this would prevent the workflow itself from running- Add
bot/**to the exclusion list if bot-related changes should trigger the workflow🔗 Analysis chain
Line range hint
4-11: Consider updating paths-ignore section.The
paths-ignoresection might need updating to reflect the new folder structure changes.Let's verify if there are any other folder structure changes that should be reflected here:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any recent folder structure changes git diff --name-status HEAD~5 HEAD | grep -E '^R|^D|^A' | sortLength of output: 251
Script:
#!/bin/bash # List all directories at root level echo "=== Directories at root level ===" fd --max-depth 1 --type d # List all files at root level echo -e "\n=== Files at root level ===" fd --max-depth 1 --type fLength of output: 320
🧰 Tools
🪛 actionlint (1.7.4)
28-28: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🧹 Nitpick comments (3)
client/apps/docs/utils/formatting.tsx (1)
17-17: Simplify complex nested ternary operationThe nested ternary operation makes the code hard to read and maintain. Consider refactoring into a more readable format.
- return `${(amount / EternumGlobalConfig.resources.resourcePrecision).toFixed(amount % EternumGlobalConfig.resources.resourcePrecision === 0 ? 0 : (amount % EternumGlobalConfig.resources.resourcePrecision) % 10 === 0 ? 1 : 2)}M`; + const normalizedAmount = amount / EternumGlobalConfig.resources.resourcePrecision; + const remainder = amount % EternumGlobalConfig.resources.resourcePrecision; + const decimals = remainder === 0 ? 0 : + remainder % 10 === 0 ? 1 : 2; + return `${normalizedAmount.toFixed(decimals)}M`;client/apps/game/src/ui/components/structures/construction/StructureConstructionMenu.tsx (1)
127-127: Use multiplyByPrecision utility consistentlyFor consistency with the rest of the codebase, use the imported
multiplyByPrecisionutility instead of directly multiplying withEternumGlobalConfig.resources.resourcePrecision.- amount={cost[Number(resourceId)].min_amount * EternumGlobalConfig.resources.resourcePrecision} + amount={multiplyByPrecision(cost[Number(resourceId)].min_amount)}bot/discord/src/events/battle_pillage.rs (1)
46-46: Consider adding formatting for better readability.The resource amount display could benefit from formatting for better readability, especially with the increased precision.
Consider adding thousand separators or using a dedicated formatting function:
-format!("{} {}", amount / RESOURCE_PRECISION, ResourceIds::from(*resource_id),) +format!("{:,} {}", amount / RESOURCE_PRECISION, ResourceIds::from(*resource_id),)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
bot/discord/Cargo.lockis excluded by!**/*.lockcontracts/game/Scarb.lockis excluded by!**/*.lockcontracts/season_pass/Scarb.lockis excluded by!**/*.lockcontracts/season_pass/contracts/.DS_Storeis excluded by!**/.DS_Storecontracts/season_resources/Scarb.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.github/workflows/discord-bot.yml(4 hunks)bot/discord/src/events/battle_pillage.rs(2 hunks)client/apps/docs/utils/formatting.tsx(1 hunks)client/apps/game/src/hooks/helpers/battles/__test__/__mock__.tsx(2 hunks)client/apps/game/src/ui/components/resources/realm-transfer.tsx(2 hunks)client/apps/game/src/ui/components/structures/construction/StructureConstructionMenu.tsx(2 hunks)client/apps/game/src/ui/components/trading/MarketOrderPanel.tsx(2 hunks)client/sdk/packages/eternum/src/constants/global.ts(1 hunks)contracts/game/src/constants.cairo(1 hunks)contracts/game/src/models/combat.cairo(1 hunks)contracts/game/src/systems/bank/contracts/liquidity.cairo(3 hunks)contracts/game/src/systems/bank/contracts/swap.cairo(5 hunks)contracts/game/src/systems/combat/contracts/battle_systems.cairo(0 hunks)contracts/game/src/systems/transport/tests/travel_systems_tests.cairo(2 hunks)contracts/game/src/utils/testing/config.cairo(1 hunks)contracts/package.json(0 hunks)contracts/scripts/client.sh(0 hunks)
💤 Files with no reviewable changes (3)
- contracts/scripts/client.sh
- contracts/package.json
- contracts/game/src/systems/combat/contracts/battle_systems.cairo
👮 Files not reviewed due to content moderation or server errors (4)
- contracts/game/src/constants.cairo
- contracts/game/src/systems/transport/tests/travel_systems_tests.cairo
- client/apps/game/src/ui/components/trading/MarketOrderPanel.tsx
- contracts/game/src/models/combat.cairo
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run pr agent on every pull request, respond to user comments
- GitHub Check: clippy
🔇 Additional comments (13)
client/apps/docs/utils/formatting.tsx (1)
14-14: Review the 'K' suffix usage with new precisionThe 'K' suffix might be misleading since we're now using a much higher precision (1,000,000,000). Consider using a more appropriate suffix or updating the formatting logic to reflect the new scale.
client/apps/game/src/hooks/helpers/battles/__test__/__mock__.tsx (1)
36-43: LGTM! Consistent use of resource precisionThe mock battle health values are correctly updated to use
EternumGlobalConfig.resources.resourcePrecision, maintaining consistency with the new precision standard.contracts/game/src/systems/bank/contracts/liquidity.cairo (2)
43-43: LGTM! Clear documentation updateThe comment has been correctly updated to reflect the new precision in price calculations.
149-149: Verify impact of precision change on price calculationsThe change in resource price calculation affects financial operations. Ensure all systems consuming this price value handle the new precision correctly.
✅ Verification successful
Precision handling is consistent with codebase standards
The multiplication by RESOURCE_PRECISION in the price calculation follows the established pattern used throughout the codebase, where all resource amount calculations are normalized using the same precision factor (1_000_000_000). All systems consuming these values handle the precision correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all references to market.quote_amount to verify precision handling ast-grep --pattern 'market.quote_amount($_)'Length of output: 46
Script:
#!/bin/bash # Search for quote_amount mentions rg "quote_amount" -A 2 # Search for RESOURCE_PRECISION usage rg "RESOURCE_PRECISION" -A 2 # Search for market related function calls rg "market\." -A 2Length of output: 58362
contracts/game/src/systems/bank/contracts/swap.cairo (3)
23-23: LGTM: Import of RESOURCE_PRECISION constant.The import of
RESOURCE_PRECISIONaligns with the project-wide update to resource precision.
49-49: LGTM: Updated comment for resource price precision.The comment accurately reflects that the price is now based on
RESOURCE_PRECISIONinstead of the hardcoded value.
101-101: Verify consistent resource price calculations.The price calculations have been updated to use
RESOURCE_PRECISIONconsistently across thebuy,sell, andemit_eventfunctions.Run the following script to verify consistent usage of
RESOURCE_PRECISIONin price calculations:Also applies to: 155-155, 188-188
✅ Verification successful
RESOURCE_PRECISION is used consistently in price calculations
The codebase shows consistent usage of RESOURCE_PRECISION (1_000_000_000) across all market operations, including buy, sell, and event emissions. No inconsistencies or hardcoded values were found in price calculations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of RESOURCE_PRECISION in price calculations. # Test: Search for any remaining hardcoded values (1000) in price calculations rg -A 5 $'1000.*price' # Test: Search for all price calculations to ensure they use RESOURCE_PRECISION ast-grep --pattern $'market.buy($_, $_, RESOURCE_PRECISION)'Length of output: 85
Script:
#!/bin/bash # Search for RESOURCE_PRECISION usage echo "=== RESOURCE_PRECISION usage ===" rg "RESOURCE_PRECISION" echo -e "\n=== Market function implementations ===" # Search for market function implementations rg -A 5 "fn (buy|sell|emit_event)" echo -e "\n=== Numerical literals in price calculations ===" # Search for numerical literals in price-related code rg -A 5 "price.*[0-9]+" rg -A 5 "[0-9]+.*price"Length of output: 36697
client/apps/game/src/ui/components/resources/realm-transfer.tsx (2)
9-9: LGTM: Import of EternumGlobalConfig.The import of
EternumGlobalConfigaligns with the project-wide update to resource precision.
67-67: LGTM: Updated resource amount calculation.The resource amount calculation now correctly uses
EternumGlobalConfig.resources.resourcePrecisioninstead of the hardcoded value.contracts/game/src/utils/testing/config.cairo (1)
111-111: LGTM: Updated max troop count calculation.The max troop count calculation now correctly uses
RESOURCE_PRECISIONinstead of the hardcoded value.client/sdk/packages/eternum/src/constants/global.ts (1)
49-50: Verify impact of increased resource precision.The significant increase in
RESOURCE_PRECISIONandRESOURCE_MULTIPLIERfrom 1000 to 1_000_000_000 is a breaking change that affects all resource calculations throughout the application.Run the following script to verify that all resource calculations handle the increased precision correctly:
✅ Verification successful
Resource precision change is properly handled across the codebase
The increase in
RESOURCE_PRECISIONfrom 1000 to 1_000_000_000 is safely implemented:
- All resource calculations consistently use BigInt/u128 types
- Proper normalization is applied in calculations
- Resource amounts are correctly scaled with precision
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify resource calculations with increased precision. # Test: Search for potential integer overflow issues in calculations rg -A 5 $'[*]/.*RESOURCE_PRECISION' # Test: Search for any remaining hardcoded values (1000) in resource calculations rg -A 5 $'[*]/.*1000' # Test: Search for resource-related type definitions to ensure they can handle the increased precision ast-grep --pattern $'type.*Resource'Length of output: 97
Script:
#!/bin/bash # Search for direct usage of the constants rg "RESOURCE_PRECISION|RESOURCE_MULTIPLIER" -B 2 -A 2 # Search for resource-related calculations and variables rg "resource.*=.*[*/]|[*/].*resource" -i -B 2 -A 2 # Search for numeric type definitions that might be affected ast-grep --pattern 'type $_ = number' # Search for BigInt usage which might indicate large number handling rg "BigInt|bigint" -B 2 -A 2Length of output: 76336
Script:
#!/bin/bash # Search for numeric type declarations and BigInt usage in TypeScript rg "type.*number|type.*bigint|BigInt" -g "*.ts" -B 2 -A 2 # Search for arithmetic operations with RESOURCE_PRECISION rg "RESOURCE_PRECISION.*[*/+\-]|[*/+\-].*RESOURCE_PRECISION" -B 2 -A 2 # Search for potential numeric overflow checks rg "overflow|exceed|limit|max|bound" -g "*.{ts,cairo}" -B 2 -A 2Length of output: 70154
.github/workflows/discord-bot.yml (1)
25-25: LGTM! Path updates are consistent with the new folder structure.The changes correctly update the paths from
discord-bottobot/discordacross both jobs.Also applies to: 35-35, 44-44, 57-57
bot/discord/src/events/battle_pillage.rs (1)
17-17: Verify resource precision consistency across the codebase.The new
RESOURCE_PRECISIONconstant aligns with the project-wide update to resource precision. Let's verify consistency with other implementations:✅ Verification successful
Resource precision implementation is correctly scoped
The
RESOURCE_PRECISIONconstant is appropriately implemented inbattle_pillage.rsas it's the only event handler that needs to format resource amounts for Discord messages. No other event handlers require this functionality, making the current implementation consistent and well-scoped.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for RESOURCE_PRECISION declarations and usages echo "Searching for RESOURCE_PRECISION declarations:" rg -p "RESOURCE_PRECISION.*=.*1_?0{3}(_?0{3})*" --type rust --type typescript --type cairo echo -e "\nSearching for resource amount divisions:" rg -p "/ *(?:RESOURCE_PRECISION|1_?0{3}(?:_?0{3})*)" --type rust --type typescript --type cairoLength of output: 460
Script:
#!/bin/bash # Search for precision-related constants and their usage echo "=== Rust Files ===" echo "Searching for precision constants:" rg -p "(?:RESOURCE|TOKEN|PRECISION).*=.*1_?0{3}(_?0{3})*" --type rust echo -e "\nSearching for precision divisions:" rg -p "/ *(?:RESOURCE_PRECISION|TOKEN_PRECISION|1_?0{9})" --type rust echo -e "\n=== Cairo Files ===" echo "Searching for precision constants:" rg -p "(?:RESOURCE|TOKEN|PRECISION).*=.*10\*\*[69]" --type cairo echo -e "\nSearching for precision divisions:" rg -p "/ *(?:RESOURCE_PRECISION|TOKEN_PRECISION|10\*\*[69])" --type cairo # Find all potential source files to ensure we're not missing any echo -e "\n=== Source Files ===" fd -e rs -e cairo -e tsLength of output: 15857
Script:
#!/bin/bash # Check other event handlers for resource formatting echo "=== Checking resource formatting in event handlers ===" rg -p "format.*amount.*resource" bot/discord/src/events/ # Look for similar resource-related constants in other event files echo -e "\n=== Checking resource constants in event handlers ===" rg -p "(?:const|let).*(?:RESOURCE|PRECISION)" bot/discord/src/events/Length of output: 604
…/github.com/BibliothecaDAO/eternum into contracts-folder-structure
Summary by CodeRabbit
New Features
Chores
contracts/package.jsonandcontracts/scripts/client.shRefactor