chore(katana): address! macro for creating ContractAddress instance
#2475
Conversation
WalkthroughOhayo, sensei! The changes in this pull request focus on standardizing the instantiation of contract addresses and felt values across various test files by replacing previous methods with new macros ( Changes
Possibly related PRs
Suggested reviewers
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.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (9)
crates/katana/primitives/src/contract.rs (1)
53-58: Ohayo! Theaddress!macro is a fantastic addition, sensei!This macro will greatly simplify the creation of
ContractAddressinstances throughout the codebase. It ensures that addresses are always normalized, which is a great practice for consistency.Consider adding a brief doc comment above the macro to explain its purpose and usage. For example:
/// Creates a normalized `ContractAddress` from the given value. /// /// # Example /// ``` /// let address = address!(0x1234); /// ``` #[macro_export] macro_rules! address { // ... (existing implementation) }This documentation will help other developers understand and use the macro correctly.
crates/katana/storage/provider/tests/fixtures.rs (1)
82-83: Consider applying theaddress!macro consistently, sensei.The
address!macro usage here is great. To maintain consistency, it might be worth checking if there are other instances in this file or related files whereContractAddress::from(felt!())is used and could be replaced withaddress!().crates/katana/executor/tests/fixtures/mod.rs (1)
171-173: Ohayo once more, sensei! Great consistency in address handling!The use of the
address!macro here is consistent with the previous change, which is excellent. It simplifies the code while maintaining the same address value. Well done!A small suggestion to further improve readability:
Consider aligning the closing parenthesis with the
contract_addressfield for better visual structure:contract_address: address!( "0x3ddfa445a70b927497249f94ff7431fc2e2abc761a34417fd4891beb7c2db85" ),This minor adjustment can make the code even more pleasing to the eye. What do you think, sensei?
crates/katana/storage/db/src/tables.rs (1)
287-287: Ohayo, sensei! Nice addition of theaddressimport.The new import looks good and is necessary for the changes in the test cases. However, to improve code organization, consider grouping it with other related imports from
katana_primitives.You might want to move this import up with the other
katana_primitivesimports for better readability:use katana_primitives::block::{BlockHash, BlockNumber, FinalityStatus, Header}; use katana_primitives::class::{ClassHash, CompiledClass, CompiledClassHash}; use katana_primitives::contract::{ContractAddress, GenericContractInfo}; +use katana_primitives::address; use katana_primitives::fee::TxFeeInfo;crates/saya/core/src/prover/program_input.rs (3)
Line range hint
499-586: Ohayo, sensei! LGTM with a small suggestion.The
test_deserialize_inputfunction looks great! The use ofaddress!andfelt!macros improves readability and consistency. The test covers various aspects of the deserialization process and thefill_damethod.Consider adding a comment explaining the purpose of the
fill_damethod call and its expected behavior, as it might not be immediately clear to someone unfamiliar with the code.
588-639: Ohayo again, sensei! This test is looking sharp!The
test_serialize_inputfunction is well-crafted, testing both serialization and deserialization of theProgramInputstruct. The use ofaddress!andfelt!macros enhances readability and consistency.To make the test even more robust, consider adding assertions for individual fields of the deserialized struct. This would help pinpoint any specific fields that might not serialize/deserialize correctly in the future.
Line range hint
641-683: Ohayo once more, sensei! This test is on the right track, but could use some polishing.The
test_serialize_to_prover_argsfunction is testing an important functionality and is structured well. However, there are a few suggestions to enhance its clarity and maintainability:
- Consider breaking down the large JSON input into smaller, more manageable pieces or using a builder pattern to construct the test input.
- The expected vector creation could be more explicit about why certain
u64values are used. Consider using named constants or adding comments to explain the correspondence with the input.- Add a comment explaining the purpose of the
fill_dacall and its impact on the serialization process.Here's a sketch of how you might refactor the expected vector creation:
let expected = vec![ felt!("0x65"), // prev_state_root felt!("102"), // block_number felt!("0x67"), // block_hash felt!("0x68"), // config_hash felt!("1"), // nonce_updates length felt!("0x457"), // nonce_update key felt!("0x56ce"),// nonce_update value // ... continue with other fields ... ];This approach makes it clearer how each value in the expected vector corresponds to the input JSON.
crates/katana/primitives/src/genesis/mod.rs (1)
420-420: Excellent consistency, sensei! One small suggestion though.The
address!macro is used consistently here for both the UniversalDeployerConfig and the sequencer_address. Great job maintaining readability!As a tiny nitpick, consider using hexadecimal notation for all address literals to maintain consistency. For example, change
address!("0x100")toaddress!("0x0100").- sequencer_address: address!("0x100"), + sequencer_address: address!("0x0100"),Also applies to: 433-433
crates/katana/storage/provider/src/providers/db/mod.rs (1)
899-900: Ohayo! Great job on consistent macro usage, sensei!The test functions
insert_blockandstorage_updated_correctlyhave been updated to useaddress!andfelt!macros, which is consistent with the changes in the dummy state update functions. This improves the readability and maintainability of the tests.For even better consistency, consider using the
felt!macro for theclass_hash_of_contractcalls as well. For example:let class_hash1 = state_prov.class_hash_of_contract(address!("1")).unwrap().unwrap();This small change would make the usage of macros uniform throughout the test functions.
Also applies to: 910-911, 994-995, 1000-1001
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (13)
- bin/saya/src/tests.rs (3 hunks)
- crates/katana/executor/src/implementation/blockifier/state.rs (7 hunks)
- crates/katana/executor/tests/executor.rs (2 hunks)
- crates/katana/executor/tests/fixtures/mod.rs (3 hunks)
- crates/katana/primitives/src/contract.rs (2 hunks)
- crates/katana/primitives/src/genesis/json.rs (9 hunks)
- crates/katana/primitives/src/genesis/mod.rs (6 hunks)
- crates/katana/primitives/src/lib.rs (1 hunks)
- crates/katana/storage/db/src/mdbx/mod.rs (3 hunks)
- crates/katana/storage/db/src/tables.rs (2 hunks)
- crates/katana/storage/provider/src/providers/db/mod.rs (6 hunks)
- crates/katana/storage/provider/tests/fixtures.rs (2 hunks)
- crates/saya/core/src/prover/program_input.rs (3 hunks)
Additional comments not posted (37)
crates/katana/primitives/src/lib.rs (1)
22-24: Ohayo, sensei! These changes look great!The new exports enhance the functionality of the Katana primitives module:
ContractAddress: This provides a dedicated type for contract addresses, improving type safety and clarity in the codebase.feltmacro: This utility fromstarknet::macroslikely simplifies working with field elements, potentially making the code more readable and less error-prone.The existing exports for
FeltandFromStrErrorare maintained, ensuring backward compatibility.These additions align well with the PR objective of standardizing the instantiation of contract addresses and felt values. Great job, sensei!
crates/katana/primitives/src/contract.rs (2)
17-19: Ohayo, sensei! LGTM on theContractAddressstruct changes!The removal of the
#[deref]attribute from the struct field is a good move. This change allows for more explicit control over the dereferencing behavior, which will be implemented manually later in the file.
27-33: Excellent manual implementation ofDeref, sensei!This explicit
Derefimplementation forContractAddressis a great addition. It provides more control over the dereferencing behavior and makes the code's intent clearer. The implementation correctly returns a reference to the innerFeltvalue, maintaining the expected functionality.crates/katana/storage/provider/tests/fixtures.rs (1)
5-5: Ohayo, sensei! Nice macro usage for address creation.The change from
ContractAddress::from(felt!("1"))toaddress!("1")is a great improvement. It simplifies the code and makes it more readable. This aligns well with the PR objective of standardizing address instantiation across the codebase.Also applies to: 82-83
crates/katana/executor/tests/fixtures/mod.rs (3)
24-24: Ohayo, sensei! Nice import update!The addition of
addressto the import statement is a good move. It aligns well with the changes in address instantiation we'll see later. Keep up the good work!
91-92: Ohayo again, sensei! Excellent address simplification!The use of the
address!macro here is a great improvement. It makes the code cleaner and more consistent. This change:
- Simplifies address instantiation
- Improves readability
- Maintains the same address value
Keep up this stellar work, sensei!
Line range hint
1-265: Ohayo, sensei! Let's wrap up this review!Overall, the changes in this file are excellent. Here's a summary of the improvements:
- Consistent use of the
address!macro for contract address instantiation.- Simplified and more readable code for address handling.
- Maintained functionality of test fixtures while improving code quality.
These changes align perfectly with the PR objectives of standardizing address instantiation. The code is now more consistent and easier to maintain. Great job, sensei!
Is there anything else you'd like me to look at or explain further?
bin/saya/src/tests.rs (6)
9-9: Ohayo, sensei! Nice import addition!The new import statement brings in the
addressandfeltmacros fromkatana_primitives. This is a good addition as it enables the use ofaddress!andfelt!macros in the test function, which will make the code more concise and readable.
47-48: Sugoi use of theaddress!macro, sensei!The replacement of
ContractAddresscreation with theaddress!macro is a great improvement. It makes the code more concise and easier to read. This change also ensures consistency in how addresses are created throughout the codebase.Also applies to: 52-53
59-60: Ohayo! Consistent use ofaddress!macro here too!Great job maintaining consistency by using the
address!macro forMessageToAppchainstructs as well. This change aligns perfectly with the refactoring in theMessageToStarknetstructs, creating a uniform approach to address creation throughout the test.Also applies to: 66-67
76-76: Sugoi combo ofaddress!andfelt!macros, sensei!Excellent use of both
address!andfelt!macros in thenonce_updatesmap initialization. This change not only maintains consistency with the earlier refactoring but also improves code readability. Theaddress!macro for the key andfelt!for the value make the types more explicit and the code more self-documenting.
87-87: Ohayo! Consistent macro usage continues, sensei!The use of
address!andfelt!macros in thedeployed_contractsmap initialization is spot on! This change maintains the consistency we've seen throughout the refactoring. It's great to see this uniform approach applied across different parts of the test, includingnonce_updatesand nowdeployed_contracts. This consistency will make the code easier to understand and maintain.
9-9: Sugoi refactoring, sensei! Overall assessment of the changes.Ohayo! After reviewing all the changes in this file, I'm impressed with the consistent use of the new
address!andfelt!macros throughout the test function. Here's a summary of the improvements:
- The new import statement brings in the necessary macros.
ContractAddresscreation is now more concise and readable usingaddress!.Feltvalues are created using thefelt!macro, improving type clarity.- The changes are applied consistently across different structs and data structures in the test.
These refactoring changes significantly enhance code readability and maintainability. The uniform approach to creating addresses and felt values will make it easier for other developers to understand and work with this code. Great job on improving the overall quality of the test!
Also applies to: 47-48, 52-53, 59-60, 66-67, 76-76, 87-87
crates/katana/executor/tests/executor.rs (3)
14-14: Ohayo, sensei! New import foraddressmacro.The addition of the
addressimport fromkatana_primitivesis a nice touch! This paves the way for using theaddress!macro, which we'll see in action soon.
29-30: Konnichiwa! Embracing theaddress!macro for contract addresses.Ohayo, sensei! I see you've leveled up your code by replacing the
felt!macro with the shiny newaddress!macro for creatingContractAddressinstances. This change makes the code more expressive and potentially safer. It's like upgrading from a rusty kunai to a gleaming katana!Here's what's awesome about this change:
- It directly creates
ContractAddressinstances, eliminating the need for type conversion.- It makes the intention clearer - we're dealing with addresses, not just felt values.
- It potentially provides better type checking at compile-time.
Great job on this refactoring, sensei! It's a small change, but it's these little improvements that add up to make our code base stronger and more maintainable.
Also applies to: 32-32
Line range hint
1-385: Sayonara, sensei! A job well done on this refactoring mission.Ohayo once again, esteemed sensei! After reviewing your changes, I must say I'm impressed with the subtle yet impactful improvements you've made to this test file. By introducing the
address!macro, you've not only enhanced the readability of the code but also potentially improved its type safety.These changes align perfectly with the PR objectives of standardizing the instantiation of contract addresses. While the modifications are minimal, they contribute significantly to maintaining consistency across the codebase. This is the way of the true code ninja - making small, precise cuts that have a big impact!
Keep up the excellent work, sensei. Your attention to detail and commitment to code quality are truly admirable. May your code always compile on the first try!
crates/katana/storage/db/src/tables.rs (1)
342-343: Ohayo again, sensei! Great job on updating the test cases!The changes look good! You've replaced
felt!("0x123456789")withaddress!("0x123456789")forContractAddressandContractStorageKey. This update improves type safety and readability by using the appropriate macro for contract addresses.These changes are consistent with the new
addressimport and ensure that contract addresses are created using the correct macro. Well done on enhancing the code quality!crates/katana/storage/db/src/mdbx/mod.rs (2)
351-351: Ohayo, sensei! Nice macro usage for contract address!The change from
ContractAddress::from(felt!("0x1337"))toaddress!("0x1337")is a great improvement. It makes the code more concise and directly conveys the intent of creating a contract address. This aligns well with the PR objective of standardizing contract address instantiation across the codebase.
465-465: Ohayo again, sensei! Consistent macro usage ftw!Excellent job maintaining consistency with the
address!macro usage here as well. This change, like the previous one, simplifies theContractAddressinstantiation and makes the test code more readable. It's great to see this standardization being applied uniformly across different test functions.crates/katana/executor/src/implementation/blockifier/state.rs (5)
241-241: Ohayo, sensei! LGTM on the new imports!The addition of
addressandFeltimports fromkatana_primitivesaligns perfectly with our goal of standardizing contract address and felt value instantiation. Nice work!
Line range hint
259-272: Ohayo, sensei! Excellent use of new macros!The consistent replacement of
ContractAddress::from(felt!("..."))withaddress!("...")and the use offelt!macro for value assignments greatly enhances code readability and standardization. This change aligns perfectly with our PR objectives. Keep up the great work!
Line range hint
287-293: Ohayo, sensei! Consistent macro usage in tests!The application of
address!andfelt!macros in the test function maintains excellent consistency with the changes made in thestate_providerfunction. This uniformity enhances the overall code quality and readability. Well done!
Line range hint
322-336: Ohayo, sensei! Impressive consistency in macro usage!The thorough implementation of
address!andfelt!macros throughout thecan_fetch_as_state_providertest function is commendable. This consistent approach not only aligns perfectly with our PR objectives but also significantly improves code readability and maintainability. Excellent work in maintaining this standard across the entire test suite!Also applies to: 394-402
Line range hint
457-466: Ohayo, sensei! Fantastic completion of macro standardization!The consistent use of
address!andfelt!macros in thefetch_non_existant_datatest function completes the standardization across all test functions in the file. This thorough implementation enhances code consistency, readability, and maintainability throughout the entire test suite. Your meticulous attention to detail in applying these changes uniformly is truly impressive. Great job on successfully achieving the PR objectives!crates/katana/primitives/src/genesis/mod.rs (3)
325-325: Ohayo, sensei! Nice macro import!The addition of the
address!macro import is a good practice. It improves code readability and aligns with the changes mentioned in the summary.
381-382: Sugoi use of the new macro, sensei!The
address!macro usage here simplifies the creation ofContractAddressinstances. It's a great improvement in terms of readability and consistency.
396-397: Consistency is key, sensei!Great job maintaining the use of the
address!macro here. It keeps the code consistent and easy to read throughout the allocations.crates/katana/storage/provider/src/providers/db/mod.rs (2)
809-810: Ohayo, sensei! Nice use of macros!The changes to use
address!andfelt!macros in thecreate_dummy_state_updatesfunction improve code readability and consistency. This aligns well with the PR objectives of standardizing the instantiation of contract addresses and felt values.Also applies to: 813-814, 821-822
834-835: Consistency is key, sensei!The changes in the
create_dummy_state_updates_2function mirror those increate_dummy_state_updates, usingaddress!andfelt!macros consistently. This contributes to a more uniform and readable codebase.Also applies to: 838-839, 842-843
crates/katana/primitives/src/genesis/json.rs (8)
680-680: Ohayo, sensei! LGTM on this import.The addition of the
addressimport is spot-on for using the newaddress!macro in our tests. It's neatly placed and follows our import style. Nice work!
694-694: Ohayo, sensei! Thisaddress!macro usage is on point!The replacement of
ContractAddress::from(felt!("0x55"))withaddress!("0x55")is a great move. It simplifies our code and makes it more readable. Keep up the good work!
706-707: Ohayo, sensei! Another stellar use of theaddress!macro!The switch from
ContractAddress::from(felt!(...))toaddress!(...)is spot-on. It's consistent with our new approach and makes the code cleaner. Excellent work!
714-717: Ohayo, sensei! Impressive consistency with theaddress!macro!You've done a great job applying the
address!macro across multiple lines. This consistent approach enhances our code's readability and maintainability. It's a joy to see such thorough work!
760-765: Ohayo, sensei! You're on a roll with theseaddress!macro changes!Your consistent application of the
address!macro continues to impress. These changes align perfectly with the pattern established earlier, contributing to a more uniform and readable codebase. Excellent attention to detail!
Line range hint
915-930: Ohayo, sensei! Your dedication to theaddress!macro is admirable!Your thorough application of the
address!macro throughout this section is commendable. The consistency you've maintained across the entire file really shines here. It's clear you've put in the effort to ensure a uniform approach. Fantastic work!
Line range hint
1022-1160: Ohayo, sensei! You've brought it home with these finaladdress!macro changes!Your commitment to updating every instance of
ContractAddress::from(felt!(...))toaddress!(...)is truly impressive. This comprehensive update ensures consistency across the entire file, which will greatly benefit code maintainability and readability. You've done an outstanding job seeing this change through to completion. Bravo!
Line range hint
680-1160: Ohayo, sensei! Let's wrap up this stellar review!Your comprehensive update to implement the
address!macro throughout this file is truly commendable. Here's a quick summary of the impact:
- Improved code readability by simplifying address creation syntax.
- Enhanced consistency across the entire file.
- Successful standardization of address instantiation.
These changes will make our codebase more maintainable and easier to understand for all team members. Great job on this refactoring effort!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2475 +/- ##
==========================================
- Coverage 68.46% 68.44% -0.02%
==========================================
Files 368 368
Lines 48185 48139 -46
==========================================
- Hits 32988 32951 -37
+ Misses 15197 15188 -9 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
ProgramInput.address!macro across various test cases, enhancing code clarity and consistency.Feltvalues with thefelt!macro, improving readability.