Feat/soroban event listener#197
Conversation
…h ring - Replace DEFAULT_MAX_ELO_DIFF (200) with INITIAL_ELO_RANGE (100), EXPANDED_ELO_RANGE (200), and EXPAND_AFTER_SECS (30) - Set INITIAL_ELO_RANGE on new rated requests in join_queue - Update Lua script to use max(incoming_range, opponent_range) so either player's expanded window can trigger a match - Rewrite expand_elo_ranges to expand once after 30 seconds instead of incrementing per minute; guard prevents repeated Redis writes - Add 8 unit tests covering matching conditions and acceptance criteria
- Add backend/modules/st_core/src/event_listener.rs Polls Horizon /events every 5s via tokio::time::interval Tracks matches awaiting stake by contract_id in Arc<RwLock<HashMap>> Decodes base64 ScVal topics to detect staked/stake_confirmed events Transitions match status AWAITING_STAKE → IN_PROGRESS on confirmation Broadcasts MatchStartSignal via broadcast::Sender to WebSocket handlers Cursor-based pagination prevents reprocessing the same event twice Duplicate event guard skips events for already-transitioned matches - Update src/lib.rs to expose event_listener module - Update Cargo.toml to add reqwest and log dependencies - Add 11 unit tests covering event detection and status transitions
|
@ezedike-evan Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Soroban event listener that polls Stellar Horizon and broadcasts match-start signals, integrates it into the WebSocket server, changes matchmaking Elo policy to a fixed two-stage window (100→200 after 30s), updates related tests, and includes unrelated duplicate/conflict edits in the game contract and frontend. Changes
Sequence DiagramsequenceDiagram
participant Horizon as Stellar Horizon API
participant Listener as SorobanEventListener
participant Registry as Tracked Matches (Arc<RwLock<HashMap>>)
participant Broad as Broadcast Channel
participant WS as WebSocket Server / Clients
loop Every POLL_INTERVAL_SECS
Listener->>Registry: Snapshot tracked contract IDs
Listener->>Horizon: GET /events?type=contract&cursor=...&limit=...
Horizon-->>Listener: Event records
loop For each record
Listener->>Listener: Advance cursor to paging_token
Listener->>Listener: is_stake_event(record)
alt stake event matches contract & topic
Listener->>Registry: Lookup & transition AwaitingStake→InProgress
Listener->>Broad: Broadcast MatchStartSignal(match_id, "IN_PROGRESS", tx_hash, ledger)
Broad-->>WS: Deliver to subscribed connections
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/modules/matchmaking/service.rs (1)
523-548:⚠️ Potential issue | 🔴 CriticalReplace the separate ZREM/ZADD calls with an atomic Lua script to prevent re-queuing players who were already matched or canceled.
The current code at lines 542–548 is non-atomic: a snapshot is taken at line 518–521, but another task (e.g.,
find_rated_matchorcancel_request) can remove a member from the queue between the snapshot and these operations. When ZREM fails silently (returns 0) because the member is gone, ZADD still executes, reintroducing a stale request. This leaves a player both inactive_matchesand matchable again.Use a Redis Lua script that atomically verifies the member exists via
ZSCORE, then removes and re-adds it only if present, returning a status code to confirm success:Safe atomic swap implementation
- conn.zrem::<_, _, ()>(key, &member) - .await - .map_err(|e| format!("Redis ZREM failed: {}", e))?; - - conn.zadd::<_, _, _, ()>(key, &updated_value, score) - .await - .map_err(|e| format!("Redis ZADD failed: {}", e))?; + let replaced: i32 = redis::Script::new(r#" + local key = KEYS[1] + local old_member = ARGV[1] + local new_member = ARGV[2] + local score = tonumber(ARGV[3]) + + if not redis.call('ZSCORE', key, old_member) then + return 0 + end + + redis.call('ZREM', key, old_member) + redis.call('ZADD', key, score, new_member) + return 1 + "#) + .key(key) + .arg(&member) + .arg(&updated_value) + .arg(score) + .invoke_async(&mut conn) + .await + .map_err(|e| format!("Redis Lua script failed: {}", e))?; + + if replaced == 0 { + continue; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/matchmaking/service.rs` around lines 523 - 548, The non-atomic remove-then-add (conn.zrem + conn.zadd) in the match expansion path (around the MatchRequest::from_redis_value -> to_redis_value flow) can reintroduce a stale request if another task (e.g., find_rated_match or cancel_request) removed the member between your snapshot and these calls; replace the two separate calls with a single Redis Lua script executed via conn.eval/eval_async that: checks ZSCORE(key, member) to ensure the member still exists at the expected score, if present performs the ZREM and ZADD with the updated_value at the same score, and returns a success code (e.g., 1) or 0 if the member was absent; then treat a 0 return as a no-op (do not re-add or error), and propagate errors on script failure. Ensure the script uses the same key, member, and score variables you currently pass (key, member, score) and update the code that currently calls conn.zrem and conn.zadd to call and handle the script result instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/modules/matchmaking/service.rs`:
- Around line 59-66: The rated-match branch currently only sets
request.max_elo_diff when None, allowing callers to bypass the two-stage Elo
window; change the logic in the MatchType::Rated branch so that
request.max_elo_diff is explicitly normalized/clamped to INITIAL_ELO_RANGE (not
left as the client's value) and ensure expand_elo_ranges() still widens to
EXPANDED_ELO_RANGE after EXPAND_AFTER_SECS; update the code paths that call
find_rated_match to use this normalized request.max_elo_diff (reference: the
MatchType::Rated block, request.max_elo_diff, INITIAL_ELO_RANGE,
EXPANDED_ELO_RANGE, expand_elo_ranges(), and find_rated_match).
- Around line 505-513: The expand_elo_ranges() method is never scheduled, so
implement a periodic background task that calls it (e.g., every 5–10s) when the
matchmaking service is constructed or started: create a tokio::spawned async
loop that uses tokio::time::interval (Duration::from_secs(5)) and on each tick
clones the service handle (Arc/clone of the struct) and awaits
service.expand_elo_ranges(). Ensure you start this task from the service
initialization function (e.g., MatchmakingService::new or
MatchmakingService::start_background_tasks), handle/ log Result errors from
expand_elo_ranges() instead of panicking, and consider using a Weak reference or
shutdown signal to avoid keeping the service alive indefinitely.
In `@backend/modules/st_core/src/event_listener.rs`:
- Around line 166-170: The current single shared cursor (cursor) causes missed
events when different contract_ids progress independently; replace the single
Arc<RwLock<String>> cursor with a per-contract map like
Arc<RwLock<HashMap<ContractId, String>>> (or HashMap<String, String> if
contract_id is a string) and update all usages: read the cursor for the specific
contract_id before making the request, initialize to "now" if absent, pass that
per-contract cursor to the request, and after handling the batch update only
that contract_id's entry with the new cursor; ensure reads/writes use the
existing Arc<RwLock<...>> pattern and clone the per-contract string when needed
so concurrency semantics remain correct.
- Around line 318-343: The current is_stake_event method incorrectly decodes the
base64 topic into UTF-8 and uses substring matching; instead decode the topic
bytes with general_purpose::STANDARD, parse the bytes as XDR ScVal using
stellar_xdr::ScVal::read_xdr (ensure the crate has ["std","base64"] features),
then match the ScVal::Symbol variant and compare that Symbol for exact equality
against your STAKE_TOPIC_SYMBOLS entries (replace the String/substring logic
around event.topic.first() and the decoded.contains(...) call); on any
decode/parse mismatch return false so only exact Symbol matches classify as
stake events.
- Around line 184-185: The poll_events() loop currently uses http_client:
reqwest::Client::new() and propagates errors with `?`, letting a single contract
failure abort the entire cycle; change this to enforce a per-contract timeout
and isolate errors so one bad contract doesn't stop the loop. For each contract
request made inside poll_events(), wrap the HTTP call in a per-request timeout
(e.g., tokio::time::timeout or reqwest client-level per-request timeout),
replace `?` propagation with per-contract match/error handling, explicitly
handle non-success HTTP statuses and JSON deserialization errors (log the
contract id/address and error), and continue to the next contract on any failure
instead of returning from poll_events(). Ensure the http_client usage and the
specific request/response parsing sites referenced in poll_events() are updated
to catch and log timeout, network, and deserialization errors and to proceed
with the remaining contracts.
In `@backend/src/socket/main.rs`:
- Line 59: The call site passes a third argument listener_handle but
websocket.rs defines handle_connection(stream: TcpStream, addr: SocketAddr) with
only two parameters; update the function signature in
backend/src/socket/websocket.rs to accept a third parameter named
listener_handle with the same type as the listener_handle variable used by the
caller (i.e., add listener_handle: <same type as caller>) and propagate that
parameter into any internal calls or mark it _listener_handle if unused so it
compiles; ensure any other call sites are updated to match the new signature.
In `@frontend/components/chess/GameHistoryPGNViewer.tsx`:
- Around line 295-304: The global keydown handler in the useEffect is listening
on window and interfering with inputs; change it to attach/detach the handler on
the component's focusable root element instead: create a container ref (e.g.,
containerRef) for the outer component, add tabIndex={0} to that container so it
can receive focus, and in useEffect add the handler to containerRef.current (not
window) and remove it on cleanup; keep using the same handler that calls goBack,
goForward, goToStart, goToEnd and still call e.preventDefault() only when the
container is focused so other page elements are unaffected.
---
Outside diff comments:
In `@backend/modules/matchmaking/service.rs`:
- Around line 523-548: The non-atomic remove-then-add (conn.zrem + conn.zadd) in
the match expansion path (around the MatchRequest::from_redis_value ->
to_redis_value flow) can reintroduce a stale request if another task (e.g.,
find_rated_match or cancel_request) removed the member between your snapshot and
these calls; replace the two separate calls with a single Redis Lua script
executed via conn.eval/eval_async that: checks ZSCORE(key, member) to ensure the
member still exists at the expected score, if present performs the ZREM and ZADD
with the updated_value at the same score, and returns a success code (e.g., 1)
or 0 if the member was absent; then treat a 0 return as a no-op (do not re-add
or error), and propagate errors on script failure. Ensure the script uses the
same key, member, and score variables you currently pass (key, member, score)
and update the code that currently calls conn.zrem and conn.zadd to call and
handle the script result instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e848063-929d-45e1-9c02-cf78ab8264fb
📒 Files selected for processing (6)
backend/modules/matchmaking/service.rsbackend/modules/st_core/Cargo.tomlbackend/modules/st_core/src/event_listener.rsbackend/modules/st_core/src/lib.rsbackend/src/socket/main.rsfrontend/components/chess/GameHistoryPGNViewer.tsx
| match request.match_type { | ||
| MatchType::Rated => { | ||
| // Set the initial Elo search window for this player. | ||
| // expand_elo_ranges() will widen it to EXPANDED_ELO_RANGE after | ||
| // EXPAND_AFTER_SECS seconds of waiting. | ||
| if request.max_elo_diff.is_none() { | ||
| request.max_elo_diff = Some(INITIAL_ELO_RANGE); | ||
| } |
There was a problem hiding this comment.
Don’t let clients opt out of the fixed 100→200 Elo window.
backend/modules/matchmaking/routes.rs:53-73 forwards req.max_elo_diff unchanged, so this is_none() check still lets a rated caller submit 500 and bypass the new two-stage policy entirely. That reopens wide immediate matches the new constants/tests are trying to prevent. Normalize rated requests here regardless of client input, or clamp them to the allowed stages before calling find_rated_match.
One straightforward normalization
- if request.max_elo_diff.is_none() {
- request.max_elo_diff = Some(INITIAL_ELO_RANGE);
- }
+ request.max_elo_diff = Some(INITIAL_ELO_RANGE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/matchmaking/service.rs` around lines 59 - 66, The rated-match
branch currently only sets request.max_elo_diff when None, allowing callers to
bypass the two-stage Elo window; change the logic in the MatchType::Rated branch
so that request.max_elo_diff is explicitly normalized/clamped to
INITIAL_ELO_RANGE (not left as the client's value) and ensure
expand_elo_ranges() still widens to EXPANDED_ELO_RANGE after EXPAND_AFTER_SECS;
update the code paths that call find_rated_match to use this normalized
request.max_elo_diff (reference: the MatchType::Rated block,
request.max_elo_diff, INITIAL_ELO_RANGE, EXPANDED_ELO_RANGE,
expand_elo_ranges(), and find_rated_match).
| /// Expand the Elo search window for players who have been waiting in the | ||
| /// rated queue for longer than EXPAND_AFTER_SECS (30 seconds). | ||
| /// | ||
| /// Called periodically by a background task (e.g. every 5–10 seconds). | ||
| /// | ||
| /// Expansion is a single step: INITIAL_ELO_RANGE → EXPANDED_ELO_RANGE. | ||
| /// This gives players a fair shot at a close match first, then guarantees | ||
| /// they will eventually be paired once the wider window opens up. | ||
| pub async fn expand_elo_ranges(&self) -> Result<(), String> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect at least one non-test call site that schedules MatchmakingService::expand_elo_ranges().
rg -nP --type=rust -C2 '\.\s*expand_elo_ranges\s*\('Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Confirm the function exists and search for background task scheduling
echo "=== Checking if expand_elo_ranges is defined ==="
rg -n "fn expand_elo_ranges" --type=rust backend/modules/matchmaking/
echo -e "\n=== Searching for background task/scheduler setup ==="
rg -n "spawn\|tokio::spawn\|background\|scheduled\|periodic" --type=rust backend/modules/matchmaking/ | head -30
echo -e "\n=== Searching for EXPAND_AFTER_SECS constant ==="
rg -n "EXPAND_AFTER_SECS" --type=rust backend/Repository: NOVUS-X/XLMate
Length of output: 992
🏁 Script executed:
#!/bin/bash
# Look at the expand_elo_ranges implementation
echo "=== Implementation of expand_elo_ranges ==="
sed -n '513,550p' backend/modules/matchmaking/service.rs
echo -e "\n=== All calls to expand_elo_ranges (any pattern) ==="
rg -n "expand_elo_ranges" --type=rust backend/
echo -e "\n=== Check main.rs or initialization code for task scheduling ==="
find backend -name "main.rs" -o -name "lib.rs" | head -5 | xargs -I {} sh -c 'echo "File: {}"; grep -n "expand_elo_ranges\|MatchmakingService\|spawn.*match" {} 2>/dev/null | head -20'
echo -e "\n=== Look for any interval/timer related code in matchmaking module ==="
rg -n "interval\|timer\|tick" --type=rust backend/modules/matchmaking/Repository: NOVUS-X/XLMate
Length of output: 2585
🏁 Script executed:
#!/bin/bash
# Search more broadly for background task management and the MatchmakingService struct
echo "=== Look for MatchmakingService usage and initialization ==="
rg -n "MatchmakingService" --type=rust backend/ | head -30
echo -e "\n=== Search for background task spawn patterns ==="
rg -n "tokio::spawn\|task::spawn\|\.spawn\|background" --type=rust backend/ | head -40
echo -e "\n=== Check for any scheduled or periodic job runners ==="
rg -n "schedule\|cron\|repeat\|loop.*expand" --type=rust backend/ | head -20
echo -e "\n=== Look at main.rs files to see how services are initialized ==="
find backend -name "main.rs" -type f | head -3 | xargs catRepository: NOVUS-X/XLMate
Length of output: 1551
🏁 Script executed:
#!/bin/bash
# Look at the server.rs file more closely, especially around MatchmakingService initialization
echo "=== Check server.rs for MatchmakingService setup and any background task logic ==="
rg -B5 -A10 "MatchmakingService::new" backend/modules/api/src/server.rs
echo -e "\n=== Look for any background task setup in the entire api module ==="
rg -n "spawn\|task\|interval\|schedule" --type=rust backend/modules/api/src/
echo -e "\n=== Check if there's any .run() or initialization pattern for MatchmakingService ==="
rg -n "\.run\|\.start\|\.init" --type=rust backend/modules/matchmaking/Repository: NOVUS-X/XLMate
Length of output: 872
🏁 Script executed:
#!/bin/bash
# Final comprehensive check - look for expand_elo_ranges being called anywhere
echo "=== Final: all occurrences of expand_elo_ranges in entire repository ==="
rg "expand_elo_ranges" --type=rust backend/ --no-heading
echo -e "\n=== Check if MatchmakingService has any background task methods ==="
rg -B2 -A5 "impl MatchmakingService" --type=rust backend/modules/matchmaking/service.rs | head -40
echo -e "\n=== Look at the entire MatchmakingService impl block to see what public methods exist ==="
sed -n '36,570p' backend/modules/matchmaking/service.rs | grep "pub" | head -20Repository: NOVUS-X/XLMate
Length of output: 1306
The expand_elo_ranges() function is never called anywhere in the codebase.
Despite the public function being defined and its docstring stating it should be "called periodically by a background task (e.g. every 5–10 seconds)", no background task exists and no code invokes this function. Rated players will remain at INITIAL_ELO_RANGE indefinitely—the 30-second widening feature is completely non-functional. You need to implement the background task that periodically calls expand_elo_ranges().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/matchmaking/service.rs` around lines 505 - 513, The
expand_elo_ranges() method is never scheduled, so implement a periodic
background task that calls it (e.g., every 5–10s) when the matchmaking service
is constructed or started: create a tokio::spawned async loop that uses
tokio::time::interval (Duration::from_secs(5)) and on each tick clones the
service handle (Arc/clone of the struct) and awaits service.expand_elo_ranges().
Ensure you start this task from the service initialization function (e.g.,
MatchmakingService::new or MatchmakingService::start_background_tasks), handle/
log Result errors from expand_elo_ranges() instead of panicking, and consider
using a Weak reference or shutdown signal to avoid keeping the service alive
indefinitely.
| /// Horizon event pagination cursor. Starts at "now" so we only process | ||
| /// events that arrive after the listener starts, then advances with each | ||
| /// batch so events are never processed twice. | ||
| cursor: Arc<RwLock<String>>, | ||
| /// Reusable HTTP client — keeps a connection pool alive across polls. |
There was a problem hiding this comment.
Use per-contract cursors; one global cursor can miss valid events.
Line 169 stores a single cursor for all contracts, then Line 277 reuses it across every contract_id request. If one contract advances further, others can skip unseen events and never transition.
💡 Suggested fix (per-contract cursor map)
pub struct SorobanEventListener {
@@
- cursor: Arc<RwLock<String>>,
+ cursors: Arc<RwLock<HashMap<String, String>>>,
@@
pub fn new(horizon_url: impl Into<String>) -> Self {
Self {
horizon_url: horizon_url.into(),
tracked_matches: Arc::new(RwLock::new(HashMap::new())),
- cursor: Arc::new(RwLock::new("now".to_string())),
+ cursors: Arc::new(RwLock::new(HashMap::new())),
http_client: reqwest::Client::new(),
}
}
@@
let mut matches = self.tracked_matches.write().await;
matches.insert(contract_id.clone(), tracked);
+ let mut cursors = self.cursors.write().await;
+ cursors.entry(contract_id.clone()).or_insert_with(|| "now".to_string());
@@
pub async fn untrack_match(&self, contract_id: &str) {
let mut matches = self.tracked_matches.write().await;
if matches.remove(contract_id).is_some() {
+ let mut cursors = self.cursors.write().await;
+ cursors.remove(contract_id);
log::info!("[EventListener] Untracked contract {}", contract_id);
}
}
@@
- let cursor = self.cursor.read().await.clone();
-
for contract_id in contract_ids {
+ let cursor = {
+ let cursors = self.cursors.read().await;
+ cursors
+ .get(&contract_id)
+ .cloned()
+ .unwrap_or_else(|| "now".to_string())
+ };
let url = format!(
"{}/events?type=contract&contract_id={}&cursor={}&limit={}",
self.horizon_url, contract_id, cursor, HORIZON_EVENT_LIMIT
);
@@
- {
- let mut cursor_guard = self.cursor.write().await;
- *cursor_guard = event.paging_token.clone();
- }
+ let mut cursors = self.cursors.write().await;
+ cursors.insert(contract_id.clone(), event.paging_token.clone());Also applies to: 277-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/st_core/src/event_listener.rs` around lines 166 - 170, The
current single shared cursor (cursor) causes missed events when different
contract_ids progress independently; replace the single Arc<RwLock<String>>
cursor with a per-contract map like Arc<RwLock<HashMap<ContractId, String>>> (or
HashMap<String, String> if contract_id is a string) and update all usages: read
the cursor for the specific contract_id before making the request, initialize to
"now" if absent, pass that per-contract cursor to the request, and after
handling the batch update only that contract_id's entry with the new cursor;
ensure reads/writes use the existing Arc<RwLock<...>> pattern and clone the
per-contract string when needed so concurrency semantics remain correct.
| http_client: reqwest::Client::new(), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the event_listener.rs file
find . -name "event_listener.rs" -path "*/st_core/*" | head -20Repository: NOVUS-X/XLMate
Length of output: 105
🏁 Script executed:
# Let's check the exact lines mentioned in the review
wc -l backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 107
🏁 Script executed:
# Read the client creation section
sed -n '175,200p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 1082
🏁 Script executed:
# Read the loop section with error handling
sed -n '275,310p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 1325
🏁 Script executed:
# Let's see the function signature and understand the context better
sed -n '260,280p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 820
🏁 Script executed:
# Let's also check where this function is called to understand if it's a critical polling loop
rg -n "poll_events" backend/modules/st_core/src/event_listener.rs | head -20Repository: NOVUS-X/XLMate
Length of output: 235
🏁 Script executed:
# Let's see what calls poll_events and how errors are handled
sed -n '250,260p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 485
🏁 Script executed:
# Let's also check the imports to see if Duration is already imported
head -30 backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 1670
🏁 Script executed:
# Let's check the imports to see what's available
sed -n '1,50p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 2270
🏁 Script executed:
# Verify the exact line numbers for the error handling locations
sed -n '283,299p' backend/modules/st_core/src/event_listener.rsRepository: NOVUS-X/XLMate
Length of output: 619
Add per-contract timeout and error isolation to prevent one failing contract from blocking the entire poll cycle.
The client at line 184 has no timeout, and the ? operators at lines 285 and 296 propagate errors up the call stack. If any single contract's HTTP request hangs, times out, or fails to deserialize, the entire poll_events() cycle aborts—skipping all remaining contracts until the next 5-second poll interval. This creates a cascading delay where a slow or flaky Horizon endpoint can prevent timely polling of healthy contracts.
The error handling at lines 289–293 only catches HTTP error status codes; network timeouts and deserialization failures bypass it entirely.
🛠️ Suggested resilience patch
pub fn new(horizon_url: impl Into<String>) -> Self {
Self {
horizon_url: horizon_url.into(),
tracked_matches: Arc::new(RwLock::new(HashMap::new())),
cursor: Arc::new(RwLock::new("now".to_string())),
- http_client: reqwest::Client::new(),
+ http_client: reqwest::Client::builder()
+ .timeout(Duration::from_secs(10))
+ .build()
+ .expect("failed to build reqwest client"),
}
}
@@
- let response = self.http_client.get(&url).send().await?;
+ let response = match self.http_client.get(&url).send().await {
+ Ok(resp) => resp,
+ Err(e) => {
+ log::warn!(
+ "[EventListener] Request failed for contract {}: {}",
+ contract_id, e
+ );
+ continue;
+ }
+ };
@@
- let body: HorizonEventsResponse = response.json().await?;
+ let body: HorizonEventsResponse = match response.json().await {
+ Ok(body) => body,
+ Err(e) => {
+ log::warn!(
+ "[EventListener] Invalid JSON for contract {}: {}",
+ contract_id, e
+ );
+ continue;
+ }
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/st_core/src/event_listener.rs` around lines 184 - 185, The
poll_events() loop currently uses http_client: reqwest::Client::new() and
propagates errors with `?`, letting a single contract failure abort the entire
cycle; change this to enforce a per-contract timeout and isolate errors so one
bad contract doesn't stop the loop. For each contract request made inside
poll_events(), wrap the HTTP call in a per-request timeout (e.g.,
tokio::time::timeout or reqwest client-level per-request timeout), replace `?`
propagation with per-contract match/error handling, explicitly handle
non-success HTTP statuses and JSON deserialization errors (log the contract
id/address and error), and continue to the next contract on any failure instead
of returning from poll_events(). Ensure the http_client usage and the specific
request/response parsing sites referenced in poll_events() are updated to catch
and log timeout, network, and deserialization errors and to proceed with the
remaining contracts.
| /// Soroban events encode topics as base64 XDR ScVal. This function decodes | ||
| /// the first topic and checks whether it contains one of the stake symbols | ||
| /// defined in `STAKE_TOPIC_SYMBOLS`. Adjust those constants to match | ||
| /// whatever your Soroban contract emits. | ||
| fn is_stake_event(&self, event: &HorizonEvent, expected_contract: &str) -> bool { | ||
| if event.event_type != "contract" { | ||
| return false; | ||
| } | ||
|
|
||
| // Verify the event came from the contract we are watching | ||
| if event.contract_id.as_deref() != Some(expected_contract) { | ||
| return false; | ||
| } | ||
|
|
||
| // Decode the first topic (ScVal symbol) and check it against our list | ||
| event.topic.first().map_or(false, |topic_b64| { | ||
| general_purpose::STANDARD | ||
| .decode(topic_b64) | ||
| .map(|bytes| { | ||
| let decoded = String::from_utf8_lossy(&bytes); | ||
| STAKE_TOPIC_SYMBOLS | ||
| .iter() | ||
| .any(|symbol| decoded.contains(symbol)) | ||
| }) | ||
| .unwrap_or(false) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the canonical way to decode Horizon /events contract topics (base64 XDR ScVal) in Rust, and how should symbol topic matching be performed exactly (equality vs substring)?
💡 Result:
The canonical way to decode Horizon /events contract topics (base64-encoded XDR ScVal) in Rust is to use the stellar-xdr crate to decode the base64 string into a Vec<ScVal> (for topics) and ScVal (for data). 1. Add stellar-xdr = "26" (with features ["std", "base64"]) to Cargo.toml. 2. Decode example: use stellar_xdr::{ScVal, ReadXdr}; let base64_topics = "topic_base64_string"; // from Horizon /events response let bytes = base64::decode(base64_topics).unwrap; let mut cursor = std::io::Cursor::new(bytes); let topics: Vec<ScVal> = ReadXdr::read_xdr(&mut cursor).unwrap; // Similarly for data: ScVal::read_xdr(&mut cursor_for_data) ScVal contains variants like Symbol(String), Address, U64, etc. Use pattern matching or TryFromVal (from soroban-sdk) for further conversion to Rust types like Symbol. For symbol topic matching: Use exact equality on the first topic (Symbol), which is the canonical event name (snake_case struct name from #[contractevent]). Do not use substring matching; events are identified by exact Symbol match on topic[0], with subsequent topics as indexed parameters. Confirmed by SEP-48 event spec discussions and soroban-sdk contractevent macro behavior (topics[23]).
Citations:
- 1: https://github.com/stellar/rs-stellar-xdr
- 2: https://crates.io/crates/stellar-xdr
- 3: https://developers.stellar.org/docs/data/analytics/hubble/data-catalog/data-dictionary/history-contract-events
- 4: https://docs.rs/soroban-sdk/latest/soroban_sdk/attr.contractevent.html
- 5: https://docs.rs/soroban-sdk/latest/soroban_sdk/events/struct.Events.html
- 6: https://docs.rs/soroban-sdk/latest/soroban_sdk/xdr/enum.ScVal.html
Use stellar-xdr crate to properly decode ScVal XDR and match topics with exact equality, not substring.
Line 337–340 incorrectly decodes base64-encoded XDR ScVal as UTF-8 text and uses substring matching. Per SEP-48 and soroban-sdk specifications, topics must be decoded using the stellar-xdr crate (with features ["std", "base64"]) and matched with exact equality on the Symbol type. Substring matching causes false positives/negatives and misclassifies events. Implement using stellar_xdr::ScVal::read_xdr() and compare the decoded Symbol exactly, not as substring.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/st_core/src/event_listener.rs` around lines 318 - 343, The
current is_stake_event method incorrectly decodes the base64 topic into UTF-8
and uses substring matching; instead decode the topic bytes with
general_purpose::STANDARD, parse the bytes as XDR ScVal using
stellar_xdr::ScVal::read_xdr (ensure the crate has ["std","base64"] features),
then match the ScVal::Symbol variant and compare that Symbol for exact equality
against your STAKE_TOPIC_SYMBOLS entries (replace the String/substring logic
around event.topic.first() and the decoded.contains(...) call); on any
decode/parse mismatch return false so only exact Symbol matches classify as
stake events.
| let listener_handle = Arc::clone(&listener); | ||
|
|
||
| tokio::spawn(async move { | ||
| if let Err(e) = handle_connection(stream, peer_addr, listener_handle).await { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== handle_connection definition =="
rg -nP 'pub\s+async\s+fn\s+handle_connection\s*\(' backend/src/socket/websocket.rs -A6 -B2
echo
echo "== handle_connection call sites =="
rg -nP '\bhandle_connection\s*\(' backend/src/socket/main.rs -A2 -B2Repository: NOVUS-X/XLMate
Length of output: 717
handle_connection signature mismatch prevents compilation.
Line 59 calls handle_connection(stream, peer_addr, listener_handle) with 3 arguments, but the function in backend/src/socket/websocket.rs accepts only 2: handle_connection(stream: TcpStream, addr: SocketAddr). Update the function signature to accept the listener_handle parameter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/socket/main.rs` at line 59, The call site passes a third argument
listener_handle but websocket.rs defines handle_connection(stream: TcpStream,
addr: SocketAddr) with only two parameters; update the function signature in
backend/src/socket/websocket.rs to accept a third parameter named
listener_handle with the same type as the listener_handle variable used by the
caller (i.e., add listener_handle: <same type as caller>) and propagate that
parameter into any internal calls or mark it _listener_handle if unused so it
compiles; ensure any other call sites are updated to match the new signature.
| useEffect(() => { | ||
| const handler = (e: KeyboardEvent) => { | ||
| if (e.key === "ArrowLeft") { e.preventDefault(); goBack(); } | ||
| if (e.key === "ArrowRight") { e.preventDefault(); goForward(); } | ||
| if (e.key === "Home") { e.preventDefault(); goToStart(); } | ||
| if (e.key === "End") { e.preventDefault(); goToEnd(); } | ||
| }; | ||
| window.addEventListener("keydown", handler); | ||
| return () => window.removeEventListener("keydown", handler); | ||
| }, [goBack, goForward, goToStart, goToEnd]); |
There was a problem hiding this comment.
Global keyboard listener may interfere with other page elements.
The keyboard handler is attached to window, which captures arrow keys globally—even when users are typing in text inputs or interacting with other components elsewhere on the page. The preventDefault() calls will break cursor navigation in input fields.
Consider scoping the listener to the component container with tabIndex={0} and focus management:
🛠️ Suggested fix: Scope keyboard navigation to focused container
+ const containerRef = useRef<HTMLDivElement>(null);
// Keyboard navigation
useEffect(() => {
const handler = (e: KeyboardEvent) => {
+ // Only handle if this component or its children are focused
+ if (!containerRef.current?.contains(document.activeElement) &&
+ document.activeElement !== containerRef.current) {
+ return;
+ }
if (e.key === "ArrowLeft") { e.preventDefault(); goBack(); }
if (e.key === "ArrowRight") { e.preventDefault(); goForward(); }
if (e.key === "Home") { e.preventDefault(); goToStart(); }
if (e.key === "End") { e.preventDefault(); goToEnd(); }
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, [goBack, goForward, goToStart, goToEnd]);Then make the outer container focusable:
return (
- <div className="rounded-xl border border-gray-700 bg-gray-900 text-white overflow-hidden">
+ <div
+ ref={containerRef}
+ tabIndex={0}
+ className="rounded-xl border border-gray-700 bg-gray-900 text-white overflow-hidden focus:outline-none focus-visible:ring-2 focus-visible:ring-indigo-400"
+ >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/chess/GameHistoryPGNViewer.tsx` around lines 295 - 304,
The global keydown handler in the useEffect is listening on window and
interfering with inputs; change it to attach/detach the handler on the
component's focusable root element instead: create a container ref (e.g.,
containerRef) for the outer component, add tabIndex={0} to that container so it
can receive focus, and in useEffect add the handler to containerRef.current (not
window) and remove it on cleanup; keep using the same handler that calls goBack,
goForward, goToStart, goToEnd and still call e.preventDefault() only when the
container is focused so other page elements are unaffected.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
backend/modules/st_core/src/event_listener.rs (3)
167-170:⚠️ Potential issue | 🔴 CriticalGlobal cursor can still drop valid events across contracts.
A single shared cursor is still used for all
contract_ids. If one contract advances farther, other contracts can skip unseen events. Use a per-contract cursor map and update only that contract’s cursor after processing its batch.Suggested fix (per-contract cursors)
- cursor: Arc<RwLock<String>>, + cursors: Arc<RwLock<HashMap<String, String>>>, @@ - cursor: Arc::new(RwLock::new("now".to_string())), + cursors: Arc::new(RwLock::new(HashMap::new())), @@ let mut matches = self.tracked_matches.write().await; matches.insert(contract_id.clone(), tracked); + let mut cursors = self.cursors.write().await; + cursors.entry(contract_id).or_insert_with(|| "now".to_string()); @@ let mut matches = self.tracked_matches.write().await; if matches.remove(contract_id).is_some() { + let mut cursors = self.cursors.write().await; + cursors.remove(contract_id); log::info!("[EventListener] Untracked contract {}", contract_id); } @@ - let cursor = self.cursor.read().await.clone(); - for contract_id in contract_ids { + let cursor = { + let cursors = self.cursors.read().await; + cursors.get(&contract_id).cloned().unwrap_or_else(|| "now".to_string()) + }; let url = format!( @@ - { - let mut cursor_guard = self.cursor.write().await; - *cursor_guard = event.paging_token.clone(); - } + let mut cursors = self.cursors.write().await; + cursors.insert(contract_id.clone(), event.paging_token.clone());Also applies to: 281-287, 306-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/st_core/src/event_listener.rs` around lines 167 - 170, The shared cursor field (cursor: Arc<RwLock<String>>) causes one contract to advance the global cursor and skip events for others; replace it with a per-contract cursor map (e.g., RwLock<HashMap<ContractId, String>> or Arc<RwLock<HashMap<ContractId, String>>>), initialize a cursor for a contract when first seen, read that contract's cursor when fetching its events, and update only that contract's entry after successfully processing its batch (references: the cursor field, the event fetching/processing loop that reads/writes the cursor, and the other uses around the markers noted in the comment). Ensure concurrency by locking the map for read to get a cursor and for write only when inserting/updating a specific contract's cursor.
185-185:⚠️ Potential issue | 🟠 MajorA single request/parse failure still aborts the whole polling cycle.
Per-contract request and JSON failures still bubble out via
?, so one bad contract can block processing of the rest until the next tick. Add timeout + per-contract error isolation (continue).#!/bin/bash # Verify abort-prone spots in poll loop rg -n 'Client::new|send\(\)\.await\?|json\(\)\.await\?' backend/modules/st_core/src/event_listener.rsAlso applies to: 289-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/st_core/src/event_listener.rs` at line 185, The poll loop currently uses reqwest::Client (http_client) and lets per-contract request/JSON failures propagate via `?` (e.g., calls to `send().await?` and `json().await?`), which aborts the whole cycle; wrap each per-contract network/parse operation in a timeout (tokio::time::timeout) and change `?` to per-contract error handling (match/if let Err) so on timeout or request/parse error you log the error and `continue` to the next contract, preserving the overall loop; apply the same fix to the other per-contract request/parse region that uses `send().await`/`json().await` so all failures are isolated.
322-347:⚠️ Potential issue | 🟠 MajorStake-topic detection still treats XDR as UTF-8 text and uses substring matching.
Horizon topics are base64-encoded XDR
ScVal; decoding to UTF-8 andcontains(...)can misclassify events. Decode XDR and compare exactSymbolequality againstSTAKE_TOPIC_SYMBOLS. The test encoder should also generate XDR-encoded topic payloads, not raw UTF-8 bytes.For the stellar-xdr version used in this repository, what is the exact Rust API to parse a base64 Horizon contract event topic into ScVal::Symbol and perform exact symbol equality matching?Also applies to: 417-419
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/st_core/src/event_listener.rs` around lines 322 - 347, The current is_stake_event function decodes the first topic as UTF-8 and uses substring matching which misparses XDR ScVal; instead decode the base64 topic bytes and XDR-deserialize into a stellar_xdr::ScVal, match ScVal::Symbol(s) exactly against entries in STAKE_TOPIC_SYMBOLS, and return true only on exact equality; update the logic in is_stake_event (and the similar code at lines ~417-419) to handle decode errors gracefully (return false) and change tests to emit proper XDR-encoded ScVal::Symbol topics rather than raw UTF-8 bytes so the new deserialization-based checks are exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/modules/st_core/src/event_listener.rs`:
- Around line 167-170: The shared cursor field (cursor: Arc<RwLock<String>>)
causes one contract to advance the global cursor and skip events for others;
replace it with a per-contract cursor map (e.g., RwLock<HashMap<ContractId,
String>> or Arc<RwLock<HashMap<ContractId, String>>>), initialize a cursor for a
contract when first seen, read that contract's cursor when fetching its events,
and update only that contract's entry after successfully processing its batch
(references: the cursor field, the event fetching/processing loop that
reads/writes the cursor, and the other uses around the markers noted in the
comment). Ensure concurrency by locking the map for read to get a cursor and for
write only when inserting/updating a specific contract's cursor.
- Line 185: The poll loop currently uses reqwest::Client (http_client) and lets
per-contract request/JSON failures propagate via `?` (e.g., calls to
`send().await?` and `json().await?`), which aborts the whole cycle; wrap each
per-contract network/parse operation in a timeout (tokio::time::timeout) and
change `?` to per-contract error handling (match/if let Err) so on timeout or
request/parse error you log the error and `continue` to the next contract,
preserving the overall loop; apply the same fix to the other per-contract
request/parse region that uses `send().await`/`json().await` so all failures are
isolated.
- Around line 322-347: The current is_stake_event function decodes the first
topic as UTF-8 and uses substring matching which misparses XDR ScVal; instead
decode the base64 topic bytes and XDR-deserialize into a stellar_xdr::ScVal,
match ScVal::Symbol(s) exactly against entries in STAKE_TOPIC_SYMBOLS, and
return true only on exact equality; update the logic in is_stake_event (and the
similar code at lines ~417-419) to handle decode errors gracefully (return
false) and change tests to emit proper XDR-encoded ScVal::Symbol topics rather
than raw UTF-8 bytes so the new deserialization-based checks are exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11d4bd48-2e50-4724-96d4-9da2d6dc6d32
📒 Files selected for processing (1)
backend/modules/st_core/src/event_listener.rs
|
@gabito1451 the ci is failing from previous error. please review |
- Define RegistryError enum for structured error reporting - Add Tournament struct and storage keys for tournament management - Implement create_tournament and register_tournament functions - Update existing functions to return Result types instead of panicking - Add comprehensive tests for tournament capacity, duplicate registration, and entry fee validation
- Extract FEN parsing to pure function and memoize with useMemo - Wrap getPieceImage, handleSquareClick, and drag handlers in useCallback - Memoize piece image mapping to prevent recreation on every render - Remove redundant boardState state, use computed memoized value instead - Prevents unnecessary re-renders during timer ticks and parent state changes Ensures smooth 60fps drag-and-drop on mobile browsers without visual stuttering. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/game_contract/src/lib.rs (1)
340-348:⚠️ Potential issue | 🔴 CriticalRemove the duplicated
player2expression in theforfeitwinner branch.The
if player == game.player1arm contains the same.as_ref().ok_or(ContractError::GameFull)?.clone()chain twice. This causes a syntax error preventing compilation. Delete one copy.Suggested fix
let winner = if player == game.player1 { game.player2 .as_ref() .ok_or(ContractError::GameFull)? .clone() - game.player2 - .as_ref() - .ok_or(ContractError::GameFull)? - .clone() } else { game.player1.clone() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 340 - 348, The winner assignment in the forfeit branch duplicates the player2 extraction chain; remove the extra repeated "game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()" so the if arm reads a single expression returning the cloned player2. Update the expression that assigns `winner` (the arm checking `if player == game.player1`) to contain only one `game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` and ensure the surrounding `if`/`else` returns properly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/game_contract/src/lib.rs`:
- Around line 183-191: Remove the stray half-resolved duplicate storage reads:
in each function (join_game, claim_draw, forfeit, payout, get_game) delete the
earlier incomplete binding "let mut games: Map<u64, Game> =
env.storage().instance().get(&GAMES)" and keep the complete form that ends with
".ok_or(ContractError::GameNotFound)?"; similarly in get_all_games remove the
duplicated tail expression so there is a single correct read of GAMES. Ensure
only one let mut games: Map<u64, Game> is present per function and that it uses
env.storage().instance().get(&GAMES).ok_or(ContractError::GameNotFound)? (or the
appropriate result-handling) so the code compiles.
- Around line 176-179: The create_game function contains leftover Git merge
conflict markers and a duplicated Ok(game_counter) return; remove the conflict
markers (<<<<<<<, =======, >>>>>>>) and the extra duplicate Ok(game_counter) so
that only a single return remains (keep the original Ok(game_counter) return
used at the end of create_game) and ensure the function compiles cleanly.
- Around line 849-853: Remove the merge conflict markers and the incorrect
initialize(...) call, replacing it with the contract's actual initializer
initialize_puzzle_rewards(...) so the call matches the exposed function;
specifically delete the <<<<<<<, =======, and >>>>>>> lines and ensure the
client.initialize_puzzle_rewards invocation (with the same arguments previously
passed to initialize) remains so the code compiles against the contract's
initialize_puzzle_rewards function.
In `@frontend/app/page.tsx`:
- Around line 71-84: Remove the orphaned/malformed useEffect fragment that
references fetchOnlinePlayers, isMounted, and PLAYER_COUNT_ENDPOINT but has no
matching useEffect opening, and also remove the duplicated redirect effect that
checks matchmakingStatus/gameId/router; locate and delete the block containing
the stray lines referencing fetchOnlinePlayers(), isMounted = false,
clearInterval(intervalId), the unmatched "}, [PLAYER_COUNT_ENDPOINT]);" and the
subsequent useEffect that redirects on matchmakingStatus === "match_found"
(duplicate of the existing effect handling matchmakingStatus/gameId/router),
leaving only the correctly formed effects elsewhere in page.tsx.
---
Outside diff comments:
In `@contracts/game_contract/src/lib.rs`:
- Around line 340-348: The winner assignment in the forfeit branch duplicates
the player2 extraction chain; remove the extra repeated
"game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()" so the if arm
reads a single expression returning the cloned player2. Update the expression
that assigns `winner` (the arm checking `if player == game.player1`) to contain
only one `game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` and
ensure the surrounding `if`/`else` returns properly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 753d89b4-8861-4629-b47c-d57e32b39942
📒 Files selected for processing (2)
contracts/game_contract/src/lib.rsfrontend/app/page.tsx
Replace with to resolve mismatched type errors. ApiError::DatabaseError expects DbErr, not String.
- Remove Git conflict markers in test setup - Fix duplicated map retrieval lines in multiple functions - Correct function call from initialize() to initialize_puzzle_rewards() - Remove duplicate return statement in get_all_games()
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/game_contract/src/lib.rs (1)
324-335:⚠️ Potential issue | 🔴 CriticalRemove duplicate winner extraction — code does not compile.
Lines 329–332 duplicate the
game.player2.as_ref()...clone()expression from lines 325–328 without a separating semicolon, causing the parse errorexpected ';', found 'game'. This is a merge-conflict artifact.🔧 Proposed fix
let winner = if player == game.player1 { game.player2 .as_ref() .ok_or(ContractError::GameFull)? .clone() - game.player2 - .as_ref() - .ok_or(ContractError::GameFull)? - .clone() } else { game.player1.clone() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 324 - 335, The winner extraction contains a duplicated expression causing a parse error; in the conditional assigning winner (the `winner` variable) remove the duplicated `game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` fragment so the `if player == game.player1 { ... } else { ... }` expression yields a single value — keep one call to `game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` inside the true branch and ensure the else branch returns `game.player1.clone()`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/modules/chess/src/rating.rs`:
- Line 67: The closure mapping DbErr to ApiError is wrong:
ApiError::DatabaseError expects a DbErr argument but the code uses `.map_err(|e|
ApiError::DatabaseError)`. Remove the `.map_err(...)` calls on the database
result chains (e.g., the expressions using map_err around database calls) and
let the `?` operator convert DbErr to ApiError via the existing `impl
From<DbErr> for ApiError`; targets include occurrences of `map_err(|e|
ApiError::DatabaseError)` and similar patterns so functions calling the DB
simply `?` the result instead.
In `@contracts/game_contract/src/lib.rs`:
- Around line 3-8: The import block in the module duplicates the same symbols
from soroban_sdk (Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract,
contracterror, contractimpl, contracttype, symbol_short); remove the duplicated
use line (the second repeated use soroban_sdk {...} entry) so each symbol is
imported only once (locate the duplicated use statement around the top-level
module where those symbols are listed and delete the repeated block).
---
Outside diff comments:
In `@contracts/game_contract/src/lib.rs`:
- Around line 324-335: The winner extraction contains a duplicated expression
causing a parse error; in the conditional assigning winner (the `winner`
variable) remove the duplicated
`game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` fragment so the
`if player == game.player1 { ... } else { ... }` expression yields a single
value — keep one call to
`game.player2.as_ref().ok_or(ContractError::GameFull)?.clone()` inside the true
branch and ensure the else branch returns `game.player1.clone()`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f96ccd12-1cc8-49d7-b4f1-c613d0e2ddee
📒 Files selected for processing (3)
backend/modules/chess/src/rating.rscontracts/game_contract/src/lib.rsfrontend/app/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/page.tsx
| use soroban_sdk::{ | ||
| Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, | ||
| contracttype, symbol_short, | ||
| Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, | ||
| contracttype, symbol_short, | ||
| }; |
There was a problem hiding this comment.
Remove duplicate imports — code does not compile.
Lines 6–7 re-import the same symbols already imported on lines 4–5, triggering E0252 errors for every duplicated identifier. This is a merge-conflict artifact.
🔧 Proposed fix
use soroban_sdk::{
Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl,
contracttype, symbol_short,
- Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl,
- contracttype, symbol_short,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use soroban_sdk::{ | |
| Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, | |
| contracttype, symbol_short, | |
| Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, | |
| contracttype, symbol_short, | |
| }; | |
| use soroban_sdk::{ | |
| Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, | |
| contracttype, symbol_short, | |
| }; |
🧰 Tools
🪛 GitHub Actions: Contracts CI
[error] 6-6: Rust error E0252: Address is defined multiple times; Address is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: Bytes is defined multiple times; Bytes is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: BytesN is defined multiple times; BytesN is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: Env is defined multiple times; Env is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: Map is defined multiple times; Map is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: Symbol is defined multiple times; Symbol is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: Vec is defined multiple times; Vec is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: contract macro is defined multiple times; contract is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: contracterror macro is defined multiple times; contracterror is reimported here (help: remove unnecessary import).
[error] 6-6: Rust error E0252: contractimpl macro is defined multiple times; contractimpl is reimported here (help: remove unnecessary import).
[error] 7-7: Rust error E0252: contracttype is defined multiple times; contracttype is reimported here (help: remove unnecessary import).
[error] 7-7: Rust error E0252: symbol_short is defined multiple times; symbol_short is reimported here (help: remove unnecessary import).
[warning] 4-7: Warning: unused imports including Address, Bytes, BytesN, Env, Map, Symbol, Vec, contract, contracterror, contractimpl, contracttype, and symbol_short.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/game_contract/src/lib.rs` around lines 3 - 8, The import block in
the module duplicates the same symbols from soroban_sdk (Address, Bytes, BytesN,
Env, Map, Symbol, Vec, contract, contracterror, contractimpl, contracttype,
symbol_short); remove the duplicated use line (the second repeated use
soroban_sdk {...} entry) so each symbol is imported only once (locate the
duplicated use statement around the top-level module where those symbols are
listed and delete the repeated block).
9 participants
Description
Adds
SorobanEventListener— a Tokio background worker that polls Horizonfor on-chain staking events and transitions matches from
AWAITING_STAKEto
IN_PROGRESSonly when the proper Soroban event fires. The start signalis broadcast to WebSocket handlers via
tokio::sync::broadcast.Related Issue
Fixes #157
Type of Change
Changes Made
backend/modules/st_core/src/event_listener.rswithSorobanEventListener,MatchStartSignal, andStakeStatus/events?type=contractevery 5s usingtokio::time::intervalArc<RwLock<HashMap>>STAKE_TOPIC_SYMBOLS(staked,stake_confirmed)InProgressmatchis a no-op
src/lib.rsto expose the new moduleCargo.tomlto addreqwest(HTTP polling) andlogTesting
cargo test -p st_coreruns all 11 unit tests without network calls.Integration testing requires a live Horizon instance with a deployed
staking contract.
Checklist
Additional Notes
STAKE_TOPIC_SYMBOLSinevent_listener.rsto match whatevertopic symbols your Soroban staking contract emits
listener.run()once at server startup in atokio::spawncallPOLL_INTERVAL_SECS = 5— adjust based on acceptable latency vs API loadSummary by CodeRabbit
New Features
Bug Fixes
Chores