From 982dae99a565da957a6c6120bf585fe285f90646 Mon Sep 17 00:00:00 2001 From: Antioch Peverell <30642645+antiochp@users.noreply.github.com> Date: Tue, 29 May 2018 17:57:11 -0400 Subject: [PATCH] make enum values explicit (#1101) --- p2p/src/lib.rs | 2 +- p2p/src/msg.rs | 95 ++++++++++++++++++------------------- p2p/src/store.rs | 6 +-- p2p/src/types.rs | 4 +- p2p/tests/enum_ser_deser.rs | 44 +++++++++++++++++ 5 files changed, 97 insertions(+), 54 deletions(-) create mode 100644 p2p/tests/enum_ser_deser.rs diff --git a/p2p/src/lib.rs b/p2p/src/lib.rs index 20280a13..83eb0090 100644 --- a/p2p/src/lib.rs +++ b/p2p/src/lib.rs @@ -53,5 +53,5 @@ pub use peer::Peer; pub use peers::Peers; pub use serv::{DummyAdapter, Server}; pub use store::{PeerData, State}; -pub use types::{Capabilities, ChainAdapter, Error, P2PConfig, PeerInfo, ReasonForBan, +pub use types::{Capabilities, ChainAdapter, Direction, Error, P2PConfig, PeerInfo, ReasonForBan, TxHashSetRead, MAX_BLOCK_HEADERS, MAX_LOCATORS, MAX_PEER_ADDRS}; diff --git a/p2p/src/msg.rs b/p2p/src/msg.rs index 91b4001b..0e47ce5b 100644 --- a/p2p/src/msg.rs +++ b/p2p/src/msg.rs @@ -41,59 +41,58 @@ const MAGIC: [u8; 2] = [0x1e, 0xc5]; /// Size in bytes of a message header pub const HEADER_LEN: u64 = 11; -/// Codes for each error that can be produced reading a message. -#[allow(dead_code)] -pub enum ErrCodes { - UnsupportedVersion = 100, -} - -/// Types of messages +/// Types of messages. +/// Note: Values here are *important* so we should only add new values at the +/// end. enum_from_primitive! { #[derive(Debug, Clone, Copy, PartialEq)] pub enum Type { - Error, - Hand, - Shake, - Ping, - Pong, - BanReason, - GetPeerAddrs, - PeerAddrs, - GetHeaders, - Header, - Headers, - GetBlock, - Block, - GetCompactBlock, - CompactBlock, - StemTransaction, - Transaction, - TxHashSetRequest, - TxHashSetArchive, + Error = 0, + Hand = 1, + Shake = 2, + Ping = 3, + Pong = 4, + GetPeerAddrs = 5, + PeerAddrs = 6, + GetHeaders = 7, + Header = 8, + Headers = 9, + GetBlock = 10, + Block = 11, + GetCompactBlock = 12, + CompactBlock = 13, + StemTransaction = 14, + Transaction = 15, + TxHashSetRequest = 16, + TxHashSetArchive = 17, + BanReason = 18, } } -const MAX_MSG_SIZES: [u64; 19] = [ - 0, // Error - 128, // Hand - 88, // Shake - 16, // Ping - 16, // Pong - 64, // BanReason - 4, // GetPeerAddrs - 4 + (1 + 16 + 2) * MAX_PEER_ADDRS as u64, // PeerAddrs, with all IPv6 - 1 + 32 * MAX_LOCATORS as u64, // GetHeaders locators - 365, // Header - 2 + 365 * MAX_BLOCK_HEADERS as u64, // Headers - 32, // GetBlock - MAX_MSG_LEN, // Block - 32, // GetCompactBlock - MAX_MSG_LEN / 10, // CompactBlock - 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS, // StemTransaction, - 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS, // Transaction, - 40, // TxHashSetRequest - 64, // TxHashSetArchive -]; +// Max msg size for each msg type. +fn max_msg_size(msg_type: Type) -> u64 { + match msg_type { + Type::Error => 0, + Type::Hand => 128, + Type::Shake => 88, + Type::Ping => 16, + Type::Pong => 16, + Type::GetPeerAddrs => 4, + Type::PeerAddrs => 4 + (1 + 16 + 2) * MAX_PEER_ADDRS as u64, + Type::GetHeaders => 1 + 32 * MAX_LOCATORS as u64, + Type::Header => 365, + Type::Headers => 2 + 365 * MAX_BLOCK_HEADERS as u64, + Type::GetBlock => 32, + Type::Block => MAX_MSG_LEN, + Type::GetCompactBlock => 32, + Type::CompactBlock => MAX_MSG_LEN / 10, + Type::StemTransaction => 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS, + Type::Transaction => 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS, + Type::TxHashSetRequest => 40, + Type::TxHashSetArchive => 64, + Type::BanReason => 64, + } +} /// The default implementation of read_exact is useless with async TcpStream as /// it will return as soon as something has been read, regardless of @@ -190,7 +189,7 @@ pub fn read_header(conn: &mut TcpStream) -> Result { let mut head = vec![0u8; HEADER_LEN as usize]; read_exact(conn, &mut head, 10000, false)?; let header = ser::deserialize::(&mut &head[..])?; - let max_len = MAX_MSG_SIZES[header.msg_type as usize]; + let max_len = max_msg_size(header.msg_type); // TODO 4x the limits for now to leave ourselves space to change things if header.msg_len > max_len * 4 { error!( diff --git a/p2p/src/store.rs b/p2p/src/store.rs index be6b4d6d..f56698e2 100644 --- a/p2p/src/store.rs +++ b/p2p/src/store.rs @@ -32,9 +32,9 @@ const PEER_PREFIX: u8 = 'p' as u8; enum_from_primitive! { #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] pub enum State { - Healthy, - Banned, - Defunct, + Healthy = 0, + Banned = 1, + Defunct = 2, } } diff --git a/p2p/src/types.rs b/p2p/src/types.rs index 044df427..1ad65a57 100644 --- a/p2p/src/types.rs +++ b/p2p/src/types.rs @@ -193,8 +193,8 @@ bitflags! { enum_from_primitive! { #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] pub enum Direction { - Inbound, - Outbound, + Inbound = 0, + Outbound = 1, } } diff --git a/p2p/tests/enum_ser_deser.rs b/p2p/tests/enum_ser_deser.rs new file mode 100644 index 00000000..a6b02ff8 --- /dev/null +++ b/p2p/tests/enum_ser_deser.rs @@ -0,0 +1,44 @@ +// Copyright 2018 The Grin Developers +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +extern crate grin_p2p as p2p; + +extern crate enum_primitive; +extern crate num; + +use num::FromPrimitive; + +// Test that Healthy == 0. +#[test] +fn test_store_state_enum() { + assert_eq!(p2p::State::from_i32(0), Some(p2p::State::Healthy)); +} + +#[test] +fn test_direction_enum() { + assert_eq!(p2p::Direction::from_i32(0), Some(p2p::Direction::Inbound)); +} + +#[test] +fn test_reason_for_ban_enum() { + assert_eq!( + p2p::types::ReasonForBan::from_i32(0), + Some(p2p::types::ReasonForBan::None) + ); +} + +#[test] +fn test_type_enum() { + assert_eq!(p2p::msg::Type::from_i32(0), Some(p2p::msg::Type::Error)); +}