Skip to content

Commit bc1e285

Browse files
authored
refactor: simplify delete_token_nft_allowance_all_serials entry handling (hiero-ledger#2294)
Signed-off-by: dosi <dosi.kolev@limechain.tech>
1 parent 2977e47 commit bc1e285

3 files changed

Lines changed: 126 additions & 9 deletions

File tree

src/hiero_sdk_python/account/account_allowance_approve_transaction.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,12 +281,6 @@ def delete_token_nft_allowance_all_serials(
281281
"""
282282
self._require_not_frozen()
283283

284-
for allowance in self.nft_allowances:
285-
if allowance.token_id == token_id and allowance.spender_account_id == spender_account_id:
286-
allowance.serial_numbers = []
287-
allowance.approved_for_all = True
288-
return self
289-
290284
self.nft_allowances.append(
291285
TokenNftAllowance(
292286
token_id=token_id,

tests/integration/account_allowance_e2e_test.py

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@
1313
AccountAllowanceDeleteTransaction,
1414
)
1515
from hiero_sdk_python.hbar import Hbar
16+
from hiero_sdk_python.query.token_nft_info_query import TokenNftInfoQuery
1617
from hiero_sdk_python.response_code import ResponseCode
1718
from hiero_sdk_python.tokens.nft_id import NftId
18-
from hiero_sdk_python.tokens.token_associate_transaction import TokenAssociateTransaction
19+
from hiero_sdk_python.tokens.token_associate_transaction import (
20+
TokenAssociateTransaction,
21+
)
1922
from hiero_sdk_python.tokens.token_mint_transaction import TokenMintTransaction
2023
from hiero_sdk_python.transaction.transaction_id import TransactionId
2124
from hiero_sdk_python.transaction.transfer_transaction import TransferTransaction
@@ -61,7 +64,9 @@ def _mint_nft(env, token_id, metadata):
6164

6265

6366
@pytest.mark.integration
64-
def test_integration_cannot_transfer_on_behalf_of_spender_without_allowance_approval(env):
67+
def test_integration_cannot_transfer_on_behalf_of_spender_without_allowance_approval(
68+
env,
69+
):
6570
"""Test that a spender cannot transfer NFTs on behalf of account without allowance approval."""
6671
spender_account, receiver_account = _create_spender_and_receiver_accounts(env)
6772

@@ -264,7 +269,9 @@ def test_integration_fungible_token_allowance(env):
264269

265270

266271
@pytest.mark.integration
267-
def test_integration_cant_transfer_on_behalf_of_spender_after_removing_the_allowance_approval(env):
272+
def test_integration_cant_transfer_on_behalf_of_spender_after_removing_the_allowance_approval(
273+
env,
274+
):
268275
"""Test that a spender cannot transfer NFTs after the allowance approval is removed."""
269276
spender_account, receiver_account = _create_spender_and_receiver_accounts(env)
270277

@@ -486,3 +493,64 @@ def test_integration_cannot_send_deleted_token_nft_serials(env):
486493
f"Transfer should have failed with SPENDER_DOES_NOT_HAVE_ALLOWANCE"
487494
f"status but got: {ResponseCode(transfer_receipt.status).name}"
488495
)
496+
497+
498+
@pytest.mark.integration
499+
def test_integration_can_approve_serial_and_delete_all_serials_in_one_transaction(env):
500+
"""Test that approve-serial + delete-all-serials in one transaction preserves the per-serial allowance."""
501+
spender_account, receiver_account = _create_spender_and_receiver_accounts(env)
502+
503+
token_id = create_nft_token(env)
504+
assert token_id is not None
505+
506+
_associate_token_with_account(env, receiver_account, token_id)
507+
508+
nft_ids = _mint_nft(env, token_id, [b"\x01", b"\x02"])
509+
nft1 = nft_ids[0]
510+
nft2 = nft_ids[1]
511+
512+
# Approve nft1 specifically and delete-all-serials for the same (token, spender)
513+
# pair in the same transaction. The per-serial approval is preserved because the
514+
# two operations produce independent NftAllowance entries.
515+
receipt = (
516+
AccountAllowanceApproveTransaction()
517+
.approve_token_nft_allowance(nft1, env.operator_id, spender_account.id)
518+
.delete_token_nft_allowance_all_serials(token_id, env.operator_id, spender_account.id)
519+
.execute(env.client)
520+
)
521+
assert receipt.status == ResponseCode.SUCCESS, (
522+
f"Allowance approval failed with status: {ResponseCode(receipt.status).name}"
523+
)
524+
525+
# Transfer nft1 (should succeed - per-serial allowance preserved)
526+
transfer_receipt = (
527+
TransferTransaction()
528+
.set_transaction_id(TransactionId.generate(spender_account.id))
529+
.add_approved_nft_transfer(nft1, env.operator_id, receiver_account.id)
530+
.freeze_with(env.client)
531+
.sign(spender_account.key)
532+
.execute(env.client)
533+
)
534+
assert transfer_receipt.status == ResponseCode.SUCCESS, (
535+
f"Transfer failed with status: {ResponseCode(transfer_receipt.status).name}"
536+
)
537+
538+
# Confirm nft1 ownership has actually moved to the receiver
539+
nft1_info = TokenNftInfoQuery(nft1).execute(env.client)
540+
assert nft1_info.account_id == receiver_account.id, (
541+
f"Expected nft1 owner to be receiver {receiver_account.id} after transfer, got {nft1_info.account_id}"
542+
)
543+
544+
# Transfer nft2 (should fail - delete-all-serials revoked any blanket allowance)
545+
transfer_receipt2 = (
546+
TransferTransaction()
547+
.set_transaction_id(TransactionId.generate(spender_account.id))
548+
.add_approved_nft_transfer(nft2, env.operator_id, receiver_account.id)
549+
.freeze_with(env.client)
550+
.sign(spender_account.key)
551+
.execute(env.client)
552+
)
553+
assert transfer_receipt2.status == ResponseCode.SPENDER_DOES_NOT_HAVE_ALLOWANCE, (
554+
f"Transfer should have failed with SPENDER_DOES_NOT_HAVE_ALLOWANCE"
555+
f"status but got: {ResponseCode(transfer_receipt2.status).name}"
556+
)

tests/unit/account_allowance_approve_transaction_test.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,61 @@ def test_delete_token_nft_allowance_all_serials(account_allowance_transaction, s
217217
assert allowance.approved_for_all is False
218218

219219

220+
def test_delete_token_nft_allowance_all_serials_preserves_prior_serial_approval(
221+
account_allowance_transaction, sample_accounts, sample_tokens
222+
):
223+
"""Test that delete-all-serials does not fold into a prior per-serial approval."""
224+
token_id = sample_tokens["token1"]
225+
owner = sample_accounts["owner"]
226+
spender = sample_accounts["spender"]
227+
nft_id = NftId(token_id, 1)
228+
229+
account_allowance_transaction.approve_token_nft_allowance(nft_id, owner, spender)
230+
account_allowance_transaction.delete_token_nft_allowance_all_serials(token_id, owner, spender)
231+
232+
assert len(account_allowance_transaction.nft_allowances) == 2, (
233+
"Expected delete-all after per-serial approval to append a second NFT allowance entry"
234+
)
235+
236+
first = account_allowance_transaction.nft_allowances[0]
237+
assert first.token_id == token_id, "Expected first entry token_id to match the approved NFT's token"
238+
assert first.owner_account_id == owner, "Expected first entry owner to match"
239+
assert first.spender_account_id == spender, "Expected first entry spender to match"
240+
assert first.serial_numbers == [1], "Expected first entry to preserve approved serial [1]"
241+
assert first.approved_for_all is False, (
242+
"Expected first entry approved_for_all to remain False (per-serial approval)"
243+
)
244+
245+
second = account_allowance_transaction.nft_allowances[1]
246+
assert second.token_id == token_id, "Expected second entry token_id to match the delete-all token"
247+
assert second.owner_account_id == owner, "Expected second entry owner to match"
248+
assert second.spender_account_id == spender, "Expected second entry spender to match"
249+
assert second.serial_numbers == [], "Expected second entry to represent delete-all with empty serials"
250+
assert second.approved_for_all is False, "Expected second entry approved_for_all to be False (revoke semantics)"
251+
252+
253+
def test_delete_token_nft_allowance_all_serials_appends_per_call(
254+
account_allowance_transaction, sample_accounts, sample_tokens
255+
):
256+
"""Test that repeated delete-all-serials calls append one entry per call."""
257+
token_id = sample_tokens["token1"]
258+
owner = sample_accounts["owner"]
259+
spender = sample_accounts["spender"]
260+
261+
account_allowance_transaction.delete_token_nft_allowance_all_serials(token_id, owner, spender)
262+
account_allowance_transaction.delete_token_nft_allowance_all_serials(token_id, owner, spender)
263+
264+
assert len(account_allowance_transaction.nft_allowances) == 2, (
265+
"Expected each delete-all call to append a separate NFT allowance entry"
266+
)
267+
for index, allowance in enumerate(account_allowance_transaction.nft_allowances):
268+
assert allowance.token_id == token_id, f"Entry {index}: token_id mismatch"
269+
assert allowance.owner_account_id == owner, f"Entry {index}: owner mismatch"
270+
assert allowance.spender_account_id == spender, f"Entry {index}: spender mismatch"
271+
assert allowance.serial_numbers == [], f"Entry {index}: expected empty serial_numbers for delete-all"
272+
assert allowance.approved_for_all is False, f"Entry {index}: expected approved_for_all=False (revoke semantics)"
273+
274+
220275
def test_add_all_token_nft_approval(account_allowance_transaction, sample_accounts, sample_tokens):
221276
"""Test adding all token NFT approval"""
222277
token_id = sample_tokens["token1"]

0 commit comments

Comments
 (0)