Conversation
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
This script posts a review comment on release PRs for CodeRabbit, ensuring it only runs for maintainer-authored PRs and avoids duplicate comments. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Refactor error handling and improve prompt loading in release PR script. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Remove message_data parameter from the constructor. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Removed tools configuration and added knowledge_base and web_search options. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Removed tools section and added web_search option under knowledge_base. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. Quick Fix for CHANGELOG.md ConflictsIf your conflict is only in CHANGELOG.md, you can resolve it easily using the GitHub web editor:
For all other merge conflicts, please read: Thank you for contributing! |
|
Hi @MonaaEid, this is **LinkBot** 👋
Linking pull requests to issues helps us significantly with reviewing pull requests and keeping the repository healthy. 🚨 This pull request does not have an issue linked. Please link an issue using the following format: 📖 Guide: If no issue exists yet, please create one: Thanks! |
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. WalkthroughRemoved chunking and multi-step flows from TopicMessageSubmitTransaction and simplified its topic ID typing; narrowed TopicInfo fields and proto parsing; changed TopicMessage.transaction_id to a formatted string; refactored TopicCreateTransaction to direct proto construction and synchronous execution; added TokenAssociation and expanded TransactionRecord to richer fields; updated review rules in .coderabbit.yaml. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant TxBuilder as TopicCreateTransaction
participant Node as TopicService (gRPC)
participant Ledger as Receipt/Consensus
Client->>TxBuilder: build transaction proto
TxBuilder->>Node: submit via client.topic_stub.createTopic
Node-->>TxBuilder: nodeTransactionPrecheckCode
alt precheck OK
TxBuilder->>Node: request/get receipt
Node-->>Ledger: produce receipt
Ledger-->>TxBuilder: return receipt
TxBuilder-->>Client: return receipt/result
else precheck FAIL
Node-->>TxBuilder: return error code
TxBuilder-->>Client: raise exception with code/name
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
| def __init__( | ||
| self, | ||
| topic_id: Optional[TopicId] = None, | ||
| topic_id: Optional[basic_types_pb2.TopicID] = None, | ||
| message: Optional[str] = None, | ||
| chunk_size: Optional[int] = None, | ||
| max_chunks: Optional[int] = None | ||
| ) -> None: | ||
| """ | ||
| Initializes a new TopicMessageSubmitTransaction instance. | ||
| Args: | ||
| topic_id (Optional[TopicId]): The ID of the topic. | ||
| topic_id (Optional[TopicID]): The ID of the topic. | ||
| message (Optional[str]): The message to submit. | ||
| chunk_size (Optional[int]): The maximum chunk size in bytes, Default: 1024. | ||
| max_chunks (Optional[int]): The maximum number of chunks allowed, Default: 20. | ||
| """ | ||
| super().__init__() | ||
| self.topic_id: Optional[TopicId] = topic_id | ||
| self.topic_id: Optional[basic_types_pb2.TopicID] = topic_id | ||
| self.message: Optional[str] = message | ||
| self.chunk_size: int = chunk_size or 1024 | ||
| self.max_chunks: int = max_chunks or 20 | ||
|
|
||
| self._current_index = 0 | ||
| self._total_chunks = self.get_required_chunks() | ||
| self._initial_transaction_id: Optional[TransactionId] = None | ||
| self._transaction_ids: List[TransactionId] = [] | ||
| self._signing_keys: List["PrivateKey"] = [] | ||
|
|
||
| def get_required_chunks(self) -> int: | ||
| """ | ||
| Returns the number of chunks required for the current message. | ||
|
|
||
| Returns: | ||
| int: Number of chunks required. | ||
| """ | ||
| if not self.message: | ||
| return 1 | ||
|
|
||
| content = self.message.encode("utf-8") | ||
| return math.ceil(len(content) / self.chunk_size) | ||
|
|
||
| def set_topic_id( | ||
| self, topic_id: TopicId | ||
| self, topic_id: basic_types_pb2.TopicID | ||
| ) -> "TopicMessageSubmitTransaction": | ||
| """ | ||
| Sets the topic ID for the message submission. | ||
|
|
||
| Args: | ||
| topic_id (TopicId): The ID of the topic to which the message is submitted. | ||
| topic_id (TopicID): The ID of the topic to which the message is submitted. | ||
|
|
||
| Returns: | ||
| TopicMessageSubmitTransaction: This transaction instance (for chaining). |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Docstrings should clarify the type is the protobuf basic_types_pb2.TopicID.
The docstrings at lines 34 and 48 reference TopicID without qualifying it. Since the SDK also has its own TopicId wrapper class, this is ambiguous and could mislead callers. Specify the fully-qualified protobuf type to match the type hints.
| return consensus_submit_message_pb2.ConsensusSubmitMessageTransactionBody( | ||
| topicID=self.topic_id._to_proto(), | ||
| message=chunk_content | ||
| message=bytes(self.message, "utf-8"), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's locate and examine the file in question
echo "=== Locating the file ==="
find . -name "topic_message_submit_transaction.py" -type f
echo ""
echo "=== Checking file size ==="
wc -l ./src/hiero_sdk_python/consensus/topic_message_submit_transaction.py 2>/dev/null || echo "File not found with that exact path"Repository: MonaaEid/hiero-sdk-python
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Let's search for any TopicId-related files and the actual file
echo "=== Searching for TopicId class definitions ==="
rg -l "class TopicId" --type py
echo ""
echo "=== Searching for topic_message_submit_transaction file ==="
git ls-files | grep -i "topic_message"Repository: MonaaEid/hiero-sdk-python
Length of output: 781
🏁 Script executed:
#!/bin/bash
# Read the actual file content around lines 120-123
if [ -f "src/hiero_sdk_python/consensus/topic_message_submit_transaction.py" ]; then
echo "=== Content around lines 115-130 ==="
sed -n '115,130p' "src/hiero_sdk_python/consensus/topic_message_submit_transaction.py" | cat -n
else
echo "File not found at expected path. Searching for it..."
find . -name "*topic_message*" -type f
fiRepository: MonaaEid/hiero-sdk-python
Length of output: 906
🏁 Script executed:
#!/bin/bash
# Search for _to_proto method definitions
echo "=== Searching for _to_proto method definitions in TopicId ==="
rg -A 5 "class TopicId" --type py
echo ""
echo "=== Searching for _to_proto method in consensus modules ==="
rg -B 2 -A 5 "_to_proto" "src/hiero_sdk_python/consensus/" --type py | head -100Repository: MonaaEid/hiero-sdk-python
Length of output: 10164
🏁 Script executed:
#!/bin/bash
echo "=== Full content of topic_message_submit_transaction.py ==="
head -50 "src/hiero_sdk_python/consensus/topic_message_submit_transaction.py" | cat -nRepository: MonaaEid/hiero-sdk-python
Length of output: 2278
🏁 Script executed:
#!/bin/bash
echo "=== Checking TopicId class and its _to_proto method ==="
sed -n '1,100p' "src/hiero_sdk_python/consensus/topic_id.py" | cat -nRepository: MonaaEid/hiero-sdk-python
Length of output: 4053
Type annotation mismatch: self.topic_id must be TopicId, not basic_types_pb2.TopicID.
Line 121 calls self.topic_id._to_proto(), but the type annotations (lines 28, 38) declare self.topic_id as basic_types_pb2.TopicID (a protobuf generated message). Protobuf messages do not have _to_proto() methods—this method exists only on the SDK's TopicId wrapper class. This mismatch will raise AttributeError: '_to_proto' at runtime.
Update the type annotations to use TopicId instead of basic_types_pb2.TopicID:
- Import
TopicIdfromhiero_sdk_python.consensus.topic_id - Change lines 28 and 38 from
basic_types_pb2.TopicIDtoTopicId - Update the
set_topic_idmethod signature (line 42) similarly
This aligns with other consensus transaction classes (topic_create_transaction.py, topic_update_transaction.py, topic_delete_transaction.py), which all follow the same pattern of calling ._to_proto() on self.topic_id.
| return consensus_submit_message_pb2.ConsensusSubmitMessageTransactionBody( | ||
| topicID=self.topic_id._to_proto(), | ||
| message=chunk_content | ||
| message=bytes(self.message, "utf-8"), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer str.encode() over bytes() constructor for encoding strings.
self.message.encode("utf-8") is more idiomatic Python than bytes(self.message, "utf-8").
♻️ Proposed fix
- message=bytes(self.message, "utf-8"),
+ message=self.message.encode("utf-8"),Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
| from hedera_sdk_python.hapi.basic_types_pb2 import Key, AccountID | ||
| from hedera_sdk_python.hapi.timestamp_pb2 import Timestamp | ||
| from hedera_sdk_python.hapi.duration_pb2 import Duration |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the actual imports in the file
cat -n src/hiero_sdk_python/consensus/topic_info.py | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 815
🏁 Script executed:
# Check the structure of the hapi package
fd -t f "\.py$" src/hiero_sdk_python/hapi/ | head -30Repository: MonaaEid/hiero-sdk-python
Length of output: 182
🏁 Script executed:
# Look for basic_types_pb2 and similar files
fd "basic_types_pb2|timestamp_pb2|duration_pb2" src/hiero_sdk_python/Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Check if there's a services subdirectory under hapi
find src/hiero_sdk_python/hapi -type d | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 129
🌐 Web query:
ConsensusTopicInfo protobuf message fields hedera-protobufs github
💡 Result:
ConsensusTopicInfo (in consensus_topic_info.proto in hashgraph/hedera-protobufs) has these fields (name → type → field #):
memo→string→1runningHash→bytes→2sequenceNumber→uint64→3expirationTime→Timestamp→4adminKey→Key→5submitKey→Key→6autoRenewPeriod→Duration→7autoRenewAccount→AccountID→8ledger_id→bytes→9(added via HIP-33 / network/ledger ID in info responses) [1][2]
Sources: Protocol docs for consensus_topic_info.proto [1], HIP-33 excerpt showing the message definition and field numbering [2].
Fix incorrect package name in imports.
The imports reference hedera_sdk_python.hapi but should reference hiero_sdk_python.hapi to match the file's location under src/hiero_sdk_python/. Update imports to:
from hiero_sdk_python.hapi.basic_types_pb2 import Key, AccountID
from hiero_sdk_python.hapi.timestamp_pb2 import Timestamp
from hiero_sdk_python.hapi.duration_pb2 import Duration🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] ModuleNotFoundError: No module named 'hedera_sdk_python' during import. Dependency may be missing in environment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check hapi imports in the consensus directory
echo "=== HAPI imports in consensus directory ==="
rg -n "from.*hapi.*import" src/hiero_sdk_python/consensus/
# Check imports in other SDK directories to understand the pattern
echo -e "\n=== HAPI imports in other directories ==="
rg -n "from.*hapi.*import" src/hiero_sdk_python/ | head -20
# Check if hedera_sdk_python exists anywhere in imports (it shouldn't)
echo -e "\n=== Any hedera_sdk_python imports (should be none) ==="
rg -n "from hedera_sdk_python" src/hiero_sdk_python/Repository: MonaaEid/hiero-sdk-python
Length of output: 5155
🏁 Script executed:
#!/bin/bash
# Check how timestamp.py imports and uses Timestamp (similar to topic_info.py)
echo "=== timestamp.py imports and usage ==="
head -20 src/hiero_sdk_python/timestamp.py
# Check the structure of topic_info.py to see the full context
echo -e "\n=== topic_info.py complete import section ==="
head -30 src/hiero_sdk_python/consensus/topic_info.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 1616
🏁 Script executed:
#!/bin/bash
# Verify the proto structure for ConsensusTopicInfo
echo "=== Checking proto field alignment ==="
# Look at how other files handle the type annotations - check if they use SDK wrappers or proto types
echo -e "\n=== Check topic_create_transaction.py for comparison ==="
grep -A 20 "class TopicCreateTransaction" src/hiero_sdk_python/consensus/topic_create_transaction.py | head -30
# Verify if there are SDK wrapper classes for Key, AccountID, Duration
echo -e "\n=== Check if SDK wrapper classes exist for these types ==="
fd -t f -e py "key\.py|account.*\.py|duration\.py" src/hiero_sdk_python/ | grep -v __pycache__
</parameter>Repository: MonaaEid/hiero-sdk-python
Length of output: 1888
🏁 Script executed:
#!/bin/bash
# Look at the SDK wrapper classes to understand the expected types
echo "=== SDK wrapper: Duration.py ==="
head -30 src/hiero_sdk_python/Duration.py
echo -e "\n=== SDK wrapper: account_id.py ==="
head -30 src/hiero_sdk_python/account/account_id.py
# Check what key wrapper exists
echo -e "\n=== Check for Key wrapper ==="
find src/hiero_sdk_python -name "*key*.py" -type f | grep -v __pycache__ | head -10
</parameter>Repository: MonaaEid/hiero-sdk-python
Length of output: 2395
🏁 Script executed:
#!/bin/bash
# Check what topic_create_transaction.py imports for Key, Duration, AccountId
echo "=== topic_create_transaction.py imports ==="
head -30 src/hiero_sdk_python/consensus/topic_create_transaction.py
echo -e "\n=== Check if there's a Key wrapper class ==="
rg -n "^class Key" src/hiero_sdk_python/ | head -5
echo -e "\n=== Check if PublicKey is used as Key wrapper ==="
head -50 src/hiero_sdk_python/crypto/public_key.py | grep -A 20 "^class PublicKey"Repository: MonaaEid/hiero-sdk-python
Length of output: 2068
🏁 Script executed:
#!/bin/bash
# Check the Timestamp wrapper class
echo "=== Timestamp wrapper class ==="
head -30 src/hiero_sdk_python/timestamp.py
# Verify what AccountId looks like (already shown it exists)
echo -e "\n=== Verify from_proto in topic_info.py to see what types it actually returns ==="
sed -n '45,65p' src/hiero_sdk_python/consensus/topic_info.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 2129
🏁 Script executed:
#!/bin/bash
# Verify the complete from_proto method to understand the scope
echo "=== Complete from_proto method ==="
sed -n '36,61p' src/hiero_sdk_python/consensus/topic_info.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 1247
Fix imports to use hiero_sdk_python package with correct paths and SDK wrapper types.
Lines 1–3 have multiple critical issues:
-
Wrong package name:
hedera_sdk_pythonshould behiero_sdk_python -
Missing path component: Imports omit
.services— correct pattern ishiero_sdk_python.hapi.services.<module>_pb2 -
Wrong types: Importing raw protobuf types instead of SDK wrapper classes that are used throughout the codebase:
Timestampshould come fromhiero_sdk_python.timestamp(nottimestamp_pb2)Durationshould come fromhiero_sdk_python.Duration(notduration_pb2)AccountIDshould beAccountIdfromhiero_sdk_python.account.account_id(notbasic_types_pb2)Keyshould come fromhiero_sdk_python.utils.key_utils(notbasic_types_pb2)
-
Type hints incomplete: Lines 13–21 don't mark
admin_key,submit_key,auto_renew_period,auto_renew_accountasOptional[...]even thoughfrom_proto()can set them toNone. -
Missing proto conversions: The
from_proto()method (lines 45–61) passes raw protobuf objects directly without converting to SDK wrapper types. UseTimestamp._from_proto(),Duration._from_proto(), etc.
Compare with topic_create_transaction.py which correctly imports and uses SDK wrappers.
🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] ModuleNotFoundError: No module named 'hedera_sdk_python' during import. Dependency may be missing in environment.
| def __init__( | ||
| self, | ||
| memo: str, | ||
| running_hash: bytes, | ||
| sequence_number: int, | ||
| expiration_time: Optional[Timestamp], | ||
| admin_key: Optional[Key], | ||
| submit_key: Optional[Key], | ||
| auto_renew_period: Optional[Duration], | ||
| auto_renew_account: Optional[AccountID], | ||
| ledger_id: Optional[bytes], | ||
| fee_schedule_key: Optional[PublicKey], | ||
| fee_exempt_keys: Optional[List[PublicKey]], | ||
| custom_fees: Optional[List[CustomFixedFee]], | ||
| ) -> None: | ||
| """ | ||
| Initializes a new instance of the TopicInfo class. | ||
| Args: | ||
| memo (str): The memo associated with the topic. | ||
| running_hash (bytes): The current running hash of the topic. | ||
| sequence_number (int): The sequence number of the topic. | ||
| expiration_time (Optional[Timestamp]): The expiration time of the topic. | ||
| admin_key (Optional[Key]): The admin key for the topic. | ||
| submit_key (Optional[Key]): The submit key for the topic. | ||
| auto_renew_period (Optional[Duration]): The auto-renew period for the topic. | ||
| auto_renew_account (Optional[AccountID]): The account ID for auto-renewal. | ||
| ledger_id (Optional[bytes]): The ledger ID associated with the topic. | ||
| fee_schedule_key (PublicKey): The fee schedule key for the topic. | ||
| fee_exempt_keys (List[PublicKey]): The fee exempt keys for the topic. | ||
| custom_fees (List[CustomFixedFee]): The custom fees for the topic. | ||
| """ | ||
| self.memo: str = memo | ||
| self.running_hash: bytes = running_hash | ||
| self.sequence_number: int = sequence_number | ||
| self.expiration_time: Optional[Timestamp] = expiration_time | ||
| self.admin_key: Optional[Key] = admin_key | ||
| self.submit_key: Optional[Key] = submit_key | ||
| self.auto_renew_period: Optional[Duration] = auto_renew_period | ||
| self.auto_renew_account: Optional[AccountID] = auto_renew_account | ||
| self.ledger_id: Optional[bytes] = ledger_id | ||
| self.fee_schedule_key: PublicKey = fee_schedule_key | ||
| self.fee_exempt_keys: List[PublicKey] = ( | ||
| list(fee_exempt_keys) if fee_exempt_keys is not None else [] | ||
| ) | ||
| self.custom_fees: List[CustomFixedFee] = ( | ||
| list(custom_fees) if custom_fees is not None else [] | ||
| ) | ||
| self, | ||
| memo: str, | ||
| running_hash: bytes, | ||
| sequence_number: int, | ||
| expiration_time: Timestamp, | ||
| admin_key: Key, | ||
| submit_key: Key, | ||
| auto_renew_period: Duration, | ||
| auto_renew_account: AccountID, | ||
| ledger_id: bytes, | ||
| ): | ||
| self.memo = memo | ||
| self.running_hash = running_hash | ||
| self.sequence_number = sequence_number | ||
| self.expiration_time = expiration_time | ||
| self.admin_key = admin_key | ||
| self.submit_key = submit_key | ||
| self.auto_renew_period = auto_renew_period | ||
| self.auto_renew_account = auto_renew_account | ||
| self.ledger_id = ledger_id |
There was a problem hiding this comment.
Type annotations don't allow None for optional fields.
from_proto (lines 48–55) passes None for admin_key, submit_key, auto_renew_period, and auto_renew_account when the proto field is absent, but the __init__ signature declares them as non-optional (Key, Duration, AccountID). This will mislead type checkers and callers.
🐛 Proposed fix — add Optional annotations
+from typing import Optional
+
class TopicInfo:
...
def __init__(
self,
memo: str,
running_hash: bytes,
sequence_number: int,
expiration_time: Timestamp,
- admin_key: Key,
- submit_key: Key,
- auto_renew_period: Duration,
- auto_renew_account: AccountID,
+ admin_key: Optional[Key],
+ submit_key: Optional[Key],
+ auto_renew_period: Optional[Duration],
+ auto_renew_account: Optional[AccountID],
ledger_id: bytes,
):🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] ModuleNotFoundError: No module named 'hedera_sdk_python' during import. Dependency may be missing in environment.
🪛 Ruff (0.15.1)
[warning] 10-10: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
| cls, | ||
| topic_info_proto: consensus_topic_info_pb2.ConsensusTopicInfo | ||
| ) -> "TopicInfo": | ||
| def from_proto(cls, topic_info_proto): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add return type annotations.
Static analysis (Ruff ANN204/ANN206) flags missing return types on from_proto and __repr__. Minor fix for consistency.
✏️ Proposed fix
`@classmethod`
- def from_proto(cls, topic_info_proto):
+ def from_proto(cls, topic_info_proto) -> "TopicInfo":- def __repr__(self):
+ def __repr__(self) -> str:Also applies to: 59-59
🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] ModuleNotFoundError: No module named 'hedera_sdk_python' during import. Dependency may be missing in environment.
🪛 Ruff (0.15.1)
[warning] 33-33: Missing return type annotation for classmethod from_proto
(ANN206)
| return cls( | ||
| memo=topic_info_proto.memo, | ||
| running_hash=topic_info_proto.runningHash, | ||
| sequence_number=topic_info_proto.sequenceNumber, | ||
| expiration_time=( | ||
| topic_info_proto.expirationTime | ||
| if topic_info_proto.HasField("expirationTime") else None | ||
| ), | ||
| admin_key=( | ||
| topic_info_proto.adminKey | ||
| if topic_info_proto.HasField("adminKey") else None | ||
| ), | ||
| submit_key=( | ||
| topic_info_proto.submitKey | ||
| if topic_info_proto.HasField("submitKey") else None | ||
| ), | ||
| auto_renew_period=( | ||
| Duration._from_proto(proto=topic_info_proto.autoRenewPeriod) | ||
| if topic_info_proto.HasField("autoRenewPeriod") else None | ||
| ), | ||
| auto_renew_account=( | ||
| topic_info_proto.autoRenewAccount | ||
| if topic_info_proto.HasField("autoRenewAccount") else None | ||
| ), | ||
| ledger_id=getattr(topic_info_proto, "ledger_id", None), | ||
| fee_schedule_key=( | ||
| PublicKey._from_proto(topic_info_proto.fee_schedule_key) | ||
| if topic_info_proto.HasField("fee_schedule_key") else None | ||
| ), | ||
| fee_exempt_keys=[PublicKey._from_proto(key) for key in topic_info_proto.fee_exempt_key_list], | ||
| custom_fees=[CustomFixedFee._from_proto(fee) for fee in topic_info_proto.custom_fees], | ||
| expiration_time=topic_info_proto.expirationTime, | ||
| admin_key=topic_info_proto.adminKey if topic_info_proto.HasField("adminKey") else None, | ||
| submit_key=topic_info_proto.submitKey if topic_info_proto.HasField("submitKey") else None, | ||
| auto_renew_period=topic_info_proto.autoRenewPeriod | ||
| if topic_info_proto.HasField("autoRenewPeriod") | ||
| else None, | ||
| auto_renew_account=topic_info_proto.autoRenewAccount | ||
| if topic_info_proto.HasField("autoRenewAccount") | ||
| else None, | ||
| ledger_id=topic_info_proto.ledger_id, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full file structure and imports
head -50 src/hiero_sdk_python/consensus/topic_info.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 1920
🏁 Script executed:
# Read the complete file
cat -n src/hiero_sdk_python/consensus/topic_info.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 3145
🏁 Script executed:
# Search for proto files or references to ConsensusTopicInfo
find . -name "*.proto" -type f 2>/dev/null | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🌐 Web query:
hedera-protobufs ConsensusTopicInfo expirationTime field definition
💡 Result:
In hedera-protobufs (services/consensus_topic_info.proto), the ConsensusTopicInfo field is defined as:
/** * An expiration time for this topic, in seconds since the epoch. *
* For this purpose, `epoch` SHALL be the UNIX epoch
* with 0 at `1970-01-01T00:00:00.000Z`. */
Timestamp expirationTime = 4;So, expirationTime is a Timestamp field with protobuf field number 4. [1]
Sources:
[1] (raw.githubusercontent.com)
Citations:
🌐 Web query:
ConsensusTopicInfo full proto definition adminKey submitKey autoRenewPeriod autoRenewAccount expirationTime
💡 Result:
Here is the full ConsensusTopicInfo proto (from services/consensus_topic_info.proto), including the fields you called out: adminKey, submitKey, autoRenewPeriod, autoRenewAccount, expirationTime. [1]
syntax = "proto3";
package proto;
option java_package = "com.hederahashgraph.api.proto.java";
option java_multiple_files = true;
import "basic_types.proto";
import "custom_fees.proto";
import "duration.proto";
import "timestamp.proto";
/**
* A query response describing the current state of a topic for the Hedera
* Consensus Service (HCS).
*/
message ConsensusTopicInfo {
string memo = 1;
bytes runningHash = 2;
uint64 sequenceNumber = 3;
Timestamp expirationTime = 4;
Key adminKey = 5;
Key submitKey = 6;
Duration autoRenewPeriod = 7;
AccountID autoRenewAccount = 8;
bytes ledger_id = 9;
Key fee_schedule_key = 10;
repeated Key fee_exempt_key_list = 11;
repeated FixedCustomFee custom_fees = 12;
}Source: [1]
expiration_time is not guarded with HasField, unlike the other message-type fields.
In the proto definition, expirationTime is a Timestamp message field (field 4), and in proto3 all fields are implicitly optional. Lines 48–55 correctly guard adminKey, submitKey, autoRenewPeriod, and autoRenewAccount with HasField to return None when absent. However, line 47 assigns expirationTime unconditionally, which means if the proto field was not set, it silently stores the default Timestamp(seconds=0, nanos=0) instead of None. This inconsistency with how other message-type fields are handled could mask missing data or lead to subtle bugs. Guard expirationTime the same way as the other optional message fields.
🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] ModuleNotFoundError: No module named 'hedera_sdk_python' during import. Dependency may be missing in environment.
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
| def from_proto( | ||
| cls, | ||
| response_or_responses: Union[mirror_proto.ConsensusTopicResponse, List[mirror_proto.ConsensusTopicResponse]], | ||
| chunking_enabled: bool = False | ||
| ) -> "TopicMessage": |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and check its structure
ls -la src/hiero_sdk_python/consensus/topic_message.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 165
🏁 Script executed:
# Find all callers of _from_proto and from_proto across the codebase
rg -n --type py "(_from_proto|from_proto)" --glob '!src/hiero_sdk_python/consensus/topic_message.py'Repository: MonaaEid/hiero-sdk-python
Length of output: 50381
🏁 Script executed:
# Read the actual implementation at lines 121-148
sed -n '121,148p' src/hiero_sdk_python/consensus/topic_message.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 1375
🏁 Script executed:
# Check for hapi imports in the file (per coding guidelines)
head -50 src/hiero_sdk_python/consensus/topic_message.py | grep -n "from.*hapi"Repository: MonaaEid/hiero-sdk-python
Length of output: 149
🏁 Script executed:
# Check if there are any integration tests or examples using TopicMessage.from_proto
grep -r "TopicMessage\." tests/ examples/ 2>/dev/null | grep -E "(from_proto|_from_proto)"Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Check if TopicMessage is exported in any __init__ files
find src/hiero_sdk_python -name "__init__.py" -exec grep -l "TopicMessage" {} \;Repository: MonaaEid/hiero-sdk-python
Length of output: 101
🏁 Script executed:
# Check how TopicMessage is exported in the main __init__.py
grep -A 5 -B 5 "TopicMessage" src/hiero_sdk_python/__init__.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 1309
🏁 Script executed:
# Check if there's any changelog or migration guide mentioning this rename
find . -name "CHANGELOG*" -o -name "MIGRATION*" -o -name "*.md" | xargs grep -l "TopicMessage\|from_proto" 2>/dev/null | head -5Repository: MonaaEid/hiero-sdk-python
Length of output: 276
🏁 Script executed:
# Check CHANGELOG for TopicMessage or from_proto changes
grep -A 10 -B 2 "TopicMessage\|from_proto" CHANGELOG.md | head -50Repository: MonaaEid/hiero-sdk-python
Length of output: 4128
🏁 Script executed:
# Check if TopicMessageQuery uses TopicMessage or TopicMessage.from_proto
grep -A 20 "class TopicMessageQuery" src/hiero_sdk_python/query/topic_message_query.py | grep -E "(from_proto|TopicMessage)"Repository: MonaaEid/hiero-sdk-python
Length of output: 134
🏁 Script executed:
# Verify TopicMessage is not exported by checking if it's in the consensus module's __init__.py
cat src/hiero_sdk_python/consensus/__init__.py 2>/dev/null || echo "No consensus __init__.py found"Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Check the topic_message.py file for the old _from_proto method to see if it was truly renamed or both exist
grep -n "def.*from_proto" src/hiero_sdk_python/consensus/topic_message.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 92
Make chunking_enabled parameter keyword-only to comply with FBT001/FBT002.
The boolean positional argument chunking_enabled: bool = False allows ambiguous call sites. Add * to make it keyword-only:
🛠️ Suggested fix
`@classmethod`
def from_proto(
cls,
response_or_responses: Union[mirror_proto.ConsensusTopicResponse, List[mirror_proto.ConsensusTopicResponse]],
+ *,
chunking_enabled: bool = False
) -> "TopicMessage":🧰 Tools
🪛 Ruff (0.15.1)
[warning] 123-123: Use X | Y for type annotations
Convert to X | Y
(UP007)
[error] 124-124: Boolean-typed positional argument in function definition
(FBT001)
[error] 124-124: Boolean default positional argument in function definition
(FBT002)
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Updated knowledge base settings and added schedule review instructions. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Refactor TopicCreateTransaction class by removing unused methods and comments, updating constructor parameters, and modifying transaction fee. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
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/consensus/topic_message.py (1)
1-196:⚠️ Potential issue | 🟠 MajorNo tests for
TopicMessage.of_single/of_manycovering the new string-basedtransaction_id.The transaction ID construction logic changed from a
TransactionIdobject to a hand-formatted string, yet no tests assert on the produced string format, correctness of field access, or edge cases (e.g., missingchunkInfo). Per coding guidelines, missing critical tests for proto-dependent logic is MAJOR.As per coding guidelines, "Prefer tests that assert on produced protobufs (fields set, oneofs, ids). Cover: valid submit, missing topic id, empty message… Missing critical tests is MAJOR."
| self.memo = memo or "" | ||
| self.admin_key = admin_key | ||
| self.submit_key = submit_key | ||
| self.auto_renew_period: Duration = auto_renew_period or Duration(7890000) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "auto_renew|7890000|7776000" --type py -A 2 -B 2 | head -100Repository: MonaaEid/hiero-sdk-python
Length of output: 8896
🏁 Script executed:
cat -n src/hiero_sdk_python/consensus/topic_create_transaction.py | head -50Repository: MonaaEid/hiero-sdk-python
Length of output: 2151
🏁 Script executed:
rg -n "7890000|7776000" --type py src/hiero_sdk_python/consensus/ src/hiero_sdk_python/account/ src/hiero_sdk_python/contract/Repository: MonaaEid/hiero-sdk-python
Length of output: 599
🏁 Script executed:
find . -name "*test*" -type f -name "*.py" | xargs rg -l "auto_renew.*7890000|auto_renew.*7776000|auto_renew.*period" | head -5Repository: MonaaEid/hiero-sdk-python
Length of output: 270
🏁 Script executed:
cat -n ./tests/unit/topic_create_transaction_test.py 2>/dev/null || cat -n ./tests/unit/topic_update_transaction_test.py | grep -A 5 -B 5 "7890000\|7776000\|auto_renew" | head -30Repository: MonaaEid/hiero-sdk-python
Length of output: 23963
🏁 Script executed:
find . -type f \( -name "*.proto" -o -name "*.md" -o -name "README*" \) | xargs rg -l "7776000|7890000|auto.*renew" 2>/dev/null | head -5Repository: MonaaEid/hiero-sdk-python
Length of output: 120
🏁 Script executed:
grep -n "7776000\|7890000\|auto.renew" ./CHANGELOG.md | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 310
🏁 Script executed:
rg -n "90.*day|7776000|renew.*period" --type py -B 3 -A 3 | grep -A 5 -B 5 "duration\|Duration\|default" | head -50Repository: MonaaEid/hiero-sdk-python
Length of output: 4021
🏁 Script executed:
web_search
What is the Hedera standard auto-renew period default in seconds for entities like topics and tokens?Repository: MonaaEid/hiero-sdk-python
Length of output: 162
🌐 Web query:
Hedera auto-renew period default 90 days seconds
💡 Result:
Hedera’s default auto-renew period is 90 days, which is 7,776,000 seconds. [1]
Fix auto-renew period default to match Hedera standard (7,776,000 seconds = 90 days).
The SDK currently defaults to 7,890,000 seconds (91.3 days), which diverges from Hedera's standard 90-day auto-renew period of 7,776,000 seconds. This inconsistency is replicated across topic, account, and token transaction types and contradicts the example in examples/topic_info.py (line 99), which uses the correct value. Update all occurrences:
src/hiero_sdk_python/consensus/topic_create_transaction.py(line 12)src/hiero_sdk_python/consensus/topic_update_transaction.py(line 36)src/hiero_sdk_python/account/account_create_transaction.py(line 23)src/hiero_sdk_python/account/account_update_transaction.py(line 23)src/hiero_sdk_python/tokens/token_create_transaction.py(line 32)
Reference: Hedera standard auto-renew period is 7,776,000 seconds (exactly 90 days).
| if response.nodeTransactionPrecheckCode != ResponseCode.OK: | ||
| error_code = response.nodeTransactionPrecheckCode | ||
| error_message = ResponseCode.get_name(error_code) | ||
| raise Exception(f"Error during transaction submission: {error_code} ({error_message})") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Uses deprecated ResponseCode.get_name() — will emit FutureWarning.
ResponseCode.get_name() is deprecated (scheduled for removal in v0.1.4 per the source). Replace with the recommended ResponseCode(code).name.
Also, raising bare Exception is not ideal (Ruff TRY002/TRY003). Consider a dedicated SDK exception type if one exists.
♻️ Proposed fix
if response.nodeTransactionPrecheckCode != ResponseCode.OK:
error_code = response.nodeTransactionPrecheckCode
- error_message = ResponseCode.get_name(error_code)
- raise Exception(f"Error during transaction submission: {error_code} ({error_message})")
+ error_name = ResponseCode(error_code).name
+ raise Exception(f"Error during transaction submission: {error_code} ({error_name})")🧰 Tools
🪛 Ruff (0.15.1)
[warning] 86-86: Create your own exception
(TRY002)
[warning] 86-86: Avoid specifying long messages outside the exception class
(TRY003)
| transaction_id: Optional[str] = None | ||
| if response.HasField("chunkInfo") and response.chunkInfo.HasField("initialTransactionID"): | ||
| transaction_id = TransactionId._from_proto(response.chunkInfo.initialTransactionID) | ||
| tx_id = response.chunkInfo.initialTransactionID | ||
| transaction_id = ( | ||
| f"{tx_id.shardNum}.{tx_id.realmNum}.{tx_id.accountNum}-" | ||
| f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos}" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated transaction ID formatting into a helper method.
The transaction ID string construction logic is duplicated verbatim in both of_single (lines 77–81) and of_many (lines 117–121). Extract to a @staticmethod to ensure consistency and single point of maintenance.
♻️ Proposed refactor
# Add as a static method on TopicMessage:
`@staticmethod`
def _format_transaction_id(tx_id) -> str:
"""Format a protobuf TransactionID as a canonical string."""
acct = tx_id.accountID
return (
f"{acct.shardNum}.{acct.realmNum}.{acct.accountNum}@"
f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos:09d}"
)Then replace both occurrences with:
transaction_id = cls._format_transaction_id(tx_id)Also applies to: 117-121
|
Hi there! I'm the LinkedIssueBot.
Thank you, |
Add detailed review instructions for Protobuf alignment in Python files. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
.coderabbit.yaml (1)
21-23:⚠️ Potential issue | 🟠 MajorMove
knowledge_baseto the top level; current placement is schema-invalid.Line 21 nests
knowledge_baseunderreviews, but the schema definesknowledge_baseas a top-level key. This was previously flagged and appears reintroduced.🛠️ Proposed fix
reviews: profile: "assertive" ... - knowledge_base: - web_search: - enabled: true + +knowledge_base: + web_search: + enabled: true#!/bin/bash set -euo pipefail echo "Nested knowledge_base under reviews (should be absent):" rg -n '^\s{2}knowledge_base:\s*$' .coderabbit.yaml || true echo "Top-level knowledge_base (should be present):" rg -n '^knowledge_base:\s*$' .coderabbit.yaml || true
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.coderabbit.yamlsrc/hiero_sdk_python/consensus/topic_create_transaction.py
| def build_transaction_body(self): | ||
| """ | ||
| Builds and returns the protobuf transaction body for topic creation. | ||
|
|
||
| Returns: | ||
| TransactionBody: The protobuf transaction body containing the topic creation details. | ||
|
|
||
| Raises: | ||
| ValueError: If required fields are missing. | ||
| """ | ||
| consensus_create_body = self._build_proto_body() | ||
| transaction_body = self.build_base_transaction_body() | ||
| transaction_body.consensusCreateTopic.CopyFrom(consensus_create_body) | ||
| transaction_body.consensusCreateTopic.CopyFrom(consensus_create_topic_pb2.ConsensusCreateTopicTransactionBody( | ||
| adminKey=self.admin_key._to_proto() if self.admin_key is not None else None, | ||
| submitKey=self.submit_key._to_proto() if self.submit_key is not None else None, | ||
| autoRenewPeriod=self.auto_renew_period._to_proto() if self.auto_renew_period is not None else None, | ||
| autoRenewAccount=self.auto_renew_account._to_proto() if self.auto_renew_account is not None else None, | ||
| memo=self.memo | ||
| )) | ||
|
|
||
| return transaction_body |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual SDK file to see its current state
cat -n src/hiero_sdk_python/consensus/topic_create_transaction.py | head -80Repository: MonaaEid/hiero-sdk-python
Length of output: 3747
🏁 Script executed:
# Search for HIP-991 or custom_fees references in the codebase
rg -i "hip.991|custom_fees|fee_schedule_key|fee_exempt" --type py src/hiero_sdk_python/consensus/Repository: MonaaEid/hiero-sdk-python
Length of output: 3422
🏁 Script executed:
# Check if there's a local proto file or schema reference
fd "consensus.*proto|topic.*proto" --type fRepository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Look for the proto imports at the top of the topic_create_transaction.py file
head -20 src/hiero_sdk_python/consensus/topic_create_transaction.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 896
🏁 Script executed:
# Look for the proto definition or any documentation
fd "consensus.*pb2" --type f src/hiero_sdk_python/Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Check if there's any README or documentation about HIP-991
rg -i "HIP.991|custom.*fee" README.md CHANGELOG.md docs/ 2>/dev/null | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 2605
🏁 Script executed:
# Look at the topic_update_transaction to see the full implementation pattern
cat -n src/hiero_sdk_python/consensus/topic_update_transaction.py | head -100Repository: MonaaEid/hiero-sdk-python
Length of output: 4816
🏁 Script executed:
# Search for any comment or issue about TopicCreateTransaction not supporting custom fees
rg -i "TopicCreateTransaction|consensusCreateTopic" --type py -A 3 -B 3 | head -50Repository: MonaaEid/hiero-sdk-python
Length of output: 4259
🌐 Web query:
hedera-protobufs ConsensusCreateTopicTransactionBody custom_fees fee_schedule_key proto schema
💡 Result:
In hedera-protobufs, the proto3 schema for ConsensusCreateTopicTransactionBody (HCS topic create) defines fee_schedule_key and custom_fees like this (from services/consensus_create_topic.proto):
Key fee_schedule_key = 8;- “Access control for update or delete of custom fees.” If unset at create, it cannot be set later via update; if set at create, it may be changed via update. (raw.githubusercontent.com)
repeated Key fee_exempt_key_list = 9;(related) (raw.githubusercontent.com)repeated FixedCustomFee custom_fees = 10;- Custom fixed fees assessed for each
ConsensusSubmitMessageto the topic; bounded byMAX_CUSTOM_FEE_ENTRIES_FOR_TOPICS. (raw.githubusercontent.com)
- Custom fixed fees assessed for each
Imports / types used
- The file imports
basic_types.proto(forKey,AccountID),custom_fees.proto(forFixedCustomFee), andduration.proto. (raw.githubusercontent.com)
Spec context
- HIP-991 describes adding topic-level
fee_schedule_keyandcustom_fees(and exempt keys) to consensus topics. (hips.hedera.com)
Citations:
- 1: https://raw.githubusercontent.com/hashgraph/hedera-protobufs/main/services/consensus_create_topic.proto
- 2: https://raw.githubusercontent.com/hashgraph/hedera-protobufs/main/services/consensus_create_topic.proto
- 3: https://raw.githubusercontent.com/hashgraph/hedera-protobufs/main/services/consensus_create_topic.proto
- 4: https://raw.githubusercontent.com/hashgraph/hedera-protobufs/main/services/consensus_create_topic.proto
- 5: https://hips.hedera.com/HIP/hip-991.html?utm_source=openai
BLOCKER (Protobuf schema alignment): HIP-991 fields missing from ConsensusCreateTopicTransactionBody construction.
The proto schema at https://github.com/hashgraph/hedera-protobufs/blob/main/services/consensus_create_topic.proto defines three non-deprecated fields for ConsensusCreateTopicTransactionBody:
custom_fees(field 10,repeated FixedCustomFee)fee_schedule_key(field 8,Key)fee_exempt_key_list(field 9,repeated Key)
TopicCreateTransaction does not expose setters or constructor parameters for these fields, nor does it set them in build_transaction_body(). This violates the Protobuf Schema Alignment guideline: every non-deprecated proto field must have a corresponding SDK attribute.
Compare with TopicUpdateTransaction, which correctly implements all three fields with setters and includes them in the protobuf construction. The existing unit tests also expect these fields to be populated in the protobuf message.
Add custom_fees, fee_schedule_key, and fee_exempt_keys as constructor parameters and attributes, along with corresponding setters, and populate them in build_transaction_body() using the same pattern as TopicUpdateTransaction.
Updated the TransactionRecord class to include additional attributes and improved documentation. Refactored type hints and enhanced the __repr__ method for better readability. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
This module defines the TokenAssociation class, which represents a token-account association as reported by the Hedera network. It includes methods for conversion to and from protobuf representation. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.coderabbit.yaml (2)
285-294:⚠️ Potential issue | 🟠 Major
schedule_review_instructionsis an invalid key underreviews— move to document root.The
reviewsobject in CodeRabbit's schema sets"additionalProperties": false, so custom keys likeschedule_review_instructionswill fail schema validation. While the YAML anchor mechanism still works, CodeRabbit may reject or ignore this configuration.Move the anchor definition to the document root level (outside
reviews:), where it remains accessible for aliasing inpath_instructions:🛠️ Proposed fix
language: "en-US" + +schedule_review_instructions: &schedule_review_instructions | + You are acting as a senior maintainer reviewing schedule-related code + ... + knowledge_base: web_search: enabled: true reviews: profile: "assertive" ... - # SCHEDULE REVIEW INSTRUCTIONS - schedule_review_instructions: &schedule_review_instructions | - You are acting as a senior maintainer reviewing schedule-related code - ...,
455-469:⚠️ Potential issue | 🟠 Major
contract_review_instructionsis also an invalid key underreviews.Same schema violation as
schedule_review_instructions. Move this anchor definition to the document root level alongside the schedule instructions fix.,
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.coderabbit.yamlsrc/hiero_sdk_python/tokens/token_association.pysrc/hiero_sdk_python/transaction/transaction_record.py
| from typing import Optional | ||
|
|
||
| from hiero_sdk_python.account.account_id import AccountId | ||
| from hiero_sdk_python.tokens.token_id import TokenId | ||
| from hiero_sdk_python.hapi.services.basic_types_pb2 import TokenAssociation as TokenAssociationProto | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class TokenAssociation: | ||
| """ | ||
| A lightweight immutable representation of a token ↔ account association. | ||
|
|
||
| Used in: | ||
| - TransactionRecord.automatic_token_associations (list of associations | ||
| created automatically during transaction execution) | ||
|
|
||
| Attributes: | ||
| token_id: The identifier of the token being associated. | ||
| account_id: The identifier of the account receiving the association. | ||
|
|
||
| See usage examples in: | ||
| examples/tokens/token_association.py | ||
| """ | ||
|
|
||
| token_id: Optional[TokenId] = None | ||
| """The identifier of the token being associated.""" | ||
|
|
||
| account_id: Optional[AccountId] = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, verify the file exists and read the relevant lines
echo "=== Reading token_association.py (lines 1-60) ==="
head -60 src/hiero_sdk_python/tokens/token_association.py | cat -n
echo ""
echo "=== Checking file length ==="
wc -l src/hiero_sdk_python/tokens/token_association.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 2821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check codebase style: find usage of Optional vs | None syntax
echo "=== Optional usage in tokens directory ==="
rg 'from typing import.*Optional' src/hiero_sdk_python/tokens/ | head -20
echo ""
echo "=== Union syntax (X | None) usage in tokens directory ==="
rg ': \w+ \| None' src/hiero_sdk_python/tokens/ | head -20
echo ""
echo "=== Check broader SDK codebase style ==="
echo "Optional imports in whole SDK:"
rg 'from typing import.*Optional' src/hiero_sdk_python/ --type py | wc -l
echo "Union syntax (X | None) in SDK:"
rg ': \w+ \| None' src/hiero_sdk_python/ --type py | wc -lRepository: MonaaEid/hiero-sdk-python
Length of output: 2118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the proto schema for TokenAssociation
echo "=== Checking for hedera-protobufs reference or local proto definitions ==="
# Look for the canonical proto file or references
fd -e proto -e py | xargs grep -l "TokenAssociation" 2>/dev/null | head -10
echo ""
echo "=== Checking imports in token_association.py ==="
grep -n "import" src/hiero_sdk_python/tokens/token_association.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the complete token_association.py file
echo "=== Complete token_association.py ==="
cat -n src/hiero_sdk_python/tokens/token_association.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 3924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the coding guidelines more closely for context on union syntax
# Look for Python version requirement or style guide references
echo "=== Checking for Python version requirement ==="
fd -e toml -e cfg -e txt | xargs grep -i "python.*3\." 2>/dev/null | head -10
echo ""
echo "=== Checking for any style or lint config ==="
fd -e pylintrc -e pyproject.toml -e setup.cfg | head -5
echo ""
echo "=== Check if token_id.py gives context ==="
cat -n src/hiero_sdk_python/tokens/token_id.py | head -80Repository: MonaaEid/hiero-sdk-python
Length of output: 3672
Replace Optional[...] with X | None union syntax per codebase style guidelines.
Lines 26, 50, and 53 use Optional[TokenId] and Optional[AccountId] annotations.
Per the established codebase convention for Python 3.10+, use the union syntax TokenId | None and AccountId | None instead.
Proposed fix
-from typing import Optional
@@
- token_id: Optional[TokenId] = None
+ token_id: TokenId | None = None
@@
- account_id: Optional[AccountId] = None
+ account_id: AccountId | None = None📝 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.
| from typing import Optional | |
| from hiero_sdk_python.account.account_id import AccountId | |
| from hiero_sdk_python.tokens.token_id import TokenId | |
| from hiero_sdk_python.hapi.services.basic_types_pb2 import TokenAssociation as TokenAssociationProto | |
| @dataclass(frozen=True) | |
| class TokenAssociation: | |
| """ | |
| A lightweight immutable representation of a token ↔ account association. | |
| Used in: | |
| - TransactionRecord.automatic_token_associations (list of associations | |
| created automatically during transaction execution) | |
| Attributes: | |
| token_id: The identifier of the token being associated. | |
| account_id: The identifier of the account receiving the association. | |
| See usage examples in: | |
| examples/tokens/token_association.py | |
| """ | |
| token_id: Optional[TokenId] = None | |
| """The identifier of the token being associated.""" | |
| account_id: Optional[AccountId] = None | |
| from hiero_sdk_python.account.account_id import AccountId | |
| from hiero_sdk_python.tokens.token_id import TokenId | |
| from hiero_sdk_python.hapi.services.basic_types_pb2 import TokenAssociation as TokenAssociationProto | |
| `@dataclass`(frozen=True) | |
| class TokenAssociation: | |
| """ | |
| A lightweight immutable representation of a token ↔ account association. | |
| Used in: | |
| - TransactionRecord.automatic_token_associations (list of associations | |
| created automatically during transaction execution) | |
| Attributes: | |
| token_id: The identifier of the token being associated. | |
| account_id: The identifier of the account receiving the association. | |
| See usage examples in: | |
| examples/tokens/token_association.py | |
| """ | |
| token_id: TokenId | None = None | |
| """The identifier of the token being associated.""" | |
| account_id: AccountId | None = None |
| transaction_id: TransactionId | None = None | ||
| transaction_hash: bytes | None = None | ||
| transaction_memo: str | None = None | ||
| transaction_fee: int | None = None | ||
| receipt: TransactionReceipt | None = None | ||
| call_result: ContractFunctionResult | None = None | ||
| token_transfers: defaultdict[ | ||
| TokenId, defaultdict[AccountId, int] | ||
| ] = field(default_factory=lambda: defaultdict(lambda: defaultdict(int))) | ||
| nft_transfers: defaultdict[ | ||
| TokenId, list[TokenNftTransfer] | ||
| ] = field(default_factory=lambda: defaultdict(list)) | ||
| transfers: defaultdict[AccountId, int] = field(default_factory=lambda: defaultdict(int)) | ||
| new_pending_airdrops: list[PendingAirdropRecord] = field(default_factory=list) | ||
|
|
||
| prng_number: Optional[int] = None | ||
| prng_bytes: Optional[bytes] = None | ||
| prng_number: int | None = None | ||
| prng_bytes: bytes | None = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if file exists and get its current content
if [ ! -f src/hiero_sdk_python/transaction/transaction_record.py ]; then
echo "File not found"
exit 1
fi
# Verify current SDK handling of entropy/bytes presence.
echo "=== Checking entropy/bytes handling in SDK ==="
rg -n 'transaction_hash=|prng_number=|prng_bytes=|WhichOneof\("entropy"\)|HasField\("prng_' src/hiero_sdk_python/transaction/transaction_record.py || true
echo -e "\n=== Checking _from_proto around line 248, 257-258 ==="
sed -n '245,265p' src/hiero_sdk_python/transaction/transaction_record.py
echo -e "\n=== Checking _to_proto around line 365-382 ==="
sed -n '360,390p' src/hiero_sdk_python/transaction/transaction_record.py
echo -e "\n=== Checking oneof entropy constraint in proto (field 19, 20) ==="Repository: MonaaEid/hiero-sdk-python
Length of output: 2973
🌐 Web query:
hashgraph hedera-protobufs transaction_record.proto oneof entropy
💡 Result:
In hashgraph/hedera-protobufs’s services/transaction_record.proto, TransactionRecord defines a oneof entropy for the UtilPrng transaction output, meaning exactly one of these fields is set in any given record: prng_bytes (384-bit bytes) or prng_number (32-bit int). prng_bytes is used when the PRNG transaction has no positive range, and prng_number is used when a positive range is provided. [1][2]
Where it is in the proto (field numbers):
oneof entropy { bytes prng_bytes = 19; int32 prng_number = 20; }[1]
How it maps to the request (UtilPrngTransactionBody):
range > 0⇒ record returns a 32-bit number (prng_number)range <= 0/unset ⇒ record returns 384-bit bytes (prng_bytes) [2]
Sources
services/transaction_record.proto(raw) [1]services/util_prng.proto(raw) [2]
Fix entropy oneof handling and transactionHash bytes normalization in round-trip mapping.
Lines 248, 257–258, and 380–381 currently violate protobuf presence semantics:
- Proto fields:
transactionHash(2, bytes),prng_bytes(19),prng_number(20) - Issue type: oneof violation | Bytes not normalized
_from_proto must detect which entropy branch is active using WhichOneof("entropy") instead of reading both fields. _to_proto must prevent setting both entropy fields together with a ValueError guard (consistent with the existing body oneof guard). Also normalize transactionHash to None when empty to match the bytes | None type.
Proposed fix
@@
- return cls(
+ entropy_field = proto.WhichOneof("entropy")
+
+ return cls(
transaction_id=tx_id,
- transaction_hash=proto.transactionHash,
+ transaction_hash=proto.transactionHash or None,
transaction_memo=proto.memo,
transaction_fee=proto.transactionFee,
@@
- prng_number=proto.prng_number,
- prng_bytes=proto.prng_bytes,
+ prng_number=proto.prng_number if entropy_field == "prng_number" else None,
+ prng_bytes=proto.prng_bytes if entropy_field == "prng_bytes" else None,
@@
+ # Defensive guard for protobuf oneof entropy
+ if self.prng_number is not None and self.prng_bytes is not None:
+ raise ValueError(
+ "prng_number and prng_bytes are mutually exclusive "
+ "(protobuf oneof entropy)"
+ )
+
record_proto = transaction_record_pb2.TransactionRecord(
transactionHash=self.transaction_hash,
memo=self.transaction_memo,
transactionFee=self.transaction_fee,
receipt=self.receipt._to_proto() if self.receipt else None,
contractCallResult=(
self.call_result._to_proto() if self.call_result else None
),
- prng_number=self.prng_number,
- prng_bytes=self.prng_bytes,
+ prng_number=self.prng_number if self.prng_number is not None else 0,
+ prng_bytes=self.prng_bytes if self.prng_bytes is not None else b"",
|
Hi there! I'm the LinkedIssueBot.
Thank you, |
1 participant
No description provided.