add review comments for GRETH-001 ~ GRETH-010#4
Open
AshinGau wants to merge 22 commits intoRichard1048576:bugfix/security-fixesfrom
Open
add review comments for GRETH-001 ~ GRETH-010#4AshinGau wants to merge 22 commits intoRichard1048576:bugfix/security-fixesfrom
AshinGau wants to merge 22 commits intoRichard1048576:bugfix/security-fixesfrom
Conversation
AshinGau
force-pushed
the
bugfix/security-fixes
branch
from
e5e072e to
490cca5
Compare
AshinGau
force-pushed
the
bugfix/security-fixes
branch
from
490cca5 to
7180fe5
Compare
AshinGau
changed the title
…cursor drop order GRETH-003 (GRAV-001): Replace hardcoded AUTHORIZED_CALLER with JWK_MANAGER_ADDR in mint_precompile.rs. The old address (0x595475...4e) was a test address that never matched the production JWK Manager system address (0x...1625f4001), allowing any holder of the test address to mint arbitrary native G tokens. Add unit test to verify the constant stays correct. GRETH-004: Remove #[cfg(debug_assertions)] gate from the validate_block() call in make_executed_block_canonical(). Previously block validation was silently skipped in release builds, allowing malformed blocks to be committed to the canonical chain without the secondary validation check. GRETH-002: Fix unsafe drop-order soundness issue in RocksDB Cursor. The iterator field held a DBRawIterator<'static> that borrows from the Arc<DB>, but the db field was declared first, meaning it dropped before the iterator. If the Cursor was the last Arc<DB> holder this was use-after-free. Fix: move the iterator field before db so Rust's field-declaration drop order drops the iterator first. Add detailed SAFETY comment explaining the invariant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n, relayer hardening GRETH-001: RPC signer recovery — replace Address::ZERO fallback with SYSTEM_CALLER (0x00000000000000000000000000000001625f0000) in transaction.rs and receipt.rs. Prevents unsigned/failed-signature txs from being attributed to the zero address. GRETH-005: Parallel DB write error handling — add explicit error! logging when state or trie persist tasks fail, so DB divergence is surfaced rather than silently ignored. GRETH-006: Path traversal in sharding config — validate that all DB shard paths are absolute and contain no ".." components before opening RocksDB instances. GRETH-007: Parallel EVM state root — add TODO documenting that parallel_execute does not verify the state root supplied by consensus against EVM output. GRETH-008: Recovery from canonical tip — add NOTE documenting that recovery trusts the persisted canonical tip's state root without re-verification. GRETH-009: Immediate finalization — add architectural note in tree/mod.rs explaining why gravity-reth marks every block immediately safe+finalized (BFT consensus provides the finality guarantee externally). GRETH-010: Filter-only gravity event extraction — add FIXME in extract_gravity_events_from_receipts noting it currently processes all receipts including user transactions (should filter by system contract sender). GRETH-011: Relayer log verification — add topic[0] (MessageSent::SIGNATURE_HASH) and emitting-address checks before parsing cross-chain event logs. GRETH-012: Relayer reorg detection — add last_confirmed_block_hash field to SourceState for future reorg guard; add TODO at get_finalized_block_number. GRETH-013: Pool discard batch size — cap discard-on-new-block loop at MAX_DISCARD_PER_BATCH=1000 with warn! logging to prevent O(n) stalls. GRETH-014: RocksDB cursor read-your-writes — document that WriteBatch writes are not visible to subsequent reads on the same cursor within the same tx. GRETH-015: BLS precompile strict length — change `< EXPECTED_INPUT_LEN` to `!= EXPECTED_INPUT_LEN` so over-length inputs are also rejected. GRETH-016: filter_invalid_txs doc — clarify it is a performance-only pre-filter that does not replace EVM-level validation. GRETH-017: Relayer persistence state — add last_confirmed_block_hash to SourceState (foundation for future HMAC integrity check). GRETH-018: CLI arg range validation — add clap value_parser ranges to pipe_block_gas_limit (1M–500G), cache_capacity (1K–100M), trie_parallel_levels (1–64). GRETH-019: DKG transcript size guard — reject empty or oversized (>512KB) DKG transcripts in construct_dkg_transaction before submitting on-chain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ering, reorg detection, checksum GRETH-007: verify_executed_block_hash — add explicit warn! log when consensus passes None (no block hash to check). The assert_eq path already verifies the state root by comparing block hashes; the None path was silently skipping it. Also update the comment in parallel_execute.rs to reflect that verification happens in verify_executed_block_hash. GRETH-010: extract_gravity_events_from_receipts — at the call site, slice the receipts array to only pass system transaction receipts (metadata + validators, inserted at the front). User transaction receipts are now excluded, preventing a user-controlled log with a matching ABI signature from injecting oracle data even if NativeOracle.record() lacks on-chain caller restrictions. GRETH-012: Reorg detection in BlockchainEventSource::poll() — at the start of each poll, fetch the current hash of the cursor block and compare with the hash stored on the previous poll. A mismatch indicates a chain reorganization past the finalized marker; poll() returns an error and logs an operator alert. Add EthHttpCli::get_block_hash_at() to support this check. Add SourceState::last_confirmed_block_hash (persisted) for use by higher-level code when restoring cursor state. GRETH-017: State file integrity checksum — add SourceState::checksum (keccak256 over all mutable fields, hex-encoded). Computed on every RelayerState::update() call, verified on every RelayerState::load() call. Detects accidental file corruption (bit flips, partial writes, truncation). Add test for corrupted state detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Records all 28 security findings across gravity-reth, gravity-sdk, and gravity_chain_core_contracts with fix commit hashes, type (code fix / mitigation / design doc), and current status. Branches: gravity-reth bugfix/security-fixes — cbccf02, 14b6ce5, f16d356 gravity-sdk bugfix/security-fixes — a0bf499 Remaining open: GRAV-005 (contracts repo, separate agent). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add cross-verification of eth_getLogs results against eth_getBlockReceipts to detect a compromised RPC server that fabricates event logs. - EthHttpCli::get_block_receipts(block_number) — new method using same retry-with-backoff pattern as existing RPC calls - BlockchainEventSource::verify_logs_against_receipts() — groups logs by block, fetches receipts once per block, verifies each log by matching address + topics + data against the canonical receipt list - Fail-closed: logs whose block receipts cannot be fetched are dropped rather than silently accepted - Integrated into poll() between eth_getLogs and event processing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security audit report covering GRETH-001 through GRETH-019: - 4 CRITICAL (signer recovery, cursor lifetime, mint precompile, block validation) - 9 HIGH (persistence, path traversal, state root, oracle, relayer, tx pool) - 6 MEDIUM (read-your-writes, BLS input, tx filter, state integrity, CLI, DKG) Design documents detail the fix approach for each severity level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds automated security review on PRs using anthropics/claude-code-security-review. Runs on non-draft PRs, uses claude-sonnet-4-6, excludes docs/tests/benches/.github. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xe#272) Security audit round 2 covering GRETH-020 through GRETH-028: - 2 HIGH (debug-only validation, silent system txn failure) - 3 MEDIUM (mint precompile trailing bytes, unsafe Send impl, unwrap panic) - 4 LOW/INFO (nonce truncation, state merge overwrite, same-RPC verification, timestamp check) Design documents detail the problem and fix approach for each severity level.
Updated review comments to reflect acceptance of Plan A for the `OracleRelayerManager` changes.
Remove code rejected by reviewers: - GRETH-011: receipt cross-verification via same RPC (ineffective) - GRETH-012: reorg detection for finalized blocks (unnecessary) - GRETH-013: MAX_DISCARD_PER_BATCH truncation (breaks semantics) - GRETH-017: keyless keccak256 checksum (ineffective without secret) - GRETH-025: remove assert! on system txn (DKG/JWK can legit fail), keep receipt.success = result.is_success() Fix GRETH-016: correct filter_invalid_txs docstring — filter produces false positives (not false negatives). Revert files already merged to main: gravity.rs CLI validators, bls_precompile.rs strict length, transaction.rs SYSTEM_CALLER. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Multi-agent parallel audit identified 48 new findings across the execution pipeline, parallel EVM, state integrity, oracle relayer, and GCEI protocol. Includes 2 CRITICAL (pipeline deadlock, cache eviction), 14 HIGH, 21 MEDIUM, and 11 LOW severity issues not covered in the 2026-02-23 audit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Richard1048576
force-pushed
the
bugfix/security-fixes
branch
from
d7ed509 to
a92a2fc
Compare
48 security fixes across CRITICAL/HIGH/MEDIUM/LOW severities: - GRETH-029: Add timeout to all pipeline barriers + JoinHandle monitoring - GRETH-030: Cap cache eviction height at persist_height - GRETH-031: Implement block_hash_ref() (was unimplemented!()) - GRETH-032/033: Fix precompile state merge on epoch-change paths - GRETH-034/035: Add TODO for RocksDB snapshot coordination - GRETH-036: Replace unbounded channels with bounded(64) - GRETH-037/038: Replace type-erased event bus with typed OnceLock - GRETH-040/041: Add system txn target allowlist, deduplicate definition - GRETH-042: Add failed_proposer_indices field - GRETH-043: Replace nonce truncation with checked conversion - GRETH-046: Reorder trie cache writes (insert-before-remove) - GRETH-049: Add per-call mint cap - GRETH-050/051: Fix precompile state merge + deep-merge system txn state - GRETH-052/053: Structured block hash verification for post-genesis - GRETH-054: Add timeout to wait_for_block_persistence - GRETH-057: Use SeqCst for epoch/height atomics - GRETH-058: Add optional TLS certificate pinning - GRETH-059/060: Fix JWK type parsing, replace expect() with error propagation - GRETH-062: Use Acquire/Release ordering on cursor atomics - GRETH-063: Make fromBlock required parameter - GRETH-064: Add minimum voting power threshold - GRETH-065/066: Increase BLS/mint precompile gas costs - GRETH-068: Replace panic with error propagation in make_canonical - GRETH-069~075: Recovery, pruning, validator-only, and relayer fixes Skipped INVALID findings: GRETH-047, 048, 055, 056, 061, 067 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AshinGau
force-pushed
the
bugfix/security-fixes
branch
from
7180fe5 to
d636a0d
Compare
Richard1048576
force-pushed
the
bugfix/security-fixes
branch
from
a92a2fc to
4dc683c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.