fix(katana): include msg sender in the tx calldata #2550
Conversation
WalkthroughOhayo, sensei! This pull request introduces modifications to the Changes
Possibly related PRs
🪧 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.githubusercontent.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 (3)
crates/katana/rpc/rpc-types/src/message.rs (1)
30-33: Ohayo, sensei! LGTM with a small suggestion.The changes look good and correctly implement the L1 handler transaction calldata structure. The added comment provides valuable context.
Consider renaming
calldatatol1_handler_calldatafor improved clarity:- let mut calldata = vec![Felt::from(self.0.from_address)]; - calldata.extend(self.0.payload); + let mut l1_handler_calldata = vec![Felt::from(self.0.from_address)]; + l1_handler_calldata.extend(self.0.payload);This change would make it more explicit that we're dealing with L1 handler-specific calldata.
crates/katana/rpc/rpc/tests/messaging.rs (2)
343-355: Ohayo, sensei! Your code is as sharp as a katana!The changes to the
estimate_message_feetest formsg_handler_valueare well-implemented. The use of theMsgFromL1struct provides a clear structure for the message data, and the test now correctly estimates the fee for the simplified message.For even greater clarity, consider adding a brief comment explaining why
payloadonly contains one value now:-let payload = vec![felt!("123")]; +// Only one value in payload as `from_address` is automatically injected +let payload = vec![felt!("123")];
Line range hint
357-380: Ohayo, sensei! Your code is evolving like a true ninja!The addition of the test case for
msg_handler_structis an excellent improvement to the test coverage. It demonstrates how to handle structured payloads in L1 handler functions, which is a valuable scenario to test.For consistency with the previous test case, consider adding a brief comment explaining the payload structure:
-// [ MyData.a , MyData.b ] -let payload = vec![felt!("1"), felt!("2")]; +// Payload represents MyData struct: [MyData.a, MyData.b] +let payload = vec![felt!("1"), felt!("2")];This small change will make it even clearer for other developers reading the test.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- crates/katana/rpc/rpc-types/src/message.rs (1 hunks)
- crates/katana/rpc/rpc/tests/messaging.rs (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
crates/katana/rpc/rpc-types/src/message.rs (1)
38-38: Ohayo again, sensei! This change looks solid.The
calldatafield is correctly set to use the newly constructed vector that includes the sender's address.To ensure consistency across the codebase, let's verify that all L1 handler transactions are using this updated structure:
This will help us confirm that the new calldata structure is consistently applied throughout the project.
✅ Verification successful
Ohayo again, sensei! This change passes verification.
The
calldatafield is consistently set to the newly constructed vector across allL1HandlerTxinstances, ensuring it includes the sender's address as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for L1 handler transaction calldata construction # Search for L1 handler transaction creation or processing rg --type rust -e "L1HandlerTx" -e "l1_handler" -A 10 -B 5Length of output: 75387
crates/katana/rpc/rpc/tests/messaging.rs (1)
333-340: Ohayo, sensei! These comments are a treasure trove of wisdom!The added comments provide excellent context for the
msg_handler_valuefunction, explaining the automatic injection offrom_addressand linking to the Starknet documentation. This clarity will be invaluable for future developers working with L1 handler functions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2550 +/- ##
==========================================
- Coverage 69.55% 69.55% -0.01%
==========================================
Files 388 388
Lines 49959 49964 +5
==========================================
- Hits 34751 34750 -1
- Misses 15208 15214 +6 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation