fix: address CodeRabbit review — chunk info restoration, key types, metadata sentinel, schedule admin key

Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>

Author: Mounil2005

src/hiero_sdk_python/consensus/topic_message_submit_transaction.py

@@ -240,8 +240,15 @@ def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map):
             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 = body.chunkInfo
+                transaction._total_chunks = chunk.total
+                transaction._current_chunk_index = chunk.number - 1
+                if chunk.HasField("initialTransactionID"):
+                    transaction._initial_transaction_id = TransactionId._from_proto(chunk.initialTransactionID)
+            else:
+                transaction._total_chunks = transaction.get_required_chunks()
         return transaction
 
     def sign(self, private_key: PrivateKey) -> TopicMessageSubmitTransaction:

src/hiero_sdk_python/file/file_create_transaction.py

@@ -201,6 +201,8 @@ def _from_proto(self, proto: file_create_pb2.FileCreateTransactionBody) -> FileC
         """
         self.keys = [PublicKey._from_proto(key) for key in proto.keys.keys] if proto.keys.keys else []
         self.contents = proto.contents
-        self.expiration_time = Timestamp._from_protobuf(proto.expirationTime) if proto.expirationTime else None
+        self.expiration_time = (
+            Timestamp._from_protobuf(proto.expirationTime) if proto.HasField("expirationTime") else None
+        )
         self.file_memo = proto.memo
         return self

src/hiero_sdk_python/file/file_update_transaction.py

@@ -7,7 +7,6 @@
 
 from hiero_sdk_python.channels import _Channel
 from hiero_sdk_python.crypto.key import Key
-from hiero_sdk_python.crypto.public_key import PublicKey
 from hiero_sdk_python.executable import _Method
 from hiero_sdk_python.file.file_id import FileId
 from hiero_sdk_python.hapi.services.basic_types_pb2 import KeyList as KeyListProto
@@ -40,7 +39,7 @@ class FileUpdateTransaction(Transaction):
     def __init__(
         self,
         file_id: FileId | None = None,
-        keys: list[PublicKey] | None = None,
+        keys: list[Key] | None = None,
         contents: str | bytes | None = None,
         expiration_time: Timestamp | None = None,
         file_memo: str | None = None,
@@ -50,7 +49,7 @@ def __init__(
 
         Args:
             file_id (FileId, optional): The ID of the file to update.
-            keys (Optional[list[PublicKey]], optional): The new keys that are allowed to
+            keys (Optional[list[Key]], optional): The new keys that are allowed to
             update/delete the file.
             contents (str | bytes, optional): The new contents of the file.
             Strings will be automatically encoded as UTF-8 bytes.
@@ -59,7 +58,7 @@ def __init__(
         """
         super().__init__()
         self.file_id: FileId | None = file_id
-        self.keys: list[PublicKey] | None = keys
+        self.keys: list[Key] | None = keys
         self.contents: bytes | None = self._encode_contents(contents)
         self.expiration_time: Timestamp | None = expiration_time
         self.file_memo: str | None = file_memo
@@ -95,19 +94,19 @@ def set_file_id(self, file_id: FileId | None) -> FileUpdateTransaction:
         self.file_id = file_id
         return self
 
-    def set_keys(self, keys: list[PublicKey] | None | PublicKey) -> FileUpdateTransaction:
+    def set_keys(self, keys: list[Key] | None | Key) -> FileUpdateTransaction:
         """
         Sets the new list of keys that can modify or delete the file.
 
         Args:
-            keys (list[PublicKey] | PublicKey | None): The new keys to set for the file.
-                Can be a list of PublicKey objects, a single PublicKey, or None.
+            keys (list[Key] | Key | None): The new keys to set for the file.
+                Can be a list of Key objects, a single Key, or None.
 
         Returns:
             FileUpdateTransaction: This transaction instance.
         """
         self._require_not_frozen()
-        if isinstance(keys, PublicKey):
+        if isinstance(keys, Key):
             self.keys = [keys]
         else:
             self.keys = keys

src/hiero_sdk_python/nodes/node_create_transaction.py

@@ -8,7 +8,6 @@
 from hiero_sdk_python.address_book.endpoint import Endpoint
 from hiero_sdk_python.channels import _Channel
 from hiero_sdk_python.crypto.key import Key
-from hiero_sdk_python.crypto.public_key import PublicKey
 from hiero_sdk_python.executable import _Method
 from hiero_sdk_python.hapi.services.node_create_pb2 import NodeCreateTransactionBody
 from hiero_sdk_python.hapi.services.schedulable_transaction_body_pb2 import (
@@ -30,7 +29,7 @@ class NodeCreateParams:
         service_endpoints (list[Endpoint]): The service endpoints of the node.
         gossip_ca_certificate (bytes, optional): The gossip ca certificate of the node.
         grpc_certificate_hash (bytes, optional): The grpc certificate hash of the node.
-        admin_key (PublicKey, optional): The admin key of the node.
+        admin_key (Key, optional): The admin key of the node.
         decline_reward (bool, optional): The decline reward of the node.
         grpc_web_proxy_endpoint (Endpoint, optional): The grpc web proxy endpoint of the node.
     """
@@ -41,7 +40,7 @@ class NodeCreateParams:
     service_endpoints: list[Endpoint] = field(default_factory=list)
     gossip_ca_certificate: bytes | None = None
     grpc_certificate_hash: bytes | None = None
-    admin_key: PublicKey | None = None
+    admin_key: Key | None = None
     decline_reward: bool | None = None
     grpc_web_proxy_endpoint: Endpoint | None = None
     associated_registered_nodes: list[int] = field(default_factory=list)
@@ -74,7 +73,7 @@ def __init__(self, node_create_params: NodeCreateParams | None = None):
         self.service_endpoints: list[Endpoint] = node_create_params.service_endpoints
         self.gossip_ca_certificate: bytes | None = node_create_params.gossip_ca_certificate
         self.grpc_certificate_hash: bytes | None = node_create_params.grpc_certificate_hash
-        self.admin_key: PublicKey | None = node_create_params.admin_key
+        self.admin_key: Key | None = node_create_params.admin_key
         self.decline_reward: bool | None = node_create_params.decline_reward
         self.grpc_web_proxy_endpoint: Endpoint | None = node_create_params.grpc_web_proxy_endpoint
         self.associated_registered_nodes: list[int] = node_create_params.associated_registered_nodes
@@ -169,12 +168,12 @@ def set_grpc_certificate_hash(self, grpc_certificate_hash: bytes | None) -> Node
         self.grpc_certificate_hash = grpc_certificate_hash
         return self
 
-    def set_admin_key(self, admin_key: PublicKey | None) -> NodeCreateTransaction:
+    def set_admin_key(self, admin_key: Key | None) -> NodeCreateTransaction:
         """
         Sets the admin key for this node create transaction.
 
         Args:
-            admin_key (PublicKey):
+            admin_key (Key):
                 The admin key of the node.
 
         Returns:

src/hiero_sdk_python/nodes/node_update_transaction.py

@@ -11,7 +11,6 @@
 from hiero_sdk_python.address_book.endpoint import Endpoint
 from hiero_sdk_python.channels import _Channel
 from hiero_sdk_python.crypto.key import Key
-from hiero_sdk_python.crypto.public_key import PublicKey
 from hiero_sdk_python.executable import _Method
 from hiero_sdk_python.hapi.services.node_update_pb2 import (
     AssociatedRegisteredNodeList,
@@ -37,7 +36,7 @@ class NodeUpdateParams:
         service_endpoints (list[Endpoint]): The service endpoints of the node.
         gossip_ca_certificate (bytes, None): The gossip ca certificate of the node.
         grpc_certificate_hash (bytes, None): The grpc certificate hash of the node.
-        admin_key (PublicKey, optional): The admin key of the node.
+        admin_key (Key, optional): The admin key of the node.
         decline_reward (bool, optional): The decline reward of the node.
         grpc_web_proxy_endpoint (Endpoint, optional): The grpc web proxy endpoint of the node.
     """
@@ -49,7 +48,7 @@ class NodeUpdateParams:
     service_endpoints: list[Endpoint] = field(default_factory=list)
     gossip_ca_certificate: bytes | None = None
     grpc_certificate_hash: bytes | None = None
-    admin_key: PublicKey | None = None
+    admin_key: Key | None = None
     decline_reward: bool | None = None
     grpc_web_proxy_endpoint: Endpoint | None = None
     associated_registered_nodes: list[int] | None = None
@@ -83,7 +82,7 @@ def __init__(self, node_update_params: NodeUpdateParams | None = None):
         self.service_endpoints: list[Endpoint] = node_update_params.service_endpoints
         self.gossip_ca_certificate: bytes | None = node_update_params.gossip_ca_certificate
         self.grpc_certificate_hash: bytes | None = node_update_params.grpc_certificate_hash
-        self.admin_key: PublicKey | None = node_update_params.admin_key
+        self.admin_key: Key | None = node_update_params.admin_key
         self.decline_reward: bool | None = node_update_params.decline_reward
         self.grpc_web_proxy_endpoint: Endpoint | None = node_update_params.grpc_web_proxy_endpoint
         self.associated_registered_nodes: list[int] | None = node_update_params.associated_registered_nodes
@@ -193,12 +192,12 @@ def set_grpc_certificate_hash(self, grpc_certificate_hash: bytes | None) -> Node
         self.grpc_certificate_hash = grpc_certificate_hash
         return self
 
-    def set_admin_key(self, admin_key: PublicKey | None) -> NodeUpdateTransaction:
+    def set_admin_key(self, admin_key: Key | None) -> NodeUpdateTransaction:
         """
         Sets the admin key for this node update transaction.
 
         Args:
-            admin_key (PublicKey):
+            admin_key (Key):
                 The admin key of the node.
 
         Returns:

src/hiero_sdk_python/schedule/schedule_create_transaction.py

@@ -6,6 +6,7 @@
 
 from hiero_sdk_python.account.account_id import AccountId
 from hiero_sdk_python.channels import _Channel
+from hiero_sdk_python.crypto.key import Key
 from hiero_sdk_python.crypto.public_key import PublicKey
 from hiero_sdk_python.executable import _Method
 from hiero_sdk_python.hapi.services.schedulable_transaction_body_pb2 import (
@@ -238,7 +239,7 @@ def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map):
             if body.HasField("payerAccountID"):
                 transaction.payer_account_id = AccountId._from_proto(body.payerAccountID)
             if body.HasField("adminKey"):
-                transaction.admin_key = PublicKey._from_proto(body.adminKey)
+                transaction.admin_key = Key.from_proto_key(body.adminKey)
             if body.HasField("scheduledTransactionBody"):
                 transaction.schedulable_body = body.scheduledTransactionBody
             transaction.schedule_memo = body.memo

src/hiero_sdk_python/tokens/token_mint_transaction.py

@@ -145,5 +145,5 @@ def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map):
             if body.HasField("token"):
                 transaction.token_id = TokenId._from_proto(body.token)
             transaction.amount = body.amount if body.amount else None
-            transaction.metadata = list(body.metadata)
+            transaction.metadata = list(body.metadata) if body.metadata else None
         return transaction

tests/unit/account_allowance_approve_transaction_test.py

@@ -16,6 +16,8 @@
 from hiero_sdk_python.hbar import Hbar
 from hiero_sdk_python.tokens.nft_id import NftId
 from hiero_sdk_python.tokens.token_id import TokenId
+from hiero_sdk_python.transaction.transaction import Transaction
+from hiero_sdk_python.transaction.transaction_id import TransactionId
 
 
 pytestmark = pytest.mark.unit
@@ -404,3 +406,38 @@ def test_zero_amount_allowances(account_allowance_transaction, sample_accounts,
     assert len(account_allowance_transaction.token_allowances) == 1
     assert account_allowance_transaction.hbar_allowances[0].amount == 0
     assert account_allowance_transaction.token_allowances[0].amount == 0
+
+
+def test_from_bytes(mock_account_ids):
+    """Test round-trip via _from_protobuf for AccountAllowanceApproveTransaction."""
+    operator_id, _, node_account_id, _, _ = mock_account_ids
+    owner = AccountId(0, 0, 200)
+    spender = AccountId(0, 0, 300)
+    token_id = TokenId(0, 0, 500)
+    nft_id = NftId(token_id, 1)
+
+    tx = AccountAllowanceApproveTransaction()
+    tx.approve_hbar_allowance(owner, spender, Hbar(10))
+    tx.approve_token_allowance(token_id, owner, spender, 100)
+    tx.approve_token_nft_allowance(nft_id, owner, spender)
+    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, AccountAllowanceApproveTransaction)
+    assert len(reconstructed.hbar_allowances) == 1
+    assert reconstructed.hbar_allowances[0].owner_account_id == owner
+    assert reconstructed.hbar_allowances[0].spender_account_id == spender
+    assert reconstructed.hbar_allowances[0].amount == Hbar(10).to_tinybars()
+    assert len(reconstructed.token_allowances) == 1
+    assert reconstructed.token_allowances[0].token_id == token_id
+    assert reconstructed.token_allowances[0].owner_account_id == owner
+    assert reconstructed.token_allowances[0].spender_account_id == spender
+    assert reconstructed.token_allowances[0].amount == 100
+    assert len(reconstructed.nft_allowances) == 1
+    assert reconstructed.nft_allowances[0].token_id == token_id
+    assert reconstructed.nft_allowances[0].owner_account_id == owner
+    assert reconstructed.nft_allowances[0].spender_account_id == spender
+    assert nft_id.serial_number in reconstructed.nft_allowances[0].serial_numbers

tests/unit/account_allowance_delete_transaction_test.py

@@ -17,6 +17,8 @@
 from hiero_sdk_python.tokens.nft_id import NftId
 from hiero_sdk_python.tokens.token_id import TokenId
 from hiero_sdk_python.tokens.token_nft_allowance import TokenNftAllowance
+from hiero_sdk_python.transaction.transaction import Transaction
+from hiero_sdk_python.transaction.transaction_id import TransactionId
 
 
 pytestmark = pytest.mark.unit
@@ -259,3 +261,25 @@ def test_empty_nft_wipe_list(account_allowance_delete_transaction):
 
     proto_body = account_allowance_delete_transaction._build_proto_body()
     assert len(proto_body.nftAllowances) == 0
+
+
+def test_from_bytes(mock_account_ids):
+    """Test round-trip via _from_protobuf for AccountAllowanceDeleteTransaction."""
+    operator_id, _, node_account_id, _, _ = mock_account_ids
+    owner = AccountId(0, 0, 200)
+    token_id = TokenId(0, 0, 100)
+    nft_id = NftId(token_id, 1)
+
+    tx = AccountAllowanceDeleteTransaction()
+    tx.delete_all_token_nft_allowances(nft_id, owner)
+    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, AccountAllowanceDeleteTransaction)
+    assert len(reconstructed.nft_wipe) == 1
+    assert reconstructed.nft_wipe[0].token_id == token_id
+    assert reconstructed.nft_wipe[0].owner_account_id == owner
+    assert reconstructed.nft_wipe[0].serial_numbers == [1]

tests/unit/account_create_transaction_test.py

@@ -25,6 +25,7 @@
     TransactionResponse as TransactionResponseProto,
 )
 from hiero_sdk_python.response_code import ResponseCode
+from hiero_sdk_python.transaction.transaction import Transaction
 from hiero_sdk_python.transaction.transaction_id import TransactionId
 from tests.unit.mock_server import mock_hedera_servers
 
@@ -539,3 +540,40 @@ def test_set_stake_account_id_reset_stake_node_id():
     tx.set_staked_account_id(AccountId(0, 0, 1))
     assert tx.staked_account_id == AccountId(0, 0, 1)
     assert tx.staked_node_id is None
+
+
+def test_from_bytes(mock_account_ids):
+    """Test round-trip via _from_protobuf for AccountCreateTransaction."""
+    operator_id, node_account_id = mock_account_ids
+
+    from hiero_sdk_python.hbar import Hbar
+
+    tx = AccountCreateTransaction()
+    tx.set_initial_balance(Hbar(5).to_tinybars())
+    tx.set_receiver_signature_required(True)
+    tx.transaction_id = generate_transaction_id(operator_id)
+    tx.node_account_id = node_account_id
+    tx.freeze()
+
+    reconstructed = Transaction.from_bytes(tx.to_bytes())
+
+    assert isinstance(reconstructed, AccountCreateTransaction)
+    assert reconstructed.initial_balance == Hbar(5).to_tinybars()
+    assert reconstructed.receiver_signature_required is True
+
+
+def test_from_bytes_without_auto_renew_period(mock_account_ids):
+    """When auto_renew_period is None, round-trip must preserve None (not restore the default)."""
+    operator_id, node_account_id = mock_account_ids
+
+    tx = AccountCreateTransaction(auto_renew_period=None)
+    tx.set_initial_balance(1000)
+    tx.transaction_id = generate_transaction_id(operator_id)
+    tx.node_account_id = node_account_id
+    tx.freeze()
+
+    reconstructed = Transaction.from_bytes(tx.to_bytes())
+
+    assert isinstance(reconstructed, AccountCreateTransaction)
+    assert reconstructed.initial_balance == 1000
+    assert reconstructed.auto_renew_period is None