Nudge Thunder once per broadcast, not once per tick - #33
Merged
Conversation
Every payout was costing two sidechain blocks instead of one, roughly
doubling settlement latency: ~570s observed against a ~253s mainchain
interval, and 42 Thunder blocks yielding only 25 settlements.
The cause is our own nudge. Thunder's `mine` builds a block body from the
mempool BEFORE it takes the miner lock, then parks that snapshot as its BMM
request the instant the lock frees:
let body = types::Body::new(Vec::new(), coinbase); // snapshot here
let mut miner_write = miner.write().await; // then block
miner_write.attempt_bmm(bribe.to_sat(), 0, header, body)
So a nudge issued while waiting captures a mempool that predates the next
batch, queues behind the in-flight mine(), and becomes the parked request
the moment the current batch confirms. The next batch, broadcast seconds
later on the following tick, cannot be in the block that request produces —
it waits for the one after. On drynet3 Thunder parked 14-93s ahead of the
broadcast it was meant to carry in 7 of 7 observed cycles.
Nudge once per broadcast instead, and force it past the rate limiter: that
is the nudge whose snapshot has to contain the batch just sent, and letting
it be skipped hands the parked slot to a stale body.
Keep a stall-recovery nudge for the case the post-broadcast request is not
carried at all (a BMM miss), gated on PAYOUT_NUDGE_STALL_SEC (default 300s,
~2 mainchain blocks) and still rate-limited. Worst case is therefore the
latency we have today; best case halves it.
The existing nudge tests passed only by accident — the fixture defaulted
started_at to epoch 1000, so every batch read as stalled. They now set it
explicitly and distinguish a settling batch from a stalled one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Every payout costs two sidechain blocks instead of one. Observed on drynet3: ~570s broadcast→settle against a ~253s mainchain interval, and 42 Thunder blocks producing only 25 settlements.
The cause is our own nudge, not Thunder.
Mechanism
minebuilds the block body from the mempool before taking the miner lock, then parks that snapshot the instant the lock frees (thunder-rust/app/app.rs):confirm_bmmthen waits for a mainchain block carrying that exact header hash, holding the lock the whole time. So:Confirmed in production — Thunder parked its request ahead of the broadcast it was meant to carry in 7 of 7 cycles:
The fix
Nudge once per broadcast, forced past the rate limiter. That is the nudge whose snapshot must contain the batch just sent; letting the limiter skip it hands the parked slot to a stale body. It is already bounded by the settlement cadence, so forcing it costs no extra BMM bids in the steady state.
Stop nudging on every tick while waiting. That nudge is what pre-commits the next slot with a stale body.
Keep a stall-recovery nudge for the case where the post-broadcast request is not carried at all (a BMM miss, ~4% of blocks currently), gated on
PAYOUT_NUDGE_STALL_SEC(default 300s, ~2 mainchain blocks) and still rate-limited.Risk
Bounded on the downside. If the model is wrong, the stall nudge fires at 300s and behaviour degrades to roughly what we have now; it cannot deadlock, because the recovery path does not depend on the model being right. If it's correct, latency halves — and since standing backlog is accrual rate × settlement latency (~30 BTC/h × 9.5 min ≈ the ~5 BTC currently owed), the backlog halves with it.
Tests
The existing nudge tests passed only by accident: the fixture defaulted
started_atto epoch 1000, so every batch read as stalled and always nudged. They now set it explicitly and distinguish the two cases:40/40 payout tests pass.