Duplicate signatures added to _signature_map due to missing deduplication check

[mohityadav8]

Description

Currently, when adding signature pairs to self._signature_map, duplicate entries can be inserted for the same body_bytes and pubKeyPrefix.

This happens because the code directly appends sigPair without verifying whether a signature from the same public key already exists.

Expected behavior:
Each (body_bytes, pubKeyPrefix) combination should have only one corresponding SignaturePair.

Actual behavior:
Duplicate signatures are appended, leading to redundant entries and potential inconsistencies.

Steps to reproduce

1. Create a transaction and sign it multiple times using the same private key
2. Inspect self._signature_map[body_bytes].sigPair
3. Observe duplicate SignaturePair entries with the same pubKeyPrefix

Additional context

• This issue can lead to unnecessary payload size increase
• May cause inefficiencies or unexpected behavior in downstream processing

Proposed Fix

Add a deduplication check before appending:

self._signature_map.setdefault(body_bytes, basic_types_pb2.SignatureMap())

already_signed = any(
    sp.pubKeyPrefix == public_key_bytes
    for sp in self._signature_map[body_bytes].sigPair
)

if not already_signed:
    self._signature_map[body_bytes].sigPair.append(sig_pair)

Hedera network

No response

Version

v0.2.5

Operating system

Windows

[mohityadav8]

@exploreriii i am working on this can u assign this to me

[exploreriii]

@coderabbitai plan

[github-actions]

Hello, this is CommunityCallBot.

This is a reminder that the Hiero Python SDK Community Call will begin in approximately 4 hours (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.

[coderabbitai]

Coding Plan

Summary

• Add an inline deduplication check using any() within the sign() method's loop, following the exact approach proposed in the ticket
• The check compares pubKeyPrefix bytes before appending to avoid redundant signature entries
• Add targeted unit tests to verify both the deduplication behavior and that multi-key signing continues to work correctly

Implementation Steps

Phase 1: Add Deduplication Check to Sign Method

Modify the sign() method to check whether a signature from the same public key already exists before appending a new signature pair. This prevents duplicate entries when the same key signs multiple times.

Implement Deduplication Logic in Transaction Sign Method

Add a guard check before appending signature pairs to prevent duplicates for the same public key prefix.

• Update src/hiero_sdk_python/transaction/transaction.py in the sign() method (around lines 193-196)
• After the setdefault() call that initializes the SignatureMap, add a check using any() to determine if a SignaturePair with the same pubKeyPrefix already exists
• Only append the new sig_pair if no existing entry matches the public_key_bytes
• This change should be minimal and localized within the existing loop that iterates over _transaction_body_bytes

🤖 Prompt for AI agents

Modify the `sign()` method in `src/hiero_sdk_python/transaction/transaction.py`
to prevent duplicate `SignaturePair` entries for the same public key.

- Locate the `sign()` method (around line 165) and find the inner loop that
iterates over `_transaction_body_bytes` (changes around lines 193-196)
- After the `setdefault()` call that initializes the `SignatureMap` for a given
body bytes key, add a deduplication guard:
- Use `any()` to check whether a `SignaturePair` with the same `pubKeyPrefix` as
`public_key_bytes` already exists in the current `SignatureMap`
- Only append the new `sig_pair` to the `SignatureMap` if no existing entry
matches the `public_key_bytes`
- Keep the change minimal and localized within the existing loop; do not
refactor surrounding logic

Phase 2: Add Unit Tests for Deduplication Behavior

Add test coverage to verify that signing with the same key multiple times does not produce duplicate signature entries, filling the identified gap in test coverage.

Task 1: Create Unit Test for Duplicate Signature Prevention

Add a test case that verifies the deduplication behavior works correctly.

• Add test in tests/unit/transaction_freeze_and_bytes_test.py (aligns with existing sign/freeze workflow tests in this file)
• Follow the existing Pattern A from the codebase: use mocked PrivateKey with direct _signature_map[body_bytes].sigPair assertions
• Test scenario: freeze a transaction, sign with the same private key twice, assert that len(sigPair) == 1
• Verify the single signature pair contains the expected pubKeyPrefix and signature value
• Name the test descriptively, e.g., test_sign_same_key_twice_does_not_duplicate

Task 2: Add Test for Multiple Distinct Keys Still Work

Ensure the deduplication logic doesn't break the ability to sign with multiple different keys.

• Add test in tests/unit/transaction_freeze_and_bytes_test.py
• Test scenario: freeze a transaction, sign with two different private keys, assert that len(sigPair) == 2
• This serves as a regression test to ensure deduplication only affects same-key scenarios
• Can follow existing test test_from_bytes_round_trip_multiple_signatures as a reference pattern

🤖 Prompt for AI agents

Add two new unit tests in `tests/unit/transaction_freeze_and_bytes_test.py` to
verify the deduplication behavior introduced in Phase 1.

**Test 1: `test_sign_same_key_twice_does_not_duplicate`**
- Follow the existing Pattern A: use a mocked `PrivateKey` with direct
assertions on `_signature_map[body_bytes].sigPair`
- Freeze a transaction, then call `sign()` with the same private key twice
- Assert that `len(sigPair) == 1` (no duplicate entry was added)
- Also verify the single remaining `SignaturePair` contains the expected
`pubKeyPrefix` and signature value

**Test 2: Regression test for multiple distinct keys**
- Use `test_from_bytes_round_trip_multiple_signatures` as a reference pattern
- Freeze a transaction, then call `sign()` with two different private keys
- Assert that `len(sigPair) == 2` (both signatures are present)
- This confirms deduplication only suppresses same-key duplicates and does not
affect distinct keys

Research

The codebase is a Python SDK for the Hiero/Hedera network. Transaction signing is handled in src/hiero_sdk_python/transaction/transaction.py, where the Transaction class contains a sign() method (line 165) that iterates over per-node transaction body bytes and appends SignaturePair entries to _signature_map. The _signature_map is a dictionary mapping body bytes to SignatureMap protobuf objects. Existing tests in tests/unit/ follow patterns using mocked PrivateKey objects with direct assertions on _signature_map[body_bytes].sigPair.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Modify the `sign()` method in `src/hiero_sdk_python/transaction/transaction.py`
to prevent duplicate `SignaturePair` entries for the same public key.

- Locate the `sign()` method (around line 165) and find the inner loop that
iterates over `_transaction_body_bytes` (changes around lines 193-196)
- After the `setdefault()` call that initializes the `SignatureMap` for a given
body bytes key, add a deduplication guard:
- Use `any()` to check whether a `SignaturePair` with the same `pubKeyPrefix` as
`public_key_bytes` already exists in the current `SignatureMap`
- Only append the new `sig_pair` to the `SignatureMap` if no existing entry
matches the `public_key_bytes`
- Keep the change minimal and localized within the existing loop; do not
refactor surrounding logic
===============================================================================

Task: 2

Add two new unit tests in `tests/unit/transaction_freeze_and_bytes_test.py` to
verify the deduplication behavior introduced in Phase 1.

**Test 1: `test_sign_same_key_twice_does_not_duplicate`**
- Follow the existing Pattern A: use a mocked `PrivateKey` with direct
assertions on `_signature_map[body_bytes].sigPair`
- Freeze a transaction, then call `sign()` with the same private key twice
- Assert that `len(sigPair) == 1` (no duplicate entry was added)
- Also verify the single remaining `SignaturePair` contains the expected
`pubKeyPrefix` and signature value

**Test 2: Regression test for multiple distinct keys**
- Use `test_from_bytes_round_trip_multiple_signatures` as a reference pattern
- Freeze a transaction, then call `sign()` with two different private keys
- Assert that `len(sigPair) == 2` (both signatures are present)
- This confirms deduplication only suppresses same-key duplicates and does not
affect distinct keys

💡 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!