chore: Refactored AccountInfo class to use the staking_info #1595
Conversation
Signed-off-by: mukundkumarjha <[email protected]>
WalkthroughAccountInfo was refactored to replace flattened staking fields with a single Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/hiero_sdk_python/account/account_info.py (5)
12-12: Wrong import: Using protobuf type instead of SDK class.The import pulls
StakingInfofrom the protobuf definitions, but based on the PR objectives and the SDK'sStakingInfoclass atsrc/hiero_sdk_python/staking_info.py, you should import the SDK wrapper class which has the_from_proto()and_to_proto()methods.🔧 Proposed fix
-from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo +from hiero_sdk_python.staking_info import StakingInfo
81-110: Critical bug:staking_infois parsed but never stored.The
staking_infovariable on lines 104-108 is assigned but never used (confirmed by Ruff F841). The parsed staking info is lost because it's not stored inaccount_info. Additionally, the SDK'sStakingInfoclass uses_from_proto(with underscore) per conventions.🐛 Proposed fix
Include
staking_infoin the constructor call or assign it after:account_info: "AccountInfo" = cls( account_id=AccountId._from_proto(proto.accountID) if proto.accountID else None, contract_account_id=proto.contractAccountID, is_deleted=proto.deleted, proxy_received=Hbar.from_tinybars(proto.proxyReceived), key=PublicKey._from_proto(proto.key) if proto.key else None, balance=Hbar.from_tinybars(proto.balance), receiver_signature_required=proto.receiverSigRequired, expiration_time=( Timestamp._from_protobuf(proto.expirationTime) if proto.expirationTime else None ), auto_renew_period=( Duration._from_proto(proto.autoRenewPeriod) if proto.autoRenewPeriod else None ), token_relationships=[ TokenRelationship._from_proto(relationship) for relationship in proto.tokenRelationships ], account_memo=proto.memo, owned_nfts=proto.ownedNfts, max_automatic_token_associations=proto.max_automatic_token_associations, + staking_info=( + StakingInfo._from_proto(proto.staking_info) + if proto.HasField("staking_info") + else None + ), ) - staking_info=( - StakingInfo.from_proto(proto.staking_info) - if proto.HasField("staking_info") - else None - ) - return account_info
161-179: Incorrect__str__implementation: Displaying full object instead of specific fields.Lines 161-162 both display the entire
staking_infoobject with different labels, and line 179 does the same for decline reward. The implementation should access specific fields from theStakingInfoobject.🔧 Proposed fix
simple_fields = [ (self.account_id, "Account ID"), (self.contract_account_id, "Contract Account ID"), (self.balance, "Balance"), (self.key, "Key"), (self.account_memo, "Memo"), (self.owned_nfts, "Owned NFTs"), (self.max_automatic_token_associations, "Max Automatic Token Associations"), - (self.staking_info, "Staked Account ID"), - (self.staking_info, "Staked Node ID"), + (self.staking_info.staked_account_id if self.staking_info else None, "Staked Account ID"), + (self.staking_info.staked_node_id if self.staking_info else None, "Staked Node ID"), (self.proxy_received, "Proxy Received"), (self.expiration_time, "Expiration Time"), (self.auto_renew_period, "Auto Renew Period"), ] ... if self.staking_info is not None: - lines.append(f"Decline Staking Reward: {self.staking_info}") + lines.append(f"Decline Staking Reward: {self.staking_info.decline_reward}")
186-199: Incorrect__repr__implementation: Displaying full object instead of specific fields.The
__repr__method labels fields asstaked_node_idandstaked_account_idbut displays the entirestaking_infoobject for both. It should either display the fullstaking_infoobject once, or access specific fields.🔧 Proposed fix (Option 1: Display staking_info as single field)
def __repr__(self) -> str: """Returns a string representation of the AccountInfo object for debugging.""" return ( f"AccountInfo(" f"account_id={self.account_id!r}, " f"contract_account_id={self.contract_account_id!r}, " f"is_deleted={self.is_deleted!r}, " f"balance={self.balance!r}, " f"receiver_signature_required={self.receiver_signature_required!r}, " f"owned_nfts={self.owned_nfts!r}, " f"account_memo={self.account_memo!r}, " - f"staked_node_id={self.staking_info!r}, " - f"staked_account_id={self.staking_info!r}" + f"staking_info={self.staking_info!r}" f")" )
41-44: Docstring references removed fields.The docstring still documents
staked_account_id,staked_node_id, anddecline_staking_rewardas separate attributes, but these were replaced by the unifiedstaking_infofield.📝 Proposed fix
- staked_account_id (Optional[AccountId]): The account to which this account is staked. - staked_node_id (Optional[int]): The node to which this account is staked. - decline_staking_reward (bool): Whether this account declines receiving staking rewards. + staking_info (Optional[StakingInfo]): Staking information for this account, including + the staked account/node ID, decline reward preference, and staking rewards.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hiero_sdk_python/account/account_info.py (2)
12-12: Wrong import: imports protobuf type instead of SDK class.This imports
StakingInfofrom the protobuf module, but the code expects the SDK'sStakingInfoclass which has_from_proto()and_to_proto()methods. The protobuf type doesn't have these methods and will causeAttributeErrorat runtime.🔧 Proposed fix
-from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo +from hiero_sdk_python.staking_info import StakingInfo
41-44: Docstring references removed fields.The docstring still documents
staked_account_id,staked_node_id, anddecline_staking_rewardas attributes, but these have been replaced bystaking_info. Update the docstring to match the actual fields.📝 Proposed fix for docstring
- staked_account_id (Optional[AccountId]): The account to which this account is staked. - staked_node_id (Optional[int]): The node to which this account is staked. - decline_staking_reward (bool): Whether this account declines receiving staking rewards. + staking_info (Optional[StakingInfo]): Staking-related information for this account.Also applies to: 59-60
♻️ Duplicate comments (1)
src/hiero_sdk_python/account/account_info.py (1)
142-146: Method name still incorrect and indentation inconsistent.The past review flagged that
to_proto()should be_to_proto()per SDK conventions. This appears to still be present despite being marked as addressed. The closing parenthesis indentation is also inconsistent.🔧 Proposed fix
max_automatic_token_associations=self.max_automatic_token_associations, - staking_info=( - self.staking_info.to_proto() - if self.staking_info is not None - else None - ), + staking_info=( + self.staking_info._to_proto() + if self.staking_info is not None + else None + ), )
| if staking_info.HasField('staked_node_id') else None | ||
| ) | ||
| account_info.decline_staking_reward = staking_info.decline_reward | ||
| staking_info=( |
There was a problem hiding this comment.
@mukundkumarjha, this should be added inside the AccountInfo object itself, currenlty is is not included in the object, possibly move it after line 101
| (self.max_automatic_token_associations, "Max Automatic Token Associations"), | ||
| (self.staked_account_id, "Staked Account ID"), | ||
| (self.staked_node_id, "Staked Node ID"), | ||
| (self.staking_info, "Staked Account ID"), |
There was a problem hiding this comment.
better to display this as staking info only
| f"account_memo={self.account_memo!r}, " | ||
| f"staked_node_id={self.staked_node_id!r}, " | ||
| f"staked_account_id={self.staked_account_id!r}" | ||
| f"staked_node_id={self.staking_info!r}, " |
There was a problem hiding this comment.
same with these, better to include stating_info itself
There was a problem hiding this comment.
@mukundkumarjha Thanks for the PR. I’ve added a few required changes.
Signed-off-by: mukundkumarjha <[email protected]>
mukundkumarjha
force-pushed
the
accountinfo
branch
from
38cab37 to
f633b2d
Compare
|
@manishdait thanks for the review I have updated the same |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hiero_sdk_python/account/account_info.py (2)
12-12: Critical: Wrong import - imports protobufStakingInfoinstead of SDK class.Line 12 imports from
hiero_sdk_python.hapi.services.basic_types_pb2, which is the protobuf-generated class. The SDK'sStakingInfoclass (with_from_proto()and_to_proto()methods) is located inhiero_sdk_python.staking_info. This causes runtime failures since protobuf classes don't havefrom_proto()/to_proto()methods.🔧 Proposed fix
-from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo +from hiero_sdk_python.staking_info import StakingInfo
41-44: Update docstring to reflect the refactored field.The docstring still documents the removed flattened fields (
staked_account_id,staked_node_id,decline_staking_reward). Update it to document the newstaking_infofield instead.🔧 Proposed fix
- staked_account_id (Optional[AccountId]): The account to which this account is staked. - staked_node_id (Optional[int]): The node to which this account is staked. - decline_staking_reward (bool): Whether this account declines receiving staking rewards. + staking_info (Optional[StakingInfo]): Staking-related information for this account.
♻️ Duplicate comments (5)
src/hiero_sdk_python/account/account_info.py (5)
143-147: Method name should follow SDK conventions with underscore prefix.The SDK's
StakingInfoclass uses_to_proto()(with underscore prefix) per the codebase conventions. The previous review comment flagged this issue but it appears to still be present. Also fix the inconsistent indentation.🔧 Proposed fix
staking_info=( - self.staking_info.to_proto() + self.staking_info._to_proto() if self.staking_info is not None else None - ), + ),
178-179: Incorrect value for "Decline Staking Reward".The label expects a boolean but the value is the entire
StakingInfoobject. This was flagged in a previous review. Access the specific field instead.🔧 Proposed fix
if self.staking_info is not None: - lines.append(f"Decline Staking Reward: {self.staking_info}") + lines.append(f"Decline Staking Reward: {self.staking_info.decline_reward}")
197-199: Incorrect field names in__repr__.The labels
staked_info=(typo) andstaked_account_id=reference removed/incorrect fields, and both incorrectly display the samestaking_infovalue. This was flagged in a previous review. Replace with a singlestaking_info=entry.🔧 Proposed fix
f"account_memo={self.account_memo!r}, " - f"staked_info={self.staking_info!r}, " - f"staked_account_id={self.staking_info!r}" + f"staking_info={self.staking_info!r}" f")"
162-162: Minor: Label inconsistency.The label "Staked Info" should be "Staking Info" for consistency with the field name
staking_info.🔧 Proposed fix
- (self.staking_info, "Staked Info"), + (self.staking_info, "Staking Info"),
102-106: Method name should follow SDK conventions with underscore prefix.The SDK's
StakingInfoclass uses_from_proto()(with underscore prefix) per the codebase conventions shown in the relevant code snippets. Also fix the inconsistent indentation.🔧 Proposed fix
staking_info=( - StakingInfo.from_proto(proto.staking_info) + StakingInfo._from_proto(proto.staking_info) if proto.HasField("staking_info") else None - ) + )
| decline_reward=self.decline_staking_reward | ||
| ), | ||
| staking_info=( | ||
| self.staking_info.to_proto() |
There was a problem hiding this comment.
this should be self.staking_info._to_proto()
| if self.decline_staking_reward is not None: | ||
| lines.append(f"Decline Staking Reward: {self.decline_staking_reward}") | ||
| if self.staking_info is not None: | ||
| lines.append(f"Decline Staking Reward: {self.staking_info}") |
| f"staked_node_id={self.staked_node_id!r}, " | ||
| f"staked_account_id={self.staked_account_id!r}" | ||
| f"staked_info={self.staking_info!r}, " | ||
| f"staked_account_id={self.staking_info!r}" |
There was a problem hiding this comment.
same with this, we can remove it
| f"staked_info={self.staking_info!r}, " | ||
| f"staked_account_id={self.staking_info!r}" | ||
| f")" | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Also please update the account_info unit tests to verify the changes.
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@mukundkumarjha Thank you for your contribution!
I concur with @manishdait on the changes.
Additionally, please review your changelog entry.
If you have questions or need assistance, please reach out.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
Signed-off-by: mukundkumarjha <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/account_info_test.py (1)
180-210: Add a non-None staking_info round‑trip test to protect the new behavior.PR objectives require preserving full staking data, but tests only assert
None. Add a test that sets a realStakingInfo(with one of staked_account_id/staked_node_id and decline_reward), runs_to_proto()/_from_proto(), and asserts those fields survive the round trip.
| max_automatic_token_associations: Optional[int] = None | ||
| staked_account_id: Optional[AccountId] = None | ||
| staked_node_id: Optional[int] = None | ||
| decline_staking_reward: Optional[bool] = None | ||
| staking_info: Optional[StakingInfo] = None | ||
|
|
There was a problem hiding this comment.
Align max_automatic_token_associations default with tests and expected API behavior.
Current default is None, but tests (and implied behavior) expect 10. This will fail default initialization tests and create inconsistent defaults.
🔧 Proposed fix
- max_automatic_token_associations: Optional[int] = None
+ max_automatic_token_associations: int = 10📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| max_automatic_token_associations: Optional[int] = None | |
| staked_account_id: Optional[AccountId] = None | |
| staked_node_id: Optional[int] = None | |
| decline_staking_reward: Optional[bool] = None | |
| staking_info: Optional[StakingInfo] = None | |
| max_automatic_token_associations: int = 10 | |
| staking_info: Optional[StakingInfo] = None | |
| (self.staking_info, "Staked Info"), | ||
| (self.proxy_received, "Proxy Received"), |
There was a problem hiding this comment.
Use consistent “staking_info” labeling in str / repr.
The label “Staked Info” / “staked_info” is inconsistent with the field name and PR objectives, which can confuse users.
🔧 Proposed fix
- (self.staking_info, "Staked Info"),
+ (self.staking_info, "Staking Info"),
...
- f"staked_info={self.staking_info!r}, "
+ f"staking_info={self.staking_info!r}, "Also applies to: 194-194
tests/unit/account_info_test.py
Outdated
| max_automatic_token_associations=10 | ||
| staking_info=None | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file around the problematic lines
cat -n tests/unit/account_info_test.py | head -80 | tail -40Repository: hiero-ledger/hiero-sdk-python
Length of output: 1958
🏁 Script executed:
# Check overall file syntax
python3 -m py_compile tests/unit/account_info_test.py 2>&1 || echo "Syntax check done"Repository: hiero-ledger/hiero-sdk-python
Length of output: 159
🏁 Script executed:
# Search for StakingInfo usage in the codebase to understand protobuf patterns
rg "StakingInfo" --type py -B 2 -A 2Repository: hiero-ledger/hiero-sdk-python
Length of output: 15130
🏁 Script executed:
# Check how protobuf message fields are typically assigned
rg "staking_info\s*=" --type py -B 1 -A 1Repository: hiero-ledger/hiero-sdk-python
Length of output: 2971
Fix syntax error and avoid invalid protobuf field assignment.
Line 52 is missing a comma after 10. Additionally, assigning None to a protobuf message field is not valid—either omit the field when unset or provide a real StakingInfo proto instance.
🔧 Proposed fix
- max_automatic_token_associations=10
- staking_info=None
+ max_automatic_token_associations=10,🧰 Tools
🪛 Ruff (0.14.14)
53-53: Expected ,, found name
(invalid-syntax)
Signed-off-by: mukundkumarjha <[email protected]>
mukundkumarjha
force-pushed
the
accountinfo
branch
from
2a5f281 to
fa034c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/account_info_test.py (1)
58-73: Addhasattrassertions for new public attributes to protect against breaking changes.Per coding guidelines (PRIORITY 1), tests should assert that public attributes exist. This helps catch accidental renames or removals early.
def test_account_info_initialization(account_info): """Test the initialization of the AccountInfo class""" + # Verify new public attributes exist (guards against breaking changes) + assert hasattr(account_info, 'max_automatic_token_associations') + assert hasattr(account_info, 'staking_info') + assert account_info.account_id == AccountId(0, 0, 100) # ... existing assertions ...
mukundkumarjha
force-pushed
the
accountinfo
branch
from
dfb7792 to
fa034c2
Compare
There was a problem hiding this comment.
this should be from hiero_sdk_python.staking_info import StakingInfo since we are now using the StakingInfo class
| assert account_info.account_memo == "Test account memo" | ||
| assert account_info.owned_nfts == 5 | ||
| assert account_info.max_automatic_token_associations == 10 | ||
| assert account_info.staking_info is None |
There was a problem hiding this comment.
we can also add a test with staking_info present (i.e staking_info not None)
tests/unit/account_info_test.py
Outdated
| assert account_info.account_memo == "Test account memo" | ||
| assert account_info.owned_nfts == 5 | ||
| assert account_info.max_automatic_token_associations == 10 | ||
| #assert account_info.staking_info == None |
There was a problem hiding this comment.
same for this, test for staking_info not equals None
Signed-off-by: mukundkumarjha <[email protected]>
Signed-off-by: Mukund Jha <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hiero_sdk_python/account/account_info.py (1)
41-44: Docstring references removed fields and omits new ones.The docstring still documents
staked_account_id,staked_node_id, anddecline_staking_rewardwhich were removed by this refactor. It also omits the newstaking_infoandmax_automatic_token_associationsfields.📝 Suggested fix
account_memo (Optional[str]): The memo associated with this account. owned_nfts (Optional[int]): The number of NFTs owned by this account. - staked_account_id (Optional[AccountId]): The account to which this account is staked. - staked_node_id (Optional[int]): The node to which this account is staked. - decline_staking_reward (bool): Whether this account declines receiving staking rewards. + max_automatic_token_associations (Optional[int]): Maximum number of tokens that can be + automatically associated with this account. + staking_info (Optional[StakingInfo]): Staking information for this account, including + staked account/node, pending rewards, and decline reward preference.tests/unit/account_info_test.py (1)
252-267: Add assertions forstaking_infoin string representation tests.Per coding guidelines, tests should verify that new public attributes appear correctly in string outputs. The
test_str_and_reprtest should verifystaking_infois rendered correctly, especially since the PR modifies these methods.📝 Suggested addition
def test_str_and_repr_with_staking_info(): """Test __str__ and __repr__ methods include staking_info when populated""" account_info = AccountInfo( account_id=AccountId(0, 0, 100), staking_info=StakingInfo( decline_reward=True, staked_node_id=5, ) ) info_str = str(account_info) info_repr = repr(account_info) # Verify staking_info appears in string output assert "Staking Info" in info_str # or "Staked Info" per current impl assert "staking_info=" in info_repr # or "staked_info=" per current impl
| proto = CryptoGetInfoResponse.AccountInfo( | ||
| accountID=AccountId(0, 0, 100)._to_proto(), | ||
| key=public_key._to_proto(), | ||
| balance=5000000, | ||
|
|
||
| staking_info={ | ||
| "decline_reward": True, | ||
| "staked_node_id": 3, | ||
| "staked_account_id": None | ||
| } | ||
| ) | ||
|
|
||
| account_info = AccountInfo._from_proto(proto) | ||
|
|
||
| assert account_info.staking_info is not None | ||
| assert account_info.staking_info.decline_reward is True | ||
| assert account_info.staking_info.staked_node_id == 3 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use proper protobuf construction for staking_info.
Using a dict to set the staking_info protobuf field relies on implicit coercion which may be fragile. For test clarity and reliability, construct the proto message explicitly.
🔧 Proposed fix
+from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo as StakingInfoProto
+
def test_from_proto_with_staking_info():
"""Test the from_proto method of the AccountInfo class with staking info"""
public_key = PrivateKey.generate_ed25519().public_key()
-
+
+ staking_info_proto = StakingInfoProto(
+ decline_reward=True,
+ staked_node_id=3
+ )
+
proto = CryptoGetInfoResponse.AccountInfo(
accountID=AccountId(0, 0, 100)._to_proto(),
key=public_key._to_proto(),
balance=5000000,
-
- staking_info={
- "decline_reward": True,
- "staked_node_id": 3,
- "staked_account_id": None
- }
+ staking_info=staking_info_proto
)
account_info = AccountInfo._from_proto(proto)
-
+
assert account_info.staking_info is not None
assert account_info.staking_info.decline_reward is True
assert account_info.staking_info.staked_node_id == 3There was a problem hiding this comment.
apart form this does rest look good to you.
|
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, |
Signed-off-by: mukundkumarjha <[email protected]>
| def test_from_proto_with_staking_info(): | ||
| """Test the from_proto method of the AccountInfo class with staking info""" | ||
| public_key = PrivateKey.generate_ed25519().public_key() | ||
|
|
||
| staking_info={ | ||
| "decline_reward": True, | ||
| "staked_node_id": 3, | ||
| "staked_account_id": None | ||
| } | ||
|
|
||
| proto = CryptoGetInfoResponse.AccountInfo( | ||
| accountID=AccountId(0, 0, 100)._to_proto(), | ||
| key=public_key._to_proto(), | ||
| balance=5000000, | ||
|
|
||
|
|
||
| ) | ||
|
|
||
| account_info = AccountInfo._from_proto(proto) | ||
|
|
||
| assert account_info.staking_info is not None | ||
| assert account_info.staking_info.decline_reward is True | ||
| assert account_info.staking_info.staked_node_id == 3 |
There was a problem hiding this comment.
Test is broken: staking_info dict is created but never used.
The staking_info dictionary defined on lines 80-84 is never passed to the proto constructor. The test creates a proto without staking info, then asserts that account_info.staking_info is not None—this will fail.
Additionally, even if you intended to use it, passing a dict to a protobuf message field is fragile. Construct the proto message explicitly.
🔧 Proposed fix
def test_from_proto_with_staking_info():
"""Test the from_proto method of the AccountInfo class with staking info"""
public_key = PrivateKey.generate_ed25519().public_key()
-
- staking_info={
- "decline_reward": True,
- "staked_node_id": 3,
- "staked_account_id": None
- }
-
+ from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo as StakingInfoProto
+
+ staking_info_proto = StakingInfoProto(
+ decline_reward=True,
+ staked_node_id=3
+ )
+
proto = CryptoGetInfoResponse.AccountInfo(
accountID=AccountId(0, 0, 100)._to_proto(),
key=public_key._to_proto(),
balance=5000000,
-
-
+ staking_info=staking_info_proto
)
account_info = AccountInfo._from_proto(proto)
-
+
assert account_info.staking_info is not None
assert account_info.staking_info.decline_reward is True
assert account_info.staking_info.staked_node_id == 3🧰 Tools
🪛 Ruff (0.14.14)
80-80: Local variable staking_info is assigned to but never used
Remove assignment to unused variable staking_info
(F841)
| assert account_info.max_automatic_token_associations is None | ||
| assert account_info.staking_info is None |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a type validation test for staking_info.
Per coding guidelines for comprehensive coverage, consider adding a test that verifies passing an invalid type for staking_info (e.g., a dict instead of StakingInfo) raises an appropriate error, if such validation exists in the implementation.
prajeeta15
left a comment
prajeeta15
left a comment
There was a problem hiding this comment.
@mukundkumarjha please address the coderabbitai issues and merge conflicts.
and let us know if you need any assistance :)
CHANGELOG.md
Outdated
| - Added dry-run support and refactored `.github/workflows/bot-workflows.yml` to use dedicated script `.github/scripts/bot-workflows.js` for improved maintainability and testability. (`#1288`) | ||
|
|
||
| ### Changed | ||
| - Refactored AccountInfo class to use the staking_info |
There was a problem hiding this comment.
please write a more descriptive CHANGELOG.md entry with the issue number that is being addressed through this PR.
Signed-off-by: mukundkumarjha <[email protected]>
Signed-off-by: Mukund Jha <[email protected]>
|
@mukundkumarjha Thank you for your contribution! I concur with @prajeeta15 on the suggestions. Do not hesitate to reach out if you need assistance. |
Signed-off-by: Mukund Jha <[email protected]>
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Are there any breaking changes in this and what is the migration plan?
5 participants
Description:
Related issue(s):
Fixes #1366
Notes for reviewer:
Checklist