fix: allow studio mode1 message fee buckets#1650
Conversation
|
Warning Review limit reached
More reviews will be available in 16 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR removes the ChangesMode1 GenVM Bucket-Only Fee Allocation and Transaction Value Coercion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/test_node_state_proxy_metrics.py (1)
449-453: ⚡ Quick winConsider using
GENVM_UNMETERED_DATA_FEE_BUCKETconstant instead of magic number.The magic number
(1 << 256) - 1appears inline, buttest_studio_fees.pyimports and uses theGENVM_UNMETERED_DATA_FEE_BUCKETconstant for the same value (see line 1526 in test_studio_fees.py). Using the constant would improve readability and maintainability.📖 Suggested refactor
+from backend.protocol_rpc.fees import ( + GENVM_UNMETERED_DATA_FEE_BUCKET, StudioFeePolicy, create_fee_accounting, required_fee_deposit, )assert fee_context.bucket_totals == [ - (1 << 256) - 1, - (1 << 256) - 1, + GENVM_UNMETERED_DATA_FEE_BUCKET, + GENVM_UNMETERED_DATA_FEE_BUCKET, 55, ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_node_state_proxy_metrics.py` around lines 449 - 453, Replace the repeated magic number (1 << 256) - 1 in the assertion for fee_context.bucket_totals with the existing GENVM_UNMETERED_DATA_FEE_BUCKET constant used elsewhere (e.g., in test_studio_fees.py); update the assertion so the first two elements reference GENVM_UNMETERED_DATA_FEE_BUCKET instead of the literal to improve readability and maintainability while keeping the expected third element (55) unchanged.backend/database_handler/transactions_processor.py (1)
93-93: ⚡ Quick winAdd return type hint.
The method signature lacks a return type hint. As per coding guidelines, all Python code should include type hints.
📝 Suggested fix
`@staticmethod` -def _parse_transaction_data(transaction_data: Transactions) -> dict: +def _parse_transaction_data(transaction_data: Transactions) -> dict[str, any]:As per coding guidelines: "Include type hints in all Python code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/database_handler/transactions_processor.py` at line 93, The function _parse_transaction_data currently lacks an explicit return type; update its signature to include a precise type hint (for example -> Dict[str, Any] or -> dict[str, Any]) and ensure typing imports (Dict, Any or use built-in generic dict with str/Any) are added or adjusted at the top of the module; modify the signature for _parse_transaction_data(transaction_data: Transactions) to include the chosen return type and update any related type imports to satisfy the project's typing guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/database_handler/transactions_processor.py`:
- Line 93: The function _parse_transaction_data currently lacks an explicit
return type; update its signature to include a precise type hint (for example ->
Dict[str, Any] or -> dict[str, Any]) and ensure typing imports (Dict, Any or use
built-in generic dict with str/Any) are added or adjusted at the top of the
module; modify the signature for _parse_transaction_data(transaction_data:
Transactions) to include the chosen return type and update any related type
imports to satisfy the project's typing guidelines.
In `@tests/unit/test_node_state_proxy_metrics.py`:
- Around line 449-453: Replace the repeated magic number (1 << 256) - 1 in the
assertion for fee_context.bucket_totals with the existing
GENVM_UNMETERED_DATA_FEE_BUCKET constant used elsewhere (e.g., in
test_studio_fees.py); update the assertion so the first two elements reference
GENVM_UNMETERED_DATA_FEE_BUCKET instead of the literal to improve readability
and maintainability while keeping the expected third element (55) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 469391a7-6e5e-4216-a50a-4076c86bd2a0
📒 Files selected for processing (4)
backend/database_handler/transactions_processor.pybackend/protocol_rpc/fees.pytests/unit/test_node_state_proxy_metrics.pytests/unit/test_studio_fees.py
|
|
🎉 This PR is included in version 0.121.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Description
Allows fee-bearing Mode 1 message transactions in Studio to reach GenVM with a positive message bucket and an empty allocation list. Consensus v0.6 now defines Mode 1 as bucket-only, with per-emission feeParams/declaredBudget supplied by GenVM emissions, so Studio should not reject it before execution.
Updates unit coverage for:
Verification
Summary by CodeRabbit
Bug Fixes
Tests