Memo collision in TopicCreateTransaction and TopicUpdateTransaction

[exploreriii]

TopicCreateTransaction line 66 has the identical self.memo: str = memo or "" ... colliding with the base Transaction.memo, which for example, can clear its memo

This is also in 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

• Align TopicCreateTransaction and TopicUpdateTransaction with the SDK-wide <entity>_memo / set_<entity>_memo convention by storing the topic memo in a new topic_memo attribute and renaming set_memo to set_topic_memo, so the inherited Transaction.memo is never shadowed.
• Update _build_proto_body() in both classes to source the topic body memo from topic_memo, leaving the base envelope memo independent.
• Update internal callers (TCK handler, examples) and unit tests to the new API, and add a regression test that verifies transaction-level and topic-level memos no longer interfere.
• The pre-existing _from_proto round-trip gap for topic memo is intentionally left out of scope as it is a separate defect not covered by this ticket.

Design Choices

Design Choice 1: Should the existing public memo attribute and set_memo() method on topic transactions be preserved for backward compatibility, or replaced outright?

Options Considered:

1. Strict rename — Replace memo/set_memo with topic_memo/set_topic_memo everywhere and update all call sites/tests. Cleanest and fully consistent with the rest of the SDK, but is a breaking change for any external consumer.
2. Rename with deprecated aliases — Introduce topic_memo/set_topic_memo as the canonical API, but keep set_memo() and a memo accessor as thin deprecated aliases delegating to the new attribute. Preserves backward compatibility at the cost of retaining outlier names.

Chosen Option: 1

Rationale: The other entity transactions never provided set_memo/memo aliases, so a strict rename keeps the SDK internally consistent and directly removes the colliding attribute; the constructor keyword can remain memo= for ergonomic parity with siblings while the stored attribute becomes topic_memo.

💡 User Tips

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

Implementation Steps

Phase 1: Decouple the topic memo from the transaction memo

This phase resolves the collision at its source by ensuring the topic transactions no longer overwrite the inherited Transaction.memo attribute. The goal is to give the topic memo its own storage and builder API that mirrors the established <entity>_memo convention, so that the transaction-level memo (set_transaction_memo / TransactionBody.memo) and the topic-level memo become fully independent.

Task 1: Introduce a dedicated topic memo attribute in TopicCreateTransaction

Stop shadowing the base memo and store the topic memo under its own name.

• Update src/hiero_sdk_python/consensus/topic_create_transaction.py so the constructor assigns the incoming memo to a new self.topic_memo attribute instead of self.memo, leaving the inherited self.memo (set by super().__init__()) intact for the transaction-level memo.
• Rename the builder method set_memo() to set_topic_memo(), following the set_<entity>_memo convention; it should set self.topic_memo and keep the _require_not_frozen() guard and fluent return.
• Update _build_proto_body() to read self.topic_memo when populating ConsensusCreateTopicTransactionBody.memo, so the topic body no longer depends on the base envelope memo.
• Keep the constructor keyword named memo for ergonomic parity with other entity transactions, but ensure it flows into topic_memo.

Task 2: Introduce a dedicated topic memo attribute in TopicUpdateTransaction

Apply the same decoupling to the update transaction.

• Update src/hiero_sdk_python/consensus/topic_update_transaction.py so the constructor assigns the incoming memo to self.topic_memo rather than self.memo.
• Rename set_memo() to set_topic_memo(), preserving the frozen-check and fluent return.
• Update _build_proto_body() to build the StringValue wrapper for ConsensusUpdateTopicTransactionBody.memo from self.topic_memo.
• Confirm the inherited set_transaction_memo() and base envelope memo now behave independently for this transaction type.

🤖 Prompt for AI agents

Implement the decoupling of the topic memo from the base transaction memo in
both consensus topic transaction classes.

For `src/hiero_sdk_python/consensus/topic_create_transaction.py`:
- Change the constructor so the incoming memo value is assigned to a new
`self.topic_memo` attribute instead of overwriting `self.memo` (the inherited
attribute set via `super().__init__()` must remain intact for the
transaction-level memo).
- Rename the builder method `set_memo()` to `set_topic_memo()`, following the
SDK-wide `set_<entity>_memo` convention. It must set `self.topic_memo`, retain
the `_require_not_frozen()` guard, and keep the fluent return (`return self`).
- Update `_build_proto_body()` to source
`ConsensusCreateTopicTransactionBody.memo` from `self.topic_memo` rather than
the base memo.
- Keep the constructor keyword argument named `memo` for ergonomic parity with
sibling entity transactions, but make sure it is stored into `self.topic_memo`.

For `src/hiero_sdk_python/consensus/topic_update_transaction.py`:
- Apply the same change: assign the incoming memo to `self.topic_memo` in the
constructor instead of `self.memo`.
- Rename `set_memo()` to `set_topic_memo()`, preserving the frozen-check and
fluent return behavior.
- Update `_build_proto_body()` so the `StringValue` wrapper for
`ConsensusUpdateTopicTransactionBody.memo` is built from `self.topic_memo`.
- Verify that the inherited `set_transaction_memo()` and the base envelope memo
now operate independently of the topic memo for this transaction type.

Phase 2: Align callers and tests with the new topic memo API

This phase updates every consumer of the old colliding names so the codebase remains consistent and the fix is verified. It also adds coverage that explicitly proves the transaction-level memo and topic memo are now independent.

Task 1: Update internal callers of the topic memo API

Ensure no remaining references to the removed set_memo / memo names on topic transactions.

• Update tck/handlers/topic.py to call set_topic_memo(...) instead of set_memo(...) where it configures topic create/update transactions.
• Search the rest of the codebase (examples, helpers) for set_memo( or .memo usage against topic transactions and switch them to the new attribute/method.

Task 2: Update and extend unit tests

Adjust existing assertions to the new API and add a regression test for the collision.

• In tests/unit/topic_create_transaction_test.py, update assertions that read tx.memo (e.g., the constructor and default-value tests) to reference tx.topic_memo, and change set_memo(...) calls to set_topic_memo(...). Protobuf-level assertions on consensusCreateTopic.memo remain unchanged.
• In tests/unit/topic_update_transaction_test.py, update set_memo(...) builder calls to set_topic_memo(...). Protobuf-level assertions on consensusUpdateTopic.memo.value remain unchanged.
• Add a regression test (in both topic test modules) that sets a topic memo via set_topic_memo() and a distinct transaction memo via set_transaction_memo(), then builds the transaction body and asserts that TransactionBody.memo (envelope) and the nested topic memo hold their respective, independent values — directly proving the collision is resolved.

🤖 Prompt for AI agents

Propagate the renamed topic memo API to all internal callers and tests, and add
regression coverage proving the fix.

- Update `tck/handlers/topic.py` to call `set_topic_memo(...)` instead of
`set_memo(...)` wherever it configures topic create/update transactions.
- Search the rest of the codebase (examples, helpers) for any remaining
`set_memo(` or `.memo` usage against topic transactions and switch these to the
new `set_topic_memo(...)` / `topic_memo` attribute.
- In `tests/unit/topic_create_transaction_test.py`: update assertions reading
`tx.memo` (including constructor and default-value tests) to instead read
`tx.topic_memo`, and change `set_memo(...)` calls to `set_topic_memo(...)`.
Leave protobuf-level assertions on `consensusCreateTopic.memo` unchanged.
- In `tests/unit/topic_update_transaction_test.py`: update `set_memo(...)`
builder calls to `set_topic_memo(...)`. Leave protobuf-level assertions on
`consensusUpdateTopic.memo.value` unchanged.
- In both topic test modules, add a regression test that sets a topic memo via
`set_topic_memo()` and a distinct transaction-level memo via
`set_transaction_memo()`, builds the transaction body, and asserts that the
envelope `TransactionBody.memo` and the nested topic memo each hold their own
independent values, proving the collision is resolved.

Research

The base Transaction class (src/hiero_sdk_python/transaction/transaction.py) owns self.memo, which maps to the protobuf TransactionBody.memo envelope field and is exposed via set_transaction_memo(). Entity-specific transaction subclasses (account, contract, file, schedule, token) each store their own entity memo in a distinctly prefixed attribute (account_memo, contract_memo, etc.) with a matching set_<entity>_memo() setter, leaving the base self.memo untouched. The two consensus topic transactions in src/hiero_sdk_python/consensus/ are the sole outliers: they overwrite self.memo in their constructors and expose a set_memo() builder, causing the reported collision. Unit tests in tests/unit/ and the TCK handler in tck/handlers/topic.py currently exercise the topic memo through the colliding memo / set_memo names.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement the decoupling of the topic memo from the base transaction memo in
both consensus topic transaction classes.

For `src/hiero_sdk_python/consensus/topic_create_transaction.py`:
- Change the constructor so the incoming memo value is assigned to a new
`self.topic_memo` attribute instead of overwriting `self.memo` (the inherited
attribute set via `super().__init__()` must remain intact for the
transaction-level memo).
- Rename the builder method `set_memo()` to `set_topic_memo()`, following the
SDK-wide `set_<entity>_memo` convention. It must set `self.topic_memo`, retain
the `_require_not_frozen()` guard, and keep the fluent return (`return self`).
- Update `_build_proto_body()` to source
`ConsensusCreateTopicTransactionBody.memo` from `self.topic_memo` rather than
the base memo.
- Keep the constructor keyword argument named `memo` for ergonomic parity with
sibling entity transactions, but make sure it is stored into `self.topic_memo`.

For `src/hiero_sdk_python/consensus/topic_update_transaction.py`:
- Apply the same change: assign the incoming memo to `self.topic_memo` in the
constructor instead of `self.memo`.
- Rename `set_memo()` to `set_topic_memo()`, preserving the frozen-check and
fluent return behavior.
- Update `_build_proto_body()` so the `StringValue` wrapper for
`ConsensusUpdateTopicTransactionBody.memo` is built from `self.topic_memo`.
- Verify that the inherited `set_transaction_memo()` and the base envelope memo
now operate independently of the topic memo for this transaction type.
===============================================================================

Task: 2

Propagate the renamed topic memo API to all internal callers and tests, and add
regression coverage proving the fix.

- Update `tck/handlers/topic.py` to call `set_topic_memo(...)` instead of
`set_memo(...)` wherever it configures topic create/update transactions.
- Search the rest of the codebase (examples, helpers) for any remaining
`set_memo(` or `.memo` usage against topic transactions and switch these to the
new `set_topic_memo(...)` / `topic_memo` attribute.
- In `tests/unit/topic_create_transaction_test.py`: update assertions reading
`tx.memo` (including constructor and default-value tests) to instead read
`tx.topic_memo`, and change `set_memo(...)` calls to `set_topic_memo(...)`.
Leave protobuf-level assertions on `consensusCreateTopic.memo` unchanged.
- In `tests/unit/topic_update_transaction_test.py`: update `set_memo(...)`
builder calls to `set_topic_memo(...)`. Leave protobuf-level assertions on
`consensusUpdateTopic.memo.value` unchanged.
- In both topic test modules, add a regression test that sets a topic memo via
`set_topic_memo()` and a distinct transaction-level memo via
`set_transaction_memo()`, builds the transaction body, and asserts that the
envelope `TransactionBody.memo` and the nested topic memo each hold their own
independent values, proving the collision is resolved.

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