feat: implement Transaction.from_bytes() deserialization for all transaction types - #2438
Mounil2005 wants to merge 12 commits into
Conversation
github-actions
Bot
added
lang: python
There was a problem hiding this comment.
Pull request overview
This PR fixes Transaction.from_bytes() round-trip behavior by adding _from_protobuf deserialization support across many transaction subclasses and by correcting/improving the transaction-type dispatch map used to select the appropriate concrete transaction class.
Changes:
- Adds
_from_protobufimplementations across account, token, consensus, file, schedule, contract, system, node, and PRNG transaction types to properly hydrate transaction-specific fields from protobuf. - Reworks the transaction type dispatch table into a module-level
_TRANSACTION_TYPE_MAP, fixing several keys/module paths and adding missing entries. - Improves protobuf field handling in a few places (e.g., memo defaults, composite-key handling via
Key.from_proto_key, and topic fee parsing helpers).
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hiero_sdk_python/transaction/transaction.py | Moves/expands transaction type dispatch into a module-level map and adjusts base-body memo handling. |
| src/hiero_sdk_python/tokens/token_wipe_transaction.py | Implements _from_protobuf for TokenWipeTransaction field hydration. |
| src/hiero_sdk_python/tokens/token_update_transaction.py | Implements _from_protobuf for TokenUpdateTransaction (including keys/memo/metadata). |
| src/hiero_sdk_python/tokens/token_update_nfts_transaction.py | Implements _from_protobuf for TokenUpdateNftsTransaction. |
| src/hiero_sdk_python/tokens/token_unpause_transaction.py | Implements _from_protobuf for TokenUnpauseTransaction (snake_case proto field). |
| src/hiero_sdk_python/tokens/token_unfreeze_transaction.py | Implements _from_protobuf for TokenUnfreezeTransaction. |
| src/hiero_sdk_python/tokens/token_revoke_kyc_transaction.py | Implements _from_protobuf for TokenRevokeKycTransaction. |
| src/hiero_sdk_python/tokens/token_reject_transaction.py | Implements _from_protobuf for TokenRejectTransaction (fungible + NFT rejection parsing). |
| src/hiero_sdk_python/tokens/token_pause_transaction.py | Implements _from_protobuf for TokenPauseTransaction (snake_case proto field). |
| src/hiero_sdk_python/tokens/token_mint_transaction.py | Implements _from_protobuf for TokenMintTransaction (amount + metadata). |
| src/hiero_sdk_python/tokens/token_grant_kyc_transaction.py | Implements _from_protobuf for TokenGrantKycTransaction. |
| src/hiero_sdk_python/tokens/token_freeze_transaction.py | Implements _from_protobuf for TokenFreezeTransaction. |
| src/hiero_sdk_python/tokens/token_fee_schedule_update_transaction.py | Implements _from_protobuf for TokenFeeScheduleUpdateTransaction. |
| src/hiero_sdk_python/tokens/token_dissociate_transaction.py | Implements _from_protobuf for TokenDissociateTransaction. |
| src/hiero_sdk_python/tokens/token_delete_transaction.py | Implements _from_protobuf for TokenDeleteTransaction. |
| src/hiero_sdk_python/tokens/token_create_transaction.py | Implements _from_protobuf for TokenCreateTransaction, hydrating token params and keys. |
| src/hiero_sdk_python/tokens/token_burn_transaction.py | Implements _from_protobuf for TokenBurnTransaction (amount + serials). |
| src/hiero_sdk_python/tokens/token_associate_transaction.py | Implements _from_protobuf for TokenAssociateTransaction. |
| src/hiero_sdk_python/tokens/token_airdrop_transaction.py | Adds airdrop transfer-list validation and implements _from_protobuf for transfers/NFT transfers. |
| src/hiero_sdk_python/tokens/token_airdrop_transaction_cancel.py | Implements _from_protobuf for TokenCancelAirdropTransaction pending-airdrop IDs. |
| src/hiero_sdk_python/tokens/token_airdrop_claim.py | Implements _from_protobuf for TokenClaimAirdropTransaction pending-airdrop IDs + validation. |
| src/hiero_sdk_python/tokens/custom_fixed_fee.py | Tightens fixed-fee proto validation and adds a topic-fee proto helper constructor. |
| src/hiero_sdk_python/system/freeze_transaction.py | Implements _from_protobuf for FreezeTransaction fields. |
| src/hiero_sdk_python/schedule/schedule_sign_transaction.py | Implements _from_protobuf for ScheduleSignTransaction. |
| src/hiero_sdk_python/schedule/schedule_delete_transaction.py | Implements _from_protobuf for ScheduleDeleteTransaction. |
| src/hiero_sdk_python/schedule/schedule_create_transaction.py | Implements _from_protobuf for ScheduleCreateTransaction (payer/admin/schedulable body fields). |
| src/hiero_sdk_python/prng_transaction.py | Implements _from_protobuf for PrngTransaction. |
| src/hiero_sdk_python/nodes/node_update_transaction.py | Switches admin key parsing to Key.from_proto_key for composite-key support. |
| src/hiero_sdk_python/nodes/node_delete_transaction.py | Implements _from_protobuf for NodeDeleteTransaction. |
| src/hiero_sdk_python/nodes/node_create_transaction.py | Switches admin key parsing to Key.from_proto_key for composite-key support. |
| src/hiero_sdk_python/file/file_update_transaction.py | Implements _from_protobuf for FileUpdateTransaction (file ID, keys, contents, memo, expiry). |
| src/hiero_sdk_python/file/file_delete_transaction.py | Implements _from_protobuf for FileDeleteTransaction. |
| src/hiero_sdk_python/file/file_create_transaction.py | Implements _from_protobuf for FileCreateTransaction via existing _from_proto. |
| src/hiero_sdk_python/file/file_append_transaction.py | Implements _from_protobuf for FileAppendTransaction and improves _from_proto field guards. |
| src/hiero_sdk_python/contract/ethereum_transaction.py | Implements _from_protobuf for EthereumTransaction fields. |
| src/hiero_sdk_python/contract/contract_update_transaction.py | Implements _from_protobuf for ContractUpdateTransaction (memo oneof, keys, staking fields). |
| src/hiero_sdk_python/contract/contract_execute_transaction.py | Implements _from_protobuf for ContractExecuteTransaction (contract ID, gas, amount, params). |
| src/hiero_sdk_python/contract/contract_delete_transaction.py | Implements _from_protobuf for ContractDeleteTransaction (obtainers oneof). |
| src/hiero_sdk_python/contract/contract_create_transaction.py | Implements _from_protobuf for ContractCreateTransaction (admin key, initcode source, staking). |
| src/hiero_sdk_python/consensus/topic_update_transaction.py | Adjusts defaults and implements _from_protobuf including topic fee helpers and key lists. |
| src/hiero_sdk_python/consensus/topic_message_submit_transaction.py | Improves docs and implements _from_protobuf for topic message submissions. |
| src/hiero_sdk_python/consensus/topic_delete_transaction.py | Implements _from_protobuf for TopicDeleteTransaction. |
| src/hiero_sdk_python/consensus/topic_create_transaction.py | Implements _from_protobuf for TopicCreateTransaction including fee parsing. |
| src/hiero_sdk_python/account/account_update_transaction.py | Implements _from_protobuf for AccountUpdateTransaction (keys, staking fields, memo wrapper). |
| src/hiero_sdk_python/account/account_delete_transaction.py | Implements _from_protobuf for AccountDeleteTransaction (account + transfer IDs). |
| src/hiero_sdk_python/account/account_create_transaction.py | Makes autoRenewPeriod optional in proto build and implements _from_protobuf for account creation fields. |
| src/hiero_sdk_python/account/account_allowance_delete_transaction.py | Implements _from_protobuf for AccountAllowanceDeleteTransaction NFT allowance wipes. |
| src/hiero_sdk_python/account/account_allowance_approve_transaction.py | Implements _from_protobuf for AccountAllowanceApproveTransaction allowance lists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Raises: | ||
| ValueError: If required fields (message) are missing. | ||
| ValueError: If required fields (topic_id, message) are missing. | ||
| """ | ||
| if self.message is None or self.message == "": | ||
| raise ValueError("Missing required fields: message.") |
| # and ensures that the correct signatures are used when submitting transactions | ||
| self._signature_map: dict[bytes, basic_types_pb2.SignatureMap] = {} | ||
| # changed from int: 2_000_000 to Hbar: 2 | ||
| # changed from int: 2_000_000 to Hbar: 0.02 |
WalkthroughThis change adds protobuf deserialization for transaction classes across account, contract, consensus, file, schedule, system, and token modules. It centralizes transaction dispatch, normalizes optional protobuf values, adds validation, and introduces broad serialization round-trip tests. ChangesTransaction deserialization
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hiero_sdk_python/transaction/transaction.py (1)
860-905: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winMissing
batch_keyrestoration breaks the batch-execution guard after round-trip.
build_base_transaction_body()setstransaction_body.batch_keywhenself.batch_keyis set (line 542-543), but_from_protobuf()never restores it. AfterTransaction.from_bytes(), a batchified inner transaction comes back withbatch_key = None(the__init__default), even though its serialized body still hasbatch_keypopulated on the wire. That silently defeats the guard inexecute():if self.batch_key and not isinstance(self, (BatchTransaction)): raise ValueError("Cannot execute batchified transaction outside of BatchTransaction.")A restored inner transaction could now be executed directly outside
BatchTransaction, bypassing an intentional safety check. This is exactly the kind of round-trip/asymmetry gap the batch-transaction lifecycle guidelines call out as security critical.🐛 Proposed fix
if sig_map and sig_map.sigPair: transaction._signature_map[body_bytes] = sig_map + if transaction_body.HasField("batch_key"): + transaction.batch_key = Key.from_proto_key(transaction_body.batch_key) + return transactionAs per path instructions for
src/hiero_sdk_python/transaction/**/*.py: "Verify: ... Protobuf packing of inner signedTransactionBytes and atomic_batch field is preserved ... batchify()/set_batch_key() flow does not bypass lifecycle" and the global instruction's "3f._from_proto/_to_protosymmetry: ... Flag any asymmetry that is not accompanied by a code comment explaining why."Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 228952b4-d803-444f-b888-ae4eaeed15b8
📒 Files selected for processing (48)
src/hiero_sdk_python/account/account_allowance_approve_transaction.pysrc/hiero_sdk_python/account/account_allowance_delete_transaction.pysrc/hiero_sdk_python/account/account_create_transaction.pysrc/hiero_sdk_python/account/account_delete_transaction.pysrc/hiero_sdk_python/account/account_update_transaction.pysrc/hiero_sdk_python/consensus/topic_create_transaction.pysrc/hiero_sdk_python/consensus/topic_delete_transaction.pysrc/hiero_sdk_python/consensus/topic_message_submit_transaction.pysrc/hiero_sdk_python/consensus/topic_update_transaction.pysrc/hiero_sdk_python/contract/contract_create_transaction.pysrc/hiero_sdk_python/contract/contract_delete_transaction.pysrc/hiero_sdk_python/contract/contract_execute_transaction.pysrc/hiero_sdk_python/contract/contract_update_transaction.pysrc/hiero_sdk_python/contract/ethereum_transaction.pysrc/hiero_sdk_python/file/file_append_transaction.pysrc/hiero_sdk_python/file/file_create_transaction.pysrc/hiero_sdk_python/file/file_delete_transaction.pysrc/hiero_sdk_python/file/file_update_transaction.pysrc/hiero_sdk_python/nodes/node_create_transaction.pysrc/hiero_sdk_python/nodes/node_delete_transaction.pysrc/hiero_sdk_python/nodes/node_update_transaction.pysrc/hiero_sdk_python/prng_transaction.pysrc/hiero_sdk_python/schedule/schedule_create_transaction.pysrc/hiero_sdk_python/schedule/schedule_delete_transaction.pysrc/hiero_sdk_python/schedule/schedule_sign_transaction.pysrc/hiero_sdk_python/system/freeze_transaction.pysrc/hiero_sdk_python/tokens/custom_fixed_fee.pysrc/hiero_sdk_python/tokens/token_airdrop_claim.pysrc/hiero_sdk_python/tokens/token_airdrop_transaction.pysrc/hiero_sdk_python/tokens/token_airdrop_transaction_cancel.pysrc/hiero_sdk_python/tokens/token_associate_transaction.pysrc/hiero_sdk_python/tokens/token_burn_transaction.pysrc/hiero_sdk_python/tokens/token_create_transaction.pysrc/hiero_sdk_python/tokens/token_delete_transaction.pysrc/hiero_sdk_python/tokens/token_dissociate_transaction.pysrc/hiero_sdk_python/tokens/token_fee_schedule_update_transaction.pysrc/hiero_sdk_python/tokens/token_freeze_transaction.pysrc/hiero_sdk_python/tokens/token_grant_kyc_transaction.pysrc/hiero_sdk_python/tokens/token_mint_transaction.pysrc/hiero_sdk_python/tokens/token_pause_transaction.pysrc/hiero_sdk_python/tokens/token_reject_transaction.pysrc/hiero_sdk_python/tokens/token_revoke_kyc_transaction.pysrc/hiero_sdk_python/tokens/token_unfreeze_transaction.pysrc/hiero_sdk_python/tokens/token_unpause_transaction.pysrc/hiero_sdk_python/tokens/token_update_nfts_transaction.pysrc/hiero_sdk_python/tokens/token_update_transaction.pysrc/hiero_sdk_python/tokens/token_wipe_transaction.pysrc/hiero_sdk_python/transaction/transaction.py
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("consensusSubmitMessage"): | ||
| body = transaction_body.consensusSubmitMessage | ||
| if body.HasField("topicID"): | ||
| transaction.topic_id = TopicId._from_proto(body.topicID) | ||
| transaction.message = body.message.decode("utf-8") if body.message else None | ||
| transaction._total_chunks = transaction.get_required_chunks() | ||
| return transaction | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
BLOCKER: _from_protobuf doesn't restore chunkInfo, breaking multi-chunk round-trip.
_build_proto_body only emits chunkInfo for multi-chunk messages, but _from_protobuf never reads it back. Restoring a serialized chunk N of M transaction currently:
- Leaves
_initial_transaction_id = None,_current_chunk_index = 0,_transaction_ids = []— all multi-chunk position/context is lost. - Recomputes
_total_chunksfrom just that one chunk's decoded bytes (get_required_chunks()), which will normally collapse back to1, silently mis-representing a multi-chunk message as single-chunk. - Calls
body.message.decode("utf-8")on a byte slice that may split a multi-byte UTF-8 character at the chunk boundary, risking aUnicodeDecodeError.
This directly hits the consensus module's flagged BLOCKER concern around _initial_transaction_id null-safety and multi-chunk metadata for this exact class.
🐛 Proposed fix
`@classmethod`
def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map):
transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map)
if transaction_body.HasField("consensusSubmitMessage"):
body = transaction_body.consensusSubmitMessage
if body.HasField("topicID"):
transaction.topic_id = TopicId._from_proto(body.topicID)
- transaction.message = body.message.decode("utf-8") if body.message else None
- transaction._total_chunks = transaction.get_required_chunks()
+ transaction.message = body.message.decode("utf-8", errors="replace") if body.message else None
+ if body.HasField("chunkInfo"):
+ chunk_info = body.chunkInfo
+ transaction._total_chunks = chunk_info.total
+ transaction._current_chunk_index = max(chunk_info.number - 1, 0)
+ transaction._initial_transaction_id = (
+ TransactionId._from_proto(chunk_info.initialTransactionID)
+ if chunk_info.HasField("initialTransactionID")
+ else None
+ )
+ else:
+ transaction._total_chunks = transaction.get_required_chunks()
return transactionNote: even with this fix, message on a restored non-first chunk still only reflects that chunk's bytes, not the full original message — the wire format simply doesn't carry the other chunks' bytes in a single TransactionBody. Worth documenting that limitation explicitly (or restricting round-trip guarantees to single-chunk/first-chunk cases) rather than leaving it implicit.
As per path instructions for src/hiero_sdk_python/consensus/**/*.py: "_initial_transaction_id null-safety ... An explicit if self._initial_transaction_id is None guard with a descriptive ValueError MUST be present. Flag as BLOCKER." and "Nanos-overflow guard... Protobuf Timestamp.nanos is bounded..." (the same §3a section governing multi-chunk TransactionID safety for this class).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | |
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | |
| if transaction_body.HasField("consensusSubmitMessage"): | |
| body = transaction_body.consensusSubmitMessage | |
| if body.HasField("topicID"): | |
| transaction.topic_id = TopicId._from_proto(body.topicID) | |
| transaction.message = body.message.decode("utf-8") if body.message else None | |
| transaction._total_chunks = transaction.get_required_chunks() | |
| return transaction | |
| `@classmethod` | |
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | |
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | |
| if transaction_body.HasField("consensusSubmitMessage"): | |
| body = transaction_body.consensusSubmitMessage | |
| if body.HasField("topicID"): | |
| transaction.topic_id = TopicId._from_proto(body.topicID) | |
| transaction.message = body.message.decode("utf-8", errors="replace") if body.message else None | |
| if body.HasField("chunkInfo"): | |
| chunk_info = body.chunkInfo | |
| transaction._total_chunks = chunk_info.total | |
| transaction._current_chunk_index = max(chunk_info.number - 1, 0) | |
| transaction._initial_transaction_id = ( | |
| TransactionId._from_proto(chunk_info.initialTransactionID) | |
| if chunk_info.HasField("initialTransactionID") | |
| else None | |
| ) | |
| else: | |
| transaction._total_chunks = transaction.get_required_chunks() | |
| return transaction |
Source: Path instructions
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("ethereumTransaction"): | ||
| body = transaction_body.ethereumTransaction | ||
| transaction.ethereum_data = body.ethereum_data if body.ethereum_data else None | ||
| if body.HasField("call_data"): | ||
| transaction.call_data = FileId._from_proto(body.call_data) | ||
| transaction.max_gas_allowed = body.max_gas_allowance if body.max_gas_allowance else None | ||
| return transaction | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistent int-field normalization vs sibling transaction classes.
max_gas_allowed is coerced from 0 to None, same pattern flagged in contract_execute_transaction.py. See consolidated comment.
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("tokenAirdrop"): | ||
| for transfer in transaction_body.tokenAirdrop.token_transfers: | ||
| if not transfer.HasField("token"): | ||
| continue | ||
| token_id = TokenId._from_proto(transfer.token) | ||
| for t in transfer.transfers: | ||
| if not t.HasField("accountID"): | ||
| continue | ||
| account_id = AccountId._from_proto(t.accountID) | ||
| expected_decimals = ( | ||
| transfer.expected_decimals.value if transfer.HasField("expected_decimals") else None | ||
| ) | ||
| transaction.token_transfers[token_id].append( | ||
| TokenTransfer(token_id, account_id, t.amount, expected_decimals, t.is_approval) | ||
| ) | ||
| for n in transfer.nftTransfers: | ||
| if not n.HasField("senderAccountID") or not n.HasField("receiverAccountID"): | ||
| continue | ||
| sender_id = AccountId._from_proto(n.senderAccountID) | ||
| receiver_id = AccountId._from_proto(n.receiverAccountID) | ||
| transaction.nft_transfers[token_id].append( | ||
| TokenNftTransfer(token_id, sender_id, receiver_id, n.serialNumber, n.is_approval) | ||
| ) | ||
| return transaction | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicate protobuf→SDK mapping logic vs. the existing _from_proto classmethod.
This new _from_protobuf reimplements the same token_transfers/nft_transfers extraction that _from_proto (Lines 95-113) already performs via TokenTransfer._from_proto(transfer) / TokenNftTransfer._from_proto(transfer). Having two independently-maintained implementations of the same mapping increases the risk of silent divergence (as already seen: one path validates/skips differently than the other). Consider having _from_protobuf delegate to the same helper methods used by _from_proto for consistency.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== token_airdrop_transaction.py ==\n'
sed -n '1,220p' src/hiero_sdk_python/tokens/token_airdrop_transaction.py | cat -n
printf '\n== token_transfer.py ==\n'
fd -a 'token_transfer.py' src/hiero_sdk_python | while read -r f; do
printf '\nFILE: %s\n' "$f"
sed -n '1,260p' "$f" | cat -n
done
printf '\n== token_nft_transfer.py ==\n'
fd -a 'token_nft_transfer.py' src/hiero_sdk_python | while read -r f; do
printf '\nFILE: %s\n' "$f"
sed -n '1,240p' "$f" | cat -n
done
printf '\n== transaction base methods around from_protobuf/from_proto ==\n'
fd -a 'transaction.py' src/hiero_sdk_python | while read -r f; do
printf '\nFILE: %s\n' "$f"
rg -n "def _from_protobuf|def _from_proto|def _to_proto|def build_transaction_body|def _build_proto_body" "$f"
doneRepository: hiero-ledger/hiero-sdk-python
Length of output: 24000
Fail fast on malformed transfer entries
src/hiero_sdk_python/tokens/token_airdrop_transaction.py:72-87 silently drops any transfer entry missing token, accountID, senderAccountID, or receiverAccountID. That turns corrupt or future-incompatible protobuf bytes into a truncated transaction instead of surfacing an error. Raise a ValueError (or similar) here so invalid input does not deserialize into partial state.
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("tokenBurn"): | ||
| body = transaction_body.tokenBurn | ||
| if body.HasField("token"): | ||
| transaction.token_id = TokenId._from_proto(body.token) | ||
| transaction.amount = body.amount if body.amount else None | ||
| transaction.serials = list(body.serialNumbers) | ||
| return transaction | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
amount normalization diverges from the sibling _from_proto method.
All mapped proto fields align with the schema at https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/token_burn.proto, but the new _from_protobuf normalizes amount == 0 to None (Line 170), while the existing _from_proto (Line 185) keeps proto.amount as-is. Parsing identical wire bytes for an NFT-only burn (serials set, amount unset → defaults to 0 on the wire) through the two entry points now yields different in-memory states (amount=None vs amount=0).
🐛 Align the two deserializers (or document the intentional difference)
- transaction.amount = body.amount if body.amount else None
+ # Match `_from_proto`'s behavior for consistency across deserialization entry points
+ transaction.amount = body.amount📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | |
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | |
| if transaction_body.HasField("tokenBurn"): | |
| body = transaction_body.tokenBurn | |
| if body.HasField("token"): | |
| transaction.token_id = TokenId._from_proto(body.token) | |
| transaction.amount = body.amount if body.amount else None | |
| transaction.serials = list(body.serialNumbers) | |
| return transaction | |
| `@classmethod` | |
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | |
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | |
| if transaction_body.HasField("tokenBurn"): | |
| body = transaction_body.tokenBurn | |
| if body.HasField("token"): | |
| transaction.token_id = TokenId._from_proto(body.token) | |
| # Match `_from_proto`'s behavior for consistency across deserialization entry points | |
| transaction.amount = body.amount | |
| transaction.serials = list(body.serialNumbers) | |
| return transaction |
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): # noqa: PLR0912 | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("tokenUpdate"): | ||
| body = transaction_body.tokenUpdate | ||
| if body.HasField("token"): | ||
| transaction.token_id = TokenId._from_proto(body.token) | ||
| if body.HasField("treasury"): | ||
| transaction.treasury_account_id = AccountId._from_proto(body.treasury) | ||
| transaction.token_name = body.name if body.name else None | ||
| transaction.token_symbol = body.symbol if body.symbol else None | ||
| transaction.token_memo = body.memo.value if body.HasField("memo") else None | ||
| transaction.metadata = body.metadata.value if body.HasField("metadata") else None | ||
| if body.HasField("expiry"): | ||
| transaction.expiration_time = Timestamp._from_protobuf(body.expiry) | ||
| if body.HasField("autoRenewAccount"): | ||
| transaction.auto_renew_account_id = AccountId._from_proto(body.autoRenewAccount) | ||
| if body.HasField("autoRenewPeriod"): | ||
| transaction.auto_renew_period = Duration._from_proto(body.autoRenewPeriod) | ||
| transaction.token_key_verification_mode = TokenKeyValidation._from_proto(body.key_verification_mode) | ||
| if body.HasField("adminKey"): | ||
| transaction.admin_key = Key.from_proto_key(body.adminKey) | ||
| if body.HasField("freezeKey"): | ||
| transaction.freeze_key = Key.from_proto_key(body.freezeKey) | ||
| if body.HasField("wipeKey"): | ||
| transaction.wipe_key = Key.from_proto_key(body.wipeKey) | ||
| if body.HasField("supplyKey"): | ||
| transaction.supply_key = Key.from_proto_key(body.supplyKey) | ||
| if body.HasField("metadata_key"): | ||
| transaction.metadata_key = Key.from_proto_key(body.metadata_key) | ||
| if body.HasField("pause_key"): | ||
| transaction.pause_key = Key.from_proto_key(body.pause_key) | ||
| if body.HasField("kycKey"): | ||
| transaction.kyc_key = Key.from_proto_key(body.kycKey) | ||
| if body.HasField("fee_schedule_key"): | ||
| transaction.fee_schedule_key = Key.from_proto_key(body.fee_schedule_key) | ||
| return transaction | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add round-trip tests for the new _from_protobuf deserializers.
This PR's stated goal is to fix incomplete Transaction.from_bytes() deserialization, but none of the seven reviewed token transaction classes ship a test asserting the round trip (build → to_bytes() → from_bytes() → assert all fields equal). Without this, a future refactor could silently reintroduce the exact bug this PR fixes.
src/hiero_sdk_python/tokens/token_update_transaction.py#L474-L511: add a test round-tripping token id, treasury, name/symbol/memo/metadata, expiry/auto-renew, key verification mode, and all eight key fields.src/hiero_sdk_python/tokens/token_associate_transaction.py#L105-L114: add a test round-trippingaccount_idandtoken_ids.src/hiero_sdk_python/tokens/token_burn_transaction.py#L163-L173: add a test round-trippingtoken_id,amount, andserials(both the fungible-amount and NFT-serials cases).src/hiero_sdk_python/tokens/token_delete_transaction.py#L97-L105: add a test round-trippingtoken_id.src/hiero_sdk_python/tokens/token_dissociate_transaction.py#L80-L89: add a test round-trippingaccount_idandtoken_ids.src/hiero_sdk_python/tokens/token_grant_kyc_transaction.py#L130-L140: add a test round-trippingtoken_idandaccount_id.src/hiero_sdk_python/tokens/token_unfreeze_transaction.py#L119-L129: add a test round-trippingtoken_idandaccount_id.
📍 Affects 7 files
src/hiero_sdk_python/tokens/token_update_transaction.py#L474-L511(this comment)src/hiero_sdk_python/tokens/token_associate_transaction.py#L105-L114src/hiero_sdk_python/tokens/token_burn_transaction.py#L163-L173src/hiero_sdk_python/tokens/token_delete_transaction.py#L97-L105src/hiero_sdk_python/tokens/token_dissociate_transaction.py#L80-L89src/hiero_sdk_python/tokens/token_grant_kyc_transaction.py#L130-L140src/hiero_sdk_python/tokens/token_unfreeze_transaction.py#L119-L129
Source: Path instructions
github-actions
Bot
added
open to community review
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #2438 +/- ##
==========================================
+ Coverage 95.39% 96.07% +0.67%
==========================================
Files 165 165
Lines 10559 11186 +627
==========================================
+ Hits 10073 10747 +674
+ Misses 486 439 -47 🚀 New features to boost your workflow:
|
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours will begin in approximately 3 hours and 28 minutes (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
0d19c27 to
a9a4fed
Compare
|
Hi, this is WorkflowBot.
|
3c1484a to
2e1052a
Compare
830c151 to
30ef09a
Compare
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/hiero_sdk_python/transaction/transaction.py (1)
875-894: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winRestore
batch_keyduring transaction deserialization.
build_base_transaction_body()writesTransactionBody.batch_key, butTransaction._from_protobuf()never reconstructs it. A deserialized inner signed transaction therefore hasbatch_key == NoneandBatchTransaction.add_inner_transaction(restored)raisesBatch key needs to be set.
Transaction._from_protobuf()insrc/hiero_sdk_python/transaction/transaction.pyaftermemo = transaction_body.memo.transaction.memo = transaction_body.memo + if transaction_body.HasField("batch_key"): + transaction.batch_key = Key.from_proto_key(transaction_body.batch_key) if transaction_body.max_custom_fees:Add a signed batch inner-transaction round-trip test that asserts
restored.batch_keyis set andBatchTransaction.add_inner_transaction(restored)succeeds.Source: Path instructions
src/hiero_sdk_python/tokens/token_reject_transaction.py (1)
55-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMAJOR — Validate bulk rejection identifiers before storing them.
__init__,set_token_ids, andset_nft_idsaccept invalid element types. Serialization then fails with a protobuf type error or an attribute error. Validate every element with the same rules asadd_token_idandadd_nft_id.
src/hiero_sdk_python/tokens/token_reject_transaction.py#L55-L112: Validate constructor and bulk-setter entries againstTokenIdorNftId. Proto fields:TokenReference.fungible_token(#1) andTokenReference.nft(#2). Issue type: Wrong type. Schema:https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/token_reject.proto. (raw.githubusercontent.com)tests/unit/token_reject_transaction_test.py#L203-L255: Add constructor and bulk-setter tests that assert the sameTypeErrormessages as the incremental add methods.Proposed validation pattern
def set_token_ids(self, token_ids: list[TokenId]) -> TokenRejectTransaction: self._require_not_frozen() + if not all(isinstance(token_id, TokenId) for token_id in token_ids): + raise TypeError("token_ids must contain only TokenId instances.") self.token_ids = list(token_ids) return selfSource: Path instructions
src/hiero_sdk_python/file/file_append_transaction.py (1)
224-242: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject empty file append contents before serialization.
Proto field:
contents(4).
Issue type: Wrong default.
contentsis required and must not be empty. The constructor and_from_proto()permitNoneorb"", and_build_proto_body()serializes either as empty bytes. This creates a transaction that the node must reject. (github.com)Validate non-empty contents in
_build_proto_body(). Reject empty protobuf contents in_from_proto(). Add tests for both paths.Proposed fix
def _build_proto_body(self) -> file_append_pb2.FileAppendTransactionBody: if self.file_id is None: raise ValueError("Missing required FileID") + if not self.contents: + raise ValueError("Missing required file contents")def _from_proto(self, proto): + if not proto.contents: + raise ValueError("File append contents must not be empty") self.file_id = FileId._from_proto(proto.fileID) if proto.HasField("fileID") else None - self.contents = proto.contents if proto.contents else None + self.contents = proto.contentsSource: Path instructions
tests/unit/topic_update_transaction_test.py (1)
264-274: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis test now accepts a silently omitted
topicID, which makes the consensus classes inconsistent.The test previously asserted a client-side
ValueError. It now asserts thattopicIDis simply absent from the body.TopicDeleteTransactionstill raisesValueError, match="Missing required fields"for the same condition (tests/unit/topic_delete_transaction_test.pyLine 71). After this change, a caller who forgetsset_topic_id()on an update gets a network precheck failure instead of an immediate, actionable error.Confirm that this relaxation is intentional. If it is, add a comment in
TopicUpdateTransaction._build_proto_bodythat records why validation is deferred to the node.Source: Path instructions
src/hiero_sdk_python/consensus/topic_message_submit_transaction.py (1)
185-190: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe docstring lists
topic_idas required, but the code does not validate it.Line 185 states that missing
topic_idraisesValueError. The guard at Line 187 checks onlymessage. Line 197 passesNonefortopicIDwhentopic_idis unset, so the node rejects the transaction instead of the SDK. Validatetopic_idhere, or remove it from the docstring.🛡️ Proposed fix
+ if self.topic_id is None: + raise ValueError("Missing required fields: topic_id.") if not self.message: raise ValueError("Missing required fields: message.")As per path instructions for
src/hiero_sdk_python/consensus/**/*.py: "Ensure required fields are enforced before freezing/signing (if the SDK has a freeze step)."Source: Path instructions
src/hiero_sdk_python/tokens/token_dissociate_transaction.py (1)
112-119: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMAJOR — Validate required identifiers before protobuf construction.
Each serializer silently omits fields that define the requested token operation. This allows freezing and signing an invalid transaction before the node rejects it.
src/hiero_sdk_python/tokens/token_dissociate_transaction.py#L112-L119: Reject a missingaccountfield (#1) and an empty or invalidtokensfield (#2). Issue type: Missing field. Schema:https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/token_dissociate.proto. (raw.githubusercontent.com)src/hiero_sdk_python/tokens/token_grant_kyc_transaction.py#L80-L86: Reject a missingtokenfield (#1) oraccountfield (#2). Issue type: Missing field. Schema:https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/token_grant_kyc.proto. (raw.githubusercontent.com)src/hiero_sdk_python/tokens/token_revoke_kyc_transaction.py#L81-L87: Reject a missingtokenfield (#1) oraccountfield (#2). Issue type: Missing field. Schema:https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/token_revoke_kyc.proto. (raw.githubusercontent.com)tests/unit/token_dissociate_transaction_test.py#L235-L291: Replace successful-body assertions for missing IDs andNonetoken entries with descriptiveValueErrorassertions.tests/unit/token_grant_kyc_transaction_test.py#L40-L54: Replace successful-body assertions for missing IDs with descriptiveValueErrorassertions.Proposed validation pattern
def _build_proto_body(self): + if self.token_id is None: + raise ValueError("Missing required token ID") + if self.account_id is None: + raise ValueError("Missing required account ID") return TokenGrantKycTransactionBody( - **kwargs + token=self.token_id._to_proto(), + account=self.account_id._to_proto(), )Source: Path instructions
src/hiero_sdk_python/tokens/token_update_transaction.py (1)
418-418: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the required token identifier before body construction. The implementation permits an invalid token-update body, and the unit test accepts that result.
src/hiero_sdk_python/tokens/token_update_transaction.py#L418-L418: raiseValueError("Missing token ID")before constructingTokenUpdateTransactionBody.tests/unit/token_update_transaction_test.py#L123-L129: replace the absent-field assertion with an assertion for the requiredValueError.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 960e5181-4d5a-4f2f-af08-31959fa8f5b8
📒 Files selected for processing (66)
src/hiero_sdk_python/consensus/topic_create_transaction.pysrc/hiero_sdk_python/consensus/topic_message_submit_transaction.pysrc/hiero_sdk_python/consensus/topic_update_transaction.pysrc/hiero_sdk_python/contract/contract_execute_transaction.pysrc/hiero_sdk_python/contract/ethereum_transaction.pysrc/hiero_sdk_python/file/file_append_transaction.pysrc/hiero_sdk_python/file/file_create_transaction.pysrc/hiero_sdk_python/file/file_update_transaction.pysrc/hiero_sdk_python/schedule/schedule_create_transaction.pysrc/hiero_sdk_python/tokens/token_airdrop_transaction.pysrc/hiero_sdk_python/tokens/token_burn_transaction.pysrc/hiero_sdk_python/tokens/token_dissociate_transaction.pysrc/hiero_sdk_python/tokens/token_grant_kyc_transaction.pysrc/hiero_sdk_python/tokens/token_mint_transaction.pysrc/hiero_sdk_python/tokens/token_reject_transaction.pysrc/hiero_sdk_python/tokens/token_revoke_kyc_transaction.pysrc/hiero_sdk_python/tokens/token_update_transaction.pysrc/hiero_sdk_python/transaction/transaction.pytests/unit/account_allowance_approve_transaction_test.pytests/unit/account_allowance_delete_transaction_test.pytests/unit/account_create_transaction_test.pytests/unit/account_delete_transaction_test.pytests/unit/account_update_transaction_test.pytests/unit/contract_create_transaction_test.pytests/unit/contract_delete_transaction_test.pytests/unit/contract_execute_transaction_test.pytests/unit/contract_update_transaction_test.pytests/unit/custom_fee_test.pytests/unit/ethereum_transaction_test.pytests/unit/file_append_transaction_test.pytests/unit/file_create_transaction_test.pytests/unit/file_delete_transaction_test.pytests/unit/file_update_transaction_test.pytests/unit/freeze_transaction_test.pytests/unit/node_create_transaction_test.pytests/unit/node_delete_transaction_test.pytests/unit/node_update_transaction_test.pytests/unit/prng_transaction_test.pytests/unit/schedule_create_transaction_test.pytests/unit/schedule_delete_transaction_test.pytests/unit/schedule_sign_transaction_test.pytests/unit/token_airdrop_claim_test.pytests/unit/token_airdrop_transaction_cancel_test.pytests/unit/token_airdrop_transaction_test.pytests/unit/token_associate_transaction_test.pytests/unit/token_burn_transaction_test.pytests/unit/token_create_transaction_test.pytests/unit/token_delete_transaction_test.pytests/unit/token_dissociate_transaction_test.pytests/unit/token_fee_schedule_update_transaction_test.pytests/unit/token_freeze_transaction_test.pytests/unit/token_grant_kyc_transaction_test.pytests/unit/token_mint_transaction_test.pytests/unit/token_pause_transaction_test.pytests/unit/token_reject_transaction_test.pytests/unit/token_revoke_kyc_transaction_test.pytests/unit/token_unfreeze_transaction_test.pytests/unit/token_unpause_transaction_test.pytests/unit/token_update_nfts_transaction_test.pytests/unit/token_update_transaction_test.pytests/unit/token_wipe_transaction_test.pytests/unit/topic_create_transaction_test.pytests/unit/topic_delete_transaction_test.pytests/unit/topic_message_submit_transaction_test.pytests/unit/topic_update_transaction_test.pytests/unit/transaction_dispatch_test.py
| body = transaction_body.consensusSubmitMessage | ||
| if body.HasField("topicID"): | ||
| transaction.topic_id = TopicId._from_proto(body.topicID) | ||
| transaction.message = body.message.decode("utf-8", errors="replace") if body.message else None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Decoding to str breaks the round trip for binary messages.
message accepts bytes | str (Line 31, Line 46, Line 96). _from_protobuf always decodes the payload to str with errors="replace". For a binary payload, every invalid sequence becomes U+FFFD. _message_as_bytes() then re-encodes those replacement characters, so Transaction.from_bytes(tx.to_bytes()).to_bytes() emits different wire bytes than the original.
Keep the raw bytes and decode only when the payload is valid UTF-8.
🐛 Proposed fix
- transaction.message = body.message.decode("utf-8", errors="replace") if body.message else None
+ if body.message:
+ try:
+ transaction.message = body.message.decode("utf-8")
+ except UnicodeDecodeError:
+ # Preserve binary payloads verbatim so the round trip stays byte-exact.
+ transaction.message = body.message
+ else:
+ transaction.message = NoneNote: for a restored non-first chunk, message still holds only that chunk's bytes. A single TransactionBody does not carry the other chunks. Document that limitation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| transaction.message = body.message.decode("utf-8", errors="replace") if body.message else None | |
| if body.message: | |
| try: | |
| transaction.message = body.message.decode("utf-8") | |
| except UnicodeDecodeError: | |
| # Preserve binary payloads verbatim so the round trip stays byte-exact. | |
| transaction.message = body.message | |
| else: | |
| transaction.message = None |
| self.keys = KeyList.from_proto(proto.keys).keys | ||
| self.contents = proto.contents | ||
| self.expiration_time = Timestamp._from_protobuf(proto.expirationTime) if proto.expirationTime else None | ||
| self.file_memo = proto.memo | ||
| self.contents = proto.contents or None | ||
| self.expiration_time = ( | ||
| Timestamp._from_protobuf(proto.expirationTime) if proto.HasField("expirationTime") else None | ||
| ) | ||
| self.file_memo = proto.memo or None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
MAJOR — Restore all non-deprecated file-create fields.
FileCreateTransactionBody also defines shardID (field 5) and realmID (field 6). The new parser drops both fields, so Transaction.from_bytes() does not reconstruct all transaction-specific state. The canonical schema is https://github.com/hashgraph/hedera-protobufs/blob/v0.72.0-rc.2/services/file_create.proto. (raw.githubusercontent.com)
src/hiero_sdk_python/file/file_create_transaction.py#L202-L207: Add nullableshard_idandrealm_idstate. Parse both fields withHasField(...). Copy both fields into_build_proto_body()when present.tests/unit/file_create_transaction_test.py#L324-L347: Add a byte round-trip case that sets and assertsshard_idandrealm_id.
Proposed preservation pattern
+# __init__
+self.shard_id = None
+self.realm_id = None
+
+# _from_proto
+self.shard_id = copy.deepcopy(proto.shardID) if proto.HasField("shardID") else None
+self.realm_id = copy.deepcopy(proto.realmID) if proto.HasField("realmID") else None
+
+# _build_proto_body, after constructing `body`
+if self.shard_id is not None:
+ body.shardID.CopyFrom(self.shard_id)
+if self.realm_id is not None:
+ body.realmID.CopyFrom(self.realm_id)📍 Affects 2 files
src/hiero_sdk_python/file/file_create_transaction.py#L202-L207(this comment)tests/unit/file_create_transaction_test.py#L324-L347
Source: Path instructions
| @classmethod | ||
| def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): | ||
| transaction = super()._from_protobuf(transaction_body, body_bytes, sig_map) | ||
| if transaction_body.HasField("fileUpdate"): | ||
| body = transaction_body.fileUpdate | ||
| if body.HasField("fileID"): | ||
| transaction.file_id = FileId._from_proto(body.fileID) | ||
| transaction.keys = [Key.from_proto_key(k) for k in body.keys.keys] if body.keys.keys else None | ||
| transaction.contents = body.contents if body.contents else None | ||
| if body.HasField("expirationTime"): | ||
| transaction.expiration_time = Timestamp._from_protobuf(body.expirationTime) | ||
| if body.HasField("memo"): | ||
| transaction.file_memo = body.memo.value |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the present-empty keys field.
FileUpdateTransactionBody.keys field 3 distinguishes an absent field from a present empty KeyList. The latter makes the file immutable. The current deserializer changes that value to None, and the serializer then omits it. (github.com)
src/hiero_sdk_python/file/file_update_transaction.py#L218-L230: Usebody.HasField("keys")to preserve a present empty list as[]. Change the body builder condition fromif self.keystoif self.keys is not None.tests/unit/file_update_transaction_test.py#L365-L390: Add a round-trip test forkeys=[]. Assertreconstructed.keys == []andreconstructed._build_proto_body().HasField("keys").
Proposed fix
- keys=(KeyListProto(keys=[key._to_proto() for key in self.keys]) if self.keys else None),
+ keys=(
+ KeyListProto(keys=[key._to_proto() for key in self.keys])
+ if self.keys is not None
+ else None
+ ),- transaction.keys = [Key.from_proto_key(k) for k in body.keys.keys] if body.keys.keys else None
+ transaction.keys = (
+ [Key.from_proto_key(key) for key in body.keys.keys]
+ if body.HasField("keys")
+ else None
+ )📍 Affects 2 files
src/hiero_sdk_python/file/file_update_transaction.py#L218-L230(this comment)tests/unit/file_update_transaction_test.py#L365-L390
Source: Path instructions
| for transfer in transaction_body.tokenAirdrop.token_transfers: | ||
| if not transfer.HasField("token"): | ||
| raise ValueError("Malformed TokenAirdropTransactionBody: token_transfer missing token field") | ||
| token_id = TokenId._from_proto(transfer.token) | ||
| for t in transfer.transfers: | ||
| if not t.HasField("accountID"): | ||
| raise ValueError("Malformed TokenAirdropTransactionBody: fungible transfer missing accountID") | ||
| account_id = AccountId._from_proto(t.accountID) | ||
| expected_decimals = ( | ||
| transfer.expected_decimals.value if transfer.HasField("expected_decimals") else None | ||
| ) | ||
| transaction.token_transfers[token_id].append( | ||
| TokenTransfer(token_id, account_id, t.amount, expected_decimals, t.is_approval) | ||
| ) | ||
| for n in transfer.nftTransfers: | ||
| if not n.HasField("senderAccountID") or not n.HasField("receiverAccountID"): | ||
| raise ValueError( | ||
| "Malformed TokenAirdropTransactionBody: NFT transfer missing sender or receiver" | ||
| ) | ||
| sender_id = AccountId._from_proto(n.senderAccountID) | ||
| receiver_id = AccountId._from_proto(n.receiverAccountID) | ||
| transaction.nft_transfers[token_id].append( | ||
| TokenNftTransfer(token_id, sender_id, receiver_id, n.serialNumber, n.is_approval) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'hook_call|allowance_hook|class TokenTransfer|class TokenNftTransfer' \
src/hiero_sdk_pythonRepository: hiero-ledger/hiero-sdk-python
Length of output: 1965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant token transfer/Airdrop files and any generated proto field names.
printf 'Relevant file list:\n'
fd -a 'token_(airdrop|transfer|transfer_list|nft_transfer).*\.py$|.*_pb2.*\.py$' src/hiero_sdk_python | sed 's#^\./##'
printf '\nAirdrop transaction lines 1-140:\n'
cat -n src/hiero_sdk_python/tokens/token_airdrop_transaction.py | sed -n '1,140p'
printf '\nToken transfer files:\n'
for f in src/hiero_sdk_python/tokens/token_transfer.py src/hiero_sdk_python/tokens/token_transfer_list.py src/hiero_sdk_python/tokens/token_nft_transfer.py; do
echo "===== $f ====="
cat -n "$f" | sed -n '1,220p'
done
printf '\nSearch token transfer proto usage:\n'
rg -n -C 3 'token_transfers|transfers|nftTransfers|TransferList|_from_proto|_to_proto' \
src/hiero_sdk_python/tokens src/hiero_sdk_python -g '*.py'Repository: hiero-ledger/hiero-sdk-python
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '===== token_transfer_list proto serialization ====='
cat -n src/hiero_sdk_python/tokens/token_transfer_list.py | sed -n '60,95p'
echo '===== token_airstrop mapping and body builders ====='
cat -n src/hiero_sdk_python/tokens/token_airdrop_transaction.py | sed -n '1,135p'
echo '===== token_transfer / token_nft_transfer proto data ====='
cat -n src/hiero_sdk_python/tokens/token_transfer.py | sed -n '1,160p'
echo '-----'
cat -n src/hiero_sdk_python/tokens/token_nft_transfer.py | sed -n '1,160p'
echo '===== exact token_transfer_list_proto field/oneof mentions in source ====='
rg -n -C 2 'token_transfers|nft_transfers|is_approval|senderAccountID|receiverAccountID|serialNumber|allowance|hook_call' \
src/hiero_sdk_python/tokens/basic_types_pb2.py src/hiero_sdk_python/tokens -g '*.py' | sed -n '1,220p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 36948
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '===== located basic_types_pb2 definitions ====='
fd -a 'basic_types_pb2.py' src/hiero_sdk_python | sed 's#^\./##'
for f in $(fd 'basic_types_pb2.py' src/hiero_sdk_python); do
echo "===== $f ====="
rg -n -C 4 'class TokenTransferList|class AccountAmount|class NftTransfer|oneof|hook_call|allowance_hook' "$f" || true
done
echo '===== proto raw text snippets if files exist ====='
for f in $(fd 'basic_types_pb2.py' src/hiero_sdk_python); do
echo "--- $f ---"
rg -n 'TokenTransferList|AccountAmount|NftTransfer|hook_call|AllowanceHook|oneof' "$f" | head -n 80 || true
doneRepository: hiero-ledger/hiero-sdk-python
Length of output: 266
🌐 Web query:
Hedera protobufs v0.72.0-rc.2 basic_types.proto TokenTransferList TokenTransfer AccountAmount NftTransfer hook_call allowance_hook
💡 Result:
The Hedera protobuf definitions, including basic_types.proto, are managed in the authoritative hashgraph/hedera-protobufs repository on GitHub [1]. The requested elements are core components used to facilitate token transfers on the Hedera network [2][3]. TokenTransferList This message defines a collection of transfers for a specific token [2]. It includes a TokenID to identify the token being transferred, a list of AccountAmount entries for fungible token transfers, and a list of NftTransfer entries for non-fungible token (NFT) transfers [2][3]. A single TokenTransferList must contain either fungible transfers or NFT transfers, but not both [2]. AccountAmount This structure represents the transfer of fungible tokens [3]. It associates an account (AccountID) with a specific amount of tokens [2]. Amounts are expressed as integers, where positive values indicate a receipt of tokens and negative values indicate a transfer out of the account [3]. NftTransfer This structure facilitates the transfer of non-fungible tokens [3]. It specifies the sender and receiver accounts, as well as the serial number of the specific NFT being transferred [2][3]. Regarding hook_call and allowance_hook: There is no standard or commonly documented protobuf message or field named hook_call or allowance_hook within the official Hedera basic_types.proto or the broader Hedera protobuf schema as of August 2026 [2][3]. The field used to signify that a transfer is an approved allowance is named is_approval (a boolean field found in both AccountAmount and NftTransfer) [3]. For the most accurate and up-to-date definitions, please refer to the official hashgraph/hedera-protobufs repository [1]. Note that v0.72.0-rc.2 refers to a specific release candidate tag; you can inspect the state of the protobuf files at that specific tag by navigating to the "Tags" section of the GitHub repository [1].
Citations:
- 1: https://github.com/hashgraph/hedera-protobufs
- 2: https://github.com/hashgraph/hedera-protobufs/blob/main/services/basic_types.proto
- 3: https://hashgraph.github.io/hedera-protobufs/
🌐 Web query:
raw hashgraph hedera-protobufs v0.72.0-rc.2 services basic_types.proto TokenTransferList oneof oneof transfers nftTransfers
💡 Result:
In the Hedera protobufs (including versions such as v0.72.0-rc.2 and the current main branch), the TokenTransferList message is used to define token transfers [1]. It includes fields for both fungible token transfers and non-fungible token (NFT) transfers, structured as follows: - transfers: A repeated list of AccountAmount objects for fungible/common tokens [1][2]. - nftTransfers: A repeated list of NftTransfer objects for non-fungible/unique tokens [1][2]. While these fields exist within the same message, they are not strictly defined as a oneof in the protobuf definition; rather, they are independent fields [1][2]. However, the protocol logic imposes a constraint: a single TokenTransferList must contain either transfers or nftTransfers, but it must not contain both [1]. Specifically, the protocol rules state [1]: - Transfers for fungible tokens must specify an accountID and amount, and they must be empty if the list is used for NFTs [1][2]. - Transfers for unique tokens (NFTs) must specify a sender, receiver, and serial number, and they must be empty if the list is used for fungible tokens [1][2]. As documented in the official Hedera protobuf definitions, the TokenTransferList is designed to handle these two distinct types of token operations while maintaining the integrity of the double-entry accounting model required for Hedera's token service [1][2].
Citations:
- 1: https://github.com/hashgraph/hedera-protobufs/blob/main/services/basic_types.proto
- 2: https://docs.hedera.com/reference/protobuf/basic-types/tokentransferlist
🌐 Web query:
site:github.com/hashgraph/hedera-protobufs main services/basic_types.proto "TokenTransferList" "oneof"
💡 Result:
In the Hedera basic_types.proto service definition, the TokenTransferList message does not technically use a oneof construct to handle the exclusivity between fungible and non-fungible transfers [1]. Instead, it uses two distinct, repeated fields: transfers (for fungible tokens) and nftTransfers (for non-fungible tokens) [1]. While the protocol buffer definition does not enforce this exclusivity via a oneof block, the documentation explicitly specifies a business logic requirement: a single TokenTransferList MUST contain either transfers or nftTransfers, but MUST NOT contain both [1]. The message structure is as follows: message TokenTransferList { TokenID token = 1; repeated AccountAmount transfers = 2; repeated NftTransfer nftTransfers = 3; google.protobuf.UInt32Value expected_decimals = 4; } Per the service requirements, transfers should be set for fungible tokens and must be empty otherwise, while nftTransfers should be set for non-fungible tokens and must be empty otherwise [1].
Citations:
Reject unrepresentable transfer data before deserialization.
Proto fields: TokenTransferList.transfers (2), TokenTransferList.nftTransfers (3), AccountAmount.hook_call (4, 5), and NftTransfer allowance hooks.
Issue type: Asymmetric round-trip.
_to_proto() builds both transfer lists from TokenTransferList, so bodies containing both transfers and nftTransfers, or empty lists, can round-trip differently or drop valid data. _from_proto() also ignores populated hook allowance fields. Reject these bodies unless the SDK adds complete support for them.
Proposed fix
+ has_fungible = bool(transfer.transfers)
+ has_nft = bool(transfer.nftTransfers)
+ if has_fungible == has_nft:
+ raise ValueError(
+ "Malformed TokenAirdropTransactionBody: transfer list must contain exactly one transfer type"
+ )
+
for t in transfer.transfers:
+ if t.WhichOneof("hook_call") is not None:
+ raise ValueError(
+ "Malformed TokenAirdropTransactionBody: fungible transfer contains unsupported hook data"
+ )
if not t.HasField("accountID"):
raise ValueError(...)
...
for n in transfer.nftTransfers:
+ if (
+ n.WhichOneof("sender_allowance_hook_call") is not None
+ or n.WhichOneof("receiver_allowance_hook_call") is not None
+ ):
+ raise ValueError(
+ "Malformed TokenAirdropTransactionBody: NFT transfer contains unsupported hook data"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for transfer in transaction_body.tokenAirdrop.token_transfers: | |
| if not transfer.HasField("token"): | |
| raise ValueError("Malformed TokenAirdropTransactionBody: token_transfer missing token field") | |
| token_id = TokenId._from_proto(transfer.token) | |
| for t in transfer.transfers: | |
| if not t.HasField("accountID"): | |
| raise ValueError("Malformed TokenAirdropTransactionBody: fungible transfer missing accountID") | |
| account_id = AccountId._from_proto(t.accountID) | |
| expected_decimals = ( | |
| transfer.expected_decimals.value if transfer.HasField("expected_decimals") else None | |
| ) | |
| transaction.token_transfers[token_id].append( | |
| TokenTransfer(token_id, account_id, t.amount, expected_decimals, t.is_approval) | |
| ) | |
| for n in transfer.nftTransfers: | |
| if not n.HasField("senderAccountID") or not n.HasField("receiverAccountID"): | |
| raise ValueError( | |
| "Malformed TokenAirdropTransactionBody: NFT transfer missing sender or receiver" | |
| ) | |
| sender_id = AccountId._from_proto(n.senderAccountID) | |
| receiver_id = AccountId._from_proto(n.receiverAccountID) | |
| transaction.nft_transfers[token_id].append( | |
| TokenNftTransfer(token_id, sender_id, receiver_id, n.serialNumber, n.is_approval) | |
| ) | |
| for transfer in transaction_body.tokenAirdrop.token_transfers: | |
| if not transfer.HasField("token"): | |
| raise ValueError("Malformed TokenAirdropTransactionBody: token_transfer missing token field") | |
| has_fungible = bool(transfer.transfers) | |
| has_nft = bool(transfer.nftTransfers) | |
| if has_fungible == has_nft: | |
| raise ValueError( | |
| "Malformed TokenAirdropTransactionBody: transfer list must contain exactly one transfer type" | |
| ) | |
| token_id = TokenId._from_proto(transfer.token) | |
| for t in transfer.transfers: | |
| if t.WhichOneof("hook_call") is not None: | |
| raise ValueError( | |
| "Malformed TokenAirdropTransactionBody: fungible transfer contains unsupported hook data" | |
| ) | |
| if not t.HasField("accountID"): | |
| raise ValueError("Malformed TokenAirdropTransactionBody: fungible transfer missing accountID") | |
| account_id = AccountId._from_proto(t.accountID) | |
| expected_decimals = ( | |
| transfer.expected_decimals.value if transfer.HasField("expected_decimals") else None | |
| ) | |
| transaction.token_transfers[token_id].append( | |
| TokenTransfer(token_id, account_id, t.amount, expected_decimals, t.is_approval) | |
| ) | |
| for n in transfer.nftTransfers: | |
| if ( | |
| n.WhichOneof("sender_allowance_hook_call") is not None | |
| or n.WhichOneof("receiver_allowance_hook_call") is not None | |
| ): | |
| raise ValueError( | |
| "Malformed TokenAirdropTransactionBody: NFT transfer contains unsupported hook data" | |
| ) | |
| if not n.HasField("senderAccountID") or not n.HasField("receiverAccountID"): | |
| raise ValueError( | |
| "Malformed TokenAirdropTransactionBody: NFT transfer missing sender or receiver" | |
| ) | |
| sender_id = AccountId._from_proto(n.senderAccountID) | |
| receiver_id = AccountId._from_proto(n.receiverAccountID) | |
| transaction.nft_transfers[token_id].append( | |
| TokenNftTransfer(token_id, sender_id, receiver_id, n.serialNumber, n.is_approval) | |
| ) |
Source: Path instructions
| transaction.token_memo = body.memo.value if body.HasField("memo") else None | ||
| transaction.metadata = body.metadata.value if body.HasField("metadata") else None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve explicit empty memo and metadata updates.
HasField() distinguishes an absent wrapper from an explicitly empty wrapper. _from_protobuf() restores "" and b"", but _build_proto_body() drops both values because lines 421-422 use truthiness checks. A rebuilt scheduled body can therefore remove a requested memo or metadata clear.
Proto fields: memo (13), metadata (16). Issue type: Asymmetric round-trip. The schema uses wrapper messages for both fields, so their presence is significant. (raw.githubusercontent.com)
Proposed fix
- memo=StringValue(value=self.token_memo) if self.token_memo else None,
- metadata=BytesValue(value=self.metadata) if self.metadata else None,
+ memo=StringValue(value=self.token_memo) if self.token_memo is not None else None,
+ metadata=BytesValue(value=self.metadata) if self.metadata is not None else None,Source: Path instructions
| assert isinstance(reconstructed, TokenCreateTransaction) | ||
| assert reconstructed._token_params.token_name == "TestToken" | ||
| assert reconstructed._token_params.token_symbol == "TT" | ||
| assert reconstructed._token_params.decimals == 2 | ||
| assert reconstructed._token_params.initial_supply == 1000 | ||
| assert reconstructed._token_params.treasury_account_id == treasury_account | ||
| assert reconstructed._keys.admin_key is not None | ||
| assert reconstructed._keys.supply_key is not None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Verify restored token-key identities, not only presence. These tests can pass when a populated key field is restored with the wrong key.
tests/unit/token_create_transaction_test.py#L993-L1000: compare the restored admin and supply key bytes with the source keys.tests/unit/token_create_transaction_test.py#L1003-L1040: compare each restored optional key’s raw bytes with its source key.tests/unit/token_update_transaction_test.py#L508-L543: compare each restored optional key’s raw bytes with its source key.
📍 Affects 2 files
tests/unit/token_create_transaction_test.py#L993-L1000(this comment)tests/unit/token_create_transaction_test.py#L1003-L1040tests/unit/token_update_transaction_test.py#L508-L543
Source: Path instructions
| def test_from_bytes(mock_account_ids, new_token_data): | ||
| """Test round-trip via _from_protobuf for TokenUpdateTransaction.""" | ||
| operator_id, _, node_account_id, token_id_1, _ = mock_account_ids | ||
| key = PrivateKey.generate().public_key() | ||
|
|
||
| tx = TokenUpdateTransaction() | ||
| tx.set_token_id(token_id_1) | ||
| tx.set_token_name("NewName") | ||
| tx.set_token_symbol("NNS") | ||
| tx.set_token_memo(new_token_data["memo"]) | ||
| tx.set_metadata(new_token_data["metadata"]) | ||
| tx.set_treasury_account_id(operator_id) | ||
| tx.set_auto_renew_account_id(operator_id) | ||
| tx.set_auto_renew_period(new_token_data["auto_renew_period"]) | ||
| tx.set_expiration_time(new_token_data["expiration_time"]) | ||
| tx.set_admin_key(key) | ||
| tx.transaction_id = TransactionId.generate(operator_id) | ||
| tx.node_account_id = node_account_id | ||
| tx.freeze() | ||
|
|
||
| reconstructed = Transaction.from_bytes(tx.to_bytes()) | ||
|
|
||
| assert isinstance(reconstructed, TokenUpdateTransaction) | ||
| assert reconstructed.token_id == token_id_1 | ||
| assert reconstructed.token_name == "NewName" | ||
| assert reconstructed.token_symbol == "NNS" | ||
| assert reconstructed.token_memo == new_token_data["memo"] | ||
| assert reconstructed.metadata == new_token_data["metadata"] | ||
| assert reconstructed.treasury_account_id == operator_id | ||
| assert reconstructed.auto_renew_account_id == operator_id | ||
| assert reconstructed.auto_renew_period.seconds == new_token_data["auto_renew_period"].seconds | ||
| assert reconstructed.expiration_time.seconds == new_token_data["expiration_time"].seconds | ||
| assert reconstructed.admin_key.to_bytes_raw() == key.to_bytes_raw() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise the non-default key verification mode.
The test uses FULL_VALIDATION, which is also the constructor default. It cannot detect removal or incorrect parsing of key_verification_mode. Set NO_VALIDATION and assert that the reconstructed value matches it.
Source: Path instructions
| def test_from_bytes(topic_id): | ||
| """Test round-trip via _from_protobuf for TopicMessageSubmitTransaction.""" | ||
| tx = TopicMessageSubmitTransaction() | ||
| tx.set_topic_id(topic_id) | ||
| tx.set_message("hello world") | ||
| tx.transaction_id = TransactionId.generate(AccountId(0, 0, 1)) | ||
| tx.node_account_id = AccountId(0, 0, 3) | ||
| tx.freeze() | ||
|
|
||
| reconstructed = Transaction.from_bytes(tx.to_bytes()) | ||
|
|
||
| assert isinstance(reconstructed, TopicMessageSubmitTransaction) | ||
| assert reconstructed.topic_id == topic_id | ||
| assert reconstructed.message == "hello world" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Add a round-trip test for a binary message.
TopicMessageSubmitTransaction.message accepts bytes | str (src/hiero_sdk_python/consensus/topic_message_submit_transaction.py Line 31). This test covers only the str path. The bytes path currently loses data, because _from_protobuf decodes with errors="replace" (Line 255 of that file). A test that sets a non-UTF-8 payload and asserts reconstructed.message re-serializes to the same bytes would catch that defect.
Do you want me to generate that test?
| assert result._total_chunks == 3 | ||
| assert result._current_chunk_index == 1 | ||
| assert result._initial_transaction_id is not None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the restored _initial_transaction_id value, not just non-None.
The test name states that it covers the initialTransactionID branch. is not None passes even if _from_protobuf reconstructs the wrong transaction ID. Compare it against initial_tx_id.
💚 Proposed fix
assert result._total_chunks == 3
assert result._current_chunk_index == 1
- assert result._initial_transaction_id is not None
+ assert result._initial_transaction_id is not None
+ assert result._initial_transaction_id._to_proto() == initial_proto_id📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert result._total_chunks == 3 | |
| assert result._current_chunk_index == 1 | |
| assert result._initial_transaction_id is not None | |
| assert result._total_chunks == 3 | |
| assert result._current_chunk_index == 1 | |
| assert result._initial_transaction_id is not None | |
| assert result._initial_transaction_id._to_proto() == initial_proto_id |
Source: Path instructions
| def test_transaction_type_map_contains_expected_keys(): | ||
| keys = set(_TRANSACTION_TYPE_MAP.keys()) | ||
| expected = { | ||
| "cryptoCreateAccount", | ||
| "cryptoTransfer", | ||
| "tokenCreation", | ||
| "consensusCreateTopic", | ||
| "consensusUpdateTopic", | ||
| "fileCreate", | ||
| "contractCreateInstance", | ||
| "util_prng", | ||
| "tokenReject", | ||
| "tokenClaimAirdrop", | ||
| "tokenCancelAirdrop", | ||
| "nodeCreate", | ||
| "nodeUpdate", | ||
| "nodeDelete", | ||
| "atomic_batch", | ||
| } | ||
| assert expected.issubset(keys) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add coverage for the corrected dispatch keys.
This subset omits freeze and the corrected snake_case keys. test_all_non_none_entries_importable() only checks entries that still exist, so removal or renaming of those mappings passes unnoticed.
Add explicit key and class assertions for freeze, token_pause, token_unpause, token_fee_schedule_update, and token_update_nfts.
Proposed test additions
expected = {
+ "freeze",
+ "token_pause",
+ "token_unpause",
+ "token_fee_schedule_update",
+ "token_update_nfts",
"cryptoCreateAccount", ("atomic_batch", "BatchTransaction"),
+ ("freeze", "FreezeTransaction"),
+ ("token_pause", "TokenPauseTransaction"),
+ ("token_unpause", "TokenUnpauseTransaction"),
+ ("token_fee_schedule_update", "TokenFeeScheduleUpdateTransaction"),
+ ("token_update_nfts", "TokenUpdateNftsTransaction"),As per path instructions, unit tests must protect breaking dispatch behavior.
Also applies to: 57-75
Source: Path instructions
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi @Mounil2005 I notice this PR remains in draft mode after one month, but we have a few refactoring PRs that will land in v0.3.0
This means you can expect complicated conflicts if the PR takes quite a while --
If you have limited time my recommendation would be to split this issue into a series of issues - for instance, maybe first work on the account, then the tokens, etc.
You can turn them around faster, they can get merged faster, and avoid messy rebases that cause more delays
4853267 to
5bdd8f5
Compare
Agree with @exploreriii on this. |
…atch map Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…etadata sentinel, schedule admin key Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…each 92% patch coverage Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…ullable fields, misleading comment Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…tion Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
… airdrop entries Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…pty keys, bool sentinels, deprecated setters Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…token_id, file_append contents Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
…on test Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
1a4aecc to
c291839
Compare
|
Hi @Mounil2005, This pull request has had no activity for 10 days. Are you still working on it?
If you're no longer working on this, please comment Reach out on discord or join our office hours if you need assistance. From the Python SDK Team |
|
Closing this PR, will make more smaller issues regarding this and solve it. |
Description:
Implements
_from_protobuffor all 44 transaction types so thatTransaction.from_bytes()correctly reconstructs each transaction type instead of returning an empty object.Related issue(s):
Fixes #2179
Notes for reviewer:
Key changes:
_from_protobufto all 44 transaction types (Account, Token, Consensus, File, Schedule, System, Contract, Node, Prng)_TRANSACTION_TYPE_MAPdispatch: corrects snake_case keys (token_pause,token_unpause,token_fee_schedule_update,token_update_nfts,util_prng), adds missingfreezeentry, addstokenClaimAirdrop, fixes wrong module path forTokenUnpauseTransactionKey.from_proto_key()for all keys (handles compositeKeyList/ThresholdKey)HasField()guards for optional message fields; scalar fields assigned directlyamount=0preserved asNonefor token burn/wipe/mint (proto3 scalar 0 is semantically "not set" for these operations)Checklist