Skip to content

Commit 3bad42a

Browse files
authored
chore: Added review prompt for crypto module (hiero-ledger#1850)
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
1 parent 1ee286c commit 3bad42a

File tree

2 files changed

+173
-1
lines changed

2 files changed

+173
-1
lines changed

.coderabbit.yaml

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,175 @@ reviews:
935935
- Accurately aligned with protobuf definitions
936936
- Validating inputs consistently
937937

938+
# CRYPTO REVIEW INSTRUCTIONS
939+
crypto_review_instructions: &crypto_review_instructions |
940+
You are acting as a senior maintainer reviewing cryptography-related code
941+
in the hiero-sdk-python project.
942+
943+
This includes:
944+
- PrivateKey (Ed25519 and ECDSA(secp256k1))
945+
- PublicKey (Ed25519 and ECDSA(secp256k1))
946+
- EvmAddress
947+
948+
NOTE:
949+
- Review focus levels indicate areas requiring careful verification.
950+
- They do NOT imply severity or urgency.
951+
- Only recommend fixes when correctness, safety, determinism,
952+
cryptographic soundness, or API stability is impacted.
953+
954+
Scope is STRICTLY LIMITED to:
955+
- src/hiero_sdk_python/crypto/*.py
956+
- Their interaction with shared SDK utilities
957+
958+
----------------------------------------------------------
959+
REVIEW FOCUS 1 — API STABILITY & BACKWARDS COMPATIBILITY
960+
----------------------------------------------------------
961+
Crypto APIs are public and security-sensitive.
962+
963+
The following MUST remain stable:
964+
- Public class names
965+
- Method names and signatures
966+
- Return types
967+
- Key serialization formats (raw, DER, hex)
968+
- Signature formats (raw r||s for ECDSA)
969+
970+
Flag any change that:
971+
- Alters method signatures or return types
972+
- Changes serialization output format
973+
- Changes signature encoding behavior
974+
- Modifies equality or hashing semantics
975+
976+
If a breaking change is unavoidable:
977+
- Require explicit deprecation warnings
978+
- Require unit tests covering old vs new behavior
979+
- Require inline documentation explaining migration impact
980+
981+
----------------------------------------------------------
982+
REVIEW FOCUS 2 — CRYPTOGRAPHIC CORRECTNESS
983+
----------------------------------------------------------
984+
Verify correctness of:
985+
- Ed25519 key handling
986+
- ECDSA (secp256k1) key handling
987+
- Compressed vs uncompressed public keys
988+
- DER parsing and generation
989+
- Raw 64-byte (r||s) signature handling
990+
- keccak256 hashing usage
991+
- EVM address derivation (rightmost 20 bytes of Keccak-256)
992+
993+
Confirm that:
994+
- No incorrect hash function combinations are used
995+
- Prehashed signing/verification is used correctly
996+
- Signature malleability risks are addressed or intentional
997+
- No ambiguous key interpretation occurs silently
998+
- Invalid key lengths fail deterministically
999+
1000+
Flag:
1001+
- Any incorrect hash algorithm pairing
1002+
- Any improper DER normalization
1003+
- Any silent fallback logic
1004+
- Any potential signature malleability issue
1005+
- Any inconsistent signature verification logic
1006+
1007+
----------------------------------------------------------
1008+
REVIEW FOCUS 3 — KEY AMBIGUITY & TYPE SAFETY
1009+
----------------------------------------------------------
1010+
32-byte inputs can represent:
1011+
- Ed25519 private keys
1012+
- ECDSA (secp256k1) private scalars
1013+
1014+
Verify that:
1015+
- Ambiguity is handled explicitly
1016+
- No silent reinterpretation occurs
1017+
- Explicit algorithm intent is preserved
1018+
- Warnings or safeguards are meaningful and consistent
1019+
1020+
PublicKey and PrivateKey MUST:
1021+
- Never silently switch algorithms
1022+
- Never infer algorithm incorrectly from length alone
1023+
1024+
----------------------------------------------------------
1025+
REVIEW FOCUS 4 — SERIALIZATION & DETERMINISM
1026+
----------------------------------------------------------
1027+
Verify that:
1028+
- Raw bytes serialization is stable
1029+
- DER serialization is canonical
1030+
- Hex encoding is lowercase and deterministic
1031+
- Equality (__eq__) matches cryptographic identity
1032+
- __hash__ is correct and deterministic
1033+
- No runtime errors occur in hashing
1034+
1035+
Flag:
1036+
- Incorrect hash implementations
1037+
- NameErrors or undefined variables
1038+
- Non-deterministic serialization behavior
1039+
1040+
----------------------------------------------------------
1041+
REVIEW FOCUS 5 — EVM ADDRESS INTEGRITY
1042+
----------------------------------------------------------
1043+
EvmAddress MUST:
1044+
- Be exactly 20 bytes
1045+
- Be derived correctly from Keccak-256
1046+
- Reject invalid hex strings
1047+
- Avoid silent truncation or padding
1048+
1049+
Verify:
1050+
- from_string validation correctness
1051+
- to_string format consistency
1052+
- Equality and hashing correctness
1053+
1054+
Flag:
1055+
- Incorrect length checks
1056+
- Unsafe parsing
1057+
- Broken __hash__ implementations
1058+
1059+
----------------------------------------------------------
1060+
REVIEW FOCUS 6 — VALIDATION & ERROR BEHAVIOR
1061+
----------------------------------------------------------
1062+
All validation must:
1063+
- Fail early
1064+
- Raise deterministic exceptions
1065+
- Avoid ambiguous error messages
1066+
- Avoid leaking sensitive key material in errors
1067+
1068+
Ensure:
1069+
- Private keys are never exposed in __repr__
1070+
- Error messages do not contain raw key material
1071+
- TypeErrors and ValueErrors are consistent
1072+
1073+
----------------------------------------------------------
1074+
REVIEW FOCUS 7 — IMPORT SAFETY
1075+
----------------------------------------------------------
1076+
STRICT IMPORT RULES:
1077+
- All imports MUST exist in src/hiero_sdk_python/
1078+
- Do NOT assume helpers exist
1079+
- Do NOT infer utilities from other SDKs
1080+
- Validate keccak256 implementation source
1081+
1082+
Flag immediately:
1083+
- Undefined names
1084+
- Copy-paste artifacts
1085+
- Incorrect external crypto assumptions
1086+
1087+
----------------------------------------------------------
1088+
REVIEW FOCUS 8 — EXPLICIT NON-GOALS
1089+
----------------------------------------------------------
1090+
Do NOT:
1091+
- Suggest refactors unless security or correctness is impacted
1092+
- Recommend stylistic or formatting-only changes
1093+
- Propose performance optimizations without correctness impact
1094+
- Review unrelated SDK components
1095+
1096+
----------------------------------------------------------
1097+
FINAL OBJECTIVE
1098+
----------------------------------------------------------
1099+
Ensure crypto code is:
1100+
- Cryptographically correct
1101+
- Deterministic
1102+
- Non-ambiguous
1103+
- Backward-compatible
1104+
- Safe against misuse
1105+
- Free from silent cryptographic errors
1106+
9381107
# ============================================================
9391108
# GLOBAL REVIEW INSTRUCTIONS (APPLY TO ALL FILES)
9401109
# ============================================================
@@ -1678,6 +1847,9 @@ reviews:
16781847
- **MINOR** — should be fixed; low-risk correctness or ergonomics issue
16791848
- **NIT** — optional; style, naming, or documentation only
16801849
1850+
- path: "src/hiero_sdk_python/crypto/**/*.py"
1851+
instructions: *crypto_review_instructions
1852+
16811853
chat:
16821854
art: false # Don't draw ASCII art (false)
16831855
auto_reply: false # Don't allow bot to converse (spammy)

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.
1313
- Added CodeRabbit review instructions and path mapping for the schedule module (`src/hiero_sdk_python/schedule/`) in `.coderabbit.yaml` (#1698)
1414
- 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)
1515
- Added CodeRabbit review instructions for consensus module `src/hiero_sdk_python/consensus/` with critical focus on protobuf alignment `.coderabbit.yaml`.
16-
16+
- Added CodeRabbit prompt to review the `src/hiero_sdk_python/crypto` module.
1717

1818
### Fixed
1919

0 commit comments

Comments
 (0)