verify balance and approval strategies concurrently#4512
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces concurrent verification of approval and balance strategies up to a specified concurrency limit by adding a new find_first_ordered_concurrent helper in detector.rs. Feedback on the changes highlights three critical compilation issues: a move-out-of-index error when matching on results[drain_cursor] in detector.rs, and lifetime-related compilation errors in both approval/mod.rs and balance/mod.rs caused by explicit return type annotations on the make_fut closures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| while drain_cursor < n { | ||
| match results[drain_cursor] { | ||
| None => break, | ||
| Some(Some(_)) => { | ||
| if let Some(Some(value)) = results[drain_cursor].take() { | ||
| return Some((drain_cursor, value)); | ||
| } | ||
| } | ||
| Some(None) => { | ||
| drain_cursor += 1; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Matching on results[drain_cursor] by value will fail to compile with cannot move out of index of Vec because Option<Option<T>> is not Copy. Additionally, matching on a reference would cause borrow checker errors when calling .take() inside the match arm. Replacing the match with a direct .is_none() check and .take() avoids these issues and is more concise.
while drain_cursor < n {
if results[drain_cursor].is_none() {
break;
}
if let Some(Some(value)) = results[drain_cursor].take() {
return Some((drain_cursor, value));
}
drain_cursor += 1;
}| let make_fut = move |idx: usize| -> std::pin::Pin< | ||
| Box<dyn std::future::Future<Output = (usize, Option<ApprovalStrategy>)> + Send>, | ||
| > { |
There was a problem hiding this comment.
The explicit return type annotation on the closure make_fut defaults to + 'static for the trait object. Since the future captures self (which is not 'static), this will cause a compilation error. Omitting the return type annotation allows the compiler to correctly infer the lifetime.
let make_fut = move |idx: usize| {| let make_fut = move |idx: usize| -> std::pin::Pin< | ||
| Box<dyn std::future::Future<Output = (usize, Option<Strategy>)> + Send>, | ||
| > { |
There was a problem hiding this comment.
The explicit return type annotation on the closure make_fut defaults to + 'static for the trait object. Since the future captures self (which is not 'static), this will cause a compilation error. Omitting the return type annotation allows the compiler to correctly infer the lifetime.
let make_fut = move |idx: usize| {
|
I checked both points locally and don't see the reported issues. The results[drain_cursor] match compiles successfully. |
1 participant
Description
Improves detection latency by verifying balance and approval strategy candidates concurrently instead of sequentially. With high RPC latency or timeouts, this reduces worst-case detection time from ~60s to ~15s (assuming all 60 candidates time out at the 1s limit) while preserving priority ordering and avoiding RPC node overload.
Changes
find_first_ordered_concurrenthelper for bounded concurrent verification with priority preservationconcurrency_limitfield (default: 4) to bothbalance::Detectorandapproval::DetectorFuturesUnorderedhigher-priority candidates are still awaited to ensure the best result is returned
futurescrate dependency for stream utilities