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

feat: implement _from_protobuf for smart contract transactions (from_bytes round-trip)

Open
#2,623 3 comments 0 reactions 0 assignees View on GitHub
approved lang: python pending-review 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:

- `ContractCreateTransaction` (`src/hiero_sdk_python/contract/contract_create_transaction.py`)
- `ContractUpdateTransaction` (`src/hiero_sdk_python/contract/contract_update_transaction.py`)
- `ContractDeleteTransaction` (`src/hiero_sdk_python/contract/contract_delete_transaction.py`)
- `ContractExecuteTransaction` (`src/hiero_sdk_python/contract/contract_execute_transaction.py`)
- `EthereumTransaction` (`src/hiero_sdk_python/contract/ethereum_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.

- [ ] `ContractCreateTransaction._from_protobuf`
- [ ] `ContractUpdateTransaction._from_protobuf`
- [ ] `ContractDeleteTransaction._from_protobuf`
- [ ] `ContractExecuteTransaction._from_protobuf`
- [ ] `EthereumTransaction._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

Not listed in #2179's original table but equally affected. `ContractCreateTransaction` has an `initcode` oneof (`fileID` vs inline `initcode` bytes) and `ContractDeleteTransaction` an obtainer oneof β€” restore exactly the arm that is set. `EthereumTransaction` is mostly raw bytes plus `max_gas_allowance`.

**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 to merge, then read the five affected transaction files and their build_transaction_body() methods. Compare them with FileCreateTransaction._from_protobuf and TransferTransaction._from_protobuf, and inspect the generated messages in src/hiero_sdk_python/hapi/services/ for optional and oneof fields. Done means the round-trip harness passes for all five types, optional fields and oneof arms are preserved, and to_bytes() remains byte-identical.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend-api-design, blockchain, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
18/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.