-
Notifications
You must be signed in to change notification settings - Fork 144
refactor: use native AccountInfo string representation in examples #1269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR refactors three account creation example scripts to use the native Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
aceppaluni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drtoxic69 This is awesome work 👍
The refactor correctly replaces manual AccountInfo serialization with the SDK’s native str representation, and the examples remain clear and runnable.
Removal of examples/utils.py makes sense and is properly documented in the changelog.
One minor non-blocking note: a few example docstrings look duplicated after the refactor. This could be cleaned up later for readability, but not an issue for this PR.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
aceppaluni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great work 👍
The PR currently has merge conflicts with main.
Please rebase or merge main and resolve conflicts so we can proceed with final review/merge.
cdd164c to
9edf1e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdexamples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/utils.py
💤 Files with no reviewable changes (1)
- examples/utils.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧬 Code graph analysis (2)
examples/account/account_create_transaction_evm_alias.py (1)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_alias(220-244)
examples/account/account_create_transaction_create_with_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_key_with_alias(109-135)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/transaction/transaction_receipt.py (1)
account_id(88-100)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_evm_alias.py
52-52: Consider moving this statement to an else block
(TRY300)
53-53: Do not catch blind exception: Exception
(BLE001)
158-158: Do not catch blind exception: Exception
(BLE001)
examples/account/account_create_transaction_create_with_alias.py
48-48: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
125-125: Do not catch blind exception: Exception
(BLE001)
126-126: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
126-126: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (13)
CHANGELOG.md (1)
121-121: LGTM!The changelog entries properly document both the refactoring of the example files to use native
AccountInfo.__str__and the removal ofexamples/utils.py. Issue references are correctly included.Also applies to: 138-140
examples/account/account_create_transaction_create_with_alias.py (6)
1-28: LGTM!The module docstring clearly explains the advanced scenario with separate main and alias keys. Imports are valid and use the correct top-level package.
32-52: LGTM!The
setup_clientfunction correctly returns operator credentials for use in transaction signing. The broad exception handling is appropriate for an example script that users copy-paste.
55-84: LGTM!The function correctly generates an ED25519 main key and an ECDSA alias key. The validation that the EVM address was successfully generated is good defensive coding.
132-135: LGTM!The function correctly uses method chaining for the query and returns the result directly.
138-147: LGTM!The function correctly uses the native
AccountInfo.__str__method for display, which is the primary objective of this PR. This simplifies the example by removing the need for manual JSON serialization.
150-165: LGTM!The main function correctly orchestrates the workflow, properly unpacks the operator credentials, and follows standard example patterns with centralized error handling.
examples/account/account_create_transaction_evm_alias.py (6)
1-31: LGTM!The module docstring clearly explains the EVM alias creation workflow. The
ResponseCodeimport is correctly included for transaction status validation.
34-56: LGTM!The
setup_clientfunction is consistent with the pattern in the other example file.
58-83: LGTM!The function correctly generates an ECDSA key pair and derives the EVM address. The comments explaining the EVM address derivation are helpful for users.
122-137: LGTM!The
print_account_summaryfunction correctly leveragesAccountInfo.__str__for display, fulfilling the PR objective of using native string representation instead of manual JSON serialization.
140-164: LGTM!The main function provides a clear numbered workflow that users can follow. The error handling is consistent with project patterns.
103-120: The code is correct as written.Transaction.execute(client)returns aTransactionReceiptdirectly (not aTransactionResponse), which has both.statusand.account_idproperties. No changes needed.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1269 +/- ##
=======================================
Coverage 92.29% 92.29%
=======================================
Files 139 139
Lines 8515 8515
=======================================
Hits 7859 7859
Misses 656 656 🚀 New features to boost your workflow:
|
|
Hi, this is WorkflowBot.
|
|
Hi @drtoxic69 you should be able to run your scripts locally to test them 👍 |
tech0priyanshu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @drtoxic69, you did a fantastic job your efforts are commendable. One suggestion from my side would be to scope this down to only addressing the issue requirements (using the native str).
|
Additionally, the following items still need work:
CC: @exploreriii @aceppaluni please confirm if I missed anything |
aceppaluni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drtoxic69
This is great work 👍
Since examples/utils.py was removed in this PR, examples/account/account_create_transaction_without_alias.py still imports info_to_dict from examples.utils, which causes a ModuleNotFoundError.
To keep this PR scoped to the original goal (using print(account_info)), I think this import can be removed entirely in that example, or replaced with the native str usage as done elsewhere.
75440ae to
0c65559
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/account/account_create_transaction_evm_alias.py (1)
142-161: Main function is correct.The orchestration is sound with proper error handling.
Minor note: The comment at line 148 ("Original signature...") appears to be a development artifact and could be removed for clarity, but this is purely optional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdexamples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/utils.py
💤 Files with no reviewable changes (1)
- examples/utils.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧬 Code graph analysis (2)
examples/account/account_create_transaction_evm_alias.py (1)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_key(76-92)
examples/account/account_create_transaction_create_with_alias.py (2)
src/hiero_sdk_python/transaction/transaction_receipt.py (1)
account_id(88-100)src/hiero_sdk_python/transaction/transaction_response.py (1)
get_receipt(27-47)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_evm_alias.py
48-48: Consider moving this statement to an else block
(TRY300)
49-49: Do not catch blind exception: Exception
(BLE001)
examples/account/account_create_transaction_create_with_alias.py
117-117: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (13)
CHANGELOG.md (1)
137-137: LGTM!The changelog entries accurately document the refactoring work: using native
AccountInfo.__str__in the examples and removing the now-unusedexamples/utils.py. Both entries properly reference issue #1263.Also applies to: 157-157
examples/account/account_create_transaction_create_with_alias.py (7)
1-28: LGTM!The imports are correct and follow the SDK's public API patterns. The module docstring clearly explains what this example demonstrates and provides usage instructions.
33-48: LGTM!The addition of the
-> Clientreturn type annotation improves clarity and aligns with Python best practices. The function correctly initializes the client with operator credentials.
51-77: LGTM!The function correctly generates both keys and derives the EVM address from the ECDSA public key. Error handling for null EVM address is appropriate.
80-120: LGTM!The transaction lifecycle is correct:
freeze_with(client)is called beforesign(alias_private_key), followed byexecute(client). The fallback logic for retrievingaccount_idviaget_receipt()is appropriate, and the exception chaining withfrom errfollows best practices.
123-135: LGTM!The function correctly constructs and executes the
AccountInfoQuery. The direct return style is concise and appropriate.
138-153: LGTM!The function now leverages the native
AccountInfo.__str__()method, which is the primary objective of this PR. This simplifies the example by removing manual JSON serialization and makes the code more maintainable.
155-167: LGTM!The main function correctly orchestrates the example workflow with proper error handling and follows the standard
if __name__ == "__main__"pattern.examples/account/account_create_transaction_evm_alias.py (5)
1-32: LGTM!The imports are correct and the module docstring provides a clear explanation of what the example demonstrates, with helpful usage instructions.
34-51: LGTM!The
-> Clientreturn type annotation improves clarity. The function correctly initializes the client with operator credentials and handles missing environment variables appropriately.
54-81: LGTM!The function correctly generates an ECDSA key pair and derives the EVM address. The enhanced docstring and inline comments effectively explain the EVM alias derivation process, which is valuable for users learning the SDK.
125-127: LGTM!The function correctly executes the
AccountInfoQuerywith proper type hints.
130-140: LGTM!The function now uses the native
AccountInfo.__str__()method, which is the core objective of this PR. This simplifies the example and removes the dependency on manual serialization helpers.
0c65559 to
f1d7ba1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
examples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧬 Code graph analysis (2)
examples/account/account_create_transaction_evm_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_alias(220-244)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/crypto/private_key.py (2)
PrivateKey(14-471)public_key(305-309)
examples/account/account_create_transaction_create_with_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (1)
AccountCreateTransaction(27-380)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/query/account_info_query.py (1)
AccountInfoQuery(11-131)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_evm_alias.py
47-47: Consider moving this statement to an else block
(TRY300)
48-48: Do not catch blind exception: Exception
(BLE001)
120-120: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_create_with_alias.py
117-117: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
examples/account/account_create_transaction_create_with_alias.py (3)
13-27: LGTM: Imports are clean and correct.All imports use valid top-level paths from the SDK and are utilized in the example.
145-147: Excellent: Core PR objective achieved.The function now uses the native
AccountInfo.__str__()method, making the example simpler and more maintainable.
107-120: LGTM: Transaction lifecycle and error handling are correct.The freeze → sign → execute order is correct, and the safe retrieval pattern with exception chaining is robust. The code properly attempts
response.account_idbefore falling back toget_receipt(client).account_id.examples/account/account_create_transaction_evm_alias.py (1)
105-120: LGTM: Transaction lifecycle is correct.The freeze → sign → execute order is proper, and the safe retrieval pattern with exception chaining matches the pattern in
account_create_transaction_create_with_alias.py. The comment correctly explains thatclient.execute()attaches the operator signature automatically.
examples/account/account_create_transaction_create_with_alias.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
examples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧬 Code graph analysis (1)
examples/account/account_create_transaction_evm_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_alias(220-244)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/crypto/public_key.py (1)
PublicKey(22-556)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_evm_alias.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
121-121: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_create_with_alias.py
114-114: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
examples/account/account_create_transaction_create_with_alias.py (3)
142-143: LGTM: Core PR objective achieved.The switch to
print(account_info)successfully leverages the SDK's nativeAccountInfo.__str__()representation, replacing the previous manual JSON serialization approach. This simplifies the example and aligns with the PR objectives.
104-109: LGTM: Transaction lifecycle is correct.The transaction properly follows the required sequence:
freeze_with(client)- locks the transaction.sign(alias_private_key)- adds alias signature.execute(client)- adds operator signature and executesThe comment accurately explains that the operator signature is automatically added during execution.
99-102: Theset_key_with_aliasmethod exists and is properly implemented inAccountCreateTransaction. The example code is correct and follows the established SDK pattern confirmed by extensive unit and integration tests.examples/account/account_create_transaction_evm_alias.py (3)
85-87: LGTM: Type hints added per previous feedback.The function signature now includes complete type annotations (
public_key: PublicKey,evm_address: str,-> AccountId), addressing previous review feedback and improving code clarity for users.
134-136: LGTM: Core PR objective achieved.The direct
print(account_info)call successfully uses the nativeAccountInfo.__str__()representation, simplifying the example and achieving the PR's main goal of removing manual JSON serialization.
109-116: LGTM: Transaction lifecycle is correct.The transaction follows the proper sequence:
- Build with
.set_key(),.set_initial_balance(),.set_alias()freeze_with(client)- locks transaction parameters.sign(private_key)- adds alias key signature.execute(client)- adds operator signature and executesThe
set_aliasmethod usage is verified in the SDK codebase. Comments clearly explain that the operator signature is automatically added during execution.
exploreriii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @drtoxic69
I notice you deleted utils.py, in which case please make sure it is not needed in any other example else the run will fail 👍
❌ Example failed: examples/account/account_create_transaction_without_alias.py
************ Error Output ************
Traceback (most recent call last):
File "", line 198, in _run_module_as_main
File "", line 88, in _run_code
File "/home/runner/work/hiero-sdk-python/hiero-sdk-python/examples/account/account_create_transaction_without_alias.py", line 19, in
from examples.utils import info_to_dict
ModuleNotFoundError: No module named 'examples.utils'
331cf88 to
bf0e6f6
Compare
| @@ -1,36 +1,52 @@ | |||
| """ | |||
| Example: Create an account without using any alias. | |||
| """Example: Create an account without using any alias. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @drtoxic69 this is ready to merge, would you mind uncommiting your changes to
account_Create_transaction_without_alias?
There was a new update to that file, coincidentally, which in the feedback eventually ended up correcting the same issue as you did - but is using a more modern client set up, so prefer to keep that one.
The other 3 can stay, and we can merge this right away.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.mdexamples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_with_fallback_alias.pyexamples/account/account_create_transaction_without_alias.pyexamples/utils.py
💤 Files with no reviewable changes (1)
- examples/utils.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_without_alias.pyexamples/account/account_create_transaction_with_fallback_alias.py
🧬 Code graph analysis (2)
examples/account/account_create_transaction_create_with_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (1)
AccountCreateTransaction(27-380)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/query/account_info_query.py (1)
AccountInfoQuery(11-131)
examples/account/account_create_transaction_evm_alias.py (2)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_alias(220-244)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_create_with_alias.py
114-114: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_evm_alias.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
121-121: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_without_alias.py
14-14: typing.Tuple is deprecated, use tuple instead
(UP035)
45-45: Consider moving this statement to an else block
(TRY300)
46-46: Do not catch blind exception: Exception
(BLE001)
80-80: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Do not catch blind exception: Exception
(BLE001)
examples/account/account_create_transaction_with_fallback_alias.py
81-81: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
examples/account/account_create_transaction_create_with_alias.py (3)
1-27: LGTM - Module docstring and imports are correct.The module docstring clearly explains the example's purpose and demonstrates both
uv runandpythonexecution methods. All imports use top-level paths fromhiero_sdk_pythonwhich is acceptable per guidelines, and the imported classes/functions exist in the SDK.
80-117: LGTM - Transaction lifecycle is correct.The transaction follows the proper lifecycle:
- Construct with
AccountCreateTransaction- Set key and alias with
.set_key_with_alias(main_private_key, alias_public_key)- Freeze with
.freeze_with(client)before signing- Sign with
.sign(alias_private_key)- Execute with
.execute(client)The comment at line 105 correctly explains that the operator signature is automatically attached during
client.execute(), which helps users understand the signing model.
135-150: LGTM - Core PR objective achieved.The function correctly uses the native
AccountInfo.__str__representation (line 143) instead of manual JSON serialization, which is the primary objective of this PR. The output formatting with headers and the EVM alias verification provides a good user experience.examples/account/account_create_transaction_without_alias.py (2)
60-83: LGTM - Transaction lifecycle is correct.The transaction properly follows the required lifecycle: construct →
set_key_without_alias→freeze_with(before signing) →sign→execute. The method chaining is clear and the error handling for missing account IDs is appropriate.
102-104: LGTM - Uses native AccountInfo string representation.Correctly leverages
AccountInfo.__str__()for output (line 103), which is the core objective of this PR.examples/account/account_create_transaction_with_fallback_alias.py (2)
66-85: LGTM - Transaction lifecycle is correct.The transaction correctly demonstrates the fallback alias behavior where a single ECDSA key is used for both the account key and EVM alias derivation. The lifecycle (construct →
set_key_with_alias→freeze_with→sign→execute) is properly ordered.
95-102: LGTM - Uses native AccountInfo string representation.Correctly uses
AccountInfo.__str__()for output (line 99), achieving the PR's core objective of replacing manual JSON serialization.examples/account/account_create_transaction_evm_alias.py (2)
111-124: LGTM - Transaction lifecycle and signing flow are correct.The transaction properly follows the lifecycle: freeze → sign (with alias private key) → execute. The comments at lines 111-112 effectively explain the dual-signature model (alias key + automatic operator signature), which helps users understand why the alias private key is required.
132-141: LGTM - Uses native AccountInfo string representation.Correctly leverages
AccountInfo.__str__()for output (line 135), fulfilling the PR's core objective of replacing manual JSON serialization with the built-in string representation.CHANGELOG.md (1)
142-142: LGTM - Changelog entries accurately document the changes.The changelog properly documents:
- The refactoring of example scripts to use
AccountInfo.__str__()(line 142)- The removal of
examples/utils.py(line 162)- Proper reference to issue #1263
Both entries are appropriately categorized under "Changed" and "Removed" sections.
Also applies to: 160-162
|
Hi @drtoxic69 just this PR outstanding please 👍 |
Signed-off-by: Shivakumar <shivakumarjagadish12@gmail.com>
Signed-off-by: Shivakumar <shivakumarjagadish12@gmail.com>
Signed-off-by: Shivakumar <shivakumarjagadish12@gmail.com>
Signed-off-by: Shivakumar <shivakumarjagadish12@gmail.com>
bf0e6f6 to
661d723
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/account/account_create_transaction_create_with_alias.py (1)
33-48: Consider usingClient.from_env()for simpler client initialization.Similar to the other example file, this uses manual client setup. The
Client.from_env()method would reduce boilerplate and follow the project's recommended pattern.♻️ Simplified client setup using Client.from_env()
-def setup_client() -> Client: - """Set up the Hedera client.""" - network = Network(network_name) - print(f"Connecting to Hedera {network_name} network!") - client = Client(network) - - try: - operator_id = AccountId.from_string(os.getenv("OPERATOR_ID", "")) - operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY", "")) - client.set_operator(operator_id, operator_key) - print(f"Client set up with operator id {client.operator_account_id}") - return client - - except Exception: - print("Error: Please check OPERATOR_ID and OPERATOR_KEY in your .env file.") - sys.exit(1) +def setup_client() -> Client: + """Set up the Hedera client from environment variables.""" + try: + client = Client.from_env() + print(f"Client set up with operator id {client.operator_account_id}") + return client + except Exception: + print("Error: Please check OPERATOR_ID, OPERATOR_KEY, and NETWORK in your .env file.") + sys.exit(1)This reduces boilerplate and follows the project's recommended pattern.
Based on learnings, examples should prefer Client.from_env() for client initialization.
examples/account/account_create_transaction_with_fallback_alias.py (1)
32-46: Simplify client setup usingClient.from_env().The manual network and operator configuration can be replaced with
Client.from_env(), which automatically handlesload_dotenv(), network selection, and operator setup. This aligns with the SDK's recommended pattern for examples.Based on learnings, examples should prefer
Client.from_env().♻️ Proposed simplification
-def setup_client() -> Client: - """Set up the Hedera client.""" - network = Network(network_name) - print(f"Connecting to Hedera {network_name} network!") - client = Client(network) - - try: - operator_id = AccountId.from_string(os.getenv("OPERATOR_ID", "")) - operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY", "")) - client.set_operator(operator_id, operator_key) - print(f"Client set up with operator id {client.operator_account_id}") - return client - except Exception: - print("Error: Please check OPERATOR_ID and OPERATOR_KEY in your .env file.") - sys.exit(1) +def setup_client() -> Client: + """Set up the Hedera client from environment variables.""" + try: + client = Client.from_env(network_name) + print(f"Connecting to Hedera {network_name} network!") + print(f"Client set up with operator id {client.operator_account_id}") + return client + except ValueError as error: + print(f"Error: {error}") + print("Please check OPERATOR_ID, OPERATOR_KEY, and NETWORK in your .env file.") + sys.exit(1)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.mdexamples/account/account_create_transaction_create_with_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_with_fallback_alias.pyexamples/utils.py
💤 Files with no reviewable changes (1)
- examples/utils.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_with_fallback_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧠 Learnings (1)
📚 Learning: 2026-01-08T10:28:14.391Z
Learnt from: github-actions[bot]
Repo: hiero-ledger/hiero-sdk-python PR: 0
File: :0-0
Timestamp: 2026-01-08T10:28:14.391Z
Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
Applied to files:
CHANGELOG.mdexamples/account/account_create_transaction_with_fallback_alias.pyexamples/account/account_create_transaction_evm_alias.pyexamples/account/account_create_transaction_create_with_alias.py
🧬 Code graph analysis (1)
examples/account/account_create_transaction_with_fallback_alias.py (5)
src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/account/account_info.py (1)
AccountInfo(20-208)src/hiero_sdk_python/query/account_info_query.py (1)
AccountInfoQuery(11-131)src/hiero_sdk_python/client/client.py (1)
Client(28-255)src/hiero_sdk_python/hbar.py (1)
Hbar(18-213)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_with_fallback_alias.py
81-81: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_evm_alias.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
121-121: Avoid specifying long messages outside the exception class
(TRY003)
examples/account/account_create_transaction_create_with_alias.py
114-114: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (11)
examples/account/account_create_transaction_evm_alias.py (2)
135-136: LGTM! Native AccountInfo string representation is now used.The refactor successfully replaces manual JSON serialization with the native
AccountInfo.__str__()method, which is the core objective of this PR.
109-124: Transaction lifecycle is correct.The transaction follows the proper sequence:
- Freeze with client
- Sign with alias private key
- Execute
- Safe account_id retrieval with error handling
The comment correctly notes that operator signing is handled automatically by
client.execute().CHANGELOG.md (2)
159-160: LGTM! Changelog entry accurately documents the refactor.The entry correctly describes the change to use native
AccountInfo.__str__method and references the proper issue number.
180-183: LGTM! Removal entry is accurate.The entry correctly documents the deletion of
examples/utils.pyand explains why it's no longer needed.examples/account/account_create_transaction_create_with_alias.py (2)
142-144: LGTM! Native AccountInfo string representation is correctly used.The refactor successfully replaces manual JSON serialization with the native
AccountInfo.__str__()method, achieving the PR's objective.
104-117: Transaction lifecycle is correct.The transaction follows the proper sequence:
- Freeze with client
- Sign with alias private key
- Execute
- Safe account_id retrieval with error handling
The implementation correctly requires the alias private key to sign to authorize the alias.
examples/account/account_create_transaction_with_fallback_alias.py (5)
15-26: LGTM! Imports are correctly structured.The imports follow the top-level import pattern and all entities exist in the SDK structure. The addition of
AccountInfoandAccountInfoQueryis appropriate for the refactored output approach.
49-63: LGTM! Key generation logic is correct.The function properly generates an ECDSA key pair, derives the EVM address, and validates it before proceeding. The formatting adjustments improve readability.
66-85: LGTM! Transaction lifecycle is correct.The transaction properly follows the SDK pattern:
- Construction with
AccountCreateTransactionandset_key_with_aliasfreeze_with(client)called before signing (line 75)- Signing with the account key
- Explicit
execute(client)call- Appropriate validation of the returned
account_idType hints and error handling are well-implemented.
88-102: Excellent! Core refactoring correctly implemented.The change from JSON serialization to native
AccountInfo.__str__representation is the primary objective of this PR and is properly executed:
AccountInfoQueryusage follows the correct SDK pattern (line 92)- Direct
print(account_info)leverages the SDK's built-in string formatting (line 99)- Explicit display of
contract_account_id(EVM alias) maintains clarity for users (line 101)Type hints enhance code clarity for example users.
104-119: LGTM! Main function correctly orchestrates the example.The execution flow is clear and follows the standard pattern for SDK examples:
- Client setup
- Key generation
- Account creation
- Account info retrieval
- Results display
Error handling with
sys.exit(1)is appropriate for example scripts.
| def setup_client() -> Client: | ||
| """Initialize client and return operator credentials.""" | ||
| network = Network(network_name) | ||
| print(f"Connecting to Hedera {network_name} network!") | ||
| client = Client(network) | ||
|
|
||
| # Get the operator account from the .env file | ||
| try: | ||
| operator_id = AccountId.from_string(os.getenv("OPERATOR_ID", "")) | ||
| operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY", "")) | ||
|
|
||
| # Set the operator (payer) account for the client | ||
| client.set_operator(operator_id, operator_key) | ||
| print(f"Client set up with operator id {client.operator_account_id}") | ||
|
|
||
| return client | ||
| except Exception: | ||
| print("Error: Please check OPERATOR_ID and OPERATOR_KEY in your .env file.") | ||
| sys.exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using Client.from_env() for simpler client initialization.
The manual setup pattern (Network + set_operator) works but adds boilerplate. Based on project learnings, examples should prefer Client.from_env() which automatically reads OPERATOR_ID and OPERATOR_KEY from the environment and simplifies the code.
♻️ Simplified client setup using Client.from_env()
-def setup_client() -> Client:
- """Initialize client and return operator credentials."""
- network = Network(network_name)
- print(f"Connecting to Hedera {network_name} network!")
- client = Client(network)
-
- try:
- operator_id = AccountId.from_string(os.getenv("OPERATOR_ID", ""))
- operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY", ""))
-
- # Set the operator (payer) account for the client
- client.set_operator(operator_id, operator_key)
- print(f"Client set up with operator id {client.operator_account_id}")
-
- return client
- except Exception:
- print("Error: Please check OPERATOR_ID and OPERATOR_KEY in your .env file.")
- sys.exit(1)
+def setup_client() -> Client:
+ """Initialize client from environment variables."""
+ try:
+ client = Client.from_env()
+ print(f"Client set up with operator id {client.operator_account_id}")
+ return client
+ except Exception:
+ print("Error: Please check OPERATOR_ID, OPERATOR_KEY, and NETWORK in your .env file.")
+ sys.exit(1)This reduces boilerplate and follows the project's recommended pattern.
Based on learnings, examples should prefer Client.from_env() for client initialization.
🧰 Tools
🪛 Ruff (0.14.10)
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
Description:
This PR refactors the account creation examples to use the native
__str__method of theAccountInfoclass for displaying account details, as requested in the issue.Previously, these examples relied on a helper function in
examples/utils.pyto manually serialize the object to a dictionary and then print it as JSON. This has been replaced with the standardprint(account_info), which leverages the SDK's built-in string formatting.Consequently,
examples/utils.pyis no longer used and has been removed to clean up the codebase.Fixes #1263
Checklist