Skip to content

refactor: include Starknet version in BlockEnv#161

Merged
kariy merged 1 commit intomainfrom
refactor/starknet-version
Jul 7, 2025
Merged

refactor: include Starknet version in BlockEnv#161
kariy merged 1 commit intomainfrom
refactor/starknet-version

Conversation

@kariy
Copy link
Member

@kariy kariy commented Jul 7, 2025

No description provided.

@claude
Copy link
Contributor

claude bot commented Jul 7, 2025

Claude finished @kariy's task —— View job


Pull Request Review: Include Starknet version in BlockEnv

Review Checklist:

  • Analyze the core refactoring changes in primitives
  • Review propagation of changes across crates
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage
  • Provide overall code quality feedback
  • Include Marcus Aurelius passage

Overview

This PR refactors the codebase to include StarknetVersion in the BlockEnv struct, propagating this change across multiple crates. The refactoring touches 16 files and appears to be well-structured and consistent.

Core Changes Analysis

Clean Architecture & Design

The refactoring follows a clean architectural pattern:

  1. Core primitives update: Added starknet_version: StarknetVersion to BlockEnv in primitives/src/env.rs:22
  2. Version management: Enhanced StarknetVersion struct with proper parsing, display, and serialization
  3. Propagation pattern: Consistently updated all dependent structures and functions

Code Quality

Strengths:

  • Consistent naming: All new fields use starknet_version consistently
  • Type safety: Uses the custom StarknetVersion type instead of strings
  • Default handling: Proper default value (StarknetVersion::default() returns [0, 1, 0, 0])
  • Serialization: Human-readable string serialization with proper parsing

Well-implemented StarknetVersion features:

  • Smart Display implementation that omits trailing zeros ("1.2.3" instead of "1.2.3.0")
  • Robust parsing with validation (max 4 segments, proper error handling)
  • Both human-readable and binary serialization support

Propagation Consistency

The changes are systematically propagated across:

  • Storage layer: Database models updated (crates/storage/db/src/models/versioned/block/v6.rs)
  • RPC layer: All block types include version (crates/rpc/rpc-types/src/block.rs)
  • Execution layer: Executor tracks and uses version (crates/executor/src/implementation/blockifier/mod.rs)
  • Chain specs: Genesis blocks include version (crates/chain-spec/src/dev.rs)
  • Tests: Updated to include the new field

Potential Issues & Recommendations

⚠️ Minor Concerns

  1. Hardcoded Version in Pending Blocks:

    // crates/rpc/rpc/src/starknet/mod.rs:631
    starknet_version: CURRENT_STARKNET_VERSION,

    This hardcodes the version for pending blocks instead of using the version from the block environment. Consider using block_env.starknet_version for consistency.

  2. Missing Documentation: The new starknet_version field in BlockEnv could benefit from documentation explaining its purpose and when it should be set.

Performance Considerations

  • Minimal overhead: Adding a 4-byte array field has negligible performance impact
  • Efficient serialization: String-based serialization is reasonable for the version field
  • No breaking serialization: The changes maintain backward compatibility

Security Considerations

  • Input validation: Version parsing includes proper bounds checking (max 4 segments, u8 range)
  • No unsafe operations: All version handling uses safe Rust patterns
  • Error handling: Proper error types for invalid version formats

Test Coverage

  • Comprehensive tests: Version parsing, display, and serialization are well-tested
  • Integration tests: Storage and RPC tests updated to include the new field
  • Edge cases: Tests cover empty strings, too many segments, and various formats

Recommendations

  1. Consider using block_env.starknet_version instead of CURRENT_STARKNET_VERSION in pending block creation
  2. Add documentation to the starknet_version field in BlockEnv
  3. Ensure version consistency between BlockEnv and block headers in all code paths

Overall Assessment

This is a high-quality refactoring that demonstrates excellent software engineering practices:

  • ✅ Clean, systematic approach to a cross-cutting concern
  • ✅ Consistent naming and type usage
  • ✅ Proper error handling and validation
  • ✅ Comprehensive test coverage
  • ✅ Maintains backward compatibility
  • ✅ No security vulnerabilities introduced

The code is production-ready with only minor suggestions for improvement.


"Confine yourself to the present." 
- Marcus Aurelius, Meditations, Book VII

Just as we must focus on the present moment rather than be scattered across time, 
this refactoring demonstrates focused attention to the immediate need—
systematically including version information where it belongs, 
without introducing unnecessary complexity or scope creep.

@github-actions
Copy link

github-actions bot commented Jul 7, 2025

Binary size report 📊

Branch Size
main 64.08MiB
refactor/starknet-version 64.08MiB ( +0.00% )

@codecov
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

Attention: Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.38%. Comparing base (9bde0ae) to head (7c8fe7e).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/backend/mod.rs 66.66% 1 Missing ⚠️
crates/primitives/src/version.rs 92.85% 1 Missing ⚠️
crates/storage/db/src/models/versioned/block/v6.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #161      +/-   ##
==========================================
- Coverage   73.32%   72.38%   -0.94%     
==========================================
  Files         209      219      +10     
  Lines       23132    24134    +1002     
==========================================
+ Hits        16961    17469     +508     
- Misses       6171     6665     +494     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kariy kariy merged commit 2f2218f into main Jul 7, 2025
12 of 13 checks passed
@kariy kariy deleted the refactor/starknet-version branch July 7, 2025 09:36
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