-
Notifications
You must be signed in to change notification settings - Fork 185
feat: Added support for include_children to the TransactionRecordQuery class #1621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
4db4c75
f33daf9
dfe2039
e2e4055
f729433
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,33 @@ | ||||
| import os | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused import. The -import os📝 Committable suggestion
Suggested change
|
||||
| from hiero_sdk_python.client import Client | ||||
| from hiero_sdk_python.query.transaction_record_query import TransactionRecordQuery | ||||
|
|
||||
| def main(): | ||||
| # Setup client | ||||
| client = Client.for_testnet() | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. our for testnet mehod does not set the credentials I would recommend you use |
||||
| # ... (add operator credentials) ... | ||||
|
|
||||
| # 1. Execute a transaction that is likely to have child records | ||||
| # (e.g., a complex Smart Contract call) | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add the example, so we can run it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure how to do that, can you please co author |
||||
| print("Executing transaction...") | ||||
|
|
||||
| # 2. Query the record and explicitly request children | ||||
| tx_id = "..." # your transaction id | ||||
|
|
||||
| query = ( | ||||
| TransactionRecordQuery() | ||||
| .set_transaction_id(tx_id) | ||||
| .set_include_children(True) # The new feature! | ||||
| ) | ||||
|
|
||||
| record = query.execute(client) | ||||
|
Comment on lines
+5
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example is incomplete and will fail if copy-pasted. Per coding guidelines, examples must work verbatim for users who copy-paste them. This example has several issues:
💡 Suggested fix import os
+from dotenv import load_dotenv
from hiero_sdk_python.client import Client
from hiero_sdk_python.query.transaction_record_query import TransactionRecordQuery
+from hiero_sdk_python.transaction.transaction_id import TransactionId
+from hiero_sdk_python.account.account_id import AccountId
+from hiero_sdk_python.crypto.private_key import PrivateKey
def main():
+ load_dotenv()
+
# Setup client
client = Client.for_testnet()
- # ... (add operator credentials) ...
+ operator_id = AccountId.from_string(os.getenv("OPERATOR_ID"))
+ operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY"))
+ client.set_operator(operator_id, operator_key)
# 1. Execute a transaction that is likely to have child records
# (e.g., a complex Smart Contract call)
print("Executing transaction...")
# 2. Query the record and explicitly request children
- tx_id = "..." # your transaction id
+ # Replace with an actual transaction ID from a previous transaction
+ tx_id = TransactionId.from_string("[email protected]")
query = (
TransactionRecordQuery()
.set_transaction_id(tx_id)
.set_include_children(True) # The new feature!
) |
||||
|
|
||||
| # 3. Demonstrate accessing the children | ||||
| print(f"Parent Transaction ID: {record.transaction_id}") | ||||
| print(f"Number of child records found: {len(record.children)}") | ||||
|
|
||||
| for i, child in enumerate(record.children): | ||||
| print(f"Child {i+1} Status: {child.receipt.status}") | ||||
|
|
||||
|
Comment on lines
+23
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, find and read the file in question
fd "transaction_record_with_children.py" --type fRepository: hiero-ledger/hiero-sdk-python Length of output: 129 🏁 Script executed: # Check if the file exists and read its content
if [ -f "examples/transaction/transaction_record_with_children.py" ]; then
wc -l examples/transaction/transaction_record_with_children.py
echo "---"
cat -n examples/transaction/transaction_record_with_children.py
else
echo "File not found"
fiRepository: hiero-ledger/hiero-sdk-python Length of output: 1383 🏁 Script executed: # Verify ResponseCode exists in hiero_sdk_python
find . -type f -name "*.py" | xargs grep -l "class ResponseCode" | head -5Repository: hiero-ledger/hiero-sdk-python Length of output: 112 🏁 Script executed: # Check how ResponseCode is imported in other example files
find examples -type f -name "*.py" -exec grep -l "ResponseCode" {} \; | head -5Repository: hiero-ledger/hiero-sdk-python Length of output: 3862 🏁 Script executed: # Look for patterns of receipt.status usage in examples
rg "receipt\.status" examples --type py -B 2 -A 2 | head -40Repository: hiero-ledger/hiero-sdk-python Length of output: 3540 🏁 Script executed: # Check how ResponseCode is imported in those example files
head -20 examples/tokens/token_fee_schedule_update_transaction_fungible.py | grep -E "^(import|from)" | head -10Repository: hiero-ledger/hiero-sdk-python Length of output: 498 🏁 Script executed: # Check if there's a general pattern for error handling in examples
head -30 examples/tokens/token_create_transaction_token_fee_schedule_key.py | grep -E "^(import|from)" | head -10Repository: hiero-ledger/hiero-sdk-python Length of output: 454 🏁 Script executed: # Verify the ResponseCode import path can be used as suggested
head -5 src/hiero_sdk_python/response_code.pyRepository: hiero-ledger/hiero-sdk-python Length of output: 154 🏁 Script executed: # Check other transaction_record examples for patterns
find examples -name "*transaction_record*" -type fRepository: hiero-ledger/hiero-sdk-python Length of output: 172 🏁 Script executed: # Look at how examples handle query results without receipt checks (queries vs transactions)
rg "\.execute\(client\)" examples/query --type py -A 3 | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 988 🏁 Script executed: # Check the other transaction_record query example
cat -n examples/query/transaction_record_query.py | head -40Repository: hiero-ledger/hiero-sdk-python Length of output: 1536 🏁 Script executed: # Verify ResponseCode is used consistently in query examples
rg "ResponseCode" examples/query --type py -B 1 -A 1 | head -30Repository: hiero-ledger/hiero-sdk-python Length of output: 2088 Validate child receipt status using Line 30 prints the raw numeric status without converting to ✅ Suggested improvement from hiero_sdk_python.query.transaction_record_query import TransactionRecordQuery
+from hiero_sdk_python.response_code import ResponseCode
@@
for i, child in enumerate(record.children):
- print(f"Child {i+1} Status: {child.receipt.status}")
+ status = ResponseCode(child.receipt.status)
+ print(f"Child {i+1} Status: {status.name}")
+ if status != ResponseCode.SUCCESS:
+ print(f"Warning: Child {i+1} failed with status {status.name}") |
||||
| if __name__ == "__main__": | ||||
| main() | ||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,9 @@ | ||||||
| from typing import Optional, Any, Union | ||||||
| from hiero_sdk_python.hapi.services import query_header_pb2, transaction_get_record_pb2, query_pb2 | ||||||
| from typing import Optional, Any, Union, List | ||||||
| from hiero_sdk_python.hapi.services import ( | ||||||
| query_header_pb2, | ||||||
| transaction_get_record_pb2, | ||||||
| query_pb2, | ||||||
| ) | ||||||
| from hiero_sdk_python.query.query import Query | ||||||
| from hiero_sdk_python.response_code import ResponseCode | ||||||
| from hiero_sdk_python.transaction.transaction_id import TransactionId | ||||||
|
|
@@ -16,17 +20,22 @@ class TransactionRecordQuery(Query): | |||||
| Represents a query for a transaction record on the Hedera network. | ||||||
| """ | ||||||
|
|
||||||
| def __init__(self, transaction_id: Optional[TransactionId] = None): | ||||||
| def __init__( | ||||||
| self, | ||||||
| transaction_id: Optional[TransactionId] = None, | ||||||
| include_children: bool = False, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be |
||||||
| ): | ||||||
| """ | ||||||
| Initializes the TransactionRecordQuery with the provided transaction ID. | ||||||
| """ | ||||||
| super().__init__() | ||||||
| self.transaction_id : Optional[TransactionId] = transaction_id | ||||||
|
|
||||||
| self.transaction_id: Optional[TransactionId] = transaction_id | ||||||
| self.include_children = include_children | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing type hint
Suggested change
|
||||||
|
|
||||||
| def set_transaction_id(self, transaction_id: TransactionId): | ||||||
| """ | ||||||
| Sets the transaction ID for the query. | ||||||
|
|
||||||
| Args: | ||||||
| transaction_id (TransactionId): The ID of the transaction to query. | ||||||
| Returns: | ||||||
|
|
@@ -35,10 +44,14 @@ def set_transaction_id(self, transaction_id: TransactionId): | |||||
| self.transaction_id = transaction_id | ||||||
| return self | ||||||
|
|
||||||
| def set_include_children(self, include_children): | ||||||
| self.include_children = include_children | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor suggestion consider adding a docstring |
||||||
| return self | ||||||
|
|
||||||
| def _make_request(self): | ||||||
| """ | ||||||
| Constructs the protobuf request for the transaction record query. | ||||||
|
|
||||||
| Builds a TransactionGetRecordQuery protobuf message with the | ||||||
| appropriate header and transaction ID. | ||||||
|
|
||||||
|
|
@@ -52,18 +65,27 @@ def _make_request(self): | |||||
| """ | ||||||
| try: | ||||||
| if not self.transaction_id: | ||||||
| raise ValueError("Transaction ID must be set before making the request.") | ||||||
| raise ValueError( | ||||||
| "Transaction ID must be set before making the request." | ||||||
| ) | ||||||
|
|
||||||
| query_header = self._make_request_header() | ||||||
| transaction_get_record = transaction_get_record_pb2.TransactionGetRecordQuery() | ||||||
| transaction_get_record = ( | ||||||
| transaction_get_record_pb2.TransactionGetRecordQuery() | ||||||
| ) | ||||||
| transaction_get_record.header.CopyFrom(query_header) | ||||||
| transaction_get_record.transactionID.CopyFrom(self.transaction_id._to_proto()) | ||||||
|
|
||||||
|
|
||||||
| transaction_get_record.transactionID.CopyFrom( | ||||||
| self.transaction_id._to_proto() | ||||||
| ) | ||||||
| transaction_get_record.include_child_records = self.include_children | ||||||
| query = query_pb2.Query() | ||||||
| if not hasattr(query, 'transactionGetRecord'): | ||||||
| raise AttributeError("Query object has no attribute 'transactionGetRecord'") | ||||||
| if not hasattr(query, "transactionGetRecord"): | ||||||
| raise AttributeError( | ||||||
| "Query object has no attribute 'transactionGetRecord'" | ||||||
| ) | ||||||
| query.transactionGetRecord.CopyFrom(transaction_get_record) | ||||||
|
|
||||||
| return query | ||||||
| except Exception as e: | ||||||
| print(f"Exception in _make_request: {e}") | ||||||
|
|
@@ -72,7 +94,7 @@ def _make_request(self): | |||||
| def _get_method(self, channel: _Channel) -> _Method: | ||||||
| """ | ||||||
| Returns the appropriate gRPC method for the transaction receipt query. | ||||||
|
|
||||||
| Implements the abstract method from Query to provide the specific | ||||||
| gRPC method for getting transaction receipts. | ||||||
|
|
||||||
|
|
@@ -83,14 +105,13 @@ def _get_method(self, channel: _Channel) -> _Method: | |||||
| _Method: The method wrapper containing the query function | ||||||
| """ | ||||||
| return _Method( | ||||||
| transaction_func=None, | ||||||
| query_func=channel.crypto.getTxRecordByTxID | ||||||
| transaction_func=None, query_func=channel.crypto.getTxRecordByTxID | ||||||
| ) | ||||||
|
|
||||||
| def _should_retry(self, response: Any) -> _ExecutionState: | ||||||
| """ | ||||||
| Determines whether the query should be retried based on the response. | ||||||
|
|
||||||
| Implements the abstract method from Query to decide whether to retry | ||||||
| the query based on the response status code. First checks the header status, | ||||||
| then the receipt status. | ||||||
|
|
@@ -102,37 +123,45 @@ def _should_retry(self, response: Any) -> _ExecutionState: | |||||
| _ExecutionState: The execution state indicating what to do next | ||||||
| """ | ||||||
| status = response.transactionGetRecord.header.nodeTransactionPrecheckCode | ||||||
|
|
||||||
| retryable_statuses = { | ||||||
| ResponseCode.UNKNOWN, | ||||||
| ResponseCode.BUSY, | ||||||
| ResponseCode.RECEIPT_NOT_FOUND, | ||||||
| ResponseCode.RECORD_NOT_FOUND, | ||||||
| ResponseCode.PLATFORM_NOT_ACTIVE | ||||||
| ResponseCode.PLATFORM_NOT_ACTIVE, | ||||||
| } | ||||||
|
|
||||||
| if status == ResponseCode.OK: | ||||||
| if response.transactionGetRecord.header.responseType == query_header_pb2.ResponseType.COST_ANSWER: | ||||||
| if ( | ||||||
| response.transactionGetRecord.header.responseType | ||||||
| == query_header_pb2.ResponseType.COST_ANSWER | ||||||
| ): | ||||||
| return _ExecutionState.FINISHED | ||||||
| pass | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| elif status in retryable_statuses or status == ResponseCode.PLATFORM_TRANSACTION_NOT_CREATED: | ||||||
| elif ( | ||||||
| status in retryable_statuses | ||||||
| or status == ResponseCode.PLATFORM_TRANSACTION_NOT_CREATED | ||||||
| ): | ||||||
| return _ExecutionState.RETRY | ||||||
| else: | ||||||
| return _ExecutionState.ERROR | ||||||
|
|
||||||
| status = response.transactionGetRecord.transactionRecord.receipt.status | ||||||
|
|
||||||
| if status in retryable_statuses or status == ResponseCode.OK: | ||||||
| return _ExecutionState.RETRY | ||||||
| elif status == ResponseCode.SUCCESS: | ||||||
| return _ExecutionState.FINISHED | ||||||
| else: | ||||||
| return _ExecutionState.ERROR | ||||||
|
|
||||||
| def _map_status_error(self, response: Any) -> Union[PrecheckError,ReceiptStatusError]: | ||||||
|
|
||||||
| def _map_status_error( | ||||||
| self, response: Any | ||||||
| ) -> Union[PrecheckError, ReceiptStatusError]: | ||||||
| """ | ||||||
| Maps a response status code to an appropriate error object. | ||||||
|
|
||||||
| Implements the abstract method from Executable to create error objects | ||||||
| from response status codes. Checks both the header status and receipt status. | ||||||
|
|
||||||
|
|
@@ -150,23 +179,37 @@ def _map_status_error(self, response: Any) -> Union[PrecheckError,ReceiptStatusE | |||||
| ResponseCode.UNKNOWN, | ||||||
| ResponseCode.RECEIPT_NOT_FOUND, | ||||||
| ResponseCode.RECORD_NOT_FOUND, | ||||||
| ResponseCode.OK | ||||||
| ResponseCode.OK, | ||||||
| } | ||||||
|
|
||||||
| if status not in retryable_statuses: | ||||||
| return PrecheckError(status) | ||||||
|
|
||||||
| receipt = response.transactionGetRecord.transactionRecord.receipt | ||||||
|
|
||||||
| return ReceiptStatusError(status, self.transaction_id, TransactionReceipt._from_proto(receipt, self.transaction_id)) | ||||||
|
|
||||||
|
|
||||||
| return ReceiptStatusError( | ||||||
| status, | ||||||
| self.transaction_id, | ||||||
| TransactionReceipt._from_proto(receipt, self.transaction_id), | ||||||
| ) | ||||||
|
|
||||||
| def _map_record_list( | ||||||
| self, | ||||||
| proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse], | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. must be
Suggested change
|
||||||
| ) -> List[TransactionRecord]: | ||||||
| records: List[TransactionRecord] = [] | ||||||
| for record in proto_records: | ||||||
| records.append(TransactionRecord._from_proto(record, self.transaction_id)) | ||||||
|
|
||||||
|
Comment on lines
+196
to
+203
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, examine the file to understand the context
cat -n src/hiero_sdk_python/query/transaction_record_query.py | head -220 | tail -50Repository: hiero-ledger/hiero-sdk-python Length of output: 2151 🏁 Script executed: # Find the TransactionRecord._from_proto method to understand its signature
fd -e py -path "*/transaction_record*" | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 241 🏁 Script executed: # Search for TransactionRecord class and _from_proto method
rg -A 10 "def _from_proto" --type py -B 2 | grep -A 10 "TransactionRecord"Repository: hiero-ledger/hiero-sdk-python Length of output: 1387 🏁 Script executed: # Look at the protobuf definition for TransactionGetRecordResponse
fd -e proto | xargs rg -l "TransactionGetRecordResponse" | head -5Repository: hiero-ledger/hiero-sdk-python Length of output: 171 🏁 Script executed: # Get the full TransactionRecord._from_proto method implementation
rg -B 5 -A 30 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | head -60Repository: hiero-ledger/hiero-sdk-python Length of output: 1636 🏁 Script executed: # Check what fields are in the protobuf TransactionGetRecordResponse
fd -name "*.proto" | xargs rg "TransactionGetRecordResponse" -A 10 2>/dev/null | head -40Repository: hiero-ledger/hiero-sdk-python Length of output: 3834 🏁 Script executed: # Look for test file to understand intended behavior
cat -n tests/unit/transaction_record_query_test.py | head -100Repository: hiero-ledger/hiero-sdk-python Length of output: 4438 🏁 Script executed: # Search for how transaction_id is used in TransactionRecord class
rg "transaction_id" src/hiero_sdk_python/transaction/transaction_record.py -B 2 -A 2 | head -50Repository: hiero-ledger/hiero-sdk-python Length of output: 2216 🏁 Script executed: # Find proto files with transaction record definitions
find . -name "*.proto" -type f | xargs grep -l "TransactionGetRecordResponse\|child_transaction_records" 2>/dev/nullRepository: hiero-ledger/hiero-sdk-python Length of output: 57 🏁 Script executed: # Search for TransactionGetRecordResponse definition
find . -name "*.proto" -type f -exec grep -l "TransactionGetRecordResponse" {} \; | head -5Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Look for the actual proto definitions in generated Python code
grep -r "child_transaction_records" --include="*.py" | head -10Repository: hiero-ledger/hiero-sdk-python Length of output: 461 🏁 Script executed: # Check what type child_transaction_records actually is
rg "child_transaction_records" -B 3 -A 3 --type py | head -50Repository: hiero-ledger/hiero-sdk-python Length of output: 1789 🏁 Script executed: # Look at the full test to understand the structure
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1424 🏁 Script executed: # Read more of the test file to see if there's verification of child transaction IDs
cat tests/unit/transaction_record_query_test.py | sed -n '98,150p'Repository: hiero-ledger/hiero-sdk-python Length of output: 2312 🏁 Script executed: # Check the protobuf message structure - look at generated Python code
find . -name "*transaction_record_pb2.py" -o -name "*transaction_get_record_pb2.py" | head -3Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Look at the generated protobuf to understand child_transaction_records structure
rg "class TransactionRecord|child_transaction_records" --type py -A 2 -B 1 | grep -A 5 "child_transaction_records"Repository: hiero-ledger/hiero-sdk-python Length of output: 1097 🏁 Script executed: # Check how child records are used elsewhere in the codebase
rg "child_transaction_records\|children" src/hiero_sdk_python/query/transaction_record_query.py -B 2 -A 2Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Let's look at the full _get_query_response method to see how _map_record_list is called
cat -n src/hiero_sdk_python/query/transaction_record_query.py | sed -n '145,210p'Repository: hiero-ledger/hiero-sdk-python Length of output: 2799 🏁 Script executed: # Check what the actual type of child_transaction_records is in the protobuf
python3 << 'EOF'
import sys
sys.path.insert(0, '/root')
from hiero_sdk_python.hapi.services import transaction_get_record_pb2
# Inspect the TransactionGetRecordResponse structure
msg = transaction_get_record_pb2.TransactionGetRecordResponse()
# Check field descriptors
for field in msg.DESCRIPTOR.fields:
if 'child' in field.name.lower():
print(f"Field: {field.name}")
print(f" Type: {field.message_type.name if field.message_type else 'scalar'}")
print(f" Label: {field.label}")
EOFRepository: hiero-ledger/hiero-sdk-python Length of output: 201 🏁 Script executed: # Now let's verify if child records actually have their own transactionID
python3 << 'EOF'
import sys
sys.path.insert(0, '/root')
from hiero_sdk_python.hapi.services import transaction_record_pb2
# Check if TransactionRecord has transactionID field
msg = transaction_record_pb2.TransactionRecord()
print("TransactionRecord fields:")
for field in msg.DESCRIPTOR.fields:
if 'transaction' in field.name.lower():
print(f" {field.name}: {field.message_type.name if field.message_type else field.type}")
EOFRepository: hiero-ledger/hiero-sdk-python Length of output: 201 🏁 Script executed: # Check what happens when TransactionRecord._from_proto receives a transaction_id parameter
rg -A 50 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | head -80Repository: hiero-ledger/hiero-sdk-python Length of output: 2543 🏁 Script executed: # Look at proto files to understand the structure
find . -path "*hapi*" -name "*.proto" | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Search for child_transaction_records definition in proto
find . -path "*hapi*" -name "*.proto" -exec grep -l "child_transaction_records" {} \;Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Check generated protobuf to see field structure
grep -r "child_transaction_records" src/hiero_sdk_python/hapi --include="*.py" -B 2 -A 2 | head -30Repository: hiero-ledger/hiero-sdk-python Length of output: 131 🏁 Script executed: # Most importantly, look at what the test expects - check if child records preserve their own transaction IDs
cat tests/unit/transaction_record_query_test.py | sed -n '98,120p'Repository: hiero-ledger/hiero-sdk-python Length of output: 996 🏁 Script executed: # Check the actual implementation of TransactionRecord._from_proto to see how it handles transaction_id parameter
rg -A 100 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | grep -A 10 "transaction_id" | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 954 🏁 Script executed: # Let's check what the actual transaction_id of child records would be after mapping
# by examining if there's any assertion about transaction_id in the test
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1424 🏁 Script executed: # Check the full _from_proto implementation to understand how it handles the transaction_id parameter
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -A 200 "def _from_proto" | head -100Repository: hiero-ledger/hiero-sdk-python Length of output: 4644 🏁 Script executed: # Look for the return statement in _from_proto to see how transaction_id is assigned
rg -A 150 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | grep -B 5 "return cls"Repository: hiero-ledger/hiero-sdk-python Length of output: 275 🏁 Script executed: # Check if there's an assertion about transaction_id in the child mapping test that we might have missed
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1275 🏁 Script executed: # Look at what proto type child_transaction_records actually holds - check the protobuf structure
grep -r "child_transaction_records" . --include="*.py" | grep -v ".pyc" | grep -v "test" | head -10Repository: hiero-ledger/hiero-sdk-python Length of output: 213 🏁 Script executed: # Search for TransactionGetRecordResponse in generated protobuf to see its actual fields
find . -name "*.py" -exec grep -l "class TransactionGetRecordResponse" {} \;Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Let's check the actual behavior by looking at what happens when _from_proto is called without explicit transaction_id
rg "if proto.HasField\(\"transactionID\"\)|proto.transactionID" src/hiero_sdk_python/transaction/transaction_record.pyRepository: hiero-ledger/hiero-sdk-python Length of output: 153 🏁 Script executed: # Verify if TransactionRecord._from_proto actually extracts transaction_id from proto when not provided
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -B 5 "transaction_id=transaction_id" | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 277 🏁 Script executed: # Let's check if the test actually verifies the transaction_id of child records
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p' | grep -i "transaction_id\|assert"Repository: hiero-ledger/hiero-sdk-python Length of output: 440 🏁 Script executed: # Check if there are any assertions about transaction_id in child records
rg "assert.*transaction_id" tests/unit/transaction_record_query_test.pyRepository: hiero-ledger/hiero-sdk-python Length of output: 267 🏁 Script executed: # Look at the entire test to see all assertions
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1275 🏁 Script executed: # Check if proto.HasField works on the child records and what fields are available
rg "HasField" src/hiero_sdk_python/transaction/transaction_record.py | head -10Repository: hiero-ledger/hiero-sdk-python Length of output: 128 🏁 Script executed: # Let's check the actual type annotation issue - is it TransactionRecord or TransactionGetRecordResponse?
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '196,210p'Repository: hiero-ledger/hiero-sdk-python Length of output: 591 🏁 Script executed: # Critical check: Does TransactionRecord._from_proto actually extract transaction_id from proto when None?
cat src/hiero_sdk_python/transaction/transaction_record.py | sed -n '150,250p'Repository: hiero-ledger/hiero-sdk-python Length of output: 4742 🏁 Script executed: # Let me verify the exact test setup - what are the child_proto transaction IDs before mapping?
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1275 🏁 Script executed: # Most importantly, check what the test asserts about child transaction IDs after mapping
rg "children\[0\]\.transaction_id|assert.*transaction" tests/unit/transaction_record_query_test.py | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 527 🏁 Script executed: # Check if there are other tests that validate child record transaction IDs
rg -C 5 "child.*transaction_id\|transaction_id.*child" tests/Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Final verification: check if proto records actually have their own transactionID field
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -A 5 "def _from_proto" | head -20Repository: hiero-ledger/hiero-sdk-python Length of output: 272 🏁 Script executed: # Look for any extraction of transactionID from proto in the _from_proto method
rg "proto\.transactionID|HasField.*transactionID" src/hiero_sdk_python/transaction/transaction_record.pyRepository: hiero-ledger/hiero-sdk-python Length of output: 153 🏁 Script executed: # Check what the test file setup shows - each child has transactionID set on proto
cat tests/unit/transaction_record_query_test.py | sed -n '98,110p'Repository: hiero-ledger/hiero-sdk-python Length of output: 621 🏁 Script executed: # Confirm: the type annotation says TransactionGetRecordResponse but should be TransactionRecord
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '196,205p'Repository: hiero-ledger/hiero-sdk-python Length of output: 427 🏁 Script executed: # Let me verify the type annotation issue - what is actually in child_transaction_records?
cat tests/unit/transaction_record_query_test.py | sed -n '100,113p'Repository: hiero-ledger/hiero-sdk-python Length of output: 634 🏁 Script executed: # Search for how the Hedera SDK JavaScript or other implementations handle child records
rg -i "child.*transaction.*record\|_map_record" --type py -B 2 -A 5 | head -50Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Check the complete context around where _map_record_list is called
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '130,160p'Repository: hiero-ledger/hiero-sdk-python Length of output: 1105 🏁 Script executed: # Critical: verify the exact type by looking at the proto definitions
find . -name "*transaction_get_record_pb2.py" | xargs grep -A 10 "class TransactionGetRecordResponse" | head -30Repository: hiero-ledger/hiero-sdk-python Length of output: 55 🏁 Script executed: # Also check if there's a way to extract transactionID from the child record proto
grep -r "transactionID" src/hiero_sdk_python/transaction/transaction_record.py | head -5Repository: hiero-ledger/hiero-sdk-python Length of output: 153 🏁 Script executed: # Let's create a simple test to understand the behavior
python3 << 'EOF'
# Check if the docstring promise in _from_proto is actually implemented
import re
with open('src/hiero_sdk_python/transaction/transaction_record.py', 'r') as f:
content = f.read()
# Find the _from_proto method
match = re.search(r'def _from_proto\((.*?)\) -> "TransactionRecord":(.*?)return cls\(', content, re.DOTALL)
if match:
method_body = match.group(2)
# Check if it extracts transaction_id from proto
if 'proto.transactionID' in method_body or 'HasField("transactionID")' in method_body:
print("FOUND: extraction of transactionID from proto")
else:
print("NOT FOUND: extraction of transactionID from proto")
print("The _from_proto does NOT extract transaction_id from proto when parameter is None")
EOFRepository: hiero-ledger/hiero-sdk-python Length of output: 204 Type annotation incorrect; child records lose their own transaction IDs. 🔧 Proposed fix records: List[TransactionRecord] = []
for record in proto_records:
- records.append(TransactionRecord._from_proto(record, self.transaction_id))
+ record_tx_id = (
+ TransactionId._from_proto(record.transactionID)
+ if record.HasField("transactionID")
+ else self.transaction_id
+ )
+ records.append(TransactionRecord._from_proto(record, record_tx_id)) |
||||||
| return records | ||||||
|
Comment on lines
+196
to
+204
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect type hint for The type hint specifies This mismatch is misleading and could cause confusion for future maintainers. def _map_record_list(
self,
- proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse],
+ proto_records: List[transaction_record_pb2.TransactionRecord],
) -> List[TransactionRecord]:Also need to add the import: from hiero_sdk_python.hapi.services import (
query_header_pb2,
transaction_get_record_pb2,
query_pb2,
+ transaction_record_pb2,
) |
||||||
|
|
||||||
| def execute(self, client): | ||||||
| """ | ||||||
| Executes the transaction record query. | ||||||
|
|
||||||
| Sends the query to the Hedera network and processes the response | ||||||
| to return a TransactionRecord object. | ||||||
|
|
||||||
| This function delegates the core logic to `_execute()`, and may propagate exceptions raised by it. | ||||||
|
|
||||||
| Args: | ||||||
|
|
@@ -182,20 +225,27 @@ def execute(self, client): | |||||
| """ | ||||||
| self._before_execute(client) | ||||||
| response = self._execute(client) | ||||||
|
|
||||||
| return TransactionRecord._from_proto(response.transactionGetRecord.transactionRecord, self.transaction_id) | ||||||
| if not response.HasField("transactionGetRecord"): | ||||||
| raise AttributeError("Response does not contain 'transactionGetRecord'") | ||||||
| record_response = response.transactionGetRecord | ||||||
| children = self._map_record_list(record_response.child_transaction_records) | ||||||
| return TransactionRecord._from_proto( | ||||||
| response.transactionGetRecord.transactionRecord, | ||||||
| self.transaction_id, | ||||||
| children=children, | ||||||
| ) | ||||||
|
|
||||||
| def _get_query_response(self, response: Any): | ||||||
| """ | ||||||
| Extracts the transaction record response from the full response. | ||||||
|
|
||||||
| Implements the abstract method from Query to extract the | ||||||
| specific transaction record response object. | ||||||
|
|
||||||
| Args: | ||||||
| response: The full response from the network | ||||||
|
|
||||||
| Returns: | ||||||
| The transaction get record response object | ||||||
| """ | ||||||
| return response.transactionGetRecord | ||||||
| return response.transactionGetRecord | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding the issue reference for traceability.
Other entries in this section include issue references (e.g.,
(#1570),(#1484)). Adding the related issue number would improve traceability.📝 Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mukundkumarjha please add the issue reference in the CHANGELOG entry to keep it consistent and easy to read.