-
Notifications
You must be signed in to change notification settings - Fork 91
feat: support hbar transfer #942
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
Signed-off-by: prajeeta pal <[email protected]>
|
@tech0priyanshu if you are available for a review |
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 @prajeeta15 this branch I think was a branch from your workflow check? or a non-updated main?
You are submitting https://github.com/hiero-ledger/hiero-sdk-python/pull/942/files
two PRs: one for PR check and one for hbar
Please make sure to pull latest changes to main, ensure your origin is matching the upstream main, and then rebase your branch with that -- you may want to make a new branch
|
i switched branches (just before push) to push the latest changes so I think the last PR commit (from old branch) got here too (git push oldbranch: new branch) .. but that was already merged and I had taken latest upstream from main. |
|
Over the weekend we had to apply a hot fix, and pulled split the workflow in two |
| tinybar = amount if isinstance(amount, int) else amount.to_tinybars() | ||
|
|
||
| if tinybar == 0: | ||
| raise ValueError("Amount must be a non-zero value.") |
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.
You changed the error return to:
raise ValueError("Amount must be a non-zero value.")
Please add a Breaking Change section
and briefly document that small breaking change
aditinally, make sure your tests are changed to grab this new error message
| transfer_tx = TransferTransaction() | ||
|
|
||
| hbar_value = Hbar(1.5, HbarUnit.HBAR) | ||
| expected_tinybars = 1_500_000_000 |
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 conversion is incorrcet
| transfer_tx.add_hbar_transfer(account_id_2, micro_hbar_value) | ||
|
|
||
| transfer_2 = next(t for t in transfer_tx.hbar_transfers if t.account_id == account_id_2) | ||
| assert transfer_2.amount == expected_tinybars |
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.
maybe also edit
tests/integration/transfer_transaction_e2e_test.py
to test hbar units?
|
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. |
Add Hbar object support to TransferTransaction and add tests
Allow
_add_hbar_transfer,add_hbar_transfer, andadd_approved_hbar_transferto acceptHbarobjects in addition to raw integer tinybar amounts. NormalizeHbarinputs to tinybars immediately (viaHbar.to_tinybars()), store the resulting tinybar integer inHbarTransfer, and keep existing integer behavior intact. Add comprehensive unit tests covering the new behavior and edge cases.Union[int, Hbar]for_add_hbar_transfer,add_hbar_transfer, andadd_approved_hbar_transfer.TypeErrorfor invalidaccount_id,amount, oris_approvedtypes.Hbarto tinybars viaHbar.to_tinybars()and store the tinybar integer inHbarTransfer.ValueErrorfor zero tinybar amounts).account_idby adding tinybars.tests/unit/transaction/test_transfer_transaction.py) that cover:Hbarobjects (HBAR and TINYBAR units),Hbarobjects,Hbarand int inputs,Hbar(0),amount,Hbarunits to tinybars,Addedentry describing the feature and tests.Related issue(s):
Fixes #917
Notes for reviewer:
Hbarinputs are normalized immediately to tinybars; theHbarobject is not stored—only the tinybar integer is stored inHbarTransfer.account_idmust be anAccountIdinstance (TypeErrorotherwise).amountmust be anintorHbar(TypeErrorotherwise).is_approvedmust bebool(TypeErrorotherwise).ValueError("Amount must be a non-zero value.").HbarTransfer(ensure they expect tinybar integers) and toHbar.to_tinybars()behavior.Checklist