feat(sozo): remove the need of world_block #2863
Conversation
WalkthroughOhayo, sensei! The changes in the Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
Documentation and Community
|
![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
🧹 Nitpick comments (4)
bin/sozo/src/commands/events.rs (4)
68-70: Ohayo sensei, consider documentingMAX_BLOCK_RANGE.
DefiningMAX_BLOCK_RANGEat 50,000 is good, but a quick doc comment explaining the rationale would help maintainers.+/// Maximum number of blocks to query in a single request const MAX_BLOCK_RANGE: u64 = 50_000;
302-303: Ohayo sensei, verify error handling formodel_get.
Consider how you’ll handle potential model retrieval failures. Right now, it looks good, but if an error occurs, it might be good to log it or bubble it up.
338-338: Ohayo sensei, keep watch on the comment.
Looks like a placeholder comment for printing. If you need more data, consider implementing it soon.
357-357: Ohayo sensei, next steps for pretty print.
Same as above, a placeholder comment for printing the value. Let me know if you want some help implementing it.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bin/sozo/src/commands/events.rs(13 hunks)
🔇 Additional comments (21)
bin/sozo/src/commands/events.rs (21)
7-7: Ohayo sensei, nice import!
This import forEvent as WorldEventis straightforward and helps bring clarity to event usage.
12-12: Ohayo sensei, imports look good!
These additional imports help streamline access to Starknet types.
60-64: Ohayo sensei, great job retrieving the provider!
The call toutils::get_world_diff_and_provideris well-structured and keeps the code clean.
71-79: Ohayo sensei, watch out for the placeholder call.
self.get_current_block()is commented as a placeholder. Ensure the actual logic to fetch the current block is implemented.
81-81: Ohayo sensei, explicitNoneis clear.
ReturningNonewhenfrom_blockis not set is straightforward and matches expectations.
84-91: Ohayo sensei, check the shape of generated keys.
Wrapping the eventmapcall in an extra vector dimension might be intentional, but validate if your event filtering logic expectsVec<Vec<Felt>>or a single level.
100-104: Ohayo sensei, good usage of pagination.
Leveragingchunk_sizeandcontinuation_tokenwill help handle large event sets gracefully.
114-115: Ohayo sensei, good error handling.
Unwrapping the result with a log statement ensures we get immediate feedback if something fails.
144-144: Ohayo sensei, generic constraints look neat!
SpecifyingP: Provider + Send + Syncis precise for async contexts.
149-153: Ohayo sensei, convenient reverse mapping.
Storingcontract_selectors_from_addressis a great optimization for quick lookups.
166-174: Ohayo sensei, consistent format forWorldSpawned.
Displayingcreatorandclass_hashin a single format block is clear.
181-190: Ohayo sensei, strong clarity forModelRegistered.
Nicely done listing namespace, name, class hash, and address.
192-201: Ohayo sensei, uniform approach forEventRegistered.
Mirroring theModelRegisteredformat keeps events consistent.
203-213: Ohayo sensei, good detail forContractRegistered.
Providing namespace, name, and salt helps with debugging.
222-225: Ohayo sensei, graceful logging inModelUpgraded.
Including both the new and previous addresses is crucial for version tracking.
236-239: Ohayo sensei, consistent pattern inEventUpgraded.
Matches the structure used for other upgraded event logs.
247-247: Ohayo sensei, succinct formatting forContractUpgraded.
Concise yet informative about the updated class hash.
267-273: Ohayo sensei, good fallback for unknown writergrantee.
If the address is not recognized, showing the raw hex string is a helpful fallback.
282-288: Ohayo sensei, consistent fallback for unknown ownergrantee.
Reusing the same approach for owner updates is a nice parallel to writer updates.
365-365: Ohayo sensei, minimal but informative forStoreDelRecord.
The message is short and sweet, though consider adding context if needed.
Line range hint
370-395: Ohayo sensei, inclusive approach forEventEmitted.
The usage of contract tags or fallback hex displays is consistent with the rest of the event handling. Nicely done!
There was a problem hiding this comment.
Thanks for the contribution here, however this is not solving the issue.
In the current fixed proposed, we only set the from_block, but then the get_event function will still be called once, and not iteratively.
So having a max block range is good approach, but now this must be done iteratively for all the chunks that will be generated.
Also, please check the scripts into scripts folder to format correctly the code and avoid diffs related to linters. 👍
glihm
changed the title
Okay, I will fix it |
Description
sozo:remove the need of world-block
Related issue
Fixes #2825.
Tests
Added to documentation?
Checklist
scripts/prettier.sh,scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit
Summary by CodeRabbit