Skip to content

feat(utils): mock provider builder macro#158

Merged
kariy merged 3 commits intomainfrom
mock-provider-macro
Jul 4, 2025
Merged

feat(utils): mock provider builder macro#158
kariy merged 3 commits intomainfrom
mock-provider-macro

Conversation

@kariy
Copy link
Member

@kariy kariy commented Jul 4, 2025

Procedural macro to easily create a mock implementation of starknet::provider::Provider trait with as minimal boilerplate as possible.

@claude
Copy link
Contributor

claude bot commented Jul 4, 2025

Claude finished @kariy's task —— View job


Mock Provider Builder Macro - Code Review

Review Checklist:

  • Read and analyze core macro implementation
  • Review test coverage and quality
  • Examine examples and documentation
  • Check for code quality and best practices
  • Identify potential bugs or issues
  • Evaluate performance considerations
  • Look for security concerns
  • Assess macro hygiene and safety
  • Provide comprehensive feedback

Summary

This PR introduces a well-designed procedural macro for creating mock implementations of the Starknet Provider trait. The implementation is solid, comprehensive, and follows Rust best practices. The macro significantly reduces boilerplate for testing by allowing users to implement only the methods they need while automatically providing unimplemented!() stubs for the rest.

Code Quality Assessment

Strengths:

  • Clear Architecture: Clean separation between parsing, generation, and trait implementation
  • Comprehensive Coverage: Supports all 33 methods from the Provider trait
  • Flexible Parameter Names: Allows custom parameter names while maintaining type safety
  • Good Error Handling: Proper error propagation and descriptive panic messages
  • Well-Structured Code: Logical organization with clear functions and data structures

Areas for Improvement:

  • Hardcoded Method Definitions: The get_all_provider_methods() function hardcodes all method signatures, which could become brittle if the Provider trait evolves
  • Large Function: The generate_custom_params() function is quite long (552 lines) and could benefit from refactoring

Potential Issues

  1. Maintenance Burden: The hardcoded method definitions in mock_provider.rs:191-397 need manual updates when the Provider trait changes. Consider using trait introspection or code generation from the actual trait.

  2. Parameter Count Validation: The macro doesn't validate that the number of parameters matches the expected count for each method. This could lead to confusing compilation errors.

  3. Limited Error Context: Parse errors could be more descriptive about what went wrong in the macro syntax.

Performance Considerations

  • Compile-time Impact: The macro generates significant code but only at compile time
  • Runtime Performance: Generated code is optimal with no runtime overhead
  • Memory Usage: Generated structs are minimal (zero-sized types with derive traits)

Security Assessment

✅ No Security Concerns Identified

  • No unsafe code
  • No network operations or file system access
  • No credential handling or sensitive data processing
  • Proper input validation in parsing logic

Test Coverage

Excellent Test Coverage:

  • Basic functionality tests
  • Custom parameter name validation
  • Multiple mock provider scenarios
  • Unimplemented method behavior
  • Default/Debug/Clone trait implementations
  • Edge cases with underscore parameters

Minor Gap: Missing tests for error conditions (invalid syntax, wrong parameter counts)

Documentation Quality

Comprehensive Documentation:

  • Clear README with examples
  • Inline code documentation
  • Usage examples showing both basic and advanced scenarios
  • Good explanation of generated code structure

Specific Code Review Notes

crates/utils/macro/src/mock_provider.rs

Lines 191-397: The hardcoded method definitions are accurate but fragile:

// This approach works but requires manual maintenance
ProviderMethod {
    name: syn::parse_str("get_storage_at").unwrap(),
    params: quote! { <A, K, B>(&self, contract_address: A, key: K, block_id: B) },
    // ...
}

Lines 400-552: The parameter mapping function is comprehensive but could be simplified:

// Consider using a macro or data structure to reduce repetition
match method_name_str.as_str() {
    "get_storage_at" => {
        // Repetitive pattern that could be abstracted
    }
}

crates/utils/macro/src/lib.rs

Lines 7-53: Clean public API with good documentation examples.

Recommendations

  1. Consider Trait Introspection: Explore using syn to parse the actual Provider trait definition instead of hardcoding signatures
  2. Parameter Validation: Add validation to ensure parameter counts match expectations
  3. Error Messages: Improve parse error messages with suggestions for correct syntax
  4. Refactor Large Functions: Break down generate_custom_params() into smaller, focused functions
  5. Add Error Tests: Include tests for invalid macro syntax to ensure good error messages

Architecture Praise

The dependency cleanup (removing katana-utils from katana-gas-oracle) is excellent - test utilities should not be dependencies in production code. This change improves the separation of concerns.

Overall Assessment

This is a high-quality implementation that provides significant value for testing. The macro is well-designed, thoroughly tested, and properly documented. While there are opportunities for improvement around maintainability and error handling, the core functionality is solid and ready for production use.

Recommendation: ✅ Approve with minor suggestions

The implementation demonstrates strong Rust macro development practices and will significantly improve the testing experience for Provider implementations.


"Be like the rocky headland on which the waves constantly break. 
It stands firm, and round it the seething waters are laid to rest."
- Marcus Aurelius, Meditations

@github-actions
Copy link

github-actions bot commented Jul 4, 2025

Binary size report 📊

Branch Size
main 64.08MiB
mock-provider-macro 64.08MiB ( -0.00% )

@codecov
Copy link

codecov bot commented Jul 4, 2025

Codecov Report

Attention: Patch coverage is 83.70536% with 73 lines in your changes missing coverage. Please review.

Project coverage is 72.27%. Comparing base (9bde0ae) to head (824c6f1).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
crates/utils/macro/src/mock_provider.rs 83.59% 73 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #158      +/-   ##
==========================================
- Coverage   73.32%   72.27%   -1.05%     
==========================================
  Files         209      219      +10     
  Lines       23132    24118     +986     
==========================================
+ Hits        16961    17432     +471     
- Misses       6171     6686     +515     

☔ 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 9b3b8fe into main Jul 4, 2025
12 of 13 checks passed
@kariy kariy deleted the mock-provider-macro branch July 4, 2025 16:47
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