refactor(katana): fix feeder gateway types #2760
Conversation
WalkthroughOhayo, sensei! This pull request introduces significant enhancements 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: 1
🧹 Outside diff range and nitpick comments (2)
crates/katana/feeder-gateway/src/types/serde_utils.rs (2)
4-30: Refactor duplicated deserialization logicOhayo sensei, consider abstracting the common deserialization logic for
u64andu128to reduce code duplication and enhance maintainability.Also applies to: 32-58, 60-82, 84-106
1-106: Add unit tests for deserialization functionsOhayo sensei, to ensure the correctness and robustness of these deserialization functions, please consider adding comprehensive unit tests.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
crates/katana/feeder-gateway/src/client.rs(9 hunks)crates/katana/feeder-gateway/src/types/mod.rs(1 hunks)crates/katana/feeder-gateway/src/types/serde_utils.rs(1 hunks)crates/katana/feeder-gateway/src/types/transaction.rs(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/katana/feeder-gateway/src/types/mod.rs
🔇 Additional comments (1)
crates/katana/feeder-gateway/src/types/transaction.rs (1)
136-136: Verify handling of missing contract_address in DeployAccountTx
Ohayo sensei, when contract_address is not provided, the code defaults it to a zero address using unwrap_or_default(). Please verify that this behavior is appropriate and that downstream components can handle a zero contract_address without issues.
Also applies to: 339-339, 360-360
| #[error("failed to parse header value '{value}'")] | ||
| InvalidHeaderValue { value: String }, | ||
|
|
There was a problem hiding this comment.
Avoid leaking API keys in error messages
Ohayo sensei, including the API key in error messages may expose sensitive information. Please modify the error handling to prevent leaking the API key value.
Apply this diff to address the issue:
- #[error("failed to parse header value '{value}'")]
- InvalidHeaderValue { value: String },
+ #[error("failed to parse API key header value")]
+ InvalidHeaderValue,- .map_err(|_| Error::InvalidHeaderValue { value: key.to_string() })?;
+ .map_err(|_| Error::InvalidHeaderValue)?;Also applies to: 152-155
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2760 +/- ##
==========================================
- Coverage 54.69% 54.63% -0.07%
==========================================
Files 427 428 +1
Lines 53918 54001 +83
==========================================
+ Hits 29490 29501 +11
- Misses 24428 24500 +72 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor