Revitalize Katana #257
Revitalize Katana #257
Conversation
- refactor spawn syscall
- add fetch for free VRF
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThe changes encompass various modifications across multiple files, focusing on enhancing the functionality of the GraphQL API, refining event handling, and improving state management in the UI. Key updates include the introduction of new filters and resolvers in the GraphQL API, adjustments to event parsing, and the addition of new state variables related to a "free VRF" feature in the UI. Additionally, several cosmetic changes were made, such as code formatting and the removal of commented-out code. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant GraphQL API
participant Database
User->>UI: Interacts with onboarding button
UI->>GraphQL API: Fetches token data
GraphQL API->>Database: Queries for tokens
Database-->>GraphQL API: Returns token data
GraphQL API-->>UI: Sends token data
UI-->>User: Displays token information
Tip New review modelWe have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. New featuresWalkthrough comment now includes:
Notes:
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
ui/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (15)
- indexer/graphql/src/indexer/graphql.py (2 hunks)
- scripts/katana/Dockerfile.graph (1 hunks)
- scripts/katana/graphql/indexer/graphql.py (27 hunks)
- scripts/katana/indexer/adventurers.ts (3 hunks)
- scripts/katana/indexer/items.ts (2 hunks)
- scripts/katana/indexer/utils/encode.ts (2 hunks)
- scripts/katana/indexer/utils/events.ts (5 hunks)
- scripts/katana/indexer/utils/helpers.ts (6 hunks)
- scripts/katana/indexer/utils/parser.ts (1 hunks)
- ui/package.json (1 hunks)
- ui/src/app/components/onboarding/Intro.tsx (1 hunks)
- ui/src/app/hooks/useUIStore.ts (2 hunks)
- ui/src/app/lib/networkConfig.ts (1 hunks)
- ui/src/app/lib/utils/syscalls.ts (4 hunks)
- ui/src/app/page.tsx (3 hunks)
Files skipped from review due to trivial changes (4)
- indexer/graphql/src/indexer/graphql.py
- scripts/katana/indexer/utils/parser.ts
- ui/package.json
- ui/src/app/components/onboarding/Intro.tsx
Additional context used
Ruff
scripts/katana/graphql/indexer/graphql.py
6-6:
hashlibimported but unusedRemove unused import:
hashlib(F401)
2825-2825: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
2828-2828: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
2890-2890: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
2893-2893: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
2923-2926: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
2959-2959: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
2962-2962: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
3026-3026: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
3029-3029: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
Additional comments not posted (55)
scripts/katana/Dockerfile.graph (1)
14-14: Verify the security implications of removing the--allowed-originparameters.The removal of the
--allowed-originparameters from theENTRYPOINTcommand could potentially allow requests from any origin, which may have security implications depending on how the application handles cross-origin requests.To verify the security implications, analyze how the application handles cross-origin requests and ensure that appropriate security measures are in place to prevent unauthorized access. Consider the following:
- Are there any other mechanisms in place to restrict access based on the origin, such as CORS configuration or authentication?
- Are there any sensitive endpoints or data that could be exposed if requests from any origin are allowed?
- Have the potential risks been assessed and deemed acceptable given the specific use case and security requirements of the application?
Provide more context on the reasoning behind this change and confirm that the necessary security considerations have been taken into account.
scripts/katana/indexer/utils/encode.ts (1)
1-2: LGTM! The addition of thecomputeHashandformatStringfunctions looks good.The
computeHashfunction provides a useful utility to compute hashes using SHA-256, which can be beneficial for various purposes such as generating unique identifiers or verifying data integrity. The use of the Starknet library for hashing suggests that the computed hashes may be used in the context of Starknet-related functionality.The
formatStringhelper function ensures that the inputs tocomputeHashare properly converted to strings before hashing, handling cases where the inputs may be of different types.The code changes are well-structured and follow good practices.
Also applies to: 41-57
ui/src/app/lib/networkConfig.ts (1)
66-66: Verify the impact of updating thegameAddressin thekatanaenvironment.The
gameAddressproperty in thenetworkConfigobject for thekatanaenvironment has been updated to a new Ethereum address. This change suggests an update to the smart contract or network configuration that the application interacts with.To verify the impact of this change, consider the following:
- Ensure that the new
gameAddresscorresponds to the correct smart contract or network configuration that the application should interact with in thekatanaenvironment.- Review how the
gameAddressis utilized within the application and assess if any modifications are required in the codebase to accommodate the updated address.- Test the application thoroughly in the
katanaenvironment to confirm that the interactions with the blockchain, such as transactions or data retrievals, are functioning as expected with the newgameAddress.Provide more context on the reason for updating the
gameAddressand confirm that the necessary testing and verification steps have been performed to ensure a smooth transition.ui/src/app/hooks/useUIStore.ts (2)
145-146: LGTM!The code changes are approved. The new properties
freeVRFandsetFreeVRFare correctly added to theStatetype with appropriate types.
268-269: LGTM!The code changes are approved. The
freeVRFstate is correctly initialized tofalseand thesetFreeVRFfunction is provided to update it.scripts/katana/indexer/adventurers.ts (2)
85-85: LGTM!The code changes are approved. The
TRANSFERevent type is correctly added to thefilterobject to allow handling transfer events from theGAMEaddress.
344-353: LGTM, but verify the transfer handling.The code changes are approved. The handling of the
TRANSFERcase in thetransformfunction has been improved:
- Parsing the event data using
event.keys.slice(1)corrects how the event data is accessed.- Checking the
fromAddressbefore updating the ownership prevents updates when thefromAddressis invalid.- Deriving the
adventurerIdfromvalue.tokenIdcorrects how the adventurer's identifier is determined during a transfer.However, ensure that the updated transfer handling is thoroughly tested to confirm that it behaves as expected and handles various scenarios correctly.
Run the following script to verify the transfer handling:
Verification successful
Verification Successful: Transfer Handling in
adventurers.tsis CorrectThe handling of the
TRANSFERcase in thetransformfunction ofscripts/katana/indexer/adventurers.tshas been verified and is correctly implemented:
- The event data is parsed using
event.keys.slice(1).- The
fromAddressis checked to ensure it is not zero before updating ownership.- The
adventurerIdis derived fromvalue.tokenId.These changes align with the intended improvements and enhance the robustness of the transfer handling.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated transfer handling in the `transform` function. # Test: Search for the `TRANSFER` case handling. Expect: The updated code segment. rg --type typescript -A 10 $'case TRANSFER' # Test: Search for the usage of `event.keys.slice(1)`. Expect: Only occurrences in the `TRANSFER` case. rg --type typescript -A 5 $'event\.keys\.slice\(1\)' # Test: Search for the `fromAddress` check. Expect: The check is performed before updating the ownership. rg --type typescript -A 5 $'if \(parseInt\(value\.fromAddress\) !== 0\)' # Test: Search for the usage of `value.tokenId`. Expect: It is used to derive the `adventurerId`. rg --type typescript -A 5 $'adventurerId: value\.tokenId'Length of output: 371
Script:
#!/bin/bash # Description: Verify the updated transfer handling in the `transform` function. # Test: Search for the `TRANSFER` case handling. Expect: The updated code segment. rg -A 10 $'case TRANSFER' # Test: Search for the usage of `event.keys.slice(1)`. Expect: Only occurrences in the `TRANSFER` case. rg -A 5 $'event\.keys\.slice\(1\)' # Test: Search for the `fromAddress` check. Expect: The check is performed before updating the ownership. rg -A 5 $'if \(parseInt\(value\.fromAddress\) !== 0\)' # Test: Search for the usage of `value.tokenId`. Expect: It is used to derive the `adventurerId`. rg -A 5 $'adventurerId: value\.tokenId'Length of output: 7848
scripts/katana/indexer/items.ts (2)
303-317: LGTM!The code changes are approved. The handling of the
ITEMS_LEVELED_UPcase in thetransformfunction has been streamlined:
- Removing the conditional checks for
item.prefixesUnlockedanditem.suffixUnlockedeliminates redundancy and complexity.- Consistently including
special1,special2, andspecial3in the update object simplifies the code and ensures that the specials are always set based on the item's specials.
The changes enhance the clarity and maintainability of the code.
369-370: LGTM!The code changes are approved. The parameter passed to
updateItemsOwnerin theTRANSFERcase has been corrected:
- Deriving the
adventurerIdfromvalue.tokenIdinstead ofvalue.toAddresscorrects how the ownership is updated.- This change aligns with the updated transfer handling in the
adventurers.tsfile.scripts/katana/indexer/utils/events.ts (14)
12-12: LGTM!The addition of
parseU32to the list of imported parsers is approved.
50-50: LGTM!The addition of the
RECEIVED_LEVEL_SEEDevent constant is approved.
51-51: LGTM!The addition of the
NEW_COLLECTION_TOTALevent constant is approved.
55-55: LGTM!The addition of the
CLAIMED_FREE_GAMEevent constant is approved.
192-192: LGTM!The addition of the
launchTournamentWinnerTokenIdfield toparseAdventurerMetadatausing theparseU32parser is approved.
363-367: LGTM!The addition of the
parseReceivedLevelSeedparser is approved.
376-380: LGTM!The addition of the
parseClaimedFreeGameparser is approved.
382-385: LGTM!The addition of the
parseNewCollectionTotalparser is approved.
373-374: LGTM, but verify the data model change.The change from
amounttotokenIdinparseTransferis approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that relies on the
amountfield inparseTransferhas been updated to handle the newtokenIdfield.Run the following script to verify the data model change:
Verification successful
Change from
amounttotokenIdsuccessfully verified.The transition from
amounttotokenIdin theparseTransferfunction has been correctly implemented across the codebase. All relevant interactions now utilize thetokenIdfield, ensuring consistency and correctness in the data model.
- Verified in files:
items.ts,adventurers.ts,tokens.ts,beastTokens.ts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all interactions with `parseTransfer` handle the new data model. # Test: Search for interactions with `parseTransfer`. Expect: Only occurrences of `tokenId` instead of `amount`. rg --type typescript -A 5 $'parseTransfer'Length of output: 76
Script:
#!/bin/bash # Description: Verify all interactions with `parseTransfer` handle the new data model. # Test: Search for interactions with `parseTransfer`. Expect: Only occurrences of `tokenId` instead of `amount`. rg --type ts -A 5 $'parseTransfer'Length of output: 5398
199-199: LGTM, but verify the parameter reorganization.The change in the index for
customRendererfrom 4 to 3 is approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that relies on the order of parameters in
parseStartGamehas been updated to handle the new index.Run the following script to verify the parameter reorganization:
191-191: LGTM, but verify the data type change.The change in the parser for
goldenTokenIdfromparseU256toparseU8is approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that interacts with
goldenTokenIdhas been updated to handle the new data type.Run the following script to verify the data type change:
354-354: LGTM, but verify the data type change.The change in the parser for
daofromparseU256toparseU128is approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that interacts with the
daofield inparseRewardDistributionhas been updated to handle the new data type.Run the following script to verify the data type change:
340-340: LGTM, but verify the data type change.The change in the parser for
amountfromparseU256toparseU128is approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that interacts with the
amountfield inparsePlayerRewardhas been updated to handle the new data type.Run the following script to verify the data type change:
Verification successful
Verification Successful:
amountfield usesparseU128.The
amountfield inparsePlayerRewardis confirmed to be usingparseU128, as expected. Ensure that all related code is tested to verify compatibility with this change.
- Instances of
amountusingparseU128were found, confirming the change.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all interactions with `amount` in `parsePlayerReward` handle the new data type. # Test: Search for interactions with `amount` in `parsePlayerReward`. Expect: Only occurrences of `parseU128`. rg --type typescript -A 5 $'parsePlayerReward[^}]*amount'Length of output: 93
Script:
#!/bin/bash # Find all TypeScript files and search for interactions with `amount` in `parsePlayerReward`. fd --extension ts --exec rg -A 5 'parsePlayerReward[^}]*amount' {}Length of output: 70
Script:
#!/bin/bash # Use ast-grep to search for the `parsePlayerReward` function and verify the usage of `parseU128` with `amount`. ast-grep --lang typescript --pattern $'function parsePlayerReward($$$) { $$$ amount: { index: $_, parser: parseU128 }, $$$ }'Length of output: 132
Script:
#!/bin/bash # Search for the usage of `parseU128` within TypeScript files to verify its association with `amount`. fd --extension ts --exec rg 'parseU128' {}Length of output: 1248
345-345: LGTM, but verify the data type change.The change in the parser for
amountfromparseU256toparseU128is approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all code that interacts with the
amountfield inparseClientRewardhas been updated to handle the new data type.Run the following script to verify the data type change:
ui/src/app/lib/utils/syscalls.ts (5)
109-109: LGTM!The addition of the
freeVRFproperty to theSyscallsPropsinterface is approved.
228-228: LGTM!The addition of the
freeVRFproperty to the destructured parameters of thecreateSyscallsfunction is approved.
285-298: LGTM!The addition of the
checkBalanceshelper function is approved. It improves code readability and modularity by encapsulating the balance-checking logic.
300-407: LGTM!The addition of the
executeSpawnhelper function is approved. It improves code readability and maintainability by encapsulating the spawning logic and handling the transaction submission, event parsing, and data updates correctly.
456-473: LGTM!The changes to the
spawnfunction are approved. The utilization of thecheckBalancesandexecuteSpawnhelper functions improves the code structure and readability. The balance checks and handling of insufficient funds are implemented correctly. The addition of approval calls when thegoldenTokenIdis "0" ensures the necessary approvals are made before spawning the adventurer.scripts/katana/graphql/indexer/graphql.py (19)
56-62: LGTM!The new functions
parse_token_idandserialize_token_idare implemented correctly.
65-78: LGTM!The updates to the
serialize_stringfunction improve its robustness by handlingNonevalues and providing a fallback for decoding failures. The implementation looks good.
230-234: LGTM!The new scalar type
TokenIdValueis correctly defined using theparse_token_idandserialize_token_idfunctions.
393-404: LGTM!The new input class
HashFilteris correctly defined with appropriate fields for hash filtering.
451-471: LGTM!The new input class
TokenIdValueFilteris correctly defined with appropriate fields for filteringTokenIdValue.
Line range hint
1080-1103: LGTM!The new fields
launchTournamentWinnerTokenId,customRenderer,battleActionCount, andgoldare correctly added to theAdventurersFilterclass.
1119-1123: LGTM!The
launchTournamentWinnerTokenIdfield is correctly added to theto_dictmethod of theAdventurersFilterclass.
Line range hint
1234-1251: LGTM!The new field
poweris correctly added to theBeastsFilterclass.
1363-1374: LGTM!The new input class
CollectionTotalFilteris correctly defined with appropriate fields for filteringCollectionTotal.
1377-1394: LGTM!The new input class
TokensFilteris correctly defined with appropriate fields for filtering tokens.
1397-1414: LGTM!The new input class
BeastTokensFilteris correctly defined with appropriate fields for filtering beast tokens.
1417-1436: LGTM!The new input class
ClaimedFreeGamesFilteris correctly defined with appropriate fields for filtering claimed free games.
1439-1464: LGTM!The new input class
TokenWithFreeGameStatusFilteris correctly defined with appropriate fields for filtering tokens based on their free game status.
Line range hint
1496-1535: LGTM!The new fields
launchTournamentWinnerTokenId,customRenderer,battleActionCount, andgoldare correctly added to theAdventurersOrderByInputclass.
Line range hint
1650-1667: LGTM!The new field
poweris correctly added to theBeastsOrderByInputclass.
1781-1792: LGTM!The new input class
CollectionTotalOrderByInputis correctly defined with appropriate fields for orderingCollectionTotal.
1795-1812: LGTM!The new input class
TokensOrderByInputis correctly defined with appropriate fields for ordering tokens.
1815-1832: LGTM!The new input class
BeastTokensOrderByInputis correctly defined with appropriate fields for ordering beast tokens.
1835-1854: LGTM!The new input class
ClaimedFreeGamesOrderByInputis correctly defined with appropriate fields for ordering claimed free games.scripts/katana/indexer/utils/helpers.ts (7)
33-33: Verify the impact of the change in the handling of thenamefield.The handling of the new parameter
launchTournamentWinnerTokenIdlooks good.However, the change in the handling of the
namefield to convert it to a hexadecimal string representation usingBigIntmay affect how names are stored or processed.Please ensure that this change is intended and doesn't break any existing functionality. Verify that the rest of the codebase is updated to handle the new format of the
namefield.Also applies to: 71-71, 75-77
622-639: LGTM!The code changes in the
updateTokenOwnerfunction are approved.
641-658: LGTM!The code changes in the
updateBeastOwnerfunction are approved.
660-682: LGTM!The code changes in the
insertFreeGamefunction are approved.
684-700: LGTM!The code changes in the
updateClaimedFreeGamefunction are approved.
702-716: LGTM!The code changes in the
updateRevealedFreeGamefunction are approved.
718-733: LGTM!The code changes in the
updateCollectionTotalfunction are approved.ui/src/app/page.tsx (1)
161-162: LGTM!The code changes related to the management of the "free VRF" state in the
Homecomponent are approved.Also applies to: 680-687
| @@ -3,6 +3,7 @@ | |||
| import ssl | |||
| import json | |||
| import logging | |||
| import hashlib | |||
There was a problem hiding this comment.
Remove unused import.
The hashlib module is imported but not used in the code. Remove this import to keep the code clean.
-import hashlibCommittable 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.
| import hashlib |
Tools
Ruff
6-6:
hashlibimported but unusedRemove unused import:
hashlib(F401)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Chores