Conversation
|
Ohayo, sensei! Please find the detailed breakdown below. WalkthroughThis pull request updates the logging functionality in multiple modules by modifying the Changes
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)crates/katana/executor/src/utils.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
🪧 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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/katana/executor/src/implementation/blockifier/mod.rs(1 hunks)crates/katana/executor/src/implementation/blockifier/utils.rs(1 hunks)crates/katana/executor/src/utils.rs(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
crates/katana/executor/src/implementation/blockifier/mod.rs (2)
crates/katana/executor/src/utils.rs (1)
log_resources(12-43)crates/katana/executor/src/abstraction/mod.rs (2)
trace(153-158)receipt(145-150)
🪛 GitHub Actions: ci
crates/katana/executor/src/utils.rs
[error] 24-24: Rust formatting check failed. Run 'rustfmt' to fix code style issues in this file.
🔇 Additional comments (4)
crates/katana/executor/src/implementation/blockifier/mod.rs (1)
253-253: Ohayo! Updated function call to pass receipt to log_resourcesThe function call has been properly updated to match the new signature of
log_resourceswhich now accepts an additionalreceiptparameter. This change enhances the logging functionality by providing more detailed gas usage information.crates/katana/executor/src/implementation/blockifier/utils.rs (2)
590-590: Field mapping corrected for data availability gasGood change, sensei! You've updated the field reference from
l1_data_gastol1_gasto correctly map the data availability gas metrics. This ensures accurate gas usage reporting.
594-594: Field mapping corrected for total gas consumedConsistent with the previous change, you've properly updated the field reference for total gas consumed from
l1_data_gastol1_gas. This ensures all gas metrics are correctly tracked and reported.crates/katana/executor/src/utils.rs (1)
12-12: Function signature updated to include receipt parameterOhayo! You've correctly updated the function signature to include the additional
receiptparameter, which allows accessing fee and gas consumption data for enhanced logging.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3133 +/- ##
==========================================
- Coverage 56.87% 55.62% -1.25%
==========================================
Files 442 443 +1
Lines 61505 62976 +1471
==========================================
+ Hits 34979 35033 +54
- Misses 26526 27943 +1417 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kariy
left a comment
kariy
left a comment
There was a problem hiding this comment.
tbh not a really a fan of logging out the tx resources.. i think that's what the starknet_getReceipt rpc is for.
it make sense for local dev when there's only a few txs at a time, but when there's a bunch of txs coming in, the logs just become noise (and a waste of computation). it's fine for now tho, but i'll try to come up with a better solution in the new katana repo.
| l1_gas: exec_info.transaction_receipt.da_gas.l1_data_gas, | ||
| l1_gas: exec_info.transaction_receipt.da_gas.l1_gas, | ||
| l1_data_gas: exec_info.transaction_receipt.da_gas.l1_data_gas, | ||
| }, | ||
| total_gas_consumed: L1Gas { | ||
| l1_gas: exec_info.transaction_receipt.gas.l1_data_gas, | ||
| l1_gas: exec_info.transaction_receipt.gas.l1_gas, |
Maybe if the |
3 participants
I think there is still some issues with gas computation, l1_data_gas is always 0.
Summary by CodeRabbit
Summary by CodeRabbit