Skip to content

Conversation

d4mr
Copy link
Member

@d4mr d4mr commented Oct 9, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Stop sending webhooks when a transaction hash is still pending, reducing false alerts.
    • Provide clearer feedback when a transaction hash isn’t yet available.
  • Chores
    • Update health timestamps after confirmations and nonce resets to improve accuracy of activity and status indicators.

d4mr added 2 commits October 10, 2025 01:15
- Added logic to update health timestamps for nonce movement and confirmations in both the EoaExecutorWorker and ResetNoncesTransaction implementations.
- Ensured health data reflects the latest nonce movement even when confirmations are ahead of the latest state.
- Added a new error variant for pending transaction hashes to improve clarity in error reporting.
- Updated the error handling logic to return the new error when a transaction hash is not yet available, enhancing the overall robustness of the confirmation process.
- Adjusted webhook queuing logic to account for the new pending state, ensuring efficient handling of transaction confirmations.
Copy link

coderabbitai bot commented Oct 9, 2025

Walkthrough

Introduces a new TransactionHashPending error for EIP-7702 confirmation and adjusts NACK/webhook handling. Updates EOA components to record health timestamps (last_nonce_movement_at, last_confirmation_at) on nonce reset validation and after confirmations in the worker. No public API changes except the new error variant.

Changes

Cohort / File(s) Summary
EIP-7702 confirmation error handling
executors/src/eip7702_executor/confirm.rs
Adds Eip7702ConfirmationError::TransactionHashPending with message. Returns this on Pending bundler responses ("Transaction hash not yet available"). Excludes this variant from webhook queuing alongside ReceiptNotAvailable during NACK handling.
EOA health timestamp updates (validation + post-confirm)
executors/src/eoa/store/atomic.rs, executors/src/eoa/worker/confirm.rs
On ResetNoncesTransaction validation, after trimming nonce resets, now updates health.last_nonce_movement_at and health.last_confirmation_at to now. After confirmations, if any succeeded, fetches health, sets the same timestamps to now, persists via update_health_data; logs warning on failure.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Confirm as EIP-7702 Confirm
  participant Bundler
  participant Webhook as Webhook Queue

  Caller->>Confirm: confirm()
  Confirm->>Bundler: getStatus()
  alt Bundler: Success
    Bundler-->>Confirm: Receipt/Hash available
    Confirm-->>Caller: Ok
  else Bundler: Pending
    Bundler-->>Confirm: Pending
    Note right of Confirm: Return new error variant
    Confirm-->>Caller: Err(TransactionHashPending)
    Note over Confirm,Webhook: Do not enqueue webhook for pending hash
  else Bundler: NACK / Error
    Bundler-->>Confirm: NACK/Error
    Confirm->>Webhook: Queue (except Pending/ReceiptNotAvailable)
    Confirm-->>Caller: Err(...)
  end
Loading
sequenceDiagram
  autonumber
  actor Worker
  participant Chain as Chain/Provider
  participant Store as EOA Store
  participant Health as Health Data

  rect rgba(200,230,255,0.25)
  Worker->>Chain: Confirm transactions
  Chain-->>Worker: Results (success/failure)
  Worker->>Worker: Build report
  end

  alt Any successes
    Worker->>Health: fetch()
    Health-->>Worker: current health
    Worker->>Store: update_health_data(last_nonce_movement_at=now, last_confirmation_at=now)
    Store-->>Worker: Ok / Err
    opt On error
      Worker->>Worker: log warning
    end
  else No successes
    Worker->>Worker: skip health update
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title includes a branch prefix and only highlights silencing webhooks, missing the health timestamp updates added in the other files and containing unnecessary noise, so it does not clearly summarize the full scope of changes. Remove the “Pb/” prefix and revise the title to succinctly cover both silencing pending webhooks and updating health timestamps, for example “Silence pending transaction webhooks and update health timestamps on confirmation.”
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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/silence-more-webhooks

📜 Recent 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 436b1f4 and 64b3154.

📒 Files selected for processing (3)
  • executors/src/eip7702_executor/confirm.rs (3 hunks)
  • executors/src/eoa/store/atomic.rs (1 hunks)
  • executors/src/eoa/worker/confirm.rs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
executors/src/eoa/store/atomic.rs (1)
executors/src/eoa/store/mod.rs (1)
  • now (832-834)
executors/src/eoa/worker/confirm.rs (1)
executors/src/eoa/store/mod.rs (1)
  • now (832-834)
🔇 Additional comments (5)
executors/src/eoa/worker/confirm.rs (1)

265-280: LGTM: Health timestamp update after confirmations is appropriate.

The implementation correctly updates health timestamps after successful transaction confirmations. The use of if let Ok(mut health) silently continues if health data doesn't exist, which appears intentional since health tracking is optional. The warning-only error handling for the update failure is also appropriate, as this is a non-critical observability enhancement.

executors/src/eip7702_executor/confirm.rs (3)

84-85: LGTM: New error variant improves state distinction.

The TransactionHashPending variant appropriately distinguishes between temporary pending states and permanent errors, enabling more precise error handling and webhook silencing.


235-238: LGTM: Correct error for pending bundler response.

Returning TransactionHashPending instead of TransactionHashError when the bundler response is Pending accurately reflects the transient nature of this state and enables appropriate retry and webhook handling downstream.


340-344: LGTM: Webhook silencing for pending states reduces noise.

Excluding TransactionHashPending from webhook queuing (alongside ReceiptNotAvailable) correctly prevents spamming webhooks during normal transaction processing delays.

executors/src/eoa/store/atomic.rs (1)

719-721: No other updates to last_confirmation_at exist; updating both timestamps here aligns with current behavior. The reference to confirm.rs is incorrect—there is no such update elsewhere.

Likely an incorrect or invalid review comment.


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

@d4mr d4mr merged commit 0311adc into main Oct 9, 2025
3 checks passed
@d4mr d4mr deleted the pb/silence-more-webhooks branch October 9, 2025 22:30
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