From fbf955dd11ad4592a0f5bf8837975622cb3af2dd Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Wed, 17 Oct 2018 10:06:38 +0100 Subject: [PATCH] Commit to prev_root in block headers (#1764) * commit to prev_root in block headers * prev_root ready to go, mergeable onto testnet4 --- chain/src/chain.rs | 38 ++++++++++----- chain/src/pipe.rs | 2 + chain/src/txhashset/txhashset.rs | 82 +++++++++++++++++++++----------- core/src/core/block.rs | 21 ++++---- core/tests/block.rs | 12 ++--- servers/src/mining/mine_block.rs | 2 +- 6 files changed, 100 insertions(+), 57 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index feb4e1f19..e36ea528c 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -486,23 +486,37 @@ impl Chain { /// the current txhashset state. pub fn set_txhashset_roots(&self, b: &mut Block, is_fork: bool) -> Result<(), Error> { let mut txhashset = self.txhashset.write().unwrap(); - let (roots, sizes) = txhashset::extending_readonly(&mut txhashset, |extension| { - if is_fork { - pipe::rewind_and_apply_fork(b, extension)?; - } - extension.apply_block(b)?; - Ok((extension.roots(), extension.sizes())) - })?; + let (prev_root, roots, sizes) = + txhashset::extending_readonly(&mut txhashset, |extension| { + if is_fork { + pipe::rewind_and_apply_fork(b, extension)?; + } - // Carefully destructure these correctly... - // TODO - Maybe sizes should be a struct to add some type safety here... - let (_, output_mmr_size, _, kernel_mmr_size) = sizes; + // Retrieve the header root before we apply the new block + let prev_root = extension.header_root(); + // Apply the latest block to the chain state via the extension. + extension.apply_block(b)?; + + Ok((prev_root, extension.roots(), extension.sizes())) + })?; + + // Set the prev_root on the header. + b.header.prev_root = prev_root; + + // Set the output, rangeproof and kernel MMR roots. b.header.output_root = roots.output_root; b.header.range_proof_root = roots.rproof_root; b.header.kernel_root = roots.kernel_root; - b.header.output_mmr_size = output_mmr_size; - b.header.kernel_mmr_size = kernel_mmr_size; + + // Set the output and kernel MMR sizes. + { + // Carefully destructure these correctly... + let (_, output_mmr_size, _, kernel_mmr_size) = sizes; + b.header.output_mmr_size = output_mmr_size; + b.header.kernel_mmr_size = kernel_mmr_size; + } + Ok(()) } diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 9f7d48f66..060a0bb58 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -218,6 +218,7 @@ pub fn sync_block_headers( extension.rewind(&prev_header)?; for header in headers { + extension.validate_root(header)?; extension.apply_header(header)?; } @@ -500,6 +501,7 @@ fn verify_block_sums(b: &Block, ext: &mut txhashset::Extension) -> Result<(), Er /// Fully validate the block by applying it to the txhashset extension. /// Check both the txhashset roots and sizes are correct after applying the block. fn apply_block_to_txhashset(block: &Block, ext: &mut txhashset::Extension) -> Result<(), Error> { + ext.validate_header_root(&block.header)?; ext.apply_block(block)?; ext.validate_roots()?; ext.validate_sizes()?; diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index 6a9d40b75..1ca23f9bb 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -701,28 +701,51 @@ impl<'a> HeaderExtension<'a> { header_hashes.push(current.hash()); current = self.batch.get_block_header(¤t.previous)?; } - // Include the genesis header as we will re-apply it after truncating the extension. - header_hashes.push(genesis.hash()); header_hashes.reverse(); // Trucate the extension (back to pos 0). self.truncate()?; - debug!( - LOGGER, - "Re-applying {} headers to extension, from {:?} to {:?}.", - header_hashes.len(), - header_hashes.first().unwrap(), - header_hashes.last().unwrap(), - ); + // Re-apply the genesis header after truncation. + self.apply_header(&genesis)?; - for h in header_hashes { - let header = self.batch.get_block_header(&h)?; - // self.validate_header_root()?; - self.apply_header(&header)?; + if header_hashes.len() > 0 { + debug!( + LOGGER, + "Re-applying {} headers to extension, from {:?} to {:?}.", + header_hashes.len(), + header_hashes.first().unwrap(), + header_hashes.last().unwrap(), + ); + + for h in header_hashes { + let header = self.batch.get_block_header(&h)?; + self.validate_root(&header)?; + self.apply_header(&header)?; + } } Ok(()) } + + /// The root of the header MMR for convenience. + pub fn root(&self) -> Hash { + self.pmmr.root() + } + + /// Validate the prev_root of the header against the root of the current header MMR. + pub fn validate_root(&self, header: &BlockHeader) -> Result<(), Error> { + // If we are validating the genesis block then we have no prev_root. + // So we are done here. + if header.height == 1 { + return Ok(()); + } + + if self.root() != header.prev_root { + Err(ErrorKind::InvalidRoot.into()) + } else { + Ok(()) + } + } } /// Allows the application of new blocks on top of the sum trees in a @@ -1078,14 +1101,19 @@ impl<'a> Extension<'a> { } } + /// Get the root of the current header MMR. + pub fn header_root(&self) -> Hash { + self.header_pmmr.root() + } + /// Validate the following MMR roots against the latest header applied - /// * output /// * rangeproof /// * kernel /// - /// Note we do not validate the header MMR roots here as we need to validate - /// a header against the state of the MMR *prior* to applying it as - /// each header commits to the root of the MMR of all previous headers, + /// Note we do not validate the header MMR root here as we need to validate + /// a header against the state of the MMR *prior* to applying it. + /// Each header commits to the root of the MMR of all previous headers, /// not including the header itself. /// pub fn validate_roots(&self) -> Result<(), Error> { @@ -1107,23 +1135,19 @@ impl<'a> Extension<'a> { } } - /// Validate the provided header by comparing its "prev_root" to the + /// Validate the provided header by comparing its prev_root to the /// root of the current header MMR. - /// - /// TODO - Implement this once we commit to prev_root in block headers. - /// - pub fn validate_header_root(&self, _header: &BlockHeader) -> Result<(), Error> { - if self.header.height == 0 { + pub fn validate_header_root(&self, header: &BlockHeader) -> Result<(), Error> { + if header.height == 1 { return Ok(()); } - let _roots = self.roots(); - - // TODO - validate once we commit to header MMR root in the header - // (not just previous hash) - // if roots.header_root != header.prev_root - - Ok(()) + let roots = self.roots(); + if roots.header_root != header.prev_root { + Err(ErrorKind::InvalidRoot.into()) + } else { + Ok(()) + } } /// Validate the header, output and kernel MMR sizes against the block header. diff --git a/core/src/core/block.rs b/core/src/core/block.rs index dff2f86ee..4089235d0 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -118,6 +118,8 @@ pub struct BlockHeader { pub height: u64, /// Hash of the block previous to this in the chain. pub previous: Hash, + /// Root hash of the header MMR at the previous header. + pub prev_root: Hash, /// Timestamp at which the block was built. pub timestamp: DateTime, /// Merklish root of all the commitments in the TxHashSet @@ -143,8 +145,9 @@ fn fixed_size_of_serialized_header(_version: u16) -> usize { let mut size: usize = 0; size += mem::size_of::(); // version size += mem::size_of::(); // height + size += mem::size_of::(); // timestamp size += mem::size_of::(); // previous - size += mem::size_of::(); // timestamp + size += mem::size_of::(); // prev_root size += mem::size_of::(); // output_root size += mem::size_of::(); // range_proof_root size += mem::size_of::(); // kernel_root @@ -176,8 +179,9 @@ impl Default for BlockHeader { BlockHeader { version: 1, height: 0, - previous: ZERO_HASH, timestamp: DateTime::::from_utc(NaiveDateTime::from_timestamp(0, 0), Utc), + previous: ZERO_HASH, + prev_root: ZERO_HASH, output_root: ZERO_HASH, range_proof_root: ZERO_HASH, kernel_root: ZERO_HASH, @@ -211,9 +215,9 @@ impl Writeable for BlockHeader { /// Deserialization of a block header impl Readable for BlockHeader { fn read(reader: &mut Reader) -> Result { - let (version, height) = ser_multiread!(reader, read_u16, read_u64); + let (version, height, timestamp) = ser_multiread!(reader, read_u16, read_u64, read_i64); let previous = Hash::read(reader)?; - let timestamp = reader.read_i64()?; + let prev_root = Hash::read(reader)?; let output_root = Hash::read(reader)?; let range_proof_root = Hash::read(reader)?; let kernel_root = Hash::read(reader)?; @@ -230,8 +234,9 @@ impl Readable for BlockHeader { Ok(BlockHeader { version, height, - previous, timestamp: DateTime::::from_utc(NaiveDateTime::from_timestamp(timestamp, 0), Utc), + previous, + prev_root, output_root, range_proof_root, kernel_root, @@ -250,11 +255,9 @@ impl BlockHeader { writer, [write_u16, self.version], [write_u64, self.height], + [write_i64, self.timestamp.timestamp()], [write_fixed_bytes, &self.previous], - [write_i64, self.timestamp.timestamp()] - ); - ser_multiwrite!( - writer, + [write_fixed_bytes, &self.prev_root], [write_fixed_bytes, &self.output_root], [write_fixed_bytes, &self.range_proof_root], [write_fixed_bytes, &self.kernel_root], diff --git a/core/tests/block.rs b/core/tests/block.rs index 93997ce52..108aec1e2 100644 --- a/core/tests/block.rs +++ b/core/tests/block.rs @@ -257,7 +257,7 @@ fn empty_block_serialized_size() { let b = new_block(vec![], &keychain, &prev, &key_id); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 1_223; + let target_len = 1_255; assert_eq!(vec.len(), target_len); } @@ -270,7 +270,7 @@ fn block_single_tx_serialized_size() { let b = new_block(vec![&tx1], &keychain, &prev, &key_id); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 2_805; + let target_len = 2_837; assert_eq!(vec.len(), target_len); } @@ -283,7 +283,7 @@ fn empty_compact_block_serialized_size() { let cb: CompactBlock = b.into(); let mut vec = Vec::new(); ser::serialize(&mut vec, &cb).expect("serialization failed"); - let target_len = 1_231; + let target_len = 1_263; assert_eq!(vec.len(), target_len); } @@ -297,7 +297,7 @@ fn compact_block_single_tx_serialized_size() { let cb: CompactBlock = b.into(); let mut vec = Vec::new(); ser::serialize(&mut vec, &cb).expect("serialization failed"); - let target_len = 1_237; + let target_len = 1_269; assert_eq!(vec.len(), target_len); } @@ -316,7 +316,7 @@ fn block_10_tx_serialized_size() { let b = new_block(txs.iter().collect(), &keychain, &prev, &key_id); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 17_043; + let target_len = 17_075; assert_eq!(vec.len(), target_len,); } @@ -335,7 +335,7 @@ fn compact_block_10_tx_serialized_size() { let cb: CompactBlock = b.into(); let mut vec = Vec::new(); ser::serialize(&mut vec, &cb).expect("serialization failed"); - let target_len = 1_291; + let target_len = 1_323; assert_eq!(vec.len(), target_len,); } diff --git a/servers/src/mining/mine_block.rs b/servers/src/mining/mine_block.rs index c4110b180..b98234517 100644 --- a/servers/src/mining/mine_block.rs +++ b/servers/src/mining/mine_block.rs @@ -95,9 +95,9 @@ fn build_block( key_id: Option, wallet_listener_url: Option, ) -> Result<(core::Block, BlockFees), Error> { - // prepare the block header timestamp let head = chain.head_header()?; + // prepare the block header timestamp let mut now_sec = Utc::now().timestamp(); let head_sec = head.timestamp.timestamp(); if now_sec <= head_sec {