Header version cleanup (#2809)

* introduce HeaderVersion (was u16) for type safety
cleanup pow ser/deser (version unused)

* fixup tests for HeaderVersion

* validate header version during header deserialization

api/src/types.rs

@@ -548,7 +548,7 @@ impl BlockHeaderPrintable {
 	pub fn from_header(header: &core::BlockHeader) -> BlockHeaderPrintable {
 		BlockHeaderPrintable {
 			hash: util::to_hex(header.hash().to_vec()),
-			version: header.version,
+			version: header.version.into(),
 			height: header.height,
 			previous: util::to_hex(header.prev_hash.to_vec()),
 			prev_root: util::to_hex(header.prev_root.to_vec()),

chain/src/error.rs

@@ -102,8 +102,8 @@ pub enum ErrorKind {
 	#[fail(display = "Output is spent")]
 	OutputSpent,
 	/// Invalid block version, either a mistake or outdated software
-	#[fail(display = "Invalid Block Version: {}", _0)]
-	InvalidBlockVersion(u16),
+	#[fail(display = "Invalid Block Version: {:?}", _0)]
+	InvalidBlockVersion(block::HeaderVersion),
 	/// We've been provided a bad txhashset
 	#[fail(display = "Invalid TxHashSet: {}", _0)]
 	InvalidTxHashSet(String),

chain/src/pipe.rs

@@ -341,7 +341,7 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(
 	// check version, enforces scheduled hard fork
 	if !consensus::valid_header_version(header.height, header.version) {
 		error!(
-			"Invalid block header version received ({}), maybe update Grin?",
+			"Invalid block header version received ({:?}), maybe update Grin?",
 			header.version
 		);
 		return Err(ErrorKind::InvalidBlockVersion(header.version).into());

core/src/consensus.rs

@@ -20,6 +20,7 @@
 
 use std::cmp::{max, min};
 
+use crate::core::block::HeaderVersion;
 use crate::global;
 use crate::pow::Difficulty;
 
@@ -128,10 +129,10 @@ pub const HARD_FORK_INTERVAL: u64 = YEAR_HEIGHT / 2;
 
 /// Check whether the block version is valid at a given height, implements
 /// 6 months interval scheduled hard forks for the first 2 years.
-pub fn valid_header_version(height: u64, version: u16) -> bool {
+pub fn valid_header_version(height: u64, version: HeaderVersion) -> bool {
 	// uncomment below as we go from hard fork to hard fork
 	if height < HARD_FORK_INTERVAL {
-		version == 1
+		version == HeaderVersion::default()
 	/* } else if height < 2 * HARD_FORK_INTERVAL {
 		version == 2
 	} else if height < 3 * HARD_FORK_INTERVAL {

core/src/core/block.rs

@@ -22,7 +22,7 @@ use std::fmt;
 use std::iter::FromIterator;
 use std::sync::Arc;
 
-use crate::consensus::{reward, REWARD};
+use crate::consensus::{self, reward, REWARD};
 use crate::core::committed::{self, Committed};
 use crate::core::compact_block::{CompactBlock, CompactBlockBody};
 use crate::core::hash::{DefaultHashable, Hash, Hashed, ZERO_HASH};
@@ -168,11 +168,47 @@ impl Hashed for HeaderEntry {
 	}
 }
 
+/// Some type safety around header versioning.
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize)]
+pub struct HeaderVersion(pub u16);
+
+impl Default for HeaderVersion {
+	fn default() -> HeaderVersion {
+		HeaderVersion(1)
+	}
+}
+
+impl HeaderVersion {
+	/// Constructor taking the provided version.
+	pub fn new(version: u16) -> HeaderVersion {
+		HeaderVersion(version)
+	}
+}
+
+impl From<HeaderVersion> for u16 {
+	fn from(v: HeaderVersion) -> u16 {
+		v.0
+	}
+}
+
+impl Writeable for HeaderVersion {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		writer.write_u16(self.0)
+	}
+}
+
+impl Readable for HeaderVersion {
+	fn read(reader: &mut dyn Reader) -> Result<HeaderVersion, ser::Error> {
+		let version = reader.read_u16()?;
+		Ok(HeaderVersion(version))
+	}
+}
+
 /// Block header, fairly standard compared to other blockchains.
 #[derive(Clone, Debug, PartialEq, Serialize)]
 pub struct BlockHeader {
 	/// Version of the block
-	pub version: u16,
+	pub version: HeaderVersion,
 	/// Height of this block since the genesis block (height 0)
 	pub height: u64,
 	/// Hash of the block previous to this in the chain.
@@ -203,7 +239,7 @@ impl DefaultHashable for BlockHeader {}
 impl Default for BlockHeader {
 	fn default() -> BlockHeader {
 		BlockHeader {
-			version: 1,
+			version: HeaderVersion::default(),
 			height: 0,
 			timestamp: DateTime::<Utc>::from_utc(NaiveDateTime::from_timestamp(0, 0), Utc),
 			prev_hash: ZERO_HASH,
@@ -239,7 +275,7 @@ impl Writeable for BlockHeader {
 		if writer.serialization_mode() != ser::SerializationMode::Hash {
 			self.write_pre_pow(writer)?;
 		}
-		self.pow.write(self.version, writer)?;
+		self.pow.write(writer)?;
 		Ok(())
 	}
 }
@@ -247,7 +283,8 @@ impl Writeable for BlockHeader {
 /// Deserialization of a block header
 impl Readable for BlockHeader {
 	fn read(reader: &mut dyn Reader) -> Result<BlockHeader, ser::Error> {
-		let (version, height, timestamp) = ser_multiread!(reader, read_u16, read_u64, read_i64);
+		let version = HeaderVersion::read(reader)?;
+		let (height, timestamp) = ser_multiread!(reader, read_u64, read_i64);
 		let prev_hash = Hash::read(reader)?;
 		let prev_root = Hash::read(reader)?;
 		let output_root = Hash::read(reader)?;
@@ -255,7 +292,7 @@ impl Readable for BlockHeader {
 		let kernel_root = Hash::read(reader)?;
 		let total_kernel_offset = BlindingFactor::read(reader)?;
 		let (output_mmr_size, kernel_mmr_size) = ser_multiread!(reader, read_u64, read_u64);
-		let pow = ProofOfWork::read(version, reader)?;
+		let pow = ProofOfWork::read(reader)?;
 
 		if timestamp > MAX_DATE.and_hms(0, 0, 0).timestamp()
 			|| timestamp < MIN_DATE.and_hms(0, 0, 0).timestamp()
@@ -263,6 +300,14 @@ impl Readable for BlockHeader {
 			return Err(ser::Error::CorruptedData);
 		}
 
+		// Check the block version before proceeding any further.
+		// We want to do this here because blocks can be pretty large
+		// and we want to halt processing as early as possible.
+		// If we receive an invalid block version then the peer is not on our hard-fork.
+		if !consensus::valid_header_version(height, version) {
+			return Err(ser::Error::InvalidBlockVersion);
+		}
+
 		Ok(BlockHeader {
 			version,
 			height,
@@ -283,9 +328,9 @@ impl Readable for BlockHeader {
 impl BlockHeader {
 	/// Write the pre-hash portion of the header
 	pub fn write_pre_pow<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		self.version.write(writer)?;
 		ser_multiwrite!(
 			writer,
-			[write_u16, self.version],
 			[write_u64, self.height],
 			[write_i64, self.timestamp.timestamp()],
 			[write_fixed_bytes, &self.prev_hash],
@@ -309,7 +354,7 @@ impl BlockHeader {
 		{
 			let mut writer = ser::BinWriter::new(&mut header_buf);
 			self.write_pre_pow(&mut writer).unwrap();
-			self.pow.write_pre_pow(self.version, &mut writer).unwrap();
+			self.pow.write_pre_pow(&mut writer).unwrap();
 			writer.write_u64(self.pow.nonce).unwrap();
 		}
 		header_buf

core/src/pow/types.rs

@@ -239,9 +239,19 @@ impl Default for ProofOfWork {
 	}
 }
 
-impl ProofOfWork {
-	/// Read implementation, can't define as trait impl as we need a version
-	pub fn read(_ver: u16, reader: &mut dyn Reader) -> Result<ProofOfWork, ser::Error> {
+impl Writeable for ProofOfWork {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		if writer.serialization_mode() != ser::SerializationMode::Hash {
+			self.write_pre_pow(writer)?;
+			writer.write_u64(self.nonce)?;
+		}
+		self.proof.write(writer)?;
+		Ok(())
+	}
+}
+
+impl Readable for ProofOfWork {
+	fn read(reader: &mut dyn Reader) -> Result<ProofOfWork, ser::Error> {
 		let total_difficulty = Difficulty::read(reader)?;
 		let secondary_scaling = reader.read_u32()?;
 		let nonce = reader.read_u64()?;
@@ -253,20 +263,11 @@ impl ProofOfWork {
 			proof,
 		})
 	}
+}
 
-	/// Write implementation, can't define as trait impl as we need a version
-	pub fn write<W: Writer>(&self, ver: u16, writer: &mut W) -> Result<(), ser::Error> {
-		if writer.serialization_mode() != ser::SerializationMode::Hash {
-			self.write_pre_pow(ver, writer)?;
-			writer.write_u64(self.nonce)?;
-		}
-
-		self.proof.write(writer)?;
-		Ok(())
-	}
-
+impl ProofOfWork {
 	/// Write the pre-hash portion of the header
-	pub fn write_pre_pow<W: Writer>(&self, _ver: u16, writer: &mut W) -> Result<(), ser::Error> {
+	pub fn write_pre_pow<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
 		ser_multiwrite!(
 			writer,
 			[write_u64, self.total_difficulty.to_num()],

core/src/ser.rs

@@ -65,6 +65,8 @@ pub enum Error {
 	SortError,
 	/// Inputs/outputs/kernels must be unique.
 	DuplicateError,
+	/// Block header version (hard-fork schedule).
+	InvalidBlockVersion,
 }
 
 impl From<io::Error> for Error {
@@ -87,6 +89,7 @@ impl fmt::Display for Error {
 			Error::DuplicateError => f.write_str("duplicate"),
 			Error::TooLargeReadErr => f.write_str("too large read"),
 			Error::HexError(ref e) => write!(f, "hex error {:?}", e),
+			Error::InvalidBlockVersion => f.write_str("invalid block version"),
 		}
 	}
 }
@@ -109,6 +112,7 @@ impl error::Error for Error {
 			Error::DuplicateError => "duplicate error",
 			Error::TooLargeReadErr => "too large read",
 			Error::HexError(_) => "hex error",
+			Error::InvalidBlockVersion => "invalid block version",
 		}
 	}
 }

core/tests/block.rs

@@ -21,7 +21,9 @@ use crate::core::core::id::ShortIdentifiable;
 use crate::core::core::transaction::{self, Transaction};
 use crate::core::core::verifier_cache::{LruVerifierCache, VerifierCache};
 use crate::core::core::Committed;
-use crate::core::core::{Block, BlockHeader, CompactBlock, KernelFeatures, OutputFeatures};
+use crate::core::core::{
+	Block, BlockHeader, CompactBlock, HeaderVersion, KernelFeatures, OutputFeatures,
+};
 use crate::core::libtx::build::{self, input, output, with_fee};
 use crate::core::{global, ser};
 use crate::keychain::{BlindingFactor, ExtKeychain, Keychain};
@@ -198,6 +200,23 @@ fn remove_coinbase_kernel_flag() {
 	);
 }
 
+#[test]
+fn serialize_deserialize_header_version() {
+	let mut vec1 = Vec::new();
+	ser::serialize(&mut vec1, &1_u16).expect("serialization failed");
+
+	let mut vec2 = Vec::new();
+	ser::serialize(&mut vec2, &HeaderVersion::default()).expect("serialization failed");
+
+	// Check that a header_version serializes to a
+	// single u16 value with no extraneous bytes wrapping it.
+	assert_eq!(vec1, vec2);
+
+	// Check we can successfully deserialize a header_version.
+	let version: HeaderVersion = ser::deserialize(&mut &vec2[..]).unwrap();
+	assert_eq!(version.0, 1)
+}
+
 #[test]
 fn serialize_deserialize_block_header() {
 	let keychain = ExtKeychain::from_random_seed(false).unwrap();

core/tests/consensus.rs

@@ -15,6 +15,7 @@
 use grin_core as core;
 
 use self::core::consensus::*;
+use self::core::core::block::HeaderVersion;
 use self::core::global;
 use self::core::pow::Difficulty;
 use chrono::prelude::Utc;
@@ -617,15 +618,27 @@ fn test_secondary_pow_scale() {
 
 #[test]
 fn hard_forks() {
-	assert!(valid_header_version(0, 1));
-	assert!(valid_header_version(10, 1));
-	assert!(!valid_header_version(10, 2));
-	assert!(valid_header_version(YEAR_HEIGHT / 2 - 1, 1));
+	assert!(valid_header_version(0, HeaderVersion::new(1)));
+	assert!(valid_header_version(10, HeaderVersion::new(1)));
+	assert!(!valid_header_version(10, HeaderVersion::new(2)));
+	assert!(valid_header_version(
+		YEAR_HEIGHT / 2 - 1,
+		HeaderVersion::new(1)
+	));
 	// v2 not active yet
-	assert!(!valid_header_version(YEAR_HEIGHT / 2, 2));
-	assert!(!valid_header_version(YEAR_HEIGHT / 2, 1));
-	assert!(!valid_header_version(YEAR_HEIGHT, 1));
-	assert!(!valid_header_version(YEAR_HEIGHT / 2 + 1, 2));
+	assert!(!valid_header_version(
+		YEAR_HEIGHT / 2,
+		HeaderVersion::new(2)
+	));
+	assert!(!valid_header_version(
+		YEAR_HEIGHT / 2,
+		HeaderVersion::new(1)
+	));
+	assert!(!valid_header_version(YEAR_HEIGHT, HeaderVersion::new(1)));
+	assert!(!valid_header_version(
+		YEAR_HEIGHT / 2 + 1,
+		HeaderVersion::new(2)
+	));
 }
 
 // #[test]

core/tests/vec_backend.rs

@@ -20,7 +20,6 @@ use self::core::ser::{FixedLength, PMMRable, Readable, Reader, Writeable, Writer
 use croaring;
 use croaring::Bitmap;
 use grin_core as core;
-use std::path::Path;
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct TestElem(pub [u32; 4]);

servers/src/mining/stratumserver.rs

@@ -333,7 +333,7 @@ impl Handler {
 		{
 			let mut writer = ser::BinWriter::new(&mut header_buf);
 			bh.write_pre_pow(&mut writer).unwrap();
-			bh.pow.write_pre_pow(bh.version, &mut writer).unwrap();
+			bh.pow.write_pre_pow(&mut writer).unwrap();
 		}
 		let pre_pow = util::to_hex(header_buf);
 		let job_template = JobTemplate {