Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move MetaAddr serialization into zebra_network::protocol::external #3020

Merged
merged 3 commits into from
Nov 7, 2021
Merged
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
63 changes: 2 additions & 61 deletions zebra-network/src/meta_addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,13 @@
use std::{
cmp::{Ord, Ordering},
convert::TryInto,
io::{Read, Write},
net::SocketAddr,
time::Instant,
};

use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
use zebra_chain::serialization::{canonical_socket_addr, DateTime32};

use zebra_chain::serialization::{
canonical_socket_addr, DateTime32, ReadZcashExt, SerializationError, TrustedPreallocate,
WriteZcashExt, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize,
};

use crate::{
constants,
protocol::{external::MAX_PROTOCOL_MESSAGE_LEN, types::PeerServices},
};
use crate::{constants, protocol::types::PeerServices};

use MetaAddrChange::*;
use PeerAddrState::*;
Expand Down Expand Up @@ -910,53 +901,3 @@ impl PartialEq for MetaAddr {
}

impl Eq for MetaAddr {}

impl ZcashSerialize for MetaAddr {
fn zcash_serialize<W: Write>(&self, mut writer: W) -> Result<(), std::io::Error> {
self.last_seen()
.expect(
"unexpected MetaAddr with missing last seen time: MetaAddrs should be sanitized \
before serialization",
)
.zcash_serialize(&mut writer)?;

writer.write_u64::<LittleEndian>(
self.services
.expect(
"unexpected MetaAddr with missing peer services: MetaAddrs should be \
sanitized before serialization",
)
.bits(),
)?;

writer.write_socket_addr(self.addr)?;

Ok(())
}
}

impl ZcashDeserialize for MetaAddr {
fn zcash_deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
let untrusted_last_seen = (&mut reader).zcash_deserialize_into()?;
let untrusted_services =
PeerServices::from_bits_truncate(reader.read_u64::<LittleEndian>()?);
let addr = reader.read_socket_addr()?;

Ok(MetaAddr::new_gossiped_meta_addr(
addr,
untrusted_services,
untrusted_last_seen,
))
}
}

/// A serialized meta addr has a 4 byte time, 8 byte services, 16 byte IP addr, and 2 byte port
const META_ADDR_SIZE: usize = 4 + 8 + 16 + 2;

impl TrustedPreallocate for MetaAddr {
fn max_allocation() -> u64 {
// Since a maximal serialized Vec<MetAddr> uses at least three bytes for its length (2MB messages / 30B MetaAddr implies the maximal length is much greater than 253)
// the max allocation can never exceed (MAX_PROTOCOL_MESSAGE_LEN - 3) / META_ADDR_SIZE
((MAX_PROTOCOL_MESSAGE_LEN - 3) / META_ADDR_SIZE) as u64
}
}
1 change: 0 additions & 1 deletion zebra-network/src/meta_addr/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
mod check;
mod preallocate;
mod prop;
mod vectors;
65 changes: 0 additions & 65 deletions zebra-network/src/meta_addr/tests/preallocate.rs

This file was deleted.

4 changes: 4 additions & 0 deletions zebra-network/src/protocol/external.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
//! Network protocol types and serialization for the Zcash wire format.

/// Node address wire formats.
mod addr;
/// A Tokio codec that transforms an `AsyncRead` into a `Stream` of `Message`s.
pub mod codec;
/// Inventory items.
Expand Down
65 changes: 65 additions & 0 deletions zebra-network/src/protocol/external/addr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//! Node address types and serialization for the Zcash wire format.

use std::io::{Read, Write};

use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};

use zebra_chain::serialization::{
ReadZcashExt, SerializationError, TrustedPreallocate, WriteZcashExt, ZcashDeserialize,
ZcashDeserializeInto, ZcashSerialize,
};

use crate::{
meta_addr::MetaAddr,
protocol::external::{types::PeerServices, MAX_PROTOCOL_MESSAGE_LEN},
};

impl ZcashSerialize for MetaAddr {
fn zcash_serialize<W: Write>(&self, mut writer: W) -> Result<(), std::io::Error> {
self.last_seen()
.expect(
"unexpected MetaAddr with missing last seen time: MetaAddrs should be sanitized \
before serialization",
)
.zcash_serialize(&mut writer)?;

writer.write_u64::<LittleEndian>(
self.services
.expect(
"unexpected MetaAddr with missing peer services: MetaAddrs should be \
sanitized before serialization",
)
.bits(),
)?;

writer.write_socket_addr(self.addr)?;

Ok(())
}
}

impl ZcashDeserialize for MetaAddr {
fn zcash_deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
let untrusted_last_seen = (&mut reader).zcash_deserialize_into()?;
let untrusted_services =
PeerServices::from_bits_truncate(reader.read_u64::<LittleEndian>()?);
let addr = reader.read_socket_addr()?;

Ok(MetaAddr::new_gossiped_meta_addr(
addr,
untrusted_services,
untrusted_last_seen,
))
}
}

/// A serialized meta addr has a 4 byte time, 8 byte services, 16 byte IP addr, and 2 byte port
pub(super) const META_ADDR_SIZE: usize = 4 + 8 + 16 + 2;

impl TrustedPreallocate for MetaAddr {
fn max_allocation() -> u64 {
// Since a maximal serialized Vec<MetAddr> uses at least three bytes for its length (2MB messages / 30B MetaAddr implies the maximal length is much greater than 253)
// the max allocation can never exceed (MAX_PROTOCOL_MESSAGE_LEN - 3) / META_ADDR_SIZE
((MAX_PROTOCOL_MESSAGE_LEN - 3) / META_ADDR_SIZE) as u64
}
}
2 changes: 1 addition & 1 deletion zebra-network/src/protocol/external/codec.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! A Tokio codec mapping byte streams to Bitcoin message streams.

use std::fmt;
use std::{
cmp::min,
fmt,
io::{Cursor, Read, Write},
};

Expand Down
66 changes: 63 additions & 3 deletions zebra-network/src/protocol/external/tests/preallocate.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
//! Tests for trusted preallocation during deserialization.

use super::super::inv::InventoryHash;
use std::convert::TryInto;

use proptest::prelude::*;

use zebra_chain::serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN};

use proptest::prelude::*;
use std::convert::TryInto;
use crate::meta_addr::MetaAddr;

use super::super::{addr::META_ADDR_SIZE, inv::InventoryHash};

proptest! {
/// Confirm that each InventoryHash takes the expected size in bytes when serialized.
Expand Down Expand Up @@ -58,3 +61,60 @@ proptest! {
assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
}
}

proptest! {
/// Confirm that each MetaAddr takes exactly META_ADDR_SIZE bytes when serialized.
/// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound.
#[test]
fn meta_addr_size_is_correct(addr in MetaAddr::arbitrary()) {
zebra_test::init();

// We require sanitization before serialization
let addr = addr.sanitize();
prop_assume!(addr.is_some());
let addr = addr.unwrap();

let serialized = addr
.zcash_serialize_to_vec()
.expect("Serialization to vec must succeed");
assert!(serialized.len() == META_ADDR_SIZE)
}

/// Verifies that...
/// 1. The smallest disallowed vector of `MetaAddrs`s is too large to fit in a legal Zcash message
/// 2. The largest allowed vector is small enough to fit in a legal Zcash message
#[test]
fn meta_addr_max_allocation_is_correct(addr in MetaAddr::arbitrary()) {
zebra_test::init();

// We require sanitization before serialization
let addr = addr.sanitize();
prop_assume!(addr.is_some());
let addr = addr.unwrap();

let max_allocation: usize = MetaAddr::max_allocation().try_into().unwrap();
let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
for _ in 0..(MetaAddr::max_allocation() + 1) {
smallest_disallowed_vec.push(addr);
}
Comment on lines +96 to +99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional suggestion:

Suggested change
let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1);
for _ in 0..(MetaAddr::max_allocation() + 1) {
smallest_disallowed_vec.push(addr);
}
let mut smallest_disallowed_vec = vec![addr; max_allocation + 1];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll fix this on top of PR #3022, because it can be fixed in all the preallocate tests.
(And I need to write new preallocate tests in #3022.)

let smallest_disallowed_serialized = smallest_disallowed_vec
.zcash_serialize_to_vec()
.expect("Serialization to vec must succeed");
// Check that our smallest_disallowed_vec is only one item larger than the limit
assert!(((smallest_disallowed_vec.len() - 1) as u64) == MetaAddr::max_allocation());
// Check that our smallest_disallowed_vec is too big to send in a valid Zcash message
assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN);

// Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency)
smallest_disallowed_vec.pop();
let largest_allowed_vec = smallest_disallowed_vec;
let largest_allowed_serialized = largest_allowed_vec
.zcash_serialize_to_vec()
.expect("Serialization to vec must succeed");

// Check that our largest_allowed_vec contains the maximum number of MetaAddrs
assert!((largest_allowed_vec.len() as u64) == MetaAddr::max_allocation());
// Check that our largest_allowed_vec is small enough to fit in a Zcash message.
assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN);
}
}