Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(katana): per transaction validator #2362

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Conversation

kariy
Copy link
Member

@kariy kariy commented Aug 29, 2024

Summary by CodeRabbit

  • New Features

    • Enhanced configurability of the Starknet configuration by explicitly setting the sequencer address.
    • Introduced a new function to split Felt values into lower and upper bits.
    • Added a new asynchronous test to validate the state of the StarkNet validator.
  • Bug Fixes

    • Improved transaction validation logic to ensure proper handling of account balance constraints.
  • Documentation

    • Updated documentation comments for various components by removing lint suppression attributes.
  • Refactor

    • Streamlined nonce retrieval logic in the Starknet API.
    • Enhanced thread safety in the transaction validator by using a mutex.

Copy link

coderabbitai bot commented Aug 29, 2024

Walkthrough

Ohayo, sensei! The recent changes involve various modifications across multiple files, focusing on enhancing configuration management, updating documentation, and improving transaction validation logic. Key adjustments include refining configuration settings, altering data structures for better thread safety, and adding new testing functionalities. Additionally, some Clippy lint suppression directives have been removed to improve documentation quality.

Changes

Files Change Summary
crates/dojo-test-utils/src/sequencer.rs Modified get_default_test_starknet_config to set cfg.genesis.sequencer_address.
crates/dojo-utils/src/tx/waiter.rs, crates/dojo-world/src/config/profile_config.rs Removed #[allow(clippy::too_long_first_doc_paragraph)] from documentation comments.
crates/katana/core/src/service/block_producer.rs Removed reference operator when passing block_env to TxValidator methods.
crates/katana/pool/src/validation/stateful.rs Updated TxValidator to use Arc<Mutex<Inner>>, added pool_nonces, and modified several methods.
crates/katana/rpc/rpc/src/starknet/mod.rs Refactored nonce retrieval logic in StarknetApi for better readability.
crates/katana/rpc/rpc/tests/common/mod.rs Added split_felt function to split a Felt into lower and upper 128 bits.
crates/katana/rpc/rpc/tests/starknet.rs Introduced ensure_validator_have_valid_state async test function for validating validator state.
crates/katana/tasks/src/lib.rs, crates/sozo/ops/src/model.rs Removed #[allow(clippy::too_long_first_doc_paragraph)] from documentation comments.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TxValidator
    participant BlockEnv
    participant StateProvider

    User->>TxValidator: Create new validator
    TxValidator->>BlockEnv: Pass block environment
    TxValidator->>StateProvider: Retrieve state
    TxValidator->>User: Return validator instance

    User->>TxValidator: Update state
    TxValidator->>BlockEnv: Update with new environment
    TxValidator->>StateProvider: Update state
    TxValidator->>User: State updated
Loading

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b17a0f3 and cd6312f.

Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing (10)
  • crates/dojo-test-utils/src/sequencer.rs (2 hunks)
  • crates/dojo-utils/src/tx/waiter.rs (1 hunks)
  • crates/dojo-world/src/config/profile_config.rs (1 hunks)
  • crates/katana/core/src/service/block_producer.rs (5 hunks)
  • crates/katana/pool/src/validation/stateful.rs (3 hunks)
  • crates/katana/rpc/rpc/src/starknet/mod.rs (1 hunks)
  • crates/katana/rpc/rpc/tests/common/mod.rs (1 hunks)
  • crates/katana/rpc/rpc/tests/starknet.rs (2 hunks)
  • crates/katana/tasks/src/lib.rs (1 hunks)
  • crates/sozo/ops/src/model.rs (1 hunks)
Files skipped from review due to trivial changes (2)
  • crates/dojo-world/src/config/profile_config.rs
  • crates/katana/tasks/src/lib.rs
Files skipped from review as they are similar to previous changes (7)
  • crates/dojo-test-utils/src/sequencer.rs
  • crates/dojo-utils/src/tx/waiter.rs
  • crates/katana/core/src/service/block_producer.rs
  • crates/katana/rpc/rpc/src/starknet/mod.rs
  • crates/katana/rpc/rpc/tests/common/mod.rs
  • crates/katana/rpc/rpc/tests/starknet.rs
  • crates/sozo/ops/src/model.rs
Additional comments not posted (8)
crates/katana/pool/src/validation/stateful.rs (8)

1-1: Ohayo, sensei! Imports look good.

The new imports for HashMap and Mutex are necessary for the added functionality and thread safety.


30-31: Ohayo, sensei! Thread safety improvements look good.

The TxValidator struct now includes inner and permit fields wrapped in Arc<Mutex<>>, enhancing thread safety.


35-41: Ohayo, sensei! Nonce management improvements look good.

The Inner struct now includes a pool_nonces field, allowing for the storage and management of nonces associated with contract addresses.


49-59: Ohayo, sensei! Initialization improvements look good.

The new method now initializes the Inner struct with the pool_nonces field and accepts a BlockEnv directly, improving encapsulation.


64-67: Ohayo, sensei! State update improvements look good.

The update method now locks the inner mutex and updates the block_env and state fields directly, ensuring thread safety.


75-80: Ohayo, sensei! Nonce retrieval improvements look good.

The pool_nonce method retrieves the nonce from the pool_nonces map or falls back to the state provider, enhancing robustness.


100-145: Ohayo, sensei! Transaction validation improvements look good.

The validate method now utilizes the prepare method of the Inner struct to create a stateful validator, and the logic for handling transaction nonces has been refined, improving the overall transaction validation flow.


149-168: Ohayo, sensei! Validation function improvements look good.

The validate function correctly handles different types of transactions and maps validation errors appropriately.


Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 97.36842% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.49%. Comparing base (13cee86) to head (cd6312f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crates/katana/pool/src/validation/stateful.rs 96.72% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2362      +/-   ##
==========================================
- Coverage   67.49%   67.49%   -0.01%     
==========================================
  Files         359      359              
  Lines       46971    46965       -6     
==========================================
- Hits        31704    31698       -6     
  Misses      15267    15267              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kariy kariy merged commit adbaad7 into main Aug 29, 2024
14 of 15 checks passed
@kariy kariy deleted the katana/validator-sync-fix branch August 29, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant