feat: add staking_info fields to ContractInfo - #1744
Conversation
There was a problem hiding this comment.
Pull request overview
Adds staking-related metadata fields to ContractInfo so contract staking targets and reward preference can be round-tripped through protobuf serialization/deserialization.
Changes:
- Add
staked_account_id,staked_node_id, anddecline_staking_rewardfields toContractInfo. - Update
ContractInfo._from_proto()to deserializestaking_infointo the new fields. - Update
ContractInfo._to_proto()and add unit tests covering staking scenarios and round-trip conversions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/hiero_sdk_python/contract/contract_info.py |
Adds staking fields and (de)serialization logic for staking_info. |
tests/unit/contract_info_test.py |
Extends unit tests to validate staking field initialization and proto round-trips. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1744 +/- ##
=======================================
Coverage 93.35% 93.35%
=======================================
Files 141 141
Lines 9121 9124 +3
=======================================
+ Hits 8515 8518 +3
Misses 606 606 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds support for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. Quick Fix for CHANGELOG.md ConflictsIf your conflict is only in CHANGELOG.md, you can resolve it easily using the GitHub web editor:
For all other merge conflicts, please read: Thank you for contributing! |
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
bcbcd6f to
98115be
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/contract_info_test.py (1)
234-252: 🛠️ Refactor suggestion | 🟠 MajorAdd staking assertions to the
None-values_to_prototest.This test exercises
_to_protowith a default-constructedContractInfobut doesn't verify staking behavior. Given the_to_protobug (always emitting aStakingInfo), asserting here would document the expected behavior and catch regressions.assert proto.max_automatic_token_associations == 0 # 0 is default for protobuf int + # When all staking fields are None, staking_info should not be present + assert not proto.HasField('staking_info'), "staking_info should be absent when all staking fields are None"This assertion will currently fail, confirming the
_to_protobug. Once the production code is fixed, this test will pass and serve as a regression guard.
98115be to
8fe7319
Compare
|
Hi, this is WorkflowBot.
|
919e2ff to
4233fb1
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/hiero_sdk_python/contract/contract_info.py (1)
42-42:⚠️ Potential issue | 🟡 MinorUse
StakingInfo | Nonefor the newly added annotation.Line 59 introduces
Optional[StakingInfo]in modified code; this should follow the repository’sX | Nonestyle.Proposed fix
- staking_info (Optional[StakingInfo]): The staking information for this contract + staking_info (StakingInfo | None): The staking information for this contract ... - staking_info: Optional[StakingInfo] = None + staking_info: StakingInfo | None = None#!/bin/bash # Verify Python version supports union syntax and locate current annotation usage. rg -n 'requires-python' pyproject.toml rg -n 'staking_info\s*:\s*Optional\[StakingInfo\]' src/hiero_sdk_python/contract/contract_info.pyAs per coding guidelines, "The codebase uses
X | Noneunion syntax (Python 3.10+). Flag any use ofOptional[X]fromtypingin newly added or modified code."Also applies to: 59-59
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.mdpyproject.tomlsrc/hiero_sdk_python/contract/contract_info.pytests/integration/contract_info_query_e2e_test.pytests/unit/account_info_test.pytests/unit/contract_info_test.py
manishdait
left a comment
There was a problem hiding this comment.
@Mounil2005, Just a small change rest looks good to me
|
Have you rebased please @Mounil2005 ? |
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
…e account_info changes Signed-off-by: Mounil <mounilkankhara@gmail.com>
…ssert style Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Signed-off-by: Mounil <mounilkankhara@gmail.com>
0b58031 to
c59d031
Compare
Done! |
|
Merge away please if we have enough approvals :D |
Signed-off-by: Mounil <mounilkankhara@gmail.com>
Description:
Add staking_info fields to ContractInfo class to support staking metadata from blockchain
Related issue(s):
Fixes #1365
Notes for reviewer:
Implementation follows the same pattern used in AccountInfo for consistency. All changes are isolated to ContractInfo and its unit tests. 17 unit tests pass including:
Checklist