|
| 1 | + # Gemini PR Review Instructions — java-tron |
| 2 | + |
| 3 | +This document defines how Gemini should review **Pull Requests** in the `java-tron` repository. |
| 4 | + |
| 5 | +The goal is to identify **consensus risks, state inconsistencies, lifecycle issues, and system-level regressions**, not style problems. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## 1. Project Context |
| 10 | + |
| 11 | +`java-tron` is a **blockchain full node implementation**. |
| 12 | + |
| 13 | +Any code change may affect: |
| 14 | + |
| 15 | +* Consensus safety |
| 16 | +* State machine correctness |
| 17 | +* Cross-node determinism |
| 18 | +* Network stability |
| 19 | +* Long-running node lifecycle |
| 20 | + |
| 21 | +PR review must therefore focus on **system-level correctness and safety**, not just local logic. |
| 22 | + |
| 23 | +--- |
| 24 | + |
| 25 | +## 2. Primary Review Objectives (Highest Priority) |
| 26 | + |
| 27 | +### 2.1 Consensus & Determinism |
| 28 | + |
| 29 | +Look for changes that may cause **different nodes to produce different results given the same input**. |
| 30 | + |
| 31 | +Examples include: |
| 32 | + |
| 33 | +* Time-dependent logic |
| 34 | +* Randomness |
| 35 | +* Unstable iteration order |
| 36 | +* Floating-point arithmetic |
| 37 | +* Inconsistent handling of `Proposal / Block / View / Round` |
| 38 | + |
| 39 | +--- |
| 40 | + |
| 41 | +### 2.2 State Machine Integrity |
| 42 | + |
| 43 | +* Partial or inconsistent state updates |
| 44 | +* State mutation before all validations complete |
| 45 | +* Missing rollback or compensation on failure paths |
| 46 | + |
| 47 | +--- |
| 48 | + |
| 49 | +### 2.3 Lifecycle & Resource Safety |
| 50 | + |
| 51 | +* Thread, executor, database, or network resource leaks |
| 52 | +* Incorrect initialization, startup, or shutdown ordering |
| 53 | +* Assumptions that components are “always alive” or “single-use” |
| 54 | + |
| 55 | +--- |
| 56 | + |
| 57 | +### 2.4 Concurrency & Hidden Races |
| 58 | + |
| 59 | +* Unsynchronized shared mutable state |
| 60 | +* Reentrancy risks |
| 61 | +* Unexpected interleavings between asynchronous callbacks |
| 62 | + |
| 63 | +--- |
| 64 | + |
| 65 | +### 2.5 Silent Failure |
| 66 | + |
| 67 | +* Exceptions swallowed or only logged |
| 68 | +* Errors converted into default values |
| 69 | +* Node continues running in degraded or inconsistent state |
| 70 | + |
| 71 | +--- |
| 72 | + |
| 73 | +## 3. DoS Risk Consideration |
| 74 | + |
| 75 | +For any change involving **external input** (network messages, blocks, transactions, proposals): |
| 76 | + |
| 77 | +* Watch for CPU, memory, or IO amplification |
| 78 | +* Identify unbounded loops, queues, or retries |
| 79 | +* Highlight paths triggerable repeatedly by malicious peers |
| 80 | + |
| 81 | +--- |
| 82 | + |
| 83 | +## 4. Scope Discipline |
| 84 | + |
| 85 | +* Focus primarily on **the PR diff** |
| 86 | +* Do not re-review unrelated legacy code |
| 87 | +* Expand context only when required for correctness or safety |
| 88 | +* Avoid large refactors or style-only suggestions |
| 89 | + |
| 90 | +--- |
| 91 | + |
| 92 | +## 5. Change-Type Awareness (Critical) |
| 93 | + |
| 94 | +Before detailed review, **first classify the PR**: |
| 95 | + |
| 96 | +### 5.1 Modification (Behavior Change) |
| 97 | + |
| 98 | +When existing logic is modified, focus on: |
| 99 | + |
| 100 | +* Whether behavioral semantics remain consistent |
| 101 | +* Whether implicit invariants are broken |
| 102 | +* Whether upstream callers still satisfy new assumptions |
| 103 | +* Whether downstream behavior changes silently |
| 104 | + |
| 105 | +Even small diffs may cause major consensus or state impact. |
| 106 | + |
| 107 | +--- |
| 108 | + |
| 109 | +### 5.2 Addition (New Logic) |
| 110 | + |
| 111 | +When new logic is introduced, focus on: |
| 112 | + |
| 113 | +* Whether all required call paths integrate it correctly |
| 114 | +* Whether lifecycle, concurrency, and error handling are complete |
| 115 | +* Whether initialization, cleanup, or configuration is missing |
| 116 | +* Whether new implicit dependencies are introduced |
| 117 | + |
| 118 | +New code risk usually lies in **integration boundaries**, not internal implementation. |
| 119 | + |
| 120 | +--- |
| 121 | + |
| 122 | +## 6. Review Output Expectations |
| 123 | + |
| 124 | +When reporting issues, clearly explain: |
| 125 | + |
| 126 | +* What could go wrong |
| 127 | +* Under what conditions |
| 128 | +* Why it matters for node correctness or network safety |
| 129 | + |
| 130 | +Prefer **high-signal, concise, critical feedback**. |
| 131 | + |
| 132 | +If no major issues are found, explicitly state that the change appears safe with respect to: |
| 133 | + |
| 134 | +* Consensus |
| 135 | +* State consistency |
| 136 | +* Lifecycle safety |
| 137 | + |
| 138 | +--- |
| 139 | + |
| 140 | +## 7. What Not To Do |
| 141 | + |
| 142 | +* Do not focus on formatting or naming |
| 143 | +* Do not suggest large refactors unrelated to PR intent |
| 144 | +* Do not assume single-node or happy-path execution |
0 commit comments