Skip to content

Commit 37e0f37

Browse files
chore: add CodeRabbit review instructions and path mapping for schedule module (hiero-ledger#1816)
Signed-off-by: Siddhartha Ganguly <gangulysiddhartha22@gmail.com>
1 parent 84947b6 commit 37e0f37

File tree

2 files changed

+177
-0
lines changed

2 files changed

+177
-0
lines changed

.coderabbit.yaml

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,176 @@ reviews:
278278
- Execution-consistent
279279
- Strictly aligned with Hedera query semantics
280280

281+
# SCHEDULE REVIEW INSTRUCTIONS
282+
schedule_review_instructions: &schedule_review_instructions |
283+
You are acting as a senior maintainer reviewing schedule-related code
284+
in the hiero-sdk-python project.
285+
286+
This includes:
287+
- Schedule transaction classes (ScheduleCreateTransaction, ScheduleSignTransaction, ScheduleDeleteTransaction)
288+
- Schedule query (ScheduleInfoQuery)
289+
- Schedule data models (ScheduleId, ScheduleInfo)
290+
- ScheduleCreateParams dataclass
291+
292+
NOTE:
293+
- Review focus levels indicate areas requiring careful verification.
294+
- They do NOT imply severity or urgency.
295+
- HIGH SENSITIVITY items require elevated scrutiny — even small changes here can break user code or introduce subtle regressions.
296+
- Only recommend fixes when behavior, safety, API stability, or spec compliance is impacted.
297+
298+
Scope is STRICTLY LIMITED to:
299+
- Changes under src/hiero_sdk_python/schedule/
300+
- Their interaction with shared SDK base classes (Transaction, Query)
301+
302+
----------------------------------------------------------
303+
REVIEW FOCUS 1 — API STABILITY & BACKWARDS COMPATIBILITY
304+
(HIGH SENSITIVITY)
305+
----------------------------------------------------------
306+
Public API contracts for ScheduleId, ScheduleInfo, and all schedule transaction classes are user-facing and considered stable.
307+
308+
The following MUST remain unchanged unless explicitly justified with migration path:
309+
- Setter method signatures (set_*, build_*) and fluent chaining behavior
310+
- Constructor signatures and defaults
311+
- Public property/attribute names
312+
- Exception types and messages for invalid states
313+
314+
ScheduleCreateParams dataclass field changes (adding/removing/renaming):
315+
- Require deprecation warnings if removing or changing behavior
316+
- Must preserve existing initialization patterns
317+
318+
Flag any change that alters public method signatures, return types, defaults, or exception behavior.
319+
320+
----------------------------------------------------------
321+
REVIEW FOCUS 2 — TRANSACTION BASE CLASS CONTRACT
322+
----------------------------------------------------------
323+
All schedule transaction classes MUST extend Transaction and follow its contract:
324+
325+
Required implementations:
326+
- _build_proto_body()
327+
- build_transaction_body()
328+
- build_scheduled_body() — MUST raise ValueError for all schedule tx types
329+
- _get_method(channel) — correct service stub (createSchedule / signSchedule / deleteSchedule)
330+
331+
All setter methods MUST:
332+
- Call self._require_not_frozen() before any mutation
333+
- Return self for fluent method chaining
334+
335+
Default transaction fee MUST be Hbar(5).to_tinybars()
336+
337+
Subclasses MUST NOT:
338+
- Override execution/retry/backoff logic
339+
- Bypass lifecycle hooks
340+
- Manage gRPC deadlines manually
341+
342+
Flag deviations only if they change observable behavior.
343+
344+
----------------------------------------------------------
345+
REVIEW FOCUS 3 — ANTI-NESTING CONSTRAINT (CRITICAL)
346+
----------------------------------------------------------
347+
The anti-nesting rule is a fundamental safety constraint:
348+
349+
- build_scheduled_body() MUST raise ValueError("Cannot schedule a Schedule...Transaction")
350+
in ScheduleCreateTransaction, ScheduleDeleteTransaction, and ScheduleSignTransaction
351+
- This prevents recursive scheduling — schedule transactions cannot themselves be scheduled
352+
353+
Any change that:
354+
- Removes or weakens this raise
355+
- Implements a working build_scheduled_body() for schedule tx types
356+
is a critical defect — flag immediately and strongly.
357+
358+
Unit tests MUST verify the anti-nesting constraint raises ValueError for all three schedule transaction types.
359+
360+
----------------------------------------------------------
361+
REVIEW FOCUS 4 — SIGNATURE SEMANTICS (HIGH SENSITIVITY)
362+
----------------------------------------------------------
363+
Signature roles MUST be clearly distinguished and preserved:
364+
365+
- Payer signature: Only pays schedule creation fees; does NOT count toward execution
366+
unless the payer key is also required by the inner transaction
367+
- Admin key signature: Only required for delete/modify operations; NOT required for execution
368+
- Inner transaction signatures: Collected via ScheduleSignTransaction; trigger execution when threshold met
369+
370+
ScheduleInfo.signers MUST contain only inner transaction signers (not payer or admin unless they overlap)
371+
372+
Documentation and comments MUST clearly distinguish these roles — flag any confusion or incorrect statements.
373+
374+
----------------------------------------------------------
375+
REVIEW FOCUS 5 — SCHEDULE EXECUTION SEMANTICS
376+
----------------------------------------------------------
377+
Execution timing rules MUST be preserved:
378+
379+
- wait_for_expiry=True: Forces execution to wait until expiration_time, even if all signatures collected early
380+
- wait_for_expiry=False or None: Executes immediately once all required signatures received
381+
382+
Key points:
383+
- expiration_time has network-enforced constraints (client should validate future-dated but not rely solely on it)
384+
- Edge cases to check: expired schedules, signatures added after execution attempt, timing conflicts
385+
386+
Flag any change that alters wait_for_expiry propagation or execution triggering logic.
387+
388+
----------------------------------------------------------
389+
REVIEW FOCUS 6 — PROTOBUF ALIGNMENT
390+
----------------------------------------------------------
391+
Serialization/deserialization MUST be faithful to Hedera protobuf definitions.
392+
393+
All relevant classes MUST implement:
394+
- _to_proto()
395+
- _from_proto() (or equivalent classmethod)
396+
397+
Verify:
398+
- Field mapping exact (wait_for_expiry, adminKey, payerAccountID, scheduledTransactionBody, etc.)
399+
- Null-safe conversions using _from_proto_field() / _convert_to_proto() helpers
400+
- Underscore prefix convention for serialization methods preserved
401+
- Round-trip integrity (create → info query → fields match)
402+
403+
Flag any field loss, incorrect defaults, or divergence from protobuf spec.
404+
405+
----------------------------------------------------------
406+
REVIEW FOCUS 7 — VALIDATION & ERROR BEHAVIOR
407+
----------------------------------------------------------
408+
Validation MUST be early, deterministic, and user-friendly:
409+
410+
- Required fields raise descriptive ValueError (e.g. "Missing required ScheduleID")
411+
- ScheduleId.from_string() validates format shard.realm.schedule (exactly three integers)
412+
- Checksum validation uses shared entity_id_helper utilities
413+
- Error messages include context about what was invalid
414+
415+
Avoid:
416+
- Deferred failures
417+
- Generic messages
418+
- Silent acceptance of invalid state
419+
420+
----------------------------------------------------------
421+
REVIEW FOCUS 8 — TEST & EXAMPLE EXPECTATIONS
422+
----------------------------------------------------------
423+
Expected coverage:
424+
- Unit tests for all public methods, constructors, protobuf conversions, and edge cases
425+
- Integration tests for network interactions, execution triggering, and error scenarios
426+
- Tests MUST verify anti-nesting constraint raises ValueError for all schedule tx types
427+
- Examples MUST demonstrate proper multi-signature workflows
428+
- Examples MUST document signature semantics (payer vs admin vs inner txn roles)
429+
430+
Missing critical coverage (especially anti-nesting, wait_for_expiry paths) should be flagged.
431+
432+
----------------------------------------------------------
433+
EXPLICIT NON-GOALS
434+
----------------------------------------------------------
435+
Do NOT:
436+
- Flag style, lint, formatting, or cosmetic changes
437+
- Propose refactors unless correctness/safety/API stability impacted
438+
- Suggest unrelated features or broad cleanups
439+
440+
----------------------------------------------------------
441+
FINAL OBJECTIVE
442+
----------------------------------------------------------
443+
Ensure schedule code is:
444+
- Backward-compatible
445+
- Anti-nesting & execution semantics correct
446+
- Signature roles clearly separated
447+
- Protobuf-aligned
448+
- Validation-safe
449+
- Deterministic and protective of users
450+
281451
# CONTRACT REVIEW INSTRUCTIONS
282452
contract_review_instructions: &contract_review_instructions |
283453
You are acting as a senior maintainer reviewing smart contract-related code
@@ -507,6 +677,9 @@ reviews:
507677
- Has no precedence bugs that weaken protection
508678
- path: "src/hiero_sdk_python/tokens/**/*.py"
509679
instructions: *token_review_instructions
680+
- path: "src/hiero_sdk_python/schedule/**/*.py"
681+
instructions: *schedule_review_instructions
682+
510683
# --- CUSTOM INSTRUCTIONS FOR EXAMPLES DIRECTORY ---
511684
- path: "examples/**/*"
512685
instructions: |

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.
66

77
## [Unreleased]
88

9+
### Added
10+
11+
- Added CodeRabbit review instructions and path mapping for the schedule module (`src/hiero_sdk_python/schedule/`) in `.coderabbit.yaml` (#1698)
12+
913
### Src
1014
-
1115

0 commit comments

Comments
 (0)