Repository navigation
3.x.x: order the tips plan's scheduled block against its warm-up tips instead of racing them (#638) - #652
Conversation
… instead of racing them (#638) The real-node smoke landed no own block in 3-8% of CI runs. The scheduled block went out at 15 s and the third external tip was minted at 18 s on the wall clock alone. When the block's search and the server's asynchronous offer outlasted the gap, the tip took the block's height. The block could also be mined on a job from before the previous tip, when that tip's notify was slow. In a phase that holds both scheduled blocks and external tips (the tips plan's warm-up), the block is now sent once every mint asked of the node is on its tip and session 0 holds work on that tip. No tip is minted from the moment a block is due until it has settled. Settled means one of three things: the node answered a submitblock for that block's hash; the server refused it stale-job or low-difficulty, which prove nothing entered the ledger; or the session reported that it could not send it. A ledger-outcome-unknown answer does not settle it, because a block candidate's append can still be offered. ExternalMint gains settled_tip and block_answered for both nodes. The phase report records each ordered block and any tips left unminted. real_node.rs prints the relay and node submissions, the mints, the client failures and the node logs when pool_blocks is not 1. Four harness tests drive the real scheduler with injected stand-in delays. All four fail with the ordering switched off, three of them with prev-blk-not-found, and pass with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fbf4e28a0
ℹ️ 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".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… the next tip is not held to the phase's end (#638) A session whose task has already stopped has a closed control channel. The ScheduledBlock send then fails and no ClientFailure is ever reported for it, so the landing stayed outstanding and every later warm-up tip was held until the deadline. The refused send is now the landing's verdict: the block never left the harness. This was raised in the Codex review of #652. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796b5db1f8
ℹ️ 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".
… the node's verdict (#638) Two holes in the ordering, raised in the second Codex review of #652. A scheduled block whose socket write failed is reported both as a no-response submit and as a recorded ClientFailure. The write can fail after the line reached the server, so only an unrecorded failure proves the block was never sent. A recorded one now waits for the node's verdict on the block's hash, like any other unanswered submit. qbitd's keepalive task asked for its own mints regardless of the ordering. A keepalive requested between the settled-tip read and the block's send, or while the block was outstanding, could take the block's height. The scheduler now holds keepalives over the same span as the tips. The flag is set under the keepalive's stop_minting lock before the settled tip is read, so a keepalive is either already counted as pending or waits for the verdict. The hold is released at the phase's end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d32f4e371
ℹ️ 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".
…re releasing the keepalives (#638) The keepalive hold was dropped at the phase's end, even when the scheduled block was still searching or waiting for the node. A keepalive due during teardown, or the next phase's tips under --plan tips, could then take the block's height. This was raised in the third Codex review of #652. The phase now sees an outstanding landing through at the boundary, within the drain limit, with the keepalives still held. The wait is boundary time, like the drained restart's wait, and nothing is offered meanwhile. The hold is released only after the landing settles or the limit runs out. An aborted phase does not wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ffe574f9
ℹ️ 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".
…ince a stale-job block can still be offered (#638) A block on block-only work, retired by a same-parent payout replacement, is answered stale-job and still captured and offered (#478). Scheduled blocks bump the payout revision, so this is reachable here. Treating stale-job (or low-difficulty) as the block's verdict released the next tip before the node had answered. This was raised in the fourth Codex review of #652. A block the session sent is now settled only by the node's answer. A refused block the node never sees holds the tips to the boundary and is reported unsettled, with the held tips as unminted; that run has already failed the smoke. The harness tests pin the stale-job capture and the unsettled report. The stand-in run's commit timeout is 1 s, so the boundary's wait is short. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Closes #638.
Problem
real_node::real_node_smoke_reconciles_against_postgres_and_the_chainfailed in about 3–8% of CI runs withpool_blocks: 0: 3 external blocks above the ramp and no own block. The tips plan's warm-up placed its one scheduled block (15 s) and its three external tips (6, 12, 18 s) on the wall clock alone.Control::ScheduledBlockwent out at 15 s, and the peer was asked for the third tip at 18 s, whatever had become of the block. The block is a full network-target search in an unoptimised debug build on a 2 vCPU runner, followed by the server's asynchronous offer. When that outlasted the 3 s gap, the third tip took height 12963 first, and the block was either refused or became a losing branch.The block also raced the tip before it in the other direction. It was mined on the session's newest job whatever tip that job was on. A job that arrived late (the gate allows 5 s from a tip to the last session's notify, and one run measured 5.8 s) gave the block a parent the second tip had already replaced.
Change
In a phase that holds both scheduled blocks and external tips (only the tips plan's warm-up does), the landing is now ordered against the tips (
run::drive_phase_with_population):The block waits for settled work. It is sent only when every mint asked of the node is on the node's tip, and session 0's newest job is on that tip. It also waits for any earlier block in the phase to settle.
The tips wait for the block's verdict. No tip is minted from the moment a block is due until that block has settled. Settled means one of three things:
submitblockfor that block's hash (and, on the fake node, an accepted one is on the chain);The block's identity is the hash of the header the session submitted. Only that session's records from after the send count (EP-STATE). The server's answer never settles it (EP-ERRORS). A block candidate's append is never refused, so an answer of
ledger-outcome-unknownorledger-confirmation-failedcan still be followed by an offer. A block on block-only work is answeredstale-joband is still captured and offered (3.x.x: An own block found 9 s after the previous landing was refusedunknown-joband never reached the node #478; review round 4). A refused block the node never sees is reported unsettled, and the tips held behind it as unminted.New node readings.
ExternalMintgainssettled_tipandblock_answered. On qbitd these come from the mint log, A's watcher and the relay'ssubmitblocklog. The fake node reads its own chain.mint_onealready waits for B to hold A's tip, so a tip minted after the verdict builds on the pool block.Nothing is dropped silently (EP-OBSERVABILITY). The phase report adds
scheduled_blocks_ordered_against_tips. It has one row per block: due, sent and settled offsets, the block hash and how it settled.nullmeans the phase ended first. It also addsexternal_tips_unminted. The phase's deadline is the only bound on either wait, so a block that never settles costs the remaining tips, and the report counts them.real_node.rs. Whenpool_blocks != 1, the test printsnode.relay.submissions,node.submissions,node.mints, each phase's ordering rows,client_failures_by_kindandclient_failures_sample, and the post-ramp node logs. It also asserts that the warm-up's block was settled by the node's answer, that its hash is the accepted submission, and that no tip went unminted. Thepool_blocks == 1assertion is unchanged.README: the
--scheduled-blocksrow describes the ordering and the two report fields.Keepalives are held over the same span (review round 2). qbitd's keepalive mints are held while a block is due or outstanding. The hold is set under the keepalive's own
stop_mintinglock before the settled tip is read. A keepalive is therefore either already counted as pending, or is not asked for until the verdict. A block still outstanding at the phase's deadline is seen through at the boundary, within the drain limit, with the keepalives still held. The hold is released only after it settles (review round 3).Other plans' phases are unchanged: no other phase holds both blocks and tips.
Tests
Commands were run one binary at a time, with
CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 CARGO_INCREMENTAL=0.cargo test -p qbit-prism-load --test harness: 150 passed. Eight tests are new: the four below, plus one for each review case (a session whose task stopped, a failed write, a block still outstanding at the deadline, and a refused block the node never sees). Each drives the realrun::drive_phaseover the fake node, for a 6 s warm-up with tips at 1.2, 2.4 and 3.6 s and the block at 3 s. A stand-in session gets its delays injected: when each tip's job reaches it, how long the search takes, what the server answers, and when the block is offered. No host load is involved.the_next_warm_up_tip_waits_for_the_scheduled_block_the_node_has_not_answered: search 1.5 s plus offer 0.3 s, past the third tip's slot. The chain must be external, external, pool, external.the_scheduled_block_waits_until_its_session_holds_work_on_the_settled_tip: every job reaches the session 1 s after its tip. The block must be sent after the second tip's job arrives and land on it.a_scheduled_block_refused_with_its_outcome_unknown_still_holds_the_next_tip: the server answersledger-outcome-unknownand offers 1.5 s later. The tip must wait for the node's answer.a_scheduled_block_captured_after_a_stale_job_refusal_still_holds_the_next_tip: the server answersstale-joband offers the block 1.5 s later, as a 3.x.x: An own block found 9 s after the previous landing was refusedunknown-joband never reached the node #478 capture does. The tip must wait for the node's answer.The new tests fail without the fix. With the ordering switched off, so the scheduler behaves as on
3.x.x, all four fail. The first three end with the block refusedprev-blk-not-found, which is 3.x.x: flaky real_node_smoke: the one scheduled own block does not land (pool_blocks 0) because it races the third external tip #638's symptom. The fourth (then a stale-job test that expected a release) failed because no ordering row was recorded. All four pass with the fix. Each review-round test was also checked by reverting only its fix: it fails, and passes again with the fix restored.PRISM_TEST_PG_BIN_DIR=<PostgreSQL 16 bindir> cargo test -p qbit-prism-load --test ctv_settlement: passed (36 s). This is a fake-node tips-plan run with a real client and server, 1 tip and 1 scheduled block. It now also asserts that the warm-up's block was settled by the node's answer and that no tip went unminted.cargo test -p qbit-prism-load --lib(44, including the newqbitd::tests::a_held_keepalive_is_asked_for_only_after_its_release),--test realism(21),--test churn(6) and--test real_node(fake_node_mode_is_unchanged): all passed.cargo fmt --all -- --checkandcargo clippy -p qbit-prism-load --all-targets -- -D warnings: clean.Not run here and why
real_node_smoke_reconciles_against_postgres_and_the_chainitself. The build host has no qbitd, so the test skips itself here, and the qbitd readings (Qbitd::settled_tip,Qbitd::block_answered) were checked by reading only. This PR'sprism-native-postgresshard runs it. It should pass, and its new assertions should show the block settled bythe node answered its submitblock. One CI pass does not measure a 3–8% flake, so the evidence that the race is gone is the deterministic harness tests above. If it fails again, the new output says where the block went.Merge notes
qbit-prism-loadchanges, and the two report fields are additive.🤖 Generated with Claude Code