From 7cf2c33d0bd83700152d5e521f977c564d91557b Mon Sep 17 00:00:00 2001 From: Roman Date: Mon, 6 Jul 2026 05:31:25 +0000 Subject: [PATCH] fix(network): accept rootless Zakura header responses Allow solicited header responses without tree-aux roots to reach the reactor non-punitively, so honest peers are not disconnected when they cannot serve optional roots. --- zebra-network/src/zakura/header_sync/pipe.rs | 72 ++++++++++++++++++++ zebra-network/src/zakura/header_sync/wire.rs | 8 ++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/zebra-network/src/zakura/header_sync/pipe.rs b/zebra-network/src/zakura/header_sync/pipe.rs index c528cc1c4de..504ba9321fa 100644 --- a/zebra-network/src/zakura/header_sync/pipe.rs +++ b/zebra-network/src/zakura/header_sync/pipe.rs @@ -480,6 +480,78 @@ mod tests { } } + /// Regression test (review feedback): tree-aux roots are an optional serving capability — + /// a peer legitimately may not have roots for the requested range (it can never serve the + /// root for its own header tip). A solicited, otherwise well-formed `Headers` response that + /// carries no roots must therefore not be a *fatal protocol reject* that disconnects the + /// peer; it should be forwarded (or at worst dropped non-punitively) so the reactor can + /// retry the range elsewhere while keeping the session. + #[test] + fn deliver_rootless_solicited_headers_does_not_disconnect_the_peer() { + use zebra_chain::{orchard, sapling, serialization::ZcashDeserializeInto}; + use zebra_test::vectors::BLOCK_MAINNET_1_BYTES; + + let (handle, mut events) = test_handle(); + let expected = + ExpectedHeadersResponse::new(block::Height(1), 1, true).expect("count is valid"); + + let block_one: Arc = Arc::new( + BLOCK_MAINNET_1_BYTES + .zcash_deserialize_into() + .expect("block 1 vector parses"), + ); + // `encode()` enforces the all-or-nothing root invariant, so build a rooted frame and + // strip the roots on the wire: flip `has_roots` to 0 and drop the trailing root bytes. + let mut frame = HeaderSyncMessage::Headers { + headers: vec![block_one.header.clone()], + body_sizes: vec![0], + tree_aux_roots: vec![BlockCommitmentRoots { + height: block::Height(1), + sapling_root: sapling::tree::NoteCommitmentTree::default().root(), + orchard_root: orchard::tree::NoteCommitmentTree::default().root(), + ironwood_root: zebra_chain::ironwood::tree::NoteCommitmentTree::default().root(), + sapling_tx: 0, + orchard_tx: 0, + ironwood_tx: 0, + auth_data_root: block::merkle::AuthDataRoot::from([0u8; 32]), + }], + } + .encode_frame() + .expect("headers frame encodes"); + frame.payload[HEADER_SYNC_MESSAGE_TYPE_BYTES + HEADER_SYNC_COUNT_BYTES] = 0; + frame + .payload + .truncate(frame.payload.len() - HEADER_SYNC_BLOCK_COMMITMENT_ROOTS_BYTES); + + let flow = deliver(&handle, Some(expected), peer(), frame); + + assert!( + !matches!(flow, Flow::Reject(SinkReject::Protocol(_))), + "a rootless solicited Headers response must not be a fatal protocol reject \ + (it disconnects an honest peer that simply has no roots to serve)" + ); + match events.try_recv() { + Ok(HeaderSyncEvent::WireMessage { + msg: + HeaderSyncMessage::Headers { + headers, + tree_aux_roots, + .. + }, + .. + }) => { + assert_eq!(headers.len(), 1); + assert!( + tree_aux_roots.is_empty(), + "the rootless response is forwarded as-is for non-punitive handling" + ); + } + other => panic!( + "expected the rootless solicited response to reach the reactor, got {other:?}" + ), + } + } + /// The peer-local correlation queue is FIFO and is filled by draining ready /// commands. This is the invariant `run_peer` relies on: an expectation /// recorded by a `RecordExpectedHeaders` command is drained and available to diff --git a/zebra-network/src/zakura/header_sync/wire.rs b/zebra-network/src/zakura/header_sync/wire.rs index 04a42ed8563..af097075e3d 100644 --- a/zebra-network/src/zakura/header_sync/wire.rs +++ b/zebra-network/src/zakura/header_sync/wire.rs @@ -207,8 +207,14 @@ impl HeaderSyncMessage { for _ in 0..count { tree_aux_roots.push(BlockCommitmentRoots::zcash_deserialize(&mut reader)?); } + validate_tree_aux_roots_len(count, tree_aux_roots.len())?; } - validate_tree_aux_roots_len(count, tree_aux_roots.len())?; + // `has_roots == false` with non-empty headers is legal on the requester side: + // roots are an optional serving capability, and a solicited response reaching + // this decode is always correlated to an in-flight request (an uncorrelated + // `Headers` frame was already rejected as `UnsolicitedHeaders` above). The + // reactor decides how to handle a rootless response to a root-carrying + // request; it must not be a fatal wire reject that disconnects the peer. if let Some(requested) = context.requested { validate_tree_aux_root_heights(requested.start_height, &tree_aux_roots)?; }