Skip to content

Conversation

d4mr
Copy link
Member

@d4mr d4mr commented Oct 4, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Bug Fixes

    • Automatically cleans up stale or incomplete pending transactions, reducing “stuck” entries.
    • Improves handling of missing user requests with non-blocking warnings instead of failures.
  • Refactor

    • Streamlines pending transaction processing for better resilience and performance, reducing error impact during retrieval and processing.

Copy link

coderabbitai bot commented Oct 4, 2025

Walkthrough

Internal refactor of pending transaction retrieval in executors/src/eoa/store/mod.rs: switches from bulk JSON deserialization to per-item streaming processing with optional decoding, adds a deletion cleanup path, and updates handling of missing user_request entries to log a warning and remove the corresponding pending entry. No public API changes.

Changes

Cohort / File(s) Summary
Pending transaction retrieval refactor
executors/src/eoa/store/mod.rs
Replaced upfront JSON deserialization with per-item optional decoding; added conditional deletion/cleanup pipeline; on missing user_request, now logs a warning and removes the pending entry; shifted to streaming processing; no changes to exported/public signatures.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Store as EOA Store
  participant DB as Storage/DB

  Caller->>Store: get_pending_transactions()
  Note over Store: Iterate pending entries (stream)
  loop For each pending entry
    Store->>DB: fetch user_request by key
    alt user_request exists
      Store->>Store: attempt JSON decode (optional)
      alt decode success
        Store-->>Caller: yield EoaTransactionRequest
      else decode fails
        Store->>DB: delete pending entry (cleanup)
        Note over Store,DB: Error-tolerant cleanup on decode failure
      end
    else user_request missing
      Store->>Store: log warning
      Store->>DB: delete pending entry
    end
  end
  Store-->>Caller: stream completes
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely describes the primary change, namely the cleanup of invalid pending transactions in the peek path. It directly aligns with the code refactor that adds conditional deletion and error-tolerant handling of missing transaction data. The phrasing is clear, specific, and avoids extraneous details.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pb/hydration-fix-store-corruption-peek

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c839569 and 130eb9c.

📒 Files selected for processing (1)
  • executors/src/eoa/store/mod.rs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
executors/src/eoa/store/mod.rs (3)
executors/src/external_bundler/deployment.rs (4)
  • conn (40-42)
  • conn (54-54)
  • conn (69-71)
  • conn (197-197)
twmq/src/lib.rs (7)
  • new (132-151)
  • redis (1221-1223)
  • redis (1229-1229)
  • redis (1321-1323)
  • redis (1329-1329)
  • redis (1359-1361)
  • redis (1367-1369)
executors/src/eoa/store/hydrate.rs (1)
  • deletion_pipe (113-114)

Comment on lines +605 to +607
if !deletion_pipe.is_empty() {
deletion_pipe.query_async::<()>(&mut conn).await?;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix deletion pipeline result type to avoid runtime failure

query_async::<()> expects a Redis Nil, but a pipeline of ZREM commands returns a bulk of integer counts, so this call will throw a ResponseError as soon as cleanup runs. Capture the results as a vector (or Value) instead.

-            deletion_pipe.query_async::<()>(&mut conn).await?;
+            let _: Vec<i64> = deletion_pipe.query_async(&mut conn).await?;
📝 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.

Suggested change
if !deletion_pipe.is_empty() {
deletion_pipe.query_async::<()>(&mut conn).await?;
}
if !deletion_pipe.is_empty() {
let _: Vec<i64> = deletion_pipe.query_async(&mut conn).await?;
}
🤖 Prompt for AI Agents
In executors/src/eoa/store/mod.rs around lines 605 to 607, the pipeline call
uses query_async::<()> which expects a Redis Nil but the pipeline of ZREM
commands returns integer counts; change the query type to capture the actual
results (e.g., use query_async::<Vec<redis::Value>>() or
query_async::<Vec<i64>>()) and bind the returned value to a variable (or let _ =
...) instead of expecting unit, so the pipeline response is consumed without
causing a ResponseError.

@d4mr d4mr merged commit 029bc40 into main Oct 4, 2025
3 checks passed
@d4mr d4mr deleted the pb/hydration-fix-store-corruption-peek branch October 4, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant