Skip to content

Commit

Permalink
network: correct data modeling for headers messages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hdevalence committed Dec 1, 2020
1 parent a91d0f0 commit 6f66bb6
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 16 deletions.
2 changes: 1 addition & 1 deletion zebra-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::fmt;

pub use hash::Hash;
pub use header::BlockTimeError;
pub use header::Header;
pub use header::{Header, CountedHeader};
pub use height::Height;
pub use root_hash::RootHash;

Expand Down
9 changes: 9 additions & 0 deletions zebra-chain/src/block/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
32 changes: 27 additions & 5 deletions zebra-chain/src/block/serialize.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -79,6 +84,23 @@ impl ZcashDeserialize for Header {
}
}

impl ZcashSerialize for CountedHeader {
fn zcash_serialize<W: io::Write>(&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<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
Ok(CountedHeader {
header: (&mut reader).zcash_deserialize_into()?,
transaction_count: reader.read_compactsize()?.try_into().unwrap(),
})
}
}

impl ZcashSerialize for Block {
fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
// All block structs are validated when they are parsed.
Expand Down
8 changes: 3 additions & 5 deletions zebra-network/src/protocol/external/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,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<block::Header>),
Headers(Vec<block::CountedHeader>),

/// A `getdata` message.
///
Expand Down
2 changes: 1 addition & 1 deletion zebra-network/src/protocol/internal/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub enum Response {
BlockHashes(Vec<block::Hash>),

/// A list of block headers.
BlockHeaders(Vec<block::Header>),
BlockHeaders(Vec<block::CountedHeader>),

/// A list of transactions.
Transactions(Vec<Arc<Transaction>>),
Expand Down
2 changes: 1 addition & 1 deletion zebra-state/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ pub enum Response {
BlockHashes(Vec<block::Hash>),

/// The response to a `FindBlockHeaders` request.
BlockHeaders(Vec<block::Header>),
BlockHeaders(Vec<block::CountedHeader>),
}
10 changes: 7 additions & 3 deletions zebra-state/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,9 +644,13 @@ impl Service<Request> 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()
Expand Down

0 comments on commit 6f66bb6

Please sign in to comment.