read_message is not cancel-safe inside connection_handler's select!, desyncing miner streams and losing in-flight seals

[n13]

read_message is not cancel-safe, but connection_handler uses it as a tokio::select! branch. When the competing job-broadcast branch wins while a frame is half-read, the partially-completed read future is dropped along with the bytes it already consumed, permanently desynchronising that miner's QUIC stream.

The code

node/src/miner_server.rs:807-812:

tokio::select! {
    // Prioritize reading to detect disconnection faster
    biased;

    // Receive results from miner
    msg_result = read_message(&mut recv) => {

with the other arm being job = job_rx.recv().

miner-api/src/lib.rs:83-99:

pub async fn read_message<R: AsyncRead + Unpin>(reader: &mut R) -> std::io::Result<MinerMessage> {
    let mut len_buf = [0u8; 4];
    reader.read_exact(&mut len_buf).await?;
    let len = u32::from_be_bytes(len_buf);
    ...
    let mut buf = vec![0u8; len as usize];
    reader.read_exact(&mut buf).await?;

AsyncReadExt::read_exact is explicitly documented as cancel-unsafe: if it is used as a select! branch and another branch completes first, the buffer may have been partially filled and those bytes are lost. There are two of them here, and quinn::RecvStream implements AsyncRead, so the loss is from the stream itself.

biased; does not help — it only controls poll order. The read is polled first, returns Pending after consuming some bytes, and then the job branch becoming ready still drops it.

Failure sequence

1. A miner is sending a JobResult; the 4-byte length prefix (or part of the JSON body) has been consumed by read_exact, which then returns Pending because the rest is still in flight.
2. In the same select!, the node broadcasts a new job — which happens on every block-template rebuild, i.e. on every transaction.
3. job_rx.recv() completes, the read_message future is dropped, and the consumed bytes are gone.
4. The next loop iteration builds a fresh read_message that starts reading mid-frame. It either interprets body bytes as a length prefix (Message size N exceeds maximum 1024, InvalidData) or hands serde_json garbage.
5. connection_handler returns Err, serve_authenticated_miner calls remove_miner, and the miner is disconnected.

The seal that was in flight is lost, and the node logs it as a read error rather than as a lost result — from the operator's side it is indistinguishable from a miner that simply did not find anything. Under transaction load this becomes a repeated silent disconnect/reconnect cycle, and it is most likely to fire exactly at new-block time, when broadcasts happen.

Suggested fixes

Either:

• Pin one read_message future outside the loop and poll &mut it in the select!, so a lost race resumes the same future on the next iteration; or
• Split send and receive into two tasks, so the read is never in a select! with anything else. This also removes the related hazard that a miner which stops reading can park the handler inside write_message and stop servicing its own reads.

Context

Found while reviewing a downstream merge of public main (through 308ba838). This predates #662 — it was already there over quinn — but connection_handler was restructured in that PR (the Ready read was lifted out into authenticate_miner_connection), so this is a natural point to address it.