Skip to content

fix: stop infinite miles retry when commitment is never opened#913

Open
owen-eth wants to merge 1 commit intomainfrom
fix-miles-retry-timeout
Open

fix: stop infinite miles retry when commitment is never opened#913
owen-eth wants to merge 1 commit intomainfrom
fix-miles-retry-timeout

Conversation

@owen-eth
Copy link
Copy Markdown
Contributor

@owen-eth owen-eth commented Mar 31, 2026

Describe your changes

When tx goes through fastrpc but the commitment is never opened (builder doesn't open for whatever reason), we were previously retrying indefinitely. This change stops retrying after 15 minutes and processes with a 0 bid cost since no bid payment was sent.

Issue ticket number and link

Fixes # (issue)

Checklist before requesting a review

  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation

@passandscore
Copy link
Copy Markdown
Contributor

Edge Case to Consider:
The one scenario worth monitoring: if a transaction's blockTS is NULL (invalid), the timeout check won't trigger (r.blockTS.Valid would be false), and it will keep retrying indefinitely. You may want to add a safeguard for this, or ensure blockTS is always populated when a tx enters FastRPC.

Looking at insertEvent() in sweep.go (lines 125-162):

var blockTS *time.Time
if header != nil {
    t := time.Unix(int64(header.Time), 0).UTC()
    blockTS = &t
}

// Then in insertEvent:
var tsVal interface{} = nil
if blockTS != nil {
    tsVal = *blockTS
}
The block timestamp IS fetched from the header at insertion time (lines 86-105). So blockTS will be NULL only if the header fetch fails.

The Actual Risk:
However, there IS a latent edge case: if header fetching fails (line 98-100):

Go
if err != nil {
    logger.Warn("header fetch failed", slog.Uint64("block", blockNum), slog.Any("error", err))
}

Then blockTS stays nil, gets inserted as NULL, and the row will retry indefinitely without the 15-minute timeout.

In Practice:
This is unlikely for old blocks, but possible for:

Recent/mempool issues during indexing
RPC node failures during header fetch retries

Recommendation:
Add a safeguard: if header fetch fails after retries, either:

  • Skip the row entirely (don't insert it)
  • Use time.Now() as fallback timestamp
  • Mark it with a flag for manual review

Note: AI recommended

@passandscore passandscore self-requested a review March 31, 2026 18:32
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