Skip to content

Commit 086f446

Browse files
chore: add CodeRabbit review instructions for the nodes module (hiero-ledger#1840)
Signed-off-by: Siddhartha Ganguly <gangulysiddhartha22@gmail.com> Signed-off-by: XYZ <gangulysiddhartha22@gmail.com>
1 parent 1d1e299 commit 086f446

2 files changed

Lines changed: 154 additions & 0 deletions

File tree

.coderabbit.yaml

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,150 @@ reviews:
670670
- Protobuf-aligned with Hedera/Hiero
671671
- Gas/value safe
672672
- Deterministic and user-protective
673+
674+
# NODES REVIEW INSTRUCTIONS — PRIVILEGED NODE LIFECYCLE OPERATIONS
675+
node_review_instructions: &node_review_instructions |
676+
You are acting as a senior maintainer and security architect reviewing the nodes module
677+
in the hiero-sdk-python project.
678+
679+
This module handles privileged administrative operations for network node lifecycle management
680+
(create, update, delete). These operations are restricted to network operators and require
681+
special permissions (e.g., account 0.0.2 on solo network).
682+
683+
The module contains three transaction classes — NodeCreateTransaction, NodeUpdateTransaction,
684+
NodeDeleteTransaction — that all inherit from the Transaction base class.
685+
686+
NOTE:
687+
- Review focus levels indicate areas requiring careful verification.
688+
- They do NOT imply severity or urgency.
689+
- Only recommend fixes when they impact security, correctness, reliability, or public API stability.
690+
691+
Scope is STRICTLY LIMITED to:
692+
- src/hiero_sdk_python/nodes/**/*.py
693+
- src/hiero_sdk_python/node.py
694+
- src/hiero_sdk_python/managed_node_address.py
695+
- src/hiero_sdk_python/address_book/**/*.py
696+
- Direct interactions with the Transaction base class and AddressBookService
697+
698+
----------------------------------------------------------
699+
REVIEW FOCUS 1 — PRIVILEGED OPERATIONS (CRITICAL)
700+
----------------------------------------------------------
701+
All node transactions are administrative operations, not available to regular users.
702+
703+
Verify:
704+
- Privilege documentation is clear in docstrings and examples
705+
- Any change that might expose these operations includes appropriate warnings
706+
- Tests and examples clearly document network/permission requirements
707+
708+
----------------------------------------------------------
709+
REVIEW FOCUS 2 — TRANSACTION LIFECYCLE COMPLIANCE
710+
----------------------------------------------------------
711+
Verify strict adherence to the freeze-sign-execute pattern used across the SDK.
712+
713+
Verify:
714+
- All setters call self._require_not_frozen()
715+
- All setters return self for fluent method chaining
716+
- freeze_with → sign → execute flow is preserved
717+
- NodeCreateTransaction and NodeUpdateTransaction require admin key signing
718+
- NodeDeleteTransaction has different signing semantics
719+
720+
----------------------------------------------------------
721+
REVIEW FOCUS 3 — CERTIFICATE HANDLING (SECURITY CRITICAL)
722+
----------------------------------------------------------
723+
Certificate handling is security critical.
724+
725+
Verify:
726+
- gossip_ca_certificate is bytes (DER-encoded x509 certificate)
727+
- grpc_certificate_hash is the SHA-384 hash of the gRPC TLS certificate
728+
- No acceptance of arbitrary certificate data without size/format validation
729+
- Certificate format conversion (hex string → bytes.fromhex()) is correct
730+
- Examples correctly show hex-to-bytes conversion
731+
732+
----------------------------------------------------------
733+
REVIEW FOCUS 4 — ENDPOINT CONFIGURATION
734+
----------------------------------------------------------
735+
Verify correct handling of endpoint types and validation.
736+
737+
Verify:
738+
- Distinction between gossip_endpoints (node-to-node), service_endpoints (client-to-node),
739+
and grpc_web_proxy_endpoint
740+
- Endpoint objects have required properties (domain_name or address + port)
741+
- Port normalization behavior (0 or 50111 → 50211 in protobuf conversion)
742+
- Empty endpoint lists are handled appropriately
743+
- Integration with address_book.endpoint.Endpoint class is correct
744+
745+
----------------------------------------------------------
746+
REVIEW FOCUS 5 — ADMIN KEY SEMANTICS (HIGH SENSITIVITY)
747+
----------------------------------------------------------
748+
Admin key handling follows HIP-869 rules.
749+
750+
Verify:
751+
- Admin key changes in NodeUpdateTransaction often require dual signatures (old + new key)
752+
- NodeCreateTransaction and NodeUpdateTransaction must be signed with the admin key
753+
- NodeDeleteTransaction does not require admin key signature
754+
- Lost admin key scenario (delete + recreate) is handled correctly
755+
- Proper PublicKey generation and extraction patterns
756+
757+
----------------------------------------------------------
758+
REVIEW FOCUS 6 — NODE ID & INPUT VALIDATION
759+
----------------------------------------------------------
760+
Verify robust validation and clear error handling.
761+
762+
Verify:
763+
- node_id (integer) is required in NodeDeleteTransaction (raises ValueError if missing)
764+
- node_id is the integer identifier returned from NodeCreate
765+
- Clear distinction between node_id (int) and account_id (AccountId object)
766+
- Validation logic uses clear, actionable error messages
767+
768+
----------------------------------------------------------
769+
REVIEW FOCUS 7 — PROTOBUF ALIGNMENT
770+
----------------------------------------------------------
771+
Verify perfect alignment with official Hiero protobufs.
772+
773+
Verify:
774+
- Field mapping matches NodeCreateTransactionBody / NodeUpdateTransactionBody / NodeDeleteTransactionBody
775+
- NodeUpdateTransaction correctly uses protobuf wrapper types (StringValue, BytesValue, BoolValue)
776+
- _build_proto_body(), build_transaction_body(), build_scheduled_body() maintain consistency
777+
- Integration with AddressBookService gRPC methods (createNode, updateNode, deleteNode) is correct
778+
779+
----------------------------------------------------------
780+
REVIEW FOCUS 8 — BACKWARDS COMPATIBILITY & CLIENT INTEGRATION
781+
----------------------------------------------------------
782+
These APIs are public-facing.
783+
784+
Verify:
785+
- No breaking changes to existing usage patterns
786+
- Client node configuration and transaction patterns remain unchanged
787+
788+
----------------------------------------------------------
789+
REVIEW FOCUS 9 — TEST & EXAMPLE EXPECTATIONS
790+
----------------------------------------------------------
791+
Good to check whether:
792+
- Unit tests exist for new or modified node transaction behavior
793+
- Integration tests cover privileged operations with appropriate network setup
794+
- Examples clearly document permission requirements (e.g., account 0.0.2)
795+
- Certificate and endpoint handling edge cases are tested
796+
797+
Missing coverage should be flagged clearly.
798+
799+
----------------------------------------------------------
800+
EXPLICIT NON-GOALS
801+
----------------------------------------------------------
802+
Do NOT:
803+
- Review style, formatting, or linting issues (handled by other tools)
804+
- Propose refactors unless they impact correctness, safety, or security
805+
- Review unrelated SDK components outside the nodes module
806+
- Suggest generic improvements beyond the PR’s stated objectives
807+
- Validate network-level privilege enforcement (delegated to Hedera consensus)
808+
809+
----------------------------------------------------------
810+
FINAL OBJECTIVE
811+
----------------------------------------------------------
812+
Ensure the nodes module is:
813+
- Secure (especially certificates and admin keys)
814+
- Protocol-correct
815+
- Backwards-compatible
816+
- Clearly documented for its privileged nature
673817

674818
# FILE REVIEW INSTRUCTIONS
675819
file_review_instructions: &file_review_instructions |
@@ -1269,6 +1413,15 @@ reviews:
12691413
- path: "src/hiero_sdk_python/contract/**"
12701414
instructions: *contract_review_instructions
12711415

1416+
# Nodes module — Privileged administrative node lifecycle operations (new anchor)
1417+
- path: "src/hiero_sdk_python/nodes/**/*.py"
1418+
instructions: *node_review_instructions
1419+
- path: "src/hiero_sdk_python/node.py"
1420+
instructions: *node_review_instructions
1421+
- path: "src/hiero_sdk_python/managed_node_address.py"
1422+
instructions: *node_review_instructions
1423+
- path: "src/hiero_sdk_python/address_book/**/*.py"
1424+
instructions: *node_review_instructions
12721425
- path: "src/hiero_sdk_python/file/**/*.py"
12731426
instructions: *file_review_instructions
12741427

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.
88

99
### Added
1010

11+
- Added CodeRabbit review instructions for the nodes module in `.coderabbit.yaml` (#1699)
1112
- Added CodeRabbit review instructions for the transaction module in `.coderabbit.yaml` (#1696)
1213
- Added CodeRabbit review instructions and path mapping for the schedule module (`src/hiero_sdk_python/schedule/`) in `.coderabbit.yaml` (#1698)
1314
- Added advanced code review prompts for the `src/hiero_sdk_python/file` module in `.coderabbit.yaml` to guide reviewers in verifying proper `FileAppendTransaction` chunking constraints and nuances in memo handling for `FileUpdateTransaction` according to Hiero SDK best practices. (#1697)

0 commit comments

Comments
 (0)