merge queue: embarking main (e561ebd) and #2096 together#2120
Closed
mergify[bot] wants to merge 44 commits intomainfrom
Closed
merge queue: embarking main (e561ebd) and #2096 together#2120mergify[bot] wants to merge 44 commits intomainfrom
mergify[bot] wants to merge 44 commits intomainfrom
Conversation
Compare local ciphertext digests against on-chain consensus from the CiphertextCommits contract. Enabled via --ciphertext-commits-address. Adds early-warning logging when peer submissions diverge and structured warn logs when local digest mismatches consensus. Four new Prometheus counters provide observability. Detection-only — no recovery action.
…ft checks - P1: Propagate DB errors from handle_consensus instead of swallowing them, so the block is retried via the outer backoff loop. - P2: Queue unresolved consensus events (local digest not yet computed) in a bounded pending queue (10k cap) and retry each block tick, so late-arriving local digests are still checked. - Refactor comparison logic into try_resolve_consensus shared by both the initial check and the retry path.
- Avoid double variant_summaries() calls by binding result once - Add has_multiple_variants() for cheap filter without string allocation - Lazy-clone current_expected_senders only on new handle insertion - Remove dead missing_submission_reported field - Embed EventContext in ConsensusState instead of duplicating fields - Use fetch_optional instead of fetch_all+assert for single-row query - Hoist filter_addresses out of the polling loop
Cover gaps identified in test audit: - Grace period boundary: handle not evicted within grace, evicted at boundary - No-consensus timeout boundary: handle not evicted within window, evicted at boundary - Consensus before any submission: creates handle, evicts after grace - Equivocation: same sender different digests warns but keeps first submission - Duplicate submission: same sender same digests silently ignored - Local digest not ready: consensus defers check, completes after DB update - Local digest match: no false-positive drift when digests agree - Local check not ready eviction: consensus with pending check times out
Simplify drift_detector tests by extracting a shared constructor, removing an unused pool variable, and adding metric assertions.
…robustness - Guard CONSENSUS_LATENCY_BLOCKS_HISTOGRAM behind alerts_enabled to prevent rebuild replay from polluting latency metrics - Early-return from try_check_local_consensus when !alerts_enabled to skip unnecessary DB queries during rebuild; handles are re-checked via refresh_pending_consensus_checks once alerts resume - Change finish_if_complete threshold from == to >= so unexpected senders don't prevent handle completion (previously hung until timeout eviction) - Capture injector exit code in e2e script instead of letting set -e abort on wait; report injector failure explicitly - Add test: unexpected_sender_does_not_block_completion
…ispatch Inline trivial free functions (has_multiple_variants, seen_sender_strings, missing_sender_strings, address_strings, digest_pair_from_db_digests) at call sites and delete them. Remove redundant .or_else() in sender_seed_block computation. Convert if-else address dispatch to match. Add submit() test helper to compress repetitive observe_submission boilerplate.
- HandleState::new associated function replaces free function - Inline DeferredMetrics fields directly into DriftDetector - has_multiple_variants() avoids allocation in hot-path variant check - classify_handle + HandleDisposition enum replaces continue chains in evict_stale - end_of_batch/end_of_rebuild facade methods encapsulate multi-step protocols - Fix dead unwrap_or in finish_if_complete (consensus already checked is_some)
Address review feedback: the old wording "peer submissions or consensus mismatch" was ambiguous. Reword to make the two detection paths explicit.
- Clarify drift counter description in metrics.rs - Document all 5 drift metrics in docs/metrics/metrics.md - Bump drift timeout defaults from 50/10 to 3000/3000 blocks (~5 min at 100ms block time) to accommodate coprocessors stuck for minutes - Replace manual bail with clap requires on ciphertext_commits_address
The field name now reflects what the detector is *doing* (replaying historical logs) rather than a side-effect toggle. Inverted semantics: `replaying: true` suppresses warns and DB queries, `false` is normal operation. Also removes redundant `if self.alerts_enabled` guard in try_check_local_consensus — already short-circuited by the early return at the top of the function.
- Add `type CiphertextDigest = FixedBytes<32>` to avoid literal 32 - Document implicit upper bound on open_handles - Remove redundant early-return guard in flush_metrics - fetch_expected_coprocessor_tx_senders now requires gateway_config_address parameter; caller decides whether to call based on config
Use domain-meaningful names: HandleOutcome describes what happened to a handle, not an internal mechanism. Variants renamed: - KeepWaiting → Pending - NoConsensusTimeout → GatewayNeverReachedConsensus - ConsensusUncheckedTimeout → LocalDigestNeverAppeared - MissingSubmissionPostGrace → NotAllCoprocessorsSubmitted
…prove clarity - Fix verify_proof metrics being incremented both inside the function and at the call site (bug: double-counting) - Remove unnecessary .clone() on owned event field - Replace unreachable Pending return with unreachable!() - Replace cross-function .unwrap() with let-else in evict_stale - Collapse two identical "local digests not ready" debug branches - Use < instead of != in classify_handle (invariant is always <) - Rename fetch_expected_coprocessor_tx_senders to fetch_expected_senders - Remove redundant replaying guard in finalize_completed_without_consensus
…tener All drift detection code paths now log-and-continue instead of propagating errors to run_get_logs. A transient DB or RPC failure during drift checking should not interrupt processing of VerifyProofRequest, ActivateKey, or ActivateCrs events.
…lock Duration The gateway chain only produces blocks when there are transactions, making block count an unreliable proxy for elapsed time. Switch drift detection from block-number arithmetic to std::time::Instant/Duration: - ConfigSettings and CLI args use Duration (humantime format, e.g. "5m") - EventContext carries observed_at: Instant - HandleState/ConsensusState carry first_seen_at/received_at: Instant - classify_handle compares wall-clock elapsed time against Duration thresholds - Block numbers retained for logging/metrics (still useful diagnostics) - Removes get_block_number RPC call from rebuild_drift_detector (no longer needed)
Resolve conflict in test-suite/fhevm/fhevm-cli by taking main's version (tab indentation, negative-acl test type) and re-adding our ciphertext-drift test type.
…fy Default is test-only
- Prefix DriftDetector fields with `drift_` for consistency with ConfigSettings - Move classify_handle from free function to &self method on DriftDetector
6 tasks
Changed Lines CoverageCoverage of added/modified lines: 86.0% Per-file breakdownDiff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
coprocessor/fhevm-engine/gw-listener/src/bin/gw_listener.rscoprocessor/fhevm-engine/gw-listener/src/drift_detector.rscoprocessor/fhevm-engine/gw-listener/src/gw_listener.rs |
Author
🧪 CI InsightsHere's what we observed from your CI run for e303e77. 🟢 All jobs passed!But CI Insights is watching 👀 |
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
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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
Branch main (e561ebd) and #2096 are embarked together for merge.
This pull request has been created by Mergify to speculatively check the mergeability of #2096.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue
mainfor merge:#approved-reviews-by >= 1[🛡 GitHub branch protection]#changes-requested-reviews-by = 0[🛡 GitHub branch protection]#review-threads-unresolved = 0[🛡 GitHub branch protection]branch-protection-review-decision = APPROVED[🛡 GitHub branch protection]check-success = run-e2e-tests / fhevm-e2e-testcheck-success = common-pull-request/lint (bpr)check-neutral = common-pull-request/lint (bpr)check-skipped = common-pull-request/lint (bpr)check-skipped = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-neutral = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-success = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-success = coprocessor-cargo-test/cargo-tests (bpr)check-neutral = coprocessor-cargo-test/cargo-tests (bpr)check-skipped = coprocessor-cargo-test/cargo-tests (bpr)check-success = coprocessor-dependency-analysis/dependencies-check (bpr)check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)check-skipped = kms-connector-tests/test-connector (bpr)check-neutral = kms-connector-tests/test-connector (bpr)check-success = kms-connector-tests/test-connector (bpr)Required conditions to stay in the queue:
#approved-reviews-by >= 1[🛡 GitHub branch protection]#changes-requested-reviews-by = 0[🛡 GitHub branch protection]#review-threads-unresolved = 0[🛡 GitHub branch protection]base = mainbranch-protection-review-decision = APPROVED[🛡 GitHub branch protection]label!=do-not-mergecheck-success = common-pull-request/lint (bpr)check-neutral = common-pull-request/lint (bpr)check-skipped = common-pull-request/lint (bpr)check-skipped = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-neutral = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-success = coprocessor-cargo-listener-tests/cargo-tests (bpr)check-success = coprocessor-cargo-test/cargo-tests (bpr)check-neutral = coprocessor-cargo-test/cargo-tests (bpr)check-skipped = coprocessor-cargo-test/cargo-tests (bpr)check-success = coprocessor-dependency-analysis/dependencies-check (bpr)check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)check-skipped = kms-connector-tests/test-connector (bpr)check-neutral = kms-connector-tests/test-connector (bpr)check-success = kms-connector-tests/test-connector (bpr)