Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ def _add_token_transfer(
) -> None:
"""Adds a fungible token transfer to the transaction's list.

Transfers for the same (token_id, account_id) pair are merged into a single
entry only when their approval status matches; a regular and an approved
transfer for the same account are kept as separate entries, since spending
an allowance is semantically different from spending the account's own
balance. When merging, an explicitly provided expected_decimals updates the
entry; passing None leaves the previously set value unchanged.

Args:
token_id (TokenId): The ID of the fungible token being transferred.
account_id (AccountId): The account ID of the sender (negative amount)
Expand Down Expand Up @@ -154,9 +161,10 @@ def _add_token_transfer(
raise TypeError("is_approved must be a boolean.")

for transfer in self.token_transfers[token_id]:
if transfer.account_id == account_id:
if transfer.account_id == account_id and transfer.is_approved == is_approved:
transfer.amount += amount
transfer.expected_decimals = expected_decimals
if expected_decimals is not None:
transfer.expected_decimals = expected_decimals
Comment on lines +164 to +167
Comment on lines +164 to +167

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep expected_decimals token-scoped for approval-separated transfers.

Proto field: TokenTransferList.expected_decimals (field 4). Issue type: Asymmetric round-trip. The schema defines this value once per token transfer list, not once per account amount. (github.com)

If a regular transfer without decimals is added first and an approved transfer with decimals is added second, Line 164 prevents a merge. build_token_transfers() then serializes the first entry's None value and drops the caller's expected decimals. The transaction can skip the requested decimal validation.

  • src/hiero_sdk_python/tokens/abstract_token_transfer_transaction.py#L164-L167: propagate an explicit decimal value to the token's existing entries, and inherit the token decimal value for a new entry when the caller passes None.
  • tests/unit/transfer_transaction_test.py#L548-L583: build a transaction with a normal no-decimal transfer followed by an approved decimal transfer, then assert tokenTransfers[0].expected_decimals.value.
Proposed implementation direction
+        transfers = self.token_transfers[token_id]
+        if expected_decimals is not None:
+            for existing_transfer in transfers:
+                existing_transfer.expected_decimals = expected_decimals
+        elif transfers:
+            expected_decimals = transfers[0].expected_decimals
+
-        for transfer in self.token_transfers[token_id]:
+        for transfer in transfers:
             if transfer.account_id == account_id and transfer.is_approved == is_approved:
                 transfer.amount += amount
-                if expected_decimals is not None:
-                    transfer.expected_decimals = expected_decimals
+                transfer.expected_decimals = expected_decimals
                 return

-        self.token_transfers[token_id].append(
+        transfers.append(
             TokenTransfer(token_id, account_id, amount, expected_decimals, is_approved)
         )

As per path instructions, protobuf fields must match their schema semantics and unit tests must cover edge cases.

📍 Affects 2 files
  • src/hiero_sdk_python/tokens/abstract_token_transfer_transaction.py#L164-L167 (this comment)
  • tests/unit/transfer_transaction_test.py#L548-L583

Source: Path instructions

return

self.token_transfers[token_id].append(
Expand Down
5 changes: 2 additions & 3 deletions tests/integration/transfer_transaction_e2e_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,8 @@ def test_integration_transfer_transaction_approved_token_transfer(env):
transfer_receipt = (
TransferTransaction()
.set_transaction_id(TransactionId.generate(account.id))
.add_approved_token_transfer_with_decimals(token_id, account.id, 500, 2)
.add_approved_token_transfer_with_decimals(token_id, env.operator_id, -499, 2)
.add_token_transfer_with_decimals(token_id, account.id, -1, 2)
.add_approved_token_transfer_with_decimals(token_id, env.operator_id, -500, 2)
.add_token_transfer_with_decimals(token_id, account.id, 500, 2)
.freeze_with(env.client)
.sign(account.key)
.execute(env.client)
Expand Down
126 changes: 115 additions & 11 deletions tests/unit/transfer_transaction_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ def test_approved_token_transfer_with_decimals(mock_account_ids):


def test_approved_token_transfer_accumulation(mock_account_ids):
"""Test that approved token transfers accumulate for the same account."""
"""Test that approved token transfers are stored as separate entries from normal ones."""
account_id_1, account_id_2, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()

Expand All @@ -473,18 +473,122 @@ def test_approved_token_transfer_accumulation(mock_account_ids):
assert transfer_2.is_approved is False
assert transfer_2.expected_decimals is None

# Add approved transfer with decimals for account_1 (accumulates)
# Add approved transfer with decimals for account_1 (separate entry, not merged)
transfer_tx.add_approved_token_transfer_with_decimals(token_id_1, account_id_1, 200, 8)

# Verify accumulation
transfer_1 = transfer_tx.token_transfers[token_id_1][0]
transfer_2 = transfer_tx.token_transfers[token_id_1][1]
assert transfer_1.amount == 700 # 500 + 200
assert transfer_1.is_approved is False # unchanged
assert transfer_1.expected_decimals == 8 # updated from the accumulation
assert transfer_2.amount == 300 # unchanged
assert transfer_2.is_approved is False # unchanged
assert transfer_2.expected_decimals is None # unchanged
# Verify stored as separate entries
transfers = transfer_tx.token_transfers[token_id_1]
assert len(transfers) == 3 # account_1 normal, account_2 normal, account_1 approved

assert transfers[0].amount == 500 # unchanged
assert transfers[0].is_approved is False # unchanged
assert transfers[1].amount == 300 # unchanged
assert transfers[1].is_approved is False # unchanged
assert transfers[2].amount == 200
assert transfers[2].is_approved is True
assert transfers[2].expected_decimals == 8


def test_normal_and_approved_transfers_kept_separate(mock_account_ids):
"""Normal and approved transfers for the same account are stored as separate entries."""
account_id_1, account_id_2, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()

transfer_tx.add_token_transfer(token_id_1, account_id_1, 500)
transfer_tx.add_token_transfer(token_id_1, account_id_2, -500)
transfer_tx.add_approved_token_transfer(token_id_1, account_id_1, 200)
transfer_tx.add_token_transfer(token_id_1, account_id_2, -200)

transfers = transfer_tx.token_transfers[token_id_1]
assert len(transfers) == 3 # account_1 normal, account_2 accumulated, account_1 approved

assert transfers[0].amount == 500
assert transfers[0].is_approved is False

assert transfers[1].amount == -700 # -500 + -200 accumulated
assert transfers[1].is_approved is False

assert transfers[2].amount == 200
assert transfers[2].is_approved is True


def test_same_approved_transfers_accumulate(mock_account_ids):
"""Two approved transfers for the same account DO accumulate."""
account_id_1, account_id_2, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()

transfer_tx.add_approved_token_transfer(token_id_1, account_id_1, 300)
transfer_tx.add_approved_token_transfer(token_id_1, account_id_1, 200)
transfer_tx.add_token_transfer(token_id_1, account_id_2, -500)

transfers = transfer_tx.token_transfers[token_id_1]
assert len(transfers) == 2 # account_1 approved (merged), account_2 normal

assert transfers[0].amount == 500 # 300 + 200 accumulated
assert transfers[0].is_approved is True

assert transfers[1].amount == -500
assert transfers[1].is_approved is False


def test_add_approved_token_transfer_no_decimals(mock_account_ids):
"""add_approved_token_transfer (non-decimal variant) sets is_approved=True."""
account_id_1, account_id_2, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()

transfer_tx.add_approved_token_transfer(token_id_1, account_id_1, -1000)
transfer_tx.add_token_transfer(token_id_1, account_id_2, 1000)

transfer = transfer_tx.token_transfers[token_id_1][0]
assert transfer.amount == -1000
assert transfer.is_approved is True
assert transfer.expected_decimals is None


def test_merge_preserves_expected_decimals(mock_account_ids):
"""Merging a transfer without decimals must not clear a previously set expected_decimals."""
account_id_1, _, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()

# First transfer specifies decimals=6
transfer_tx.add_token_transfer_with_decimals(token_id_1, account_id_1, 500, 6)
transfer = transfer_tx.token_transfers[token_id_1][0]
assert transfer.amount == 500
assert transfer.expected_decimals == 6

# Second transfer for the same account without decimals (expected_decimals=None)
transfer_tx.add_token_transfer(token_id_1, account_id_1, 200)
transfer = transfer_tx.token_transfers[token_id_1][0]
assert transfer.amount == 700
assert transfer.expected_decimals == 6 # Preserved!

# Third transfer for the same account with updated decimals (expected_decimals=4)
transfer_tx.add_token_transfer_with_decimals(token_id_1, account_id_1, 100, 4)
transfer = transfer_tx.token_transfers[token_id_1][0]
assert transfer.amount == 800
assert transfer.expected_decimals == 4 # Updated!

# Now add approved transfer with decimals=8
transfer_tx.add_approved_token_transfer_with_decimals(token_id_1, account_id_1, 300, 8)
approved_transfer = transfer_tx.token_transfers[token_id_1][1]
assert approved_transfer.amount == 300
assert approved_transfer.expected_decimals == 8
assert approved_transfer.is_approved is True

# Second approved transfer for the same account without decimals
transfer_tx.add_approved_token_transfer(token_id_1, account_id_1, 100)
approved_transfer = transfer_tx.token_transfers[token_id_1][1]
assert approved_transfer.amount == 400
assert approved_transfer.expected_decimals == 8 # Preserved!
assert approved_transfer.is_approved is True


def test_add_token_transfer_invalid_is_approved_type(mock_account_ids):
"""Test _add_token_transfer with invalid type for is_approved."""
account_id_1, _, _, token_id_1, _ = mock_account_ids
transfer_tx = TransferTransaction()
with pytest.raises(TypeError, match="is_approved must be a boolean"):
transfer_tx._add_token_transfer(token_id_1, account_id_1, 100, is_approved="invalid")


@pytest.mark.parametrize(
Expand Down