fix(coprocessor): verify input via eth_getLogs#1795
fix(coprocessor): verify input via eth_getLogs#1795dartdart26 wants to merge 2 commits intorelease/0.10.xfrom
Conversation
Instead of relying on subscriptions that are not reliable in all cases, use eth_getLogs to fetch verify input requests. Also, change: * poll interval from 1 sec to 500 ms to reduce latency
🧪 CI InsightsHere's what we observed from your CI run for b68e72a. 🟢 All jobs passed!But CI Insights is watching 👀 |
e8e24d0 to
ef536f1
Compare
Since gw-listener catches up automatically via the last processed block number in the DB, call the manual process "replay" instead of "catchup". Replay is requested via the cmd line. Also, keep the old name as an alias for backward compatibility. Furthermore, replay is not KMS-specific anymore, so remove KMS from the name. Add more logging in health checks and replay. Update cargo-audit version to 0.22.0 and run `cargo update -p ruint` due to a vulnerability reported in CI.
ef536f1 to
b68e72a
Compare
There was a problem hiding this comment.
Pull request overview
This pull request refactors the gateway listener to use eth_getLogs polling instead of event subscriptions for fetching verify input requests, improving reliability. The terminology changes from "catchup" to "replay" to better reflect the manual recovery process, and the poll interval is reduced from 1 second to 500ms to reduce latency.
Changes:
- Removed subscription-based
run_input_verificationand merged input verification into the polling-basedrun_get_logsmethod - Renamed
catchup_kms_generation_from_blocktoreplay_from_blockwith backward-compatible CLI alias - Reduced
get_logs_poll_intervaldefault from 1s to 500ms and added configurable logging frequency for last processed block updates
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gw_listener.rs | Consolidated event processing into run_get_logs, removed subscription-based verification, and added enhanced logging for health checks |
| lib.rs | Updated configuration field names and defaults for replay functionality and poll interval |
| bin/gw_listener.rs | Renamed CLI argument with backward-compatible alias and added new logging configuration parameter |
| gw_listener_tests.rs | Updated test to use new replay_from_block configuration field name |
| values.yaml | Updated Helm chart with new parameter names and defaults |
| Chart.yaml | Bumped chart version from 0.7.10 to 0.7.12 |
| Cargo.lock | Updated dependency versions including ruint, cargo-audit, and various windows-sys references |
| coprocessor-dependency-analysis.yml | Updated cargo-audit version from 0.21.0 to 0.22.0 |
Comments suppressed due to low confidence (1)
coprocessor/fhevm-engine/gw-listener/src/gw_listener.rs:339
- The VERIFY_PROOF_SUCCESS_COUNTER is incremented twice for each successful verify proof request. It's incremented here in the
verify_proof_requestfunction (line 336), and then again at line 276 inrun_get_logsviaVERIFY_PROOF_SUCCESS_COUNTER.inc_by(verify_proof_success). Remove the counter increment from this function since the caller already handles metric updates after successful DB commit to avoid double-counting.
inspect(|_| {
VERIFY_PROOF_SUCCESS_COUNTER.inc();
}).inspect_err(|_| {
VERIFY_PROOF_FAIL_COUNTER.inc();
})?;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
| }, | ||
| _ => { | ||
| error!(log = ?log, "Unknown KMSGeneration event") |
There was a problem hiding this comment.
Missing period at the end of the error message. For consistency with other error messages in the codebase (e.g., lines 206, 210, 255), add a period at the end of this error message.
| error!(log = ?log, "Unknown KMSGeneration event") | |
| error!(log = ?log, "Unknown KMSGeneration event."); |
|
Already on main, we can close this |
Instead of relying on subscriptions that are not reliable in all cases, use eth_getLogs to fetch verify input requests.
Also, change:
Since gw-listener catches up automatically via the last processed block number in the DB, call the manual process "replay" instead of "catchup". Replay is requested via the cmd line. Also, keep the old name as an alias for backward compatibility. Furthermore, replay is not KMS-specific anymore, so remove KMS from the name.
Add more logging in health checks and replay.