Skip to content

Persist stuck-transaction monitoring across restarts - #4296

Open
mswilkison wants to merge 2 commits into
mainfrom
codex/persist-transaction-monitor
Open

mswilkison wants to merge 2 commits into
mainfrom
codex/persist-transaction-monitor

Conversation

@mswilkison

@mswilkison mswilkison commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Wallet transactions broadcast before a node restart disappeared from the stuck-transaction monitor. Persist the tracked set in the node's encrypted tBTC work storage and restore it synchronously during node construction, before monitoring or wallet coordination starts.

Original broadcast times and alert state survive restarts, so downtime counts toward the existing stuck threshold and maximum tracking age. Confirmed and expired entries are removed from storage. Separate initial and alerted records preserve the original registration if an alert write is interrupted. Failed saves and deletes are logged and retried in a separate local pass before the confirmation budget starts, so a slow Bitcoin backend cannot starve storage recovery. Corrupt or unreadable records do not prevent recovery of healthy entries, and the existing tracking limit remains enforced.

Update the operator documentation to explain that this work data must survive restarts and cannot be reconstructed after loss. Transactions broadcast by an older, memory-only version cannot be recovered retroactively.

Validation:

  • Monitor regression tests pass, including restart lifecycle, interrupted writes, deletion retries, partial recovery, capacity, and concurrent tracking.
  • The confirmation-timeout regression fails on the previous PR revision and passes with this fix for all three storage operations: initial registration, alert-state updates, and deletion. It verifies recovery after repeated timeouts and a restart.
  • go test -race -ldflags=-checklinkname=0 ./pkg/tbtc -run '^Test(TransactionMonitor|Node_RestoresTransactionMonitor)' -count=1 -timeout=3m passes, including node startup with encrypted storage.
  • go vet ./pkg/tbtc and staticcheck -checks=inherit,-SA1019 ./pkg/tbtc pass; changed Go files are formatted.
  • go test -ldflags=-checklinkname=0 ./pkg/tbtc -count=1 -timeout=15m passes.
  • go vet ./... reports an existing copy-of-lock warning at pkg/tecdsa/signing/protocol.go:737, an unchanged line also present on main.

Fixes #4175.

Summary by CodeRabbit

  • New Features

    • Transaction monitoring state now persists across restarts, including broadcast times and alert status.
    • Monitored transactions are automatically restored from local storage when the node starts.
    • Failed storage operations are retried to help preserve monitoring and cleanup reliability.
  • Documentation

    • Added guidance on transaction-monitor storage, recovery behavior, and the monitoring gap caused by lost records.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: b2cccd08-08ab-4081-b534-b15cdbe9ac12

📥 Commits

Reviewing files that changed from the base of the PR and between a7ac898 and d5eb33d.

📒 Files selected for processing (7)
  • docs/run-keep-node.adoc
  • pkg/tbtc/node.go
  • pkg/tbtc/node_test.go
  • pkg/tbtc/transaction_monitor.go
  • pkg/tbtc/transaction_monitor_storage.go
  • pkg/tbtc/transaction_monitor_storage_test.go
  • pkg/tbtc/transaction_monitor_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The stuck-transaction monitor now persists tracked transactions in work storage, restores them during node construction, and retries failed saves and deletions. Records retain broadcast times and alert state across restarts. Tests cover recovery, validation, failures, capacity, and concurrency.

Changes

Transaction Monitor Persistence

Layer / File(s) Summary
Durable records and restoration
pkg/tbtc/transaction_monitor_storage.go, pkg/tbtc/transaction_monitor_storage_test.go
Adds versioned transaction-monitor records, validation, restoration, duplicate handling, capacity enforcement, and durable deletion. Tests cover lifecycle recovery, malformed records, unrelated directories, and capacity limits.
Monitor lifecycle integration
pkg/tbtc/transaction_monitor.go, pkg/tbtc/node.go
Passes work persistence to the monitor. The monitor restores state at construction, persists registrations and alerts, and retries failed persistence operations before confirmation checks.
Persistence failure and concurrency validation
pkg/tbtc/transaction_monitor_storage_test.go
Tests interrupted writes, failed deletions, partial alert writes, confirmation-budget interaction, and concurrent registrations.
Startup and compatibility validation
pkg/tbtc/node_test.go, pkg/tbtc/transaction_monitor_test.go, docs/run-keep-node.adoc
Verifies node startup restoration, updates constructor calls, and documents transaction-monitor records and restart behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to d5eb3

Persistence, restoration, retry, cleanup, and restart behavior are covered without an identified merge-blocking regression.

Sequence Diagram(s)

sequenceDiagram
  participant Node
  participant TransactionMonitor
  participant WorkPersistence
  participant BitcoinChain
  Node->>TransactionMonitor: Construct with work persistence
  TransactionMonitor->>WorkPersistence: Restore tracked transactions
  TransactionMonitor->>WorkPersistence: Persist registration and alert state
  TransactionMonitor->>BitcoinChain: Check confirmation status
  TransactionMonitor->>WorkPersistence: Retry failed saves or deletions
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 6 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving stuck-transaction monitoring across node restarts.
Linked Issues check ✅ Passed The changes satisfy issue [#4175] by persisting the transaction monitor state, restoring it during node construction, preserving broadcast and alert state, retrying storage failures, and covering rest…
Out of Scope Changes check ✅ Passed The implementation, storage tests, node restart test, monitor test updates, and operator documentation directly support the linked issue and PR objectives. No unrelated changes are evident.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 6 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/persist-transaction-monitor

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.

❤️ Share

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

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.

Persist the stuck-transaction monitor's tracked set across restarts

1 participant