Skip to content

Conversation

@d4mr
Copy link
Contributor

@d4mr d4mr commented Nov 6, 2025

  • Introduced a check to skip gas bump attempts if the transaction has not been queued long enough, enhancing transaction handling efficiency.
  • Added logging to warn when a transaction is skipped due to insufficient queuing time, providing better visibility into transaction states.

Summary by CodeRabbit

  • Bug Fixes
    • Optimized gas bump handling to prevent unnecessary gas increases for recently queued transactions, improving transaction processing efficiency.

- Introduced a check to skip gas bump attempts if the transaction has not been queued long enough, enhancing transaction handling efficiency.
- Added logging to warn when a transaction is skipped due to insufficient queuing time, providing better visibility into transaction states.
@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

Adds a timing guard in the gas bump function to prevent unnecessary gas bumps when a transaction hasn't been queued long enough. The function now checks if time_since_queuing is below NONCE_STALL_LIMIT_MS and skips the gas bump operation accordingly.

Changes

Cohort / File(s) Summary
Gas bump guard logic
executors/src/eoa/worker/confirm.rs
Adds a time-based guard condition in attempt_gas_bump_for_stalled_nonce to skip gas bumping if the queued transaction hasn't reached the minimum stall duration threshold; logs warning and returns Ok(false) when condition is met

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review the timing guard logic and verify NONCE_STALL_LIMIT_MS constant is appropriately defined
  • Confirm the early return and logging behavior align with the intended gas bump prevention strategy

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 directly and clearly describes the main change: adding a nonce stall check to the gas bump logic in EoaExecutorWorker, which matches the PR's core objective.
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/gas-bump-time-limit

📜 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 4512db9 and 51318f9.

📒 Files selected for processing (1)
  • executors/src/eoa/worker/confirm.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-20T05:30:35.171Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/engine-core PR: 48
File: executors/src/eoa/worker/send.rs:20-21
Timestamp: 2025-09-20T05:30:35.171Z
Learning: In executors/src/eoa/worker/send.rs, there is a critical bug where HEALTH_CHECK_INTERVAL is defined as 300 seconds but compared against millisecond timestamps, causing balance checks to occur every 300ms instead of every 5 minutes (1000x more frequent than intended).

Applied to files:

  • executors/src/eoa/worker/confirm.rs
🧬 Code graph analysis (1)
executors/src/eoa/worker/confirm.rs (1)
executors/src/eoa/store/mod.rs (1)
  • now (832-834)
🔇 Additional comments (1)
executors/src/eoa/worker/confirm.rs (1)

379-390: LGTM! Well-designed timing guard for gas bump logic.

The implementation correctly adds a safeguard to prevent gas bumping transactions that haven't been queued long enough. The time unit consistency is correct (both timestamps and the constant use milliseconds), avoiding the unit mismatch bug mentioned in the learnings.

The dual-check design is sound:

  • Outer check (line 84): Ensures the nonce has been stalled for ≥60s overall
  • Inner check (line 381): Ensures the specific transaction has been queued for ≥60s

This handles the scenario where a nonce is stalled but a replacement transaction was recently submitted—the system appropriately waits before bumping the new transaction.

Based on learnings.


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

@d4mr d4mr merged commit c1830ef into main Nov 6, 2025
3 checks passed
@d4mr d4mr deleted the pb/gas-bump-time-limit branch November 6, 2025 08:49
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.

2 participants