hiero-ledger / hiero-ledger/hiero-sdk-python

feat: implement _from_protobuf for token lifecycle transactions (from_bytes round-trip)

Open
#2,616 4 comments 0 reactions 0 assignees View on GitHub
approved lang: python skill: advanced
Dominant language
Python
Stars
63
Forks
298
Avg merge
3d 18h
Merged PRs (30d)
38

Description

### πŸ§‘β€πŸ”¬ Advanced Issue

Welcome! This is an **[Advanced Issue](https://github.com/issues?q=is%3Aopen%20is%3Aissue%20org%3Ahiero-ledger%20archived%3Afalse%20no%3Aassignee%20(label%3A%22advanced%22%20OR%20label%3A%22skill%3A%20advanced%22)%20(repo%3Ahiero-ledger%2Fhiero-sdk-cpp%20OR%20repo%3Ahiero-ledger%2Fhiero-sdk-swift%20OR%20repo%3Ahiero-ledger%2Fhiero-sdk-python%20OR%20repo%3Ahiero-ledger%2Fhiero-sdk-js%20OR%20repo%3Ahiero-ledger%2Fhiero-website))** touching core SDK architecture.

It is designed for expert contributors who have demonstrated deep architectural understanding and a proven track record of high-quality contributions.

### 🐞 Problem Description

`Transaction.from_bytes()` returns a correctly-typed but **empty** instance for the transaction types below: they never override `_from_protobuf`, so every type-specific field is left at its default with no error or warning (common fields, signatures and the original body bytes *are* restored by the base class, so execution still works β€” inspection does not). Part of #2179; the dispatch-map fixes and the round-trip test harness land in #2614 first.

Affected classes:

- `TokenCreateTransaction` (`src/hiero_sdk_python/tokens/token_create_transaction.py`)
- `TokenUpdateTransaction` (`src/hiero_sdk_python/tokens/token_update_transaction.py`)
- `TokenDeleteTransaction` (`src/hiero_sdk_python/tokens/token_delete_transaction.py`)
- `TokenUpdateNftsTransaction` (`src/hiero_sdk_python/tokens/token_update_nfts_transaction.py`)
- `TokenFeeScheduleUpdateTransaction` (`src/hiero_sdk_python/tokens/token_fee_schedule_update_transaction.py`)

### πŸ’‘ Expected Solution

Implement `_from_protobuf` for each affected class as the exact field-for-field inverse of its `build_transaction_body()`, then remove the `xfail` markers for these types in the round-trip harness from #2614.

- [ ] `TokenCreateTransaction._from_protobuf`
- [ ] `TokenUpdateTransaction._from_protobuf`
- [ ] `TokenDeleteTransaction._from_protobuf`
- [ ] `TokenUpdateNftsTransaction._from_protobuf`
- [ ] `TokenFeeScheduleUpdateTransaction._from_protobuf`
- [ ] Un-xfail these types in the #2614 round-trip harness

### πŸ” Background Research

This is inverse-serialization contract work β€” please research the pattern before coding; **do not code from this issue title alone**, and do not ship the first AI-generated attempt that runs. Reviewers check inverse fidelity per field.

1. Issue #2179 (context) and #2614 (the test harness your PR must satisfy β€” **do not start before it is merged**).
2. The reference implementations: `FileCreateTransaction._from_protobuf` (`src/hiero_sdk_python/file/file_create_transaction.py`) and `TransferTransaction._from_protobuf` (`src/hiero_sdk_python/transaction/transfer_transaction.py`). Your implementation must follow their structure exactly: call `super()._from_protobuf(...)`, then restore type-specific fields from the body.
3. Each affected class's `build_transaction_body()` β€” your `_from_protobuf` is its exact inverse. Reuse the SDK's existing `_from_proto` converters (`AccountId`, `TokenId`, keys, …); never hand-parse a submessage that already has one.
4. The generated proto message for each body (`src/hiero_sdk_python/hapi/services/`) β€” learn which fields are `optional` (guard with `HasField` so unset stays `None`) versus plain scalars/repeated.
5. [CONTRIBUTING.md](https://github.com/hiero-ledger/hiero-sdk-python/blob/main/CONTRIBUTING.md) for test and PR conventions.

### πŸ› οΈ Implementation Notes

`TokenCreateTransaction` is the largest body in the SDK: seven keys, supply/token-type enums, custom fees, auto-renew fields. `CustomFee` subclasses already implement `_from_protobuf` (`src/hiero_sdk_python/tokens/custom_fee.py`) β€” research and reuse them. Pay attention to which proto fields are `optional` (need `HasField`) versus plain scalars, so an unset key does not come back as an empty proto key.

**Constraints / acceptance criteria:**

- For each class: build with all fields set β†’ freeze β†’ `to_bytes()` β†’ `from_bytes()` β†’ every type-specific field equals the original
- Unset optional fields come back as `None`/empty, not proto defaults
- `restored.to_bytes()` stays byte-identical to the original (no re-serialization of the body)
- No changes to public API signatures

Part of #2179. **Blocked by #2614.**

### πŸ”¬ Technical Domains

- [x] **API Client Architecture** (request β†’ serialization β†’ execution β†’ response mapping)
- [x] **Backward Compatibility** (preserving method signatures, defaults, and return types)
- [x] **Protobuf Alignment** (reading `.proto` files, `_to_proto()` / `_from_proto()` correctness)
- [x] **State & Immutability** (correct usage of guards like `_require_not_frozen`)
- [ ] **Execution Boundaries** (retry logic, backoff, node selection, gRPC deadlines)
- [x] **Testing** (unit, integration, mocking, test coverage for edge cases and failure modes)

### 🧠 Advanced Contributors β€” Prerequisites & Expectations

> [!CAUTION]
> **Advanced issues are the highest-risk work in this project. We will reject PRs that do not meet these standards.**

### 🏁 Concrete Prerequisites
- **Advanced Language:** Proficient with Python.
- **Expertise:** Deep architectural understanding of `_Executable`, `Transaction`, and `Query` base classes.
- **Proven History:** Successfully completed **β‰₯ 10 intermediate issues** in this repo.
- **Consistency:** **β‰₯ 3–4 months** of active, human-led contributions to this SDK.

### ⚠️ AI Usage Policy

- Using AI to generate code for Advanced issues is **strictly discouraged**
- AI may be used to help explain file relationships, but cannot be the main source of research.
- Submitting AI-generated or unvalidated code is grounds for **immediate closure**

### ⏱️ Timeline & Workflow
- **Typical time:** ~1 month / ~50 hours.
- πŸ”΄ Completing an advanced issue in 1–3 days is a **red flag** and will likely be rejected.
- **Suggested:** Post your proposed architectural approach as a comment and wait for explicit maintainer approval **before writing any code.**

### πŸ›‘οΈ Quality & Review Standards

Advanced PRs must be **"safe, maintainable, architecturally sound, and production-ready."**

1. **Architectural Fit:** The solution must fit naturally into the existing SDK abstractions.
2. **Security & Correctness:** Evaluate all logic for injection risks, state corruption, or thread-safety issues.
3. **Maintainability:** Code must be short and clear enough for any other maintainer to debug without your assistance.
4. **Backward Compatibility:** Public API signatures must be preserved. If a breaking change is required, it must be explicitly managed through a deprecation cycle.
5. **Comprehensive Testing:** Must include unit and integration tests covering all new logic paths, edge cases, and failure modes. AI generated tests based on AI generated code is grounds for immediate rejection.

### βœ… PR Quality Checklist

Before opening your PR, the contributor must confirm:
- [ ] I have spent the majority of my time researching the problem and solution space extensively, including reviewing relevant code, documentation, and external resources.
- [ ] I understand the system-wide impact of these changes on affected modules and performance.
- [ ] The system design fits with current Hiero SDK architectural approaches.
- [ ] I have tested my changes extensively against both local and network environments.
- [ ] I have verified naming, types, and field ordering against pinned Protobufs.
- [ ] Every line of code is personally understood and explainable.

### πŸ“š Resources & Support

**References:**
- [Hedera Protobufs](https://github.com/hashgraph/hedera-protobufs)

**Python SDK References:**
- [SDK Project Structure](https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/training/setup/project_structure.md)
- [Transaction Lifecycle](https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/training/transaction_lifecycle.md)
- [Executable Architecture](https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/training/executable.md)
- [Hedera Protobufs (services)](https://github.com/hashgraph/hedera-protobufs/tree/main/services)
- [Browse closed advanced PRs](https://github.com/hiero-ledger/hiero-sdk-python/pulls?q=is%3Apr+is%3Amerged+label%3A%22skill%3A+advanced%22) β€” see how others did it

**πŸ†˜ Stuck?**
- [Community Calls](https://zoom-lfx.platform.linuxfoundation.org/meetings/hiero?view=week)
- [Discord](https://github.com/hiero-ledger/sdk-collaboration-hub/blob/main/guides/issue-progression/for-developers/discord.md)

Contributor guide

Open the contributing guide

Research direction

Wait for #2614, then review its round-trip harness, the five affected token transaction files, and the reference implementations in file_create_transaction.py and transfer_transaction.py. Read each build_transaction_body(), the generated messages under src/hiero_sdk_python/hapi/services/, and custom_fee.py before running the harness. Done means all type-specific fields round-trip, unset optionals remain empty or None, and restored bytes remain identical.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, blockchain
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.