TopicUpdateTransaction silent state-rewrite and small consistency issues

[exploreriii]

The following TCK PR has surfaced a couple bugs in topic update:
#2462

The constructor defaults auto_renew_period=Duration(7890000) so every bare TopicUpdateTransaction().set_topic_id(...).execute(...) silently rewrites the topic's auto-renew period to ~91 days

Instead we should assume absence of duration means none, similar to the JS SDK implementation
https://github.com/hiero-ledger/hiero-sdk-js/blob/main/src/topic/TopicUpdateTransaction.js

Also we don't need to raise this error:
topicID=self.topic_id._to_proto() if self.topic_id else None,
Let the network answer INVALID_TOPIC_ID — matching the JS SDK

Additionally this type hint is incorrect
def set_expiration_time(self, expiration_time: Timestamp) -> TopicUpdateTransaction:

[github-actions]

Thanks for your submission.

This issue is awaiting approval from @hiero-ledger/hiero-sdk-python-triage and @hiero-ledger/hiero-sdk-python-committers and @hiero-ledger/hiero-sdk-python-maintainers.

[github-actions]

Approved by @hiero-ledger/hiero-sdk-python-triage and @hiero-ledger/hiero-sdk-python-committers and @hiero-ledger/hiero-sdk-python-maintainers.

[github-actions]

@coderabbitai plan

[coderabbitai]

Coding Plan

Summary

• Change auto_renew_period (and optionally memo) to default to None so omitted optional fields are true no-ops, preventing silent overwrites of on-chain topic state.
• Remove the client-side missing-topic_id ValueError and build topicID conditionally, deferring validation to the network (INVALID_TOPIC_ID), matching the JS SDK and Account/Node conventions.
• Fix set_expiration_time's type hint to reference the SDK Timestamp class actually consumed by proto building.
• Update and extend unit tests to match the corrected semantics and lock in the no-op defaults.

Design Choices

Design Choice 1: Should the sibling `memo or ""` default also be corrected in this ticket?

Options Considered:

1. Fix memo as well (default to None, keep the existing is not None conditional wrapper) so absence is a true no-op, consistent with Token/Contract/File.
2. Leave memo untouched to strictly match the explicitly enumerated ticket items.

Chosen Option: 1

Rationale: It is the same bug class the ticket title calls out ("silent state-rewrite"), aligns with the JS SDK "absence = no-op" intent and sibling-transaction conventions, and is a minimal, low-risk change; if reviewers prefer strict scope, this single change can be dropped without affecting the other fixes. The constructor coerces memo to "" (self.memo: str = memo or ""), so a bare update sends an empty StringValue and can clear the topic memo — the same silent state-rewrite class the ticket title references, though the ticket body lists only auto_renew_period, topic_id, and the type hint.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Correct TopicUpdateTransaction semantics and typing

This phase aligns TopicUpdateTransaction with the JS SDK and sibling update transactions: omitted optional fields become genuine no-ops, primary-ID validation defers to the network, and setter type hints reference the correct SDK types.

Task 1: Make auto_renew_period absence a no-op

Change the update transaction so a bare update no longer overwrites the topic's auto-renew period.

• In src/hiero_sdk_python/consensus/topic_update_transaction.py, change the __init__ parameter default for auto_renew_period from Duration(7890000) to None, so the stored attribute is None unless the caller explicitly sets it.
• Confirm _build_proto_body continues to emit autoRenewPeriod only when the field is set (the existing truthy/is not None conditional already omits it when None), so an unset period is excluded from the proto entirely.
• No changes to set_auto_renew_period (it already accepts int/Duration and remains the explicit opt-in path).

Task 2: Defer missing topic_id validation to the network

Remove the client-side ValueError so the network returns INVALID_TOPIC_ID, matching the JS SDK and the Account/Node update pattern.

• In _build_proto_body, remove the if self.topic_id is None: raise ValueError(...) guard.
• Build the topicID field conditionally, e.g. topicID=self.topic_id._to_proto() if self.topic_id else None, mirroring AccountUpdateTransaction._build_proto_body.
• Ensure the rest of the proto assembly is unaffected and that build_transaction_body/build_scheduled_body no longer surface a client-side error for a missing topic id.

Task 3: Correct set_expiration_time type hint

Align the setter's parameter type with the SDK Timestamp wrapper actually consumed by _build_proto_body.

• In set_expiration_time, change the parameter annotation from timestamp_pb2.Timestamp to the SDK Timestamp class (already imported from hiero_sdk_python.timestamp), matching the __init__ annotation and the _to_protobuf() usage in _build_proto_body.
• Remove the timestamp_pb2 import if it becomes unused after this change; otherwise leave the shared hapi-services import intact.
• (If Option 1 of the Assumptions is accepted) additionally default memo to None in __init__ instead of memo or "", keeping the existing conditional StringValue wrapper so an omitted memo is not sent.

🤖 Prompt for AI agents

Implement fixes in `src/hiero_sdk_python/consensus/topic_update_transaction.py`
to align `TopicUpdateTransaction` with the JS SDK and sibling update
transactions (Token/Contract/File, Account/Node), making omitted optional fields
true no-ops, deferring primary-ID validation to the network, and correcting a
type hint.

1. Auto-renew period no-op: In `__init__`, change the `auto_renew_period`
parameter default from `Duration(7890000)` to `None`, so the stored attribute is
`None` unless explicitly set via `set_auto_renew_period`. Verify
`_build_proto_body` already conditionally emits `autoRenewPeriod` only when the
field is not `None`/falsy, so it is fully excluded from the proto when unset. Do
not modify `set_auto_renew_period`.

2. Defer topic_id validation to network: In `_build_proto_body`, remove the `if
self.topic_id is None: raise ValueError(...)` guard entirely. Instead build the
`topicID` field conditionally (e.g. `topicID=self.topic_id._to_proto() if
self.topic_id else None`), mirroring the pattern used in
`AccountUpdateTransaction._build_proto_body`. Confirm
`build_transaction_body`/`build_scheduled_body` no longer raise a client-side
error for a missing topic id, allowing the network to return `INVALID_TOPIC_ID`
instead.

3. Fix type hint: In `set_expiration_time`, change the parameter annotation from
`timestamp_pb2.Timestamp` to the SDK `Timestamp` class (already imported from
`hiero_sdk_python.timestamp`), matching the `__init__` annotation and the
`_to_protobuf()` usage in `_build_proto_body`. Remove the `timestamp_pb2` import
if it becomes unused after this change; otherwise leave shared hapi-services
imports intact.

4. Memo no-op (per accepted assumption): Additionally default `memo` to `None`
in `__init__` instead of `memo or ""`, keeping the existing conditional
`StringValue` wrapper so an omitted memo is not sent in the proto.

Phase 2: Align unit tests with the corrected behavior

This phase updates existing tests that assert removed behavior and adds coverage proving the new no-op defaults, so the suite reflects the JS-SDK-aligned semantics.

Task 1: Replace the missing-topic_id ValueError test

Update the test that asserts a client-side error that no longer exists.

• In tests/unit/topic_update_transaction_test.py, revise test_missing_topic_id_in_update so it no longer expects a ValueError; instead assert that build_transaction_body() succeeds with an unset/absent topicID in the resulting proto, reflecting deferral to the network.

Task 2: Add coverage for no-op defaults

Guard against regression of the silent state-rewrite bug.

• Add a unit test in tests/unit/topic_update_transaction_test.py verifying that a TopicUpdateTransaction constructed with only a topic_id (no set_auto_renew_period call) produces a proto body that does not include autoRenewPeriod.
• (If Option 1 is accepted) add/adjust a test verifying that a bare update omits the memo field when no memo is provided, and that the explicit setter path still emits it.
• Verify the existing tests that explicitly call set_auto_renew_period(...) still pass unchanged.

🤖 Prompt for AI agents

Update `tests/unit/topic_update_transaction_test.py` to match the corrected
semantics implemented in Phase 1 and lock in the new no-op defaults via
regression tests.

1. Rework `test_missing_topic_id_in_update`: it should no longer expect a
`ValueError`. Instead, assert that `build_transaction_body()` succeeds and that
the resulting proto has an unset/absent `topicID`, reflecting deferral of
validation to the network.

2. Add a new test verifying that a `TopicUpdateTransaction` constructed with
only a `topic_id` (without calling `set_auto_renew_period`) produces a proto
body that does not include `autoRenewPeriod`.

3. If the memo-default assumption is accepted, add or adjust a test verifying
that a bare update (no memo provided) omits the `memo` field from the proto,
while the explicit setter path still emits it.

4. Confirm existing tests that explicitly call `set_auto_renew_period(...)`
continue to pass unchanged, ensuring no regression in explicit opt-in behavior.

Research

TopicUpdateTransaction lives in src/hiero_sdk_python/consensus/topic_update_transaction.py and follows the SDK's update-transaction convention: a constructor holding optional fields, chainable set_* setters, and a _build_proto_body() that assembles the protobuf body consumed by build_transaction_body()/build_scheduled_body().

Sibling update transactions (Token/Contract/File and Account/Node) establish two relevant conventions:

• optional fields default to None and are conditionally wrapped when building the proto
• primary-ID validation is deferred to the network (as in Account/Node)

Unit coverage lives in tests/unit/topic_update_transaction_test.py, which currently includes test_missing_topic_id_in_update asserting a ValueError.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement fixes in `src/hiero_sdk_python/consensus/topic_update_transaction.py`
to align `TopicUpdateTransaction` with the JS SDK and sibling update
transactions (Token/Contract/File, Account/Node), making omitted optional fields
true no-ops, deferring primary-ID validation to the network, and correcting a
type hint.

1. Auto-renew period no-op: In `__init__`, change the `auto_renew_period`
parameter default from `Duration(7890000)` to `None`, so the stored attribute is
`None` unless explicitly set via `set_auto_renew_period`. Verify
`_build_proto_body` already conditionally emits `autoRenewPeriod` only when the
field is not `None`/falsy, so it is fully excluded from the proto when unset. Do
not modify `set_auto_renew_period`.

2. Defer topic_id validation to network: In `_build_proto_body`, remove the `if
self.topic_id is None: raise ValueError(...)` guard entirely. Instead build the
`topicID` field conditionally (e.g. `topicID=self.topic_id._to_proto() if
self.topic_id else None`), mirroring the pattern used in
`AccountUpdateTransaction._build_proto_body`. Confirm
`build_transaction_body`/`build_scheduled_body` no longer raise a client-side
error for a missing topic id, allowing the network to return `INVALID_TOPIC_ID`
instead.

3. Fix type hint: In `set_expiration_time`, change the parameter annotation from
`timestamp_pb2.Timestamp` to the SDK `Timestamp` class (already imported from
`hiero_sdk_python.timestamp`), matching the `__init__` annotation and the
`_to_protobuf()` usage in `_build_proto_body`. Remove the `timestamp_pb2` import
if it becomes unused after this change; otherwise leave shared hapi-services
imports intact.

4. Memo no-op (per accepted assumption): Additionally default `memo` to `None`
in `__init__` instead of `memo or ""`, keeping the existing conditional
`StringValue` wrapper so an omitted memo is not sent in the proto.
===============================================================================

Task: 2

Update `tests/unit/topic_update_transaction_test.py` to match the corrected
semantics implemented in Phase 1 and lock in the new no-op defaults via
regression tests.

1. Rework `test_missing_topic_id_in_update`: it should no longer expect a
`ValueError`. Instead, assert that `build_transaction_body()` succeeds and that
the resulting proto has an unset/absent `topicID`, reflecting deferral of
validation to the network.

2. Add a new test verifying that a `TopicUpdateTransaction` constructed with
only a `topic_id` (without calling `set_auto_renew_period`) produces a proto
body that does not include `autoRenewPeriod`.

3. If the memo-default assumption is accepted, add or adjust a test verifying
that a bare update (no memo provided) omits the `memo` field from the proto,
while the explicit setter path still emits it.

4. Confirm existing tests that explicitly call `set_auto_renew_period(...)`
continue to pass unchanged, ensuring no regression in explicit opt-in behavior.

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