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

fix: correct the from_bytes transaction-type dispatch map and add a round-trip test harness

Open
#2,614 9 comments 0 reactions 1 assignee Claimed by @aceppaluni View on GitHub
approved lang: python scope: tests 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()` dispatches on `TransactionBody.WhichOneof("data")` through the map in `Transaction._get_transaction_class()` (`src/hiero_sdk_python/transaction/transaction.py`). Several map entries do not match what `WhichOneof` actually returns, so valid transaction bytes raise `ValueError: Unknown transaction type` even though the SDK class exists. Verified against the generated proto:

- Wrong-cased keys (proto field is snake_case, map key is camelCase): `token_pause`, `token_unpause`, `token_fee_schedule_update`, `token_update_nfts`, `util_prng`
- Missing entry: `tokenClaimAirdrop` β†’ `TokenClaimAirdropTransaction` (`src/hiero_sdk_python/tokens/token_airdrop_claim.py`)
- Mapped to `None` although the class now exists: `freeze` β†’ `FreezeTransaction` (`src/hiero_sdk_python/system/freeze_transaction.py`), `token_fee_schedule_update` β†’ `TokenFeeScheduleUpdateTransaction` (`src/hiero_sdk_python/tokens/token_fee_schedule_update_transaction.py`)

This issue also creates the safety net for the rest of #2179: a parametrized round-trip test harness. Most transaction classes do not override `_from_protobuf`, so `from_bytes` silently returns an object with all type-specific fields at their defaults. The harness makes that visible and regression-proof; the follow-up issues (#2615–#2624) implement `_from_protobuf` per service family and un-xfail their types.

**This issue blocks #2615–#2624 and should be completed first.**

### πŸ› οΈ Implementation Notes

Research before coding β€” do not code from this issue title alone:

1. Issue #2179 β€” the full analysis of what is broken and why.
2. `Transaction.from_bytes()` and `Transaction._get_transaction_class()` in `src/hiero_sdk_python/transaction/transaction.py` β€” understand the whole deserialization chain first.
3. Verify field names yourself against the generated proto (do not trust this issue or an AI tool): `[f.name for f in TransactionBody.DESCRIPTOR.oneofs_by_name['data'].fields]`.
4. `TransferTransaction._from_protobuf` and `FileCreateTransaction._from_protobuf` β€” the two reference implementations the harness must pass for.
5. Existing tests in `tests/unit/transaction_test.py` and [CONTRIBUTING.md](https://github.com/hiero-ledger/hiero-sdk-python/blob/main/CONTRIBUTING.md).

**Scope:**

- [ ] Correct/add the map entries listed above; keep `None` only for types the SDK genuinely does not implement (`cryptoAddLiveHash`, `systemDelete`, `systemUndelete`, …)
- [ ] Add a parametrized round-trip test: for every mapped transaction type, build an instance with representative fields set, freeze, `to_bytes()` β†’ `Transaction.from_bytes()`, assert the restored object is the right class and every type-specific field equals the original
- [ ] Mark types whose `_from_protobuf` is not yet implemented as `xfail(strict=True)` β€” follow-up PRs remove the xfail per type
- [ ] A second assertion for all types: `restored.to_bytes() == original` (this must already pass today β€” body bytes are preserved verbatim)

**Acceptance criteria:**

- [ ] `token_pause`, `token_unpause`, `token_fee_schedule_update`, `token_update_nfts`, `util_prng`, `tokenClaimAirdrop`, `freeze` bytes all dispatch to the correct class
- [ ] Harness merged with strict xfails covering the not-yet-implemented types; already-implemented types (transfer, file create, node, batch) pass without xfail

Part of #2179.

### πŸ”¬ 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

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.