Defer transaction resubmission until scanning completes - #392
Conversation
Move post-batch rebroadcast to after ensure_complete_scan_state, and refuse a tip that advanced on lightwalletd but never landed in the DB. Builds on ValarDragon's #386 deferral of resubmit while scan ranges remain.
There was a problem hiding this comment.
💡 Codex Review
vizor-wallet/rust/src/wallet/sync_engine/mod.rs
Lines 2262 to 2263 in 72e4ae7
When the last post-batch tip refresh fails, or it sees a higher lightwalletd tip but update_chain_tip fails, current_tip_height and the DB remain at the old height and the loop can still complete with no pending ranges for that stale tip. This code then fabricates the “fresh” height from final_tip_height, so authoritative_resubmit_tip cannot reject the stale DB tip and resubmit_pending_transactions can rebroadcast a transaction that was already mined or expired in the unobserved new block. Fetch/update the tip here, or carry forward the unsafe refresh state and skip final resubmit unless the DB tip is authoritative.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
resubmit_pending_transactionsuntil local scan work is drained, so already-mined wallet txs are not rebroadcast whilemined_heightis stillNULL.ensure_complete_scan_state, and refusing a tip that advanced on lightwalletd but never landed viaupdate_chain_tip.Authorship
Original investigation and fix by @ValarDragon in draft #386 (commits authored by Dev Ojha). This PR remakes that work on
chainapsis/vizor-walletand adds a small completion-path hardening follow-up.Please prefer this PR over #386; the draft can be closed once this lands.
Why
During catch-up (especially with 96-block migration retention batches), Vizor refreshed tip and rebroadcast after each batch before scanning that tip. Resubmit eligibility only checks local
mined_height IS NULL, so on-chain txs looked pending and were rebroadcast repeatedly — matching the live pending/Sending-now race analysis.Tests
cd rust && cargo test --lib resubmit_: 16 passed