add review comments for GRETH-011 ~ GRETH-018

Author: nekomoto911

docs/plans/2026-02-23-greth-high-fixes-design.md

@@ -60,6 +60,8 @@ Date: 2026-02-23
 
 **Files:** `crates/pipe-exec-layer-ext-v2/relayer/src/blockchain_source.rs`, `crates/pipe-exec-layer-ext-v2/relayer/src/eth_client.rs`
 
+**Review Comments** reviewer: neko; state: rejected; comments: Currently verify_logs_against_receipts uses self.rpc_client.get_block_receipts() to cross-verify logs returned by eth_getLogs. However, both RPC calls go through the same rpc_client (i.e., the same RPC endpoint). If the RPC node is fully compromised, an attacker can forge both eth_getLogs and eth_getBlockReceipts responses to be consistent with each other, rendering the cross-verification ineffective.
+
 ## GRETH-012: Relayer Has No Reorg Detection
 
 **Problem:** Relayer polls up to "finalized" block from RPC but does not verify the block hash remains stable across polls. A source chain reorg past finality could cause already-relayed events to be undone.
@@ -68,10 +70,14 @@ Date: 2026-02-23
 
 **Files:** `crates/pipe-exec-layer-ext-v2/relayer/src/blockchain_source.rs`
 
+**Review Comments** reviewer: neko; state: rejected; comments: Unnecessary. The eth_getLogs requests only query up to the finalized block number, so reorg detection is not needed.
+
 ## GRETH-013: Transaction Pool Discard Unbounded
 
 **Problem:** `discard_txs` event bus can remove unlimited transactions in a single message. Any crate with event bus access can drain the entire mempool.
 
 **Fix:** Added `MAX_DISCARD_PER_BATCH = 1000` limit. Batches exceeding the limit are truncated with `warn!` logging. Each discard operation logged at warn level with transaction count and sample hashes.
 
 **Files:** `crates/transaction-pool/src/maintain.rs`
+
+**Review Comments** reviewer: neko; state: rejected; comments: The proposed change alters semantics. The current behavior intentionally discards all specified transactions, and truncating to MAX_DISCARD_PER_BATCH would leave stale transactions in the pool. All discard_txs must be fully processed, not partially truncated.

docs/plans/2026-02-23-greth-medium-fixes-design.md

@@ -10,6 +10,8 @@ Date: 2026-02-23
 
 **Files:** `crates/storage/db/src/implementation/rocksdb/cursor.rs`
 
+**Review Comments** reviewer: neko; state: accepted; comments: Accepted.
+
 ## GRETH-015: BLS Precompile Over-Length Input Accepted
 
 **Problem:** BLS PoP verification precompile checks `data.len() < EXPECTED_INPUT_LEN` (144 bytes) but allows arbitrarily long input. Extra trailing bytes silently ignored. Could serve as covert channel.
@@ -18,6 +20,8 @@ Date: 2026-02-23
 
 **Files:** `crates/pipe-exec-layer-ext-v2/execute/src/bls_precompile.rs`
 
+**Review Comments** reviewer: neko; state: accepted; comments: Accepted.
+
 ## GRETH-016: filter_invalid_txs Parallel Pre-Filter (Documentation)
 
 **Problem:** `filter_invalid_txs` processes transactions per-sender in parallel using pre-block state. Cross-sender dependencies (balance transfers, shared contract state) are not visible during validation.
@@ -26,6 +30,8 @@ Date: 2026-02-23
 
 **Files:** `crates/pipe-exec-layer-ext-v2/execute/src/lib.rs`
 
+**Review Comments** reviewer: neko; state: rejected; comments: The statement "each sender's account state is a local copy of the pre-block state" does not lead to invalid transactions slipping through. On the contrary, it may cause false positives — transactions that would actually be valid during EVM execution (due to cross-sender incoming transfers) are incorrectly marked as invalid by this pre-filter. The documentation comment should be corrected to: "Cross-sender dependencies (e.g. incoming transfers from other senders in the same block) are not visible during parallel per-sender validation because each sender's account state is a local copy of the pre-block state. This means the filter may produce false positives (marking valid transactions as invalid) but will not produce false negatives (letting truly invalid transactions through)."
+
 ## GRETH-017: Relayer State File No Integrity Protection
 
 **Problem:** Relayer state (last nonce, cursor position) persisted as plain JSON with no checksum. A filesystem-level attacker can roll back `last_nonce` to cause duplicate oracle writes or advance it to skip events.
@@ -34,6 +40,8 @@ Date: 2026-02-23
 
 **Files:** `crates/pipe-exec-layer-ext-v2/relayer/src/persistence.rs`
 
+**Review Comments** reviewer: neko; state: rejected; comments: (1) Defense against attackers is invalid — keccak256(content) is a keyless public algorithm and the code is open source. Any attacker with filesystem write access can modify last_nonce and recompute a valid checksum. Defending against a real attacker requires at minimum HMAC with a node-exclusive secret key. (2) Defense against process crashes is redundant — save() already uses a write-temp-then-rename atomic write pattern, so abnormal process exit will not produce half-written files. The checksum adds no incremental value in this scenario. (3) Defense against disk bit rot is theoretically valid but practically negligible — modern hardware ECC, mainstream filesystems (ZFS/Btrfs have data checksums; even ext4 without them), and cloud block storage end-to-end verification make the probability of "a bit flip that changes a numeric value while keeping the JSON structurally valid" approach zero.
+
 ## GRETH-018: Gravity CLI Args Accept Out-of-Range Values
 
 **Problem:** `--gravity.pipe-block-gas-limit`, `--gravity.cache.capacity`, `--gravity.trie.parallel-level` accept any u64 value. Zero gas limit causes all executions to revert. Extremely large parallel level spawns millions of threads.
@@ -45,6 +53,8 @@ Date: 2026-02-23
 
 **Files:** `crates/node/core/src/args/gravity.rs`
 
+**Review Comments** reviewer: neko; state: accepted; comments: Accepted.
+
 ## GRETH-019: DKG Transcript Not Size-Validated
 
 **Problem:** DKG transcript bytes from consensus layer are passed directly to system transaction construction without size or structural validation. Oversized transcript could cause excessive gas consumption or OOM.