Skip to content

fix: keep the old sink open when a proof replacement is rejected - #2301

Open
0xrlawrence wants to merge 1 commit into
yetanotherco:stagingfrom
0xrlawrence:fix/batcher-replacement-sink-ordering
Open

fix: keep the old sink open when a proof replacement is rejected#2301
0xrlawrence wants to merge 1 commit into
yetanotherco:stagingfrom
0xrlawrence:fix/batcher-replacement-sink-ordering

Conversation

@0xrlawrence

Copy link
Copy Markdown

Description

handle_replacement_message closed the previous connection's sink before calling replacement_entry_is_valid.

replacement_entry is cloned from the queued entry, so both hold the same Arc<RwLock<...>> sink. When validation fails the function returns with the original entry still in the batch queue — but the client has already been sent ProofReplaced and the shared sink has already been closed. That proof stays queued and paid for, and can no longer report its inclusion to the user.

The fix takes the old sink aside with Option::take, runs the validity check, and only spawns the notify-and-close task once the replacement is actually going into the queue.

replacement_entry_is_valid only inspects sender, nonce and max_fee, so moving it ahead of the sink handling does not change what it decides.

Type of change

  • Bug fix

Checklist

  • “Hotfix” to testnet, everything else to staging

Verified with cargo check -p aligned-batcher, cargo clippy -p aligned-batcher and cargo fmt --check on staging. Clippy reports the same warnings before and after (both at lib.rs:365/:391, unrelated to this change).

handle_replacement_message closed the previous connection's sink before
calling replacement_entry_is_valid. The replacement entry is cloned from
the original, so the two share the same Arc sink. When validation failed
the function returned with the original entry still queued, but its sink
had already been closed and the client had already been told
ProofReplaced, so that proof could never report its inclusion.

Take the old sink aside, run the validity check, and only close it once
the replacement is actually going into the queue.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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