`_add_token_transfer` silently ignores `is_approved` when merging transfers for the same account

[mohityadav8]

Description

When _add_token_transfer() is called multiple times for the same (token_id, account_id) pair, the transfer amount is accumulated correctly, but the is_approved flag is never updated in the accumulation branch.

This causes approved transfers to silently lose their approval state depending on call order.

Example:

tx.add_token_transfer(token_a, sender, 500)
# is_approved=False (default)

tx.add_approved_token_transfer(token_a, sender, 200)
# expected accumulated transfer to become approved

### Steps to reproduce

1. Create a transaction.
2. Add a normal token transfer for a `(token_id, account_id)` pair.
3. Add an approved token transfer for the same `(token_id, account_id)` pair.
4. Inspect the accumulated transfer object.

Example:

```python
tx.add_token_transfer(token_a, sender, 500)
tx.add_approved_token_transfer(token_a, sender, 200)

Observed result:

is_approved == False

Expected result:

is_approved == True

Additional context

Root cause appears to be inside _add_token_transfer() where the accumulation branch updates amount and expected_decimals, but never updates is_approved.

Current logic:

for transfer in self.token_transfers[token_id]:
    if transfer.account_id == account_id:
        transfer.amount += amount
        transfer.expected_decimals = expected_decimals
        return

Suggested fix:

transfer.is_approved = is_approved

before returning from the accumulation branch.

This can affect allowance-based transfer flows where regular and approved transfers are combined in the same transaction.

Hedera network

No response

Version

v0.2.5

Operating system

None

[mohityadav8]

Hi @exploreriii, assign this to me I am working on this.

[github-actions]

Hello, this is CommunityCallBot.

This is a reminder that the Hiero Python SDK Community Call will begin in approximately 3 hours and 3 minutes (14:00 UTC).

The call is an open forum where contributors and users can discuss topics, raise issues, and influence the direction of the Python SDK.

Details:

• Time: 14:00 UTC
• Join Link: Zoom Meeting

Disclaimer: This is an automated reminder. Please verify the schedule here for any changes.

[github-actions]

@coderabbitai plan

[coderabbitai]

Coding Plan

Summary

• Apply a minimal, targeted fix that adds is_approved update to the existing accumulation branch, following the established pattern for expected_decimals
• Implement "last call wins" semantics for is_approved, which is consistent with how other fields are handled during accumulation
• Add tests that both verify the fix and explicitly document the expected behavior when mixing approved and non-approved transfers

Implementation Steps

Bug Fix and Test Coverage

This phase addresses the core bug and adds the necessary test coverage to prevent regression. Both tasks are tightly coupled since the fix is simple and tests should verify the corrected behavior.

Task 1: Update `_add_token_transfer` to preserve `is_approved` during accumulation

Fix the accumulation branch to update the is_approved flag, matching the existing pattern used for expected_decimals.

• Update src/hiero_sdk_python/tokens/abstract_token_transfer_transaction.py in the accumulation branch of _add_token_transfer (around line 156-157)
• Add transfer.is_approved = is_approved before the return statement, following the same pattern as the expected_decimals assignment
• This implements "last call wins" semantics, consistent with how expected_decimals is already handled

Task 2: Add unit tests for `is_approved` accumulation behavior

Add comprehensive tests to verify the fix and document the expected semantics for is_approved during transfer merging.

• Add tests in tests/unit/transfer_transaction_test.py covering:
  • Normal transfer followed by approved transfer for same (token_id, account_id) → verify is_approved becomes True
  • Approved transfer followed by normal transfer for same pair → verify is_approved becomes False (documenting "last wins" semantics)
  • Test with add_approved_token_transfer (the non-decimal variant, which currently lacks dedicated tests)
  • Verify the amount accumulation still works correctly alongside the is_approved update
• Follow the existing test patterns in the file (e.g., similar structure to test_approved_token_transfer_accumulation)

🤖 Prompt for AI agents

This phase implements a targeted bug fix in `_add_token_transfer` and adds
corresponding unit tests to prevent regression.

**Task 1: Fix `_add_token_transfer` in
`src/hiero_sdk_python/tokens/abstract_token_transfer_transaction.py`**
- Locate the accumulation branch of `_add_token_transfer` (around line 156-157)
where an existing `TokenTransfer` for the same `(token_id, account_id)` pair is
updated
- Add `transfer.is_approved = is_approved` before the `return` statement in that
branch, following the same pattern already used for `expected_decimals`
- This ensures "last call wins" semantics for `is_approved`, consistent with how
other fields are handled

**Task 2: Add unit tests in `tests/unit/transfer_transaction_test.py`**
- Add tests covering the following scenarios:
- A normal transfer followed by an approved transfer for the same `(token_id,
account_id)` pair → assert `is_approved` is `True`
- An approved transfer followed by a normal transfer for the same pair → assert
`is_approved` is `False` (documenting "last wins" semantics)
- Usage of `add_approved_token_transfer` (the non-decimal variant, which
currently lacks dedicated tests)
- Verify that amount accumulation still works correctly alongside the
`is_approved` update
- Follow existing test patterns in the file, using a structure similar to
`test_approved_token_transfer_accumulation`

Research

The project is a Python SDK for the Hiero/Hedera network. Token transfer functionality lives in src/hiero_sdk_python/tokens/, with AbstractTokenTransferTransaction providing the core _add_token_transfer method that handles transfer accumulation. The TokenTransfer class holds individual transfer data including amount, expected_decimals, and is_approved fields. Unit tests are organized in tests/unit/ with transfer_transaction_test.py containing comprehensive coverage for TransferTransaction, though the non-decimal add_approved_token_transfer variant lacks dedicated tests.

---

🚀 Next Steps

🤖 All AI agent prompts combined

This phase implements a targeted bug fix in `_add_token_transfer` and adds
corresponding unit tests to prevent regression.

**Task 1: Fix `_add_token_transfer` in
`src/hiero_sdk_python/tokens/abstract_token_transfer_transaction.py`**
- Locate the accumulation branch of `_add_token_transfer` (around line 156-157)
where an existing `TokenTransfer` for the same `(token_id, account_id)` pair is
updated
- Add `transfer.is_approved = is_approved` before the `return` statement in that
branch, following the same pattern already used for `expected_decimals`
- This ensures "last call wins" semantics for `is_approved`, consistent with how
other fields are handled

**Task 2: Add unit tests in `tests/unit/transfer_transaction_test.py`**
- Add tests covering the following scenarios:
- A normal transfer followed by an approved transfer for the same `(token_id,
account_id)` pair → assert `is_approved` is `True`
- An approved transfer followed by a normal transfer for the same pair → assert
`is_approved` is `False` (documenting "last wins" semantics)
- Usage of `add_approved_token_transfer` (the non-decimal variant, which
currently lacks dedicated tests)
- Verify that amount accumulation still works correctly alongside the
`is_approved` update
- Follow existing test patterns in the file, using a structure similar to
`test_approved_token_transfer_accumulation`

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!