Skip to content

feat: bump starknet-rs to 0.15.1#153

Merged
kariy merged 3 commits intomainfrom
chore/bump-starknet-0.15.1
Jul 2, 2025
Merged

feat: bump starknet-rs to 0.15.1#153
kariy merged 3 commits intomainfrom
chore/bump-starknet-0.15.1

Conversation

@glihm
Copy link
Contributor

@glihm glihm commented Jul 1, 2025

This PR aims at bumping starknet-rs to the latest stable version, to ease the migration of the whole stack to a common version.

@glihm glihm requested a review from kariy July 1, 2025 21:58
@claude
Copy link
Contributor

claude bot commented Jul 1, 2025

Claude finished @glihm's task —— View job


PR Review: Bump starknet-rs to 0.15.1

Progress Checklist:

  • Analyze Cargo.toml changes
  • Review Cargo.lock dependency updates
  • Examine API adaptation in error/starknet.rs
  • Check bin/katana/Cargo.toml changes
  • Assess security and performance implications
  • Provide comprehensive review feedback

Review Summary ✅

This is a well-executed dependency upgrade that properly handles API changes introduced in starknet-rs 0.15.1. The changes are focused, necessary, and maintain backward compatibility.

Code Quality & Best Practices ✅

Excellent error handling adaptation: The changes in crates/rpc/rpc-api/src/error/starknet.rs:267-278 properly handle new error variants:

  • NoTraceAvailable(_) → mapped to UnexpectedError
  • StorageProofNotSupported → properly mapped with default values
  • InsufficientResourcesForValidate → direct mapping
  • Additional subscription and filtering error variants handled correctly

Proper dependency coordination: The piltover dependency update in bin/katana/Cargo.toml:28 shows good awareness of the dependency chain, updating to a revision that supports starknet 0.15.1.

Potential Issues & Concerns ⚠️

Minor: Default value handling: In lines 270-271 of error/starknet.rs, the StorageProofNotSupported error mapping uses default values (0, 0) for oldest_block and requested_block. While this maintains compilation compatibility, the error context is lost. Consider either:

  • Logging the original error for debugging
  • Adding a comment explaining why defaults are used

Transitive dependency versioning: The Cargo.lock shows some starknet dependencies still on older versions (e.g., starknet 0.13.0 in account_sdk). This is expected for external dependencies but worth monitoring for future compatibility.

Security Considerations ✅

  • No security regressions identified: The error handling changes are defensive and don't expose sensitive information
  • Dependency upgrade follows semantic versioning: 0.15.1 is a patch/minor version that shouldn't introduce breaking security changes
  • Error mappings preserve security boundaries: No sensitive data leakage in error conversions

Performance Considerations ✅

  • No performance regressions expected: Version 0.15.1 likely includes performance improvements
  • Error handling remains efficient: The new error mappings are O(1) operations
  • Cargo.lock changes indicate optimized dependency resolution

Test Coverage 🔍

Positive: The existing comprehensive test suite in error/starknet.rs:300-448 covers error conversion scenarios and should catch any regressions.

Suggestion: Consider adding specific test cases for the newly added error variants (lines 267-278) to ensure they're properly handled in all scenarios.

Final Recommendation: ✅ APPROVE

This is a solid dependency upgrade that:

  • Properly handles API changes with comprehensive error mapping
  • Maintains backward compatibility
  • Follows Rust best practices for error handling
  • Coordinates related dependency updates (piltover)
  • Includes thorough testing infrastructure

The minor issue with default values in error mapping doesn't affect functionality and can be addressed in a follow-up if needed.


"Accept the things to which fate binds you, and love the people with whom fate brings you together."
- Marcus Aurelius, Meditations

Comment on lines +138 to +140
StarknetApiError::InvalidSubscriptionId => 66,
StarknetApiError::TooManyAddressesInFilter => 67,
StarknetApiError::TooManyBlocksBack => 68,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link

github-actions bot commented Jul 1, 2025

Binary size report 📊

Branch Size
main 66.29MiB
chore/bump-starknet-0.15.1 64.07MiB ( -3.34% )

@claude
Copy link
Contributor

claude bot commented Jul 1, 2025

Claude finished @glihm's task —— View job


PR Review: Bump starknet-rs to 0.15.1

Progress Checklist:

  • Analyze Cargo.toml and Cargo.lock changes
  • Review error handling adaptations in starknet.rs
  • Check trace and blockifier integration changes
  • Assess security and performance implications
  • Provide comprehensive review feedback

Review Summary ✅

This is a well-executed dependency upgrade that properly handles API changes introduced in starknet-rs 0.15.1. The changes are focused, necessary, and maintain backward compatibility while enabling the project to benefit from the latest starknet-rs improvements.

Code Quality & Best Practices ✅

Excellent error handling adaptation: The changes in crates/rpc/rpc-api/src/error/starknet.rs:267-278 properly handle new error variants introduced in starknet-rs 0.15.1:

  • NoTraceAvailable(_) → mapped to UnexpectedError with descriptive message
  • StorageProofNotSupported → properly mapped with default values
  • InsufficientResourcesForValidate → direct mapping
  • Additional subscription and filtering error variants handled correctly

Proper dependency coordination: The piltover dependency update in bin/katana/Cargo.toml:28 shows good awareness of the dependency chain, updating to revision 3bed7ac554259668dbdce6a5f56de5b2bf7faf43 that supports starknet 0.15.1.

Consistent type usage: The trace and blockifier files show appropriate usage of updated starknet types, maintaining API compatibility.

Potential Issues & Concerns ⚠️

Minor: Default value handling: In lines 270-271 of error/starknet.rs, the StorageProofNotSupported error mapping uses default values (0, 0) for oldest_block and requested_block. While this maintains compilation compatibility, the original error context is lost. Consider:

  • Adding a comment explaining why defaults are used
  • Logging the original error details if debugging is needed

Dependency versioning consistency: The workspace dependencies show starknet updated to 0.15.1, but some transitive dependencies may still be on older versions. This is expected but worth monitoring for future compatibility.

Security Considerations ✅

  • No security regressions identified: The error handling changes are defensive and don't expose sensitive information
  • Dependency upgrade follows semantic versioning: 0.15.1 should be backward compatible with proper error handling
  • Error mappings preserve security boundaries: No sensitive data leakage in error conversions
  • Default values in error handling are safe: Using 0 values for block numbers in error cases doesn't create security vulnerabilities

Performance Considerations ✅

  • No performance regressions expected: Version 0.15.1 likely includes performance improvements from the starknet-rs team
  • Error handling remains efficient: The new error mappings are O(1) operations with minimal overhead
  • Binary size improvement: GitHub Actions report shows a 3.34% reduction in binary size, indicating improved efficiency
  • Type conversions optimized: The trace and blockifier changes maintain efficient type conversions

Test Coverage 🔍

Positive: The existing comprehensive test suite in error/starknet.rs:300-448 covers error conversion scenarios and should catch any regressions from the API changes.

Suggestion: Consider adding specific test cases for the newly added error variants (lines 267-278) to ensure they're properly handled in all scenarios, particularly for the new subscription and filtering errors.

Architecture & Design ✅

The changes properly follow Katana's architecture patterns:

  • Error handling through the provider abstraction layer
  • Proper separation of concerns between RPC types and core logic
  • Consistent use of workspace dependency management
  • No breaking changes to public APIs

Final Recommendation: ✅ APPROVE

This is a solid dependency upgrade that:

  • Properly handles API changes with comprehensive error mapping
  • Maintains backward compatibility for Katana users
  • Follows Rust best practices for error handling and dependency management
  • Coordinates related dependency updates appropriately
  • Includes robust testing infrastructure
  • Results in improved binary size and likely performance

The minor issue with default values in error mapping doesn't affect functionality and demonstrates defensive programming practices.


"Loss is nothing else but change, and change is Nature's delight."
- Marcus Aurelius, Meditations

@codecov
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

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

Project coverage is 72.29%. Comparing base (9bde0ae) to head (34dce2a).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crates/rpc/rpc-api/src/error/starknet.rs 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
- Coverage   73.32%   72.29%   -1.03%     
==========================================
  Files         209      212       +3     
  Lines       23132    23461     +329     
==========================================
+ Hits        16961    16962       +1     
- Misses       6171     6499     +328     

☔ 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.

@claude
Copy link
Contributor

claude bot commented Jul 2, 2025

Claude finished @glihm's task —— View job


PR Review: Bump starknet-rs to 0.15.1

Progress Checklist:

  • Read and analyze Cargo.toml dependency changes
  • Review Cargo.lock for dependency resolution issues
  • Examine error handling adaptations in starknet.rs
  • Check trace and blockifier type updates
  • Assess security implications of the upgrade
  • Evaluate performance considerations
  • Review test coverage concerns (codecov report)
  • Provide comprehensive review feedback

Review Summary ✅

This is a well-executed dependency upgrade that properly handles API changes introduced in starknet-rs 0.15.1. The changes are focused, necessary, and maintain backward compatibility while enabling the project to benefit from the latest starknet-rs improvements.

Code Quality & Best Practices ✅

Excellent error handling adaptation: The changes in crates/rpc/rpc-api/src/error/starknet.rs:267-278 properly handle new error variants introduced in starknet-rs 0.15.1:

  • NoTraceAvailable(_) → mapped to UnexpectedError with descriptive message
  • StorageProofNotSupported → properly mapped with default values
  • InsufficientResourcesForValidate → direct mapping
  • Additional subscription and filtering error variants handled correctly

Proper dependency coordination: The piltover dependency update in bin/katana/Cargo.toml:28 shows good awareness of the dependency chain, updating to revision 3bed7ac554259668dbdce6a5f56de5b2bf7faf43 that supports starknet 0.15.1.

Consistent type usage: The trace and blockifier files show appropriate usage of updated starknet types, maintaining API compatibility.

Potential Issues & Concerns ⚠️

Minor: Default value handling: In lines 270-271 of error/starknet.rs, the StorageProofNotSupported error mapping uses default values (0, 0) for oldest_block and requested_block. While this maintains compilation compatibility, the original error context is lost. Consider:

  • Adding a comment explaining why defaults are used
  • Logging the original error details if debugging is needed

Dependency versioning consistency: The workspace dependencies show starknet updated to 0.15.1, but some transitive dependencies may still be on older versions (e.g., account_sdk using starknet 0.13.0). This is expected but worth monitoring for future compatibility.

Security Considerations ✅

  • No security regressions identified: The error handling changes are defensive and don't expose sensitive information
  • Dependency upgrade follows semantic versioning: 0.15.1 should be backward compatible with proper error handling
  • Error mappings preserve security boundaries: No sensitive data leakage in error conversions
  • Default values in error handling are safe: Using 0 values for block numbers in error cases doesn't create security vulnerabilities

Performance Considerations ✅

  • No performance regressions expected: Version 0.15.1 likely includes performance improvements from the starknet-rs team
  • Error handling remains efficient: The new error mappings are O(1) operations with minimal overhead
  • Binary size improvement: GitHub Actions report shows a 3.34% reduction in binary size, indicating improved efficiency
  • Type conversions optimized: The trace and blockifier changes maintain efficient type conversions

Test Coverage 🔍

Positive: The existing comprehensive test suite in error/starknet.rs:300-448 covers error conversion scenarios and should catch any regressions from the API changes.

Suggestion: Consider adding specific test cases for the newly added error variants (lines 267-278) to ensure they're properly handled in all scenarios, particularly for the new subscription and filtering errors. The codecov report shows 6 missing lines which are likely these new error mappings.

Architecture & Design ✅

The changes properly follow Katana's architecture patterns:

  • Error handling through the provider abstraction layer
  • Proper separation of concerns between RPC types and core logic
  • Consistent use of workspace dependency management
  • No breaking changes to public APIs

Final Recommendation: ✅ APPROVE

This is a solid dependency upgrade that:

  • Properly handles API changes with comprehensive error mapping
  • Maintains backward compatibility for Katana users
  • Follows Rust best practices for error handling and dependency management
  • Coordinates related dependency updates appropriately
  • Includes robust testing infrastructure
  • Results in improved binary size and likely performance

The minor issue with default values in error mapping doesn't affect functionality and demonstrates defensive programming practices.


"Loss is nothing else but change, and change is Nature's delight."
- Marcus Aurelius, Meditations

Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the message for the new errors to match the spec.

Otherwise, lgtm. Thanks!

@kariy kariy merged commit eaea715 into main Jul 2, 2025
13 of 14 checks passed
@kariy kariy deleted the chore/bump-starknet-0.15.1 branch July 2, 2025 04:08
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.

2 participants