-
Notifications
You must be signed in to change notification settings - Fork 144
fix: support public for batch_key in Transaction #1051
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
Conversation
|
Hi, this is WorkflowBot.
|
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.
Pull request overview
This PR extends the Transaction class to support both PrivateKey and PublicKey for the batch_key parameter, enabling more flexible batch transaction workflows. Previously, only PrivateKey was accepted, which required users to have access to private keys even when only public key verification was needed.
Key changes:
- Updated type annotations from
PrivateKeytoKey(Union type) for batch_key throughout Transaction class - Modified protobuf conversion to use the centralized
key_to_protoutility function - Added comprehensive unit tests verifying both key types work correctly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/unit/test_batch_key.py | New test file with unit tests verifying batch_key accepts both PrivateKey and PublicKey types |
| src/hiero_sdk_python/transaction/transaction.py | Updated batch_key type annotation from PrivateKey to Key, changed protobuf conversion to use key_to_proto utility, updated method signatures and documentation |
| CHANGELOG.md | Added entry documenting the change to allow PublicKey for batch_key in Transaction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Request review if available @hiero-ledger/hiero-sdk-python-triage |
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.
Hello @AubreyDDD
You may also want to expand eg
examples/transaction/batch_transaction.py
tests/integration/batch_transaction_e2e_test.py
tests/unit/test_batch_transaction.py
|
Request review if available @hiero-ledger/hiero-sdk-python-triage |
|
Hi @AubreyDDD |
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! From the Hiero Python SDK Team |
|
I merged all tests into |
|
Hello, this is the Office Hour Bot. 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 or receive assistance from a maintainer. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here to be notified of any changes. |
|
Request review if available @hiero-ledger/hiero-sdk-python-triage |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request adds support for using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used🧬 Code graph analysis (4)src/hiero_sdk_python/transaction/transaction.py (1)
examples/transaction/batch_transaction.py (2)
tests/unit/batch_transaction_test.py (4)
tests/integration/batch_transaction_e2e_test.py (3)
🪛 Ruff (0.14.8)tests/unit/batch_transaction_test.py372-372: Unused function argument: (ARG001) tests/integration/batch_transaction_e2e_test.py358-358: Redefinition of unused (F811) 399-399: Redefinition of unused (F811) 467-467: Redefinition of unused (F811) ⏰ 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). (8)
🔇 Additional comments (19)
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 @AubreyDDD, Please resolve merge conflicts and update the branch |
Signed-off-by: Aubrey Du <[email protected]>
Signed-off-by: Aubrey Du <[email protected]>
Signed-off-by: Aubrey Du <[email protected]>
976d942 to
54af4ed
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1051 +/- ##
=======================================
Coverage 91.19% 91.19%
=======================================
Files 139 139
Lines 8446 8447 +1
=======================================
+ Hits 7702 7703 +1
Misses 744 744 🚀 New features to boost your workflow:
|
|
Congratulations Aubrey! Nicely documented, very clear and thorough |
Signed-off-by: Aubrey Du <[email protected]> Signed-off-by: prajeeta pal <[email protected]>
Description:
Allow both PrivateKey and PublicKey for batch_key in Transaction class, enabling more flexible batch transaction workflows.
Related issue(s):
Fixes #1015
Notes for reviewer:
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.