Skip to content

Conversation

@prajeeta15
Copy link
Contributor

Add Hbar object support to TransferTransaction and add tests

Allow _add_hbar_transfer, add_hbar_transfer, and add_approved_hbar_transfer to accept Hbar objects in addition to raw integer tinybar amounts. Normalize Hbar inputs to tinybars immediately (via Hbar.to_tinybars()), store the resulting tinybar integer in HbarTransfer, and keep existing integer behavior intact. Add comprehensive unit tests covering the new behavior and edge cases.

  • Accept Union[int, Hbar] for _add_hbar_transfer, add_hbar_transfer, and add_approved_hbar_transfer.
  • Add runtime type validation and raise TypeError for invalid account_id, amount, or is_approved types.
  • Normalize Hbar to tinybars via Hbar.to_tinybars() and store the tinybar integer in HbarTransfer.
  • Reject zero-value transfers (ValueError for zero tinybar amounts).
  • Accumulate multiple transfers to the same account_id by adding tinybars.
  • Add unit tests (tests/unit/transaction/test_transfer_transaction.py) that cover:
    • constructor behavior with pre-populated transfers,
    • adding HBAR transfers with integer tinybars,
    • adding HBAR transfers with Hbar objects (HBAR and TINYBAR units),
    • approved HBAR transfers with Hbar objects,
    • accumulation for mixed Hbar and int inputs,
    • zero-value validation for ints and Hbar(0),
    • invalid-type validation for amount,
    • conversion accuracy from various Hbar units to tinybars,
    • existing token and NFT transfer behavior, building transaction and scheduled bodies, and frozen transaction behavior.
  • Update changelog with an Added entry describing the feature and tests.

Related issue(s):

Fixes #917

Notes for reviewer:

  • Backwards compatible: integer inputs behave exactly as before.
  • Hbar inputs are normalized immediately to tinybars; the Hbar object is not stored—only the tinybar integer is stored in HbarTransfer.
  • Type validation:
    • account_id must be an AccountId instance (TypeError otherwise).
    • amount must be an int or Hbar (TypeError otherwise).
    • is_approved must be bool (TypeError otherwise).
  • Zero amounts are rejected with ValueError("Amount must be a non-zero value.").
  • Tests added exercise both positive and negative amounts, edge large/small integer boundaries, token/NFT behavior, and frozen transaction immutability.
  • Suggested reviewers should pay attention to any consumers of HbarTransfer (ensure they expect tinybar integers) and to Hbar.to_tinybars() behavior.

Checklist

  • Documented (Code comments, README, changelog entry)
  • Tested (unit, integration, etc.)

Signed-off-by: prajeeta pal <prajeetapal@gmail.com>
Signed-off-by: prajeeta pal <prajeetapal@gmail.com>
Signed-off-by: prajeeta pal <prajeetapal@gmail.com>
@github-actions
Copy link

github-actions bot commented Dec 9, 2025

Hi, this is MergeConflictBot.
Your pull request cannot be merged because it contains merge conflicts.

Please resolve these conflicts locally and push the changes.

To assist you, please read:

Thank you for contributing!

From the Hiero Python SDK Team

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

Hi, this is WorkflowBot.
Your pull request cannot be merged as it is not passing all our workflow checks.
Please click on each check to review the logs and resolve issues so all checks pass.
To help you:

@exploreriii
Copy link
Contributor

Requet review if available @MonaaEid @Adityarya11

@prajeeta15 prajeeta15 marked this pull request as draft December 10, 2025 09:06
Signed-off-by: prajeeta pal <prajeetapal@gmail.com>
Signed-off-by: prajeeta  <96904203+prajeeta15@users.noreply.github.com>
@prajeeta15 prajeeta15 marked this pull request as ready for review December 10, 2025 09:11
Copy link
Contributor

@exploreriii exploreriii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prajeeta15 please rebase and also add your changelog check correctly :)
thank you

@exploreriii exploreriii marked this pull request as draft December 10, 2025 15:29
@Adityarya11
Copy link
Contributor

Adityarya11 commented Dec 11, 2025

Hi @prajeeta15 please rebase and also add your changelog check correctly :) thank you

hi @prajeeta15 if the ChangeLog is messed up you may try git checkout main -- CHANGELOG.md
then add your change then push force
but first rebase and fetch the upstream
Thanks

Copy link
Contributor

@exploreriii exploreriii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes agree, you can just copy in the main's changelog code then add your change on top and commit that.

Signed-off-by: prajeeta  <96904203+prajeeta15@users.noreply.github.com>
@prajeeta15 prajeeta15 marked this pull request as ready for review December 12, 2025 18:06
@prajeeta15
Copy link
Contributor Author

updated changelog

@Adityarya11
Copy link
Contributor

Adityarya11 commented Dec 12, 2025

updated changelog

Hi @prajeeta15
Change log should only contain your feature
Only one addition in file
You can do
Git checkout main -- CHANGELOG.md
The. Just add one line of your work , commit and push the commit
Thanks

Signed-off-by: prajeeta pal <prajeetapal@gmail.com>
@prajeeta15
Copy link
Contributor Author

prajeeta15 commented Dec 12, 2025

@Adityarya11 yeah I was having an issue during rebase so I had just copy-pasted the current changelog.md from main to this. but cool. i pulled and updated changelog with just my lines of work.

@exploreriii
Copy link
Contributor

Request review if available @hiero-ledger/hiero-sdk-python-triage


for transfer in self.hbar_transfers:
if transfer.account_id == account_id:
transfer.amount += amount
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what could happen if the amount is equal to 0 here?

Comment on lines 151 to +152
if not isinstance(amount, int) or amount == 0:
raise ValueError("Amount must be a non-zero integer.")
raise ValueError("Amount must be a integer.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the previous version, is a bit cleaner and also here you have two if with quite te same condition:

  1. if not isinstance(amount, int) or amount == 0 (look after the or)
  2. if amount == 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow TransferTransaction add_hbar_transfer to handle Hbar as an input type

5 participants