Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions zebra-network/src/zakura/header_sync/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<block::Block> = 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
Expand Down
8 changes: 7 additions & 1 deletion zebra-network/src/zakura/header_sync/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
Expand Down
Loading