From dc827ebe93e2a8652ca9b848cf213037397715cd Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Sat, 23 Jun 2018 00:36:10 +0100 Subject: [PATCH] Remove block markers, use header mmr pos instead (#1188) Also fixes `spend_in_fork_and_compact` chain test. --- chain/src/chain.rs | 40 ++++------------ chain/src/lib.rs | 2 +- chain/src/store.rs | 33 +------------ chain/src/txhashset.rs | 53 +++++---------------- chain/src/types.rs | 76 ------------------------------ chain/tests/data_file_integrity.rs | 8 ---- 6 files changed, 21 insertions(+), 191 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index e707068c4..b3d8928f8 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -31,7 +31,7 @@ use grin_store::Error::NotFoundErr; use pipe; use store; use txhashset; -use types::{BlockMarker, ChainAdapter, Error, Options, Tip}; +use types::{ChainAdapter, Error, Options, Tip}; use util::LOGGER; use util::secp::pedersen::{Commitment, RangeProof}; @@ -489,21 +489,14 @@ impl Chain { /// the required indexes for a consumer to rewind to a consistent state /// at the provided block hash. pub fn txhashset_read(&self, h: Hash) -> Result<(u64, u64, File), Error> { - // get the indexes for the block - let marker = { - let txhashset = self.txhashset.read().unwrap(); - txhashset.indexes_at(&h)? - }; - // now we want to rewind the txhashset extension and // sync a "rewound" copy of the leaf_set files to disk // so we can send these across as part of the zip file. // The fast sync client does *not* have the necessary data // to rewind after receiving the txhashset zip. + let header = self.store.get_block_header(&h)?; + let head_header = self.store.head_header()?; { - let head_header = self.store.head_header()?; - let header = self.store.get_block_header(&h)?; - let mut txhashset = self.txhashset.write().unwrap(); txhashset::extending_readonly(&mut txhashset, |extension| { extension.rewind(&header, &head_header, true, true, true)?; @@ -514,7 +507,11 @@ impl Chain { // prepares the zip and return the corresponding Read let txhashset_reader = txhashset::zip_read(self.db_root.clone())?; - Ok((marker.output_pos, marker.kernel_pos, txhashset_reader)) + Ok(( + header.output_mmr_size, + header.kernel_mmr_size, + txhashset_reader, + )) } /// Writes a reading view on a txhashset state that's been provided to us. @@ -538,18 +535,6 @@ impl Chain { let header = self.store.get_block_header(&h)?; txhashset::zip_write(self.db_root.clone(), txhashset_data)?; - { - // write the block marker so we can safely rewind to - // the pos for that block when we validate the extension below - let batch = self.store.batch()?; - let marker = BlockMarker { - output_pos: rewind_to_output, - kernel_pos: rewind_to_kernel, - }; - batch.save_block_marker(&h, &marker)?; - batch.commit()?; - } - let mut txhashset = txhashset::TxHashSet::open(self.db_root.clone(), self.store.clone(), Some(&header))?; @@ -636,8 +621,6 @@ impl Chain { Ok(b) => { count += 1; batch.delete_block(&b.hash())?; - //TODO: Validation seems to fail as block markers are being deleted? - batch.delete_block_marker(&b.hash())?; batch.delete_block_input_bitmap(&b.hash())?; } Err(NotFoundErr) => { @@ -754,13 +737,6 @@ impl Chain { .map_err(|e| Error::StoreErr(e, "chain get header".to_owned())) } - /// Get the block marker for the specified block hash. - pub fn get_block_marker(&self, bh: &Hash) -> Result { - self.store - .get_block_marker(bh) - .map_err(|e| Error::StoreErr(e, "chain get block marker".to_owned())) - } - /// Gets the block header at the provided height pub fn get_header_by_height(&self, height: u64) -> Result { self.store diff --git a/chain/src/lib.rs b/chain/src/lib.rs index bb2303bed..966857c73 100644 --- a/chain/src/lib.rs +++ b/chain/src/lib.rs @@ -48,4 +48,4 @@ pub mod types; pub use chain::{Chain, MAX_ORPHAN_SIZE}; pub use store::ChainStore; -pub use types::{BlockSums, ChainAdapter, Error, Options, Tip}; +pub use types::{ChainAdapter, Error, Options, Tip}; diff --git a/chain/src/store.rs b/chain/src/store.rs index 818db8f78..69f49db8b 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -28,7 +28,7 @@ use core::core::target::Difficulty; use core::core::{Block, BlockHeader}; use grin_store as store; use grin_store::{option_to_not_found, to_key, Error, u64_to_key}; -use types::{BlockMarker, BlockSums, Tip}; +use types::Tip; const STORE_SUBPATH: &'static str = "chain"; @@ -146,20 +146,6 @@ impl ChainStore { ) } - pub fn get_block_marker(&self, bh: &Hash) -> Result { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_MARKER_PREFIX, &mut bh.to_vec())), - ) - } - - pub fn get_block_sums(&self, bh: &Hash) -> Result { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec())), - ) - } - pub fn build_block_input_bitmap(&self, block: &Block) -> Result { let bitmap = block .inputs @@ -327,23 +313,6 @@ impl<'a> Batch<'a> { .delete(&to_key(COMMIT_POS_PREFIX, &mut commit.to_vec())) } - pub fn get_block_marker(&self, bh: &Hash) -> Result { - option_to_not_found( - self.db - .get_ser(&to_key(BLOCK_MARKER_PREFIX, &mut bh.to_vec())), - ) - } - - pub fn save_block_marker(&self, bh: &Hash, marker: &BlockMarker) -> Result<(), Error> { - self.db - .put_ser(&to_key(BLOCK_MARKER_PREFIX, &mut bh.to_vec())[..], &marker) - } - - pub fn delete_block_marker(&self, bh: &Hash) -> Result<(), Error> { - self.db - .delete(&to_key(BLOCK_MARKER_PREFIX, &mut bh.to_vec())) - } - pub fn get_block_header_db(&self, h: &Hash) -> Result { option_to_not_found( self.db diff --git a/chain/src/txhashset.rs b/chain/src/txhashset.rs index 9151b5170..8c04b901c 100644 --- a/chain/src/txhashset.rs +++ b/chain/src/txhashset.rs @@ -40,7 +40,7 @@ use grin_store; use grin_store::pmmr::PMMRBackend; use grin_store::types::prune_noop; use store::{Batch, ChainStore}; -use types::{BlockMarker, Error, TxHashSetRoots}; +use types::{Error, TxHashSetRoots}; use util::{secp_static, zip, LOGGER}; const TXHASHSET_SUBDIR: &'static str = "txhashset"; @@ -201,11 +201,6 @@ impl TxHashSet { rproof_pmmr.elements_from_insertion_index(start_index, max_count) } - /// Output and kernel MMR indexes at the end of the provided block - pub fn indexes_at(&self, bh: &Hash) -> Result { - self.commit_index.get_block_marker(bh).map_err(&From::from) - } - /// Get sum tree roots /// TODO: Return data instead of hashes pub fn roots(&mut self) -> (Hash, Hash, Hash) { @@ -235,10 +230,8 @@ impl TxHashSet { // horizon for compacting is based on current_height let horizon = current_height.saturating_sub(global::cut_through_horizon().into()); let horizon_header = self.commit_index.get_header_by_height(horizon)?; - let horizon_marker = self.commit_index.get_block_marker(&horizon_header.hash())?; - let rewind_add_pos = - output_pos_to_rewind(self.commit_index.clone(), &horizon_header, &head_header)?; + let rewind_add_pos = output_pos_to_rewind(&horizon_header, &head_header)?; let rewind_rm_pos = input_pos_to_rewind(self.commit_index.clone(), &horizon_header, &head_header)?; @@ -253,14 +246,14 @@ impl TxHashSet { }; self.output_pmmr_h.backend.check_compact( - horizon_marker.output_pos, + horizon_header.output_mmr_size, &rewind_add_pos, &rewind_rm_pos.1, clean_output_index, )?; self.rproof_pmmr_h.backend.check_compact( - horizon_marker.output_pos, + horizon_header.output_mmr_size, &rewind_add_pos, &rewind_rm_pos.1, &prune_noop, @@ -385,7 +378,6 @@ pub struct Extension<'a> { commit_index: Arc, new_output_commits: HashMap, - new_block_markers: HashMap, rollback: bool, /// Batch in which the extension occurs, public so it can be used within @@ -446,7 +438,6 @@ impl<'a> Extension<'a> { ), commit_index, new_output_commits: HashMap::new(), - new_block_markers: HashMap::new(), rollback: false, batch, } @@ -622,24 +613,14 @@ impl<'a> Extension<'a> { self.apply_kernel(kernel)?; } - // finally, recording the PMMR positions after this block for future rewind - let marker = BlockMarker { - output_pos: self.output_pmmr.unpruned_size(), - kernel_pos: self.kernel_pmmr.unpruned_size(), - }; - self.new_block_markers.insert(b.hash(), marker); Ok(()) } // Store all new output pos in the index. - // Also store any new block_markers. fn save_indexes(&self) -> Result<(), Error> { for (commit, pos) in &self.new_output_commits { self.batch.save_output_pos(commit, *pos)?; } - for (bh, marker) in &self.new_block_markers { - self.batch.save_block_marker(bh, marker)?; - } Ok(()) } @@ -775,29 +756,20 @@ impl<'a> Extension<'a> { rewind_kernel: bool, rewind_rangeproof: bool, ) -> Result<(), Error> { - let hash = block_header.hash(); trace!( LOGGER, "Rewind to header {} @ {}", block_header.height, - hash + block_header.hash(), ); - // Rewind our MMRs to the appropriate positions - // based on the block_marker. - let (output_pos, kernel_pos) = { - let marker = self.batch.get_block_marker(&hash)?; - (marker.output_pos, marker.kernel_pos) - }; - // We need to build bitmaps of added and removed output positions // so we can correctly rewind all operations applied to the output MMR // after the position we are rewinding to (these operations will be // undone during rewind). // Rewound output pos will be removed from the MMR. // Rewound input (spent) pos will be added back to the MMR. - let rewind_add_pos = - output_pos_to_rewind(self.commit_index.clone(), block_header, head_header)?; + let rewind_add_pos = output_pos_to_rewind(block_header, head_header)?; let rewind_rm_pos = input_pos_to_rewind(self.commit_index.clone(), block_header, head_header)?; if !rewind_rm_pos.0 { @@ -806,8 +778,8 @@ impl<'a> Extension<'a> { } self.rewind_to_pos( - output_pos, - kernel_pos, + block_header.output_mmr_size, + block_header.kernel_mmr_size, &rewind_add_pos, &rewind_rm_pos.1, rewind_utxo, @@ -1123,15 +1095,12 @@ pub fn zip_write(root_dir: String, txhashset_data: File) -> Result<(), Error> { /// The MMR is append-only so we can simply look for all positions added after /// the rewind pos. fn output_pos_to_rewind( - commit_index: Arc, block_header: &BlockHeader, head_header: &BlockHeader, ) -> Result { - let marker_to = commit_index.get_block_marker(&head_header.hash())?; - let marker_from = commit_index.get_block_marker(&block_header.hash())?; - Ok(((marker_from.output_pos + 1)..=marker_to.output_pos) - .map(|x| x as u32) - .collect()) + let marker_to = head_header.output_mmr_size; + let marker_from = block_header.output_mmr_size; + Ok(((marker_from + 1)..=marker_to).map(|x| x as u32).collect()) } /// Given a block header to rewind to and the block header at the diff --git a/chain/src/types.rs b/chain/src/types.rs index cabb5d713..3b0f4c4cf 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -270,79 +270,3 @@ pub struct NoopAdapter {} impl ChainAdapter for NoopAdapter { fn block_accepted(&self, _: &Block, _: Options) {} } - -/// The output and kernel positions that define the size of the MMRs for a -/// particular block. -#[derive(Debug, Clone)] -pub struct BlockMarker { - /// The output (and rangeproof) MMR position of the final output in the - /// block - pub output_pos: u64, - /// The kernel position of the final kernel in the block - pub kernel_pos: u64, -} - -impl Writeable for BlockMarker { - fn write(&self, writer: &mut W) -> Result<(), ser::Error> { - writer.write_u64(self.output_pos)?; - writer.write_u64(self.kernel_pos)?; - Ok(()) - } -} - -impl Readable for BlockMarker { - fn read(reader: &mut Reader) -> Result { - Ok(BlockMarker { - output_pos: reader.read_u64()?, - kernel_pos: reader.read_u64()?, - }) - } -} - -impl Default for BlockMarker { - fn default() -> BlockMarker { - BlockMarker { - output_pos: 0, - kernel_pos: 0, - } - } -} - -/// The output_sum and kernel_sum for a given block. -/// This is used to validate the next block being processed by applying -/// the inputs, outputs, kernels and kernel_offset from the new block -/// and checking everything sums correctly. -#[derive(Debug, Clone)] -pub struct BlockSums { - /// The total output sum so far. - pub output_sum: Commitment, - /// The total kernel sum so far. - pub kernel_sum: Commitment, -} - -impl Writeable for BlockSums { - fn write(&self, writer: &mut W) -> Result<(), ser::Error> { - writer.write_fixed_bytes(&self.output_sum)?; - writer.write_fixed_bytes(&self.kernel_sum)?; - Ok(()) - } -} - -impl Readable for BlockSums { - fn read(reader: &mut Reader) -> Result { - Ok(BlockSums { - output_sum: Commitment::read(reader)?, - kernel_sum: Commitment::read(reader)?, - }) - } -} - -impl Default for BlockSums { - fn default() -> BlockSums { - let zero_commit = secp_static::commit_to_zero_value(); - BlockSums { - output_sum: zero_commit.clone(), - kernel_sum: zero_commit.clone(), - } - } -} diff --git a/chain/tests/data_file_integrity.rs b/chain/tests/data_file_integrity.rs index 786c3bbd7..f471ce62b 100644 --- a/chain/tests/data_file_integrity.rs +++ b/chain/tests/data_file_integrity.rs @@ -97,14 +97,6 @@ fn data_files() { let head = Tip::from_block(&b.header); - // Check we have block markers for the last block and the block previous - let _cur_pmmr_md = chain - .get_block_marker(&head.last_block_h) - .expect("block marker does not exist"); - chain - .get_block_marker(&head.prev_block_h) - .expect("prev block marker does not exist"); - chain.validate(false).unwrap(); } }