From b449fe93b2023ca5999dc4dd5ffa8e5c14c8e490 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Tue, 1 Dec 2020 15:52:09 -0800 Subject: [PATCH] network: correct data modeling for headers messages We modeled a Bitcoin `headers` message as being a list of block headers. However, the actual data structure is slightly different: it's a list of (block header, transaction count) pairs. This caused zcashd to reject our headers messages. To fix this, introduce a new `CountedHeader` struct with a `block::Header` and transaction count `usize`, then thread it through the inbound service and the state. I tested this locally by running Zebra with these changes and inspecting a trace-level log of the span of a peer connection that requested a nontrivial headers packet from us, and verified that it did not reject our message. --- zebra-chain/src/block.rs | 2 +- zebra-chain/src/block/header.rs | 9 ++++++ zebra-chain/src/block/serialize.rs | 32 ++++++++++++++++--- .../src/protocol/external/message.rs | 8 ++--- .../src/protocol/internal/response.rs | 2 +- zebra-state/src/response.rs | 2 +- zebra-state/src/service.rs | 10 ++++-- 7 files changed, 49 insertions(+), 16 deletions(-) diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 5115ccdfc..0975cbe9b 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -18,7 +18,7 @@ use std::fmt; pub use hash::Hash; pub use header::BlockTimeError; -pub use header::Header; +pub use header::{CountedHeader, Header}; pub use height::Height; pub use root_hash::RootHash; diff --git a/zebra-chain/src/block/header.rs b/zebra-chain/src/block/header.rs index 4fb8bbacc..973e0fa4b 100644 --- a/zebra-chain/src/block/header.rs +++ b/zebra-chain/src/block/header.rs @@ -109,3 +109,12 @@ impl Header { } } } + +/// A header with a count of the number of transactions in its block. +/// +/// This structure is used in the Bitcoin network protocol. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CountedHeader { + pub header: Header, + pub transaction_count: usize, +} diff --git a/zebra-chain/src/block/serialize.rs b/zebra-chain/src/block/serialize.rs index d1d5dbb64..2042f12f0 100644 --- a/zebra-chain/src/block/serialize.rs +++ b/zebra-chain/src/block/serialize.rs @@ -1,12 +1,17 @@ +use std::{convert::TryInto, io}; + use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use chrono::{TimeZone, Utc}; -use std::io; -use crate::serialization::ZcashDeserializeInto; -use crate::serialization::{ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize}; -use crate::work::{difficulty::CompactDifficulty, equihash}; +use crate::{ + serialization::{ + ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashDeserializeInto, + ZcashSerialize, + }, + work::{difficulty::CompactDifficulty, equihash}, +}; -use super::{merkle, Block, Hash, Header}; +use super::{merkle, Block, CountedHeader, Hash, Header}; /// The maximum size of a Zcash block, in bytes. /// @@ -79,6 +84,23 @@ impl ZcashDeserialize for Header { } } +impl ZcashSerialize for CountedHeader { + fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { + self.header.zcash_serialize(&mut writer)?; + writer.write_compactsize(self.transaction_count as u64)?; + Ok(()) + } +} + +impl ZcashDeserialize for CountedHeader { + fn zcash_deserialize(mut reader: R) -> Result { + Ok(CountedHeader { + header: (&mut reader).zcash_deserialize_into()?, + transaction_count: reader.read_compactsize()?.try_into().unwrap(), + }) + } +} + impl ZcashSerialize for Block { fn zcash_serialize(&self, mut writer: W) -> Result<(), io::Error> { // All block structs are validated when they are parsed. diff --git a/zebra-network/src/protocol/external/message.rs b/zebra-network/src/protocol/external/message.rs index 1f6f2aa93..930faed81 100644 --- a/zebra-network/src/protocol/external/message.rs +++ b/zebra-network/src/protocol/external/message.rs @@ -182,12 +182,10 @@ pub enum Message { /// /// Returns block headers in response to a getheaders packet. /// + /// Each block header is accompanied by a transaction count. + /// /// [Bitcoin reference](https://en.bitcoin.it/wiki/Protocol_documentation#headers) - // Note that the block headers in this packet include a - // transaction count (a var_int, so there can be more than 81 - // bytes per header) as opposed to the block headers that are - // hashed by miners. - Headers(Vec), + Headers(Vec), /// A `getdata` message. /// diff --git a/zebra-network/src/protocol/internal/response.rs b/zebra-network/src/protocol/internal/response.rs index 9e0f638f6..ce7e0cc58 100644 --- a/zebra-network/src/protocol/internal/response.rs +++ b/zebra-network/src/protocol/internal/response.rs @@ -22,7 +22,7 @@ pub enum Response { BlockHashes(Vec), /// A list of block headers. - BlockHeaders(Vec), + BlockHeaders(Vec), /// A list of transactions. Transactions(Vec>), diff --git a/zebra-state/src/response.rs b/zebra-state/src/response.rs index c8d51737b..9268dbe26 100644 --- a/zebra-state/src/response.rs +++ b/zebra-state/src/response.rs @@ -40,5 +40,5 @@ pub enum Response { BlockHashes(Vec), /// The response to a `FindBlockHeaders` request. - BlockHeaders(Vec), + BlockHeaders(Vec), } diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 6c5392b03..be9de8977 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -644,9 +644,13 @@ impl Service for StateService { let res: Vec<_> = res .iter() .map(|&hash| { - self.best_block(hash.into()) - .expect("block for found hash is in the best chain") - .header + let block = self + .best_block(hash.into()) + .expect("block for found hash is in the best chain"); + block::CountedHeader { + transaction_count: block.transactions.len(), + header: block.header, + } }) .collect(); async move { Ok(Response::BlockHeaders(res)) }.boxed()