From 2552b34118dfcc2404452b1990c5d4582d025934 Mon Sep 17 00:00:00 2001 From: Antioch Peverell <30642645+antiochp@users.noreply.github.com> Date: Thu, 15 Mar 2018 15:16:34 -0400 Subject: [PATCH] remove difficulty field from block_header (derive from total_difficulty) (#777) * remove difficulty field from block_header (derive from total_difficulty) * address feedback on PR and general cleanup * rustfmt * rework pow/difficulty validation in pipe::validate_header now that we only have total_difficulty available * cleanup various todos * rustfmt * rework DifficultyIterator to track header and prev_header state * rustfmt caught some garbage syntax * cleanup --- api/src/types.rs | 3 -- chain/src/pipe.rs | 53 ++++++++++++++------------- chain/src/store.rs | 41 ++++++++++++++++----- chain/tests/data_file_integrity.rs | 1 - chain/tests/mine_simple_chain.rs | 3 +- chain/tests/test_coinbase_maturity.rs | 7 ++-- core/src/core/block.rs | 19 +++------- core/src/core/target.rs | 5 +-- core/src/genesis.rs | 2 - doc/pow/pow.md | 2 - grin/src/miner.rs | 23 +++++++----- pow/src/lib.rs | 21 ++--------- 12 files changed, 89 insertions(+), 91 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index e8a23e0c6..23dd4806b 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -511,8 +511,6 @@ pub struct BlockHeaderPrintable { pub kernel_root: String, /// Nonce increment used to mine this block. pub nonce: u64, - /// Difficulty used to mine the block. - pub difficulty: u64, /// Total accumulated difficulty since genesis block pub total_difficulty: u64, } @@ -529,7 +527,6 @@ impl BlockHeaderPrintable { range_proof_root: util::to_hex(h.range_proof_root.to_vec()), kernel_root: util::to_hex(h.kernel_root.to_vec()), nonce: h.nonce, - difficulty: h.difficulty.into_num(), total_difficulty: h.total_difficulty.into_num(), } } diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 9fb2e511d..7d7b297d3 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -272,6 +272,8 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), E )), }?; + // make sure this header has a height exactly one higher than the previous + // header if header.height != prev.height + 1 { return Err(Error::InvalidBlockHeight); } @@ -283,43 +285,44 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), E return Err(Error::InvalidBlockTime); } + // verify the proof of work and related parameters + // at this point we have a previous block header + // we know the height increased by one + // so now we can check the total_difficulty increase is also valid + // check the pow hash shows a difficulty at least as large + // as the target difficulty if !ctx.opts.contains(Options::SKIP_POW) { - // verify the proof of work and related parameters - - // explicit check to ensure we are not below the minimum difficulty - // we will also check difficulty based on next_difficulty later on - if header.difficulty < Difficulty::one() { + if header.total_difficulty.clone() <= prev.total_difficulty.clone() { return Err(Error::DifficultyTooLow); } - let diff_iter = store::DifficultyIter::from(header.previous, ctx.store.clone()); - let difficulty = - consensus::next_difficulty(diff_iter).map_err(|e| Error::Other(e.to_string()))?; + let target_difficulty = header.total_difficulty.clone() - prev.total_difficulty.clone(); + + if header.pow.clone().to_difficulty() < target_difficulty { + return Err(Error::DifficultyTooLow); + } + + // explicit check to ensure we are not below the minimum difficulty + // we will also check difficulty based on next_difficulty later on + if target_difficulty < Difficulty::one() { + return Err(Error::DifficultyTooLow); + } // explicit check to ensure total_difficulty has increased by exactly // the _network_ difficulty of the previous block // (during testnet1 we use _block_ difficulty here) - if header.total_difficulty != prev.total_difficulty.clone() + difficulty.clone() { + let diff_iter = store::DifficultyIter::from(header.previous, ctx.store.clone()); + let network_difficulty = + consensus::next_difficulty(diff_iter).map_err(|e| Error::Other(e.to_string()))?; + if target_difficulty != network_difficulty.clone() { error!( LOGGER, "validate_header: BANNABLE OFFENCE: header cumulative difficulty {} != {}", - header.difficulty.into_num(), - prev.total_difficulty.into_num() + difficulty.into_num() + target_difficulty.into_num(), + prev.total_difficulty.into_num() + network_difficulty.into_num() ); return Err(Error::WrongTotalDifficulty); } - - // now check that the difficulty is not less than that calculated by the - // difficulty iterator based on the previous block - if header.difficulty < difficulty { - error!( - LOGGER, - "validate_header: BANNABLE OFFENCE: header difficulty {} < {}", - header.difficulty.into_num(), - difficulty.into_num() - ); - return Err(Error::DifficultyTooLow); - } } Ok(()) @@ -416,7 +419,7 @@ fn update_head(b: &Block, ctx: &mut BlockContext) -> Result, Error> LOGGER, "pipe: chain head reached {} @ {} [{}]", b.header.height, - b.header.difficulty, + b.header.total_difficulty, b.hash() ); } else { @@ -424,7 +427,7 @@ fn update_head(b: &Block, ctx: &mut BlockContext) -> Result, Error> LOGGER, "pipe: chain head reached {} @ {} [{}]", b.header.height, - b.header.difficulty, + b.header.total_difficulty, b.hash() ); } diff --git a/chain/src/store.rs b/chain/src/store.rs index d45f34c4c..15141d1ef 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -252,8 +252,15 @@ impl ChainStore for ChainKVStore { /// previous difficulties). Mostly used by the consensus next difficulty /// calculation. pub struct DifficultyIter { - next: Hash, + start: Hash, store: Arc, + + // maintain state for both the "next" header in this iteration + // and its previous header in the chain ("next next" in the iteration) + // so we effectively read-ahead as we iterate through the chain back + // toward the genesis block (while maintaining current state) + header: Option, + prev_header: Option, } impl DifficultyIter { @@ -261,8 +268,10 @@ impl DifficultyIter { /// the provided block hash. pub fn from(start: Hash, store: Arc) -> DifficultyIter { DifficultyIter { - next: start, + start: start, store: store, + header: None, + prev_header: None, } } } @@ -271,13 +280,27 @@ impl Iterator for DifficultyIter { type Item = Result<(u64, Difficulty), TargetError>; fn next(&mut self) -> Option { - let bhe = self.store.get_block_header(&self.next); - match bhe { - Err(_) => None, - Ok(bh) => { - self.next = bh.previous; - Some(Ok((bh.timestamp.to_timespec().sec as u64, bh.difficulty))) - } + // Get both header and previous_header if this is the initial iteration. + // Otherwise move prev_header to header and get the next prev_header. + self.header = if self.header.is_none() { + self.store.get_block_header(&self.start).ok() + } else { + self.prev_header.clone() + }; + + // If we have a header we can do this iteration. + // Otherwise we are done. + if let Some(header) = self.header.clone() { + self.prev_header = self.store.get_block_header(&header.previous).ok(); + + let prev_difficulty = self.prev_header + .clone() + .map_or(Difficulty::zero(), |x| x.total_difficulty); + let difficulty = header.total_difficulty - prev_difficulty; + + Some(Ok((header.timestamp.to_timespec().sec as u64, difficulty))) + } else { + return None; } } } diff --git a/chain/tests/data_file_integrity.rs b/chain/tests/data_file_integrity.rs index d8fabadcf..5d14737dc 100644 --- a/chain/tests/data_file_integrity.rs +++ b/chain/tests/data_file_integrity.rs @@ -92,7 +92,6 @@ fn data_files() { core::core::Block::new(&prev, vec![], &keychain, &pk, difficulty.clone()).unwrap(); b.header.timestamp = prev.timestamp + time::Duration::seconds(60); - b.header.difficulty = difficulty.clone(); // TODO: overwrite here? really? chain.set_txhashset_roots(&mut b, false).unwrap(); pow::pow_size( diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 4a342aa06..3c675b783 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -80,7 +80,6 @@ fn mine_empty_chain() { core::core::Block::new(&prev, vec![], &keychain, &pk, difficulty.clone()).unwrap(); b.header.timestamp = prev.timestamp + time::Duration::seconds(60); - b.header.difficulty = difficulty.clone(); // TODO: overwrite here? really? chain.set_txhashset_roots(&mut b, false).unwrap(); pow::pow_size( @@ -429,6 +428,6 @@ fn prepare_block_nosum( Ok(b) => b, }; b.header.timestamp = prev.timestamp + time::Duration::seconds(60); - b.header.total_difficulty = Difficulty::from_num(diff); + b.header.total_difficulty = prev.total_difficulty.clone() + Difficulty::from_num(diff); b } diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index 31ea4daab..c1b5f1c25 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -81,7 +81,7 @@ fn test_coinbase_maturity() { block.header.timestamp = prev.timestamp + time::Duration::seconds(60); let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap(); - block.header.difficulty = difficulty.clone(); + chain.set_txhashset_roots(&mut block, false).unwrap(); pow::pow_size( @@ -138,7 +138,6 @@ fn test_coinbase_maturity() { block.header.timestamp = prev.timestamp + time::Duration::seconds(60); let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap(); - block.header.difficulty = difficulty.clone(); match chain.set_txhashset_roots(&mut block, false) { Err(Error::Transaction(transaction::Error::ImmatureCoinbase)) => (), @@ -165,7 +164,7 @@ fn test_coinbase_maturity() { block.header.timestamp = prev.timestamp + time::Duration::seconds(60); let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap(); - block.header.difficulty = difficulty.clone(); + chain.set_txhashset_roots(&mut block, false).unwrap(); pow::pow_size( @@ -200,7 +199,7 @@ fn test_coinbase_maturity() { block.header.timestamp = prev.timestamp + time::Duration::seconds(60); let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap(); - block.header.difficulty = difficulty.clone(); + chain.set_txhashset_roots(&mut block, false).unwrap(); pow::pow_size( diff --git a/core/src/core/block.rs b/core/src/core/block.rs index d6cc8b997..0de076700 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -115,8 +115,6 @@ pub struct BlockHeader { pub nonce: u64, /// Proof of work data. pub pow: Proof, - /// Difficulty used to mine the block. - pub difficulty: Difficulty, /// Total accumulated difficulty since genesis block pub total_difficulty: Difficulty, /// Total accumulated sum of kernel offsets since genesis block. @@ -133,7 +131,6 @@ impl Default for BlockHeader { height: 0, previous: ZERO_HASH, timestamp: time::at_utc(time::Timespec { sec: 0, nsec: 0 }), - difficulty: Difficulty::one(), total_difficulty: Difficulty::one(), output_root: ZERO_HASH, range_proof_root: ZERO_HASH, @@ -160,7 +157,6 @@ impl Writeable for BlockHeader { ); try!(writer.write_u64(self.nonce)); - try!(self.difficulty.write(writer)); try!(self.total_difficulty.write(writer)); try!(self.total_kernel_offset.write(writer)); @@ -181,7 +177,6 @@ impl Readable for BlockHeader { let rproof_root = Hash::read(reader)?; let kernel_root = Hash::read(reader)?; let nonce = reader.read_u64()?; - let difficulty = Difficulty::read(reader)?; let total_difficulty = Difficulty::read(reader)?; let total_kernel_offset = BlindingFactor::read(reader)?; let pow = Proof::read(reader)?; @@ -203,7 +198,6 @@ impl Readable for BlockHeader { kernel_root: kernel_root, pow: pow, nonce: nonce, - difficulty: difficulty, total_difficulty: total_difficulty, total_kernel_offset: total_kernel_offset, }) @@ -620,7 +614,6 @@ impl Block { Block { header: BlockHeader { pow: self.header.pow.clone(), - difficulty: self.header.difficulty.clone(), total_difficulty: self.header.total_difficulty.clone(), ..self.header }, @@ -1048,7 +1041,7 @@ mod test { let b = new_block(vec![], &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 1_256; + let target_len = 1_248; assert_eq!(vec.len(), target_len,); } @@ -1060,7 +1053,7 @@ mod test { let b = new_block(vec![&tx1], &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 2_900; + let target_len = 2_892; assert_eq!(vec.len(), target_len,); } @@ -1071,7 +1064,7 @@ mod test { let b = new_block(vec![], &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b.as_compact_block()).expect("serialization failed"); - let target_len = 1_264; + let target_len = 1_256; assert_eq!(vec.len(), target_len,); } @@ -1083,7 +1076,7 @@ mod test { let b = new_block(vec![&tx1], &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b.as_compact_block()).expect("serialization failed"); - let target_len = 1_270; + let target_len = 1_262; assert_eq!(vec.len(), target_len,); } @@ -1100,7 +1093,7 @@ mod test { let b = new_block(txs.iter().collect(), &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b).expect("serialization failed"); - let target_len = 17_696; + let target_len = 17_688; assert_eq!(vec.len(), target_len,); } @@ -1117,7 +1110,7 @@ mod test { let b = new_block(txs.iter().collect(), &keychain, &prev); let mut vec = Vec::new(); ser::serialize(&mut vec, &b.as_compact_block()).expect("serialization failed"); - let target_len = 1_324; + let target_len = 1_316; assert_eq!(vec.len(), target_len,); } diff --git a/core/src/core/target.rs b/core/src/core/target.rs index 78f9ff555..b1a70e5c9 100644 --- a/core/src/core/target.rs +++ b/core/src/core/target.rs @@ -42,9 +42,8 @@ impl Difficulty { Difficulty { num: 0 } } - /// Difficulty of one, which is the minumum difficulty (when the hash - /// equals the max target) - /// TODO - is this the minimum dificulty or is consensus::MINIMUM_DIFFICULTY the minimum? + /// Difficulty of one, which is the minumum difficulty + /// (when the hash equals the max target) pub fn one() -> Difficulty { Difficulty { num: 1 } } diff --git a/core/src/genesis.rs b/core/src/genesis.rs index 6e7fcad1d..fef020b64 100644 --- a/core/src/genesis.rs +++ b/core/src/genesis.rs @@ -90,7 +90,6 @@ pub fn genesis_testnet2() -> core::Block { ..time::empty_tm() }, //TODO: Check this is over-estimated at T2 launch - difficulty: Difficulty::from_num(global::initial_block_difficulty()), total_difficulty: Difficulty::from_num(global::initial_block_difficulty()), nonce: 70081, pow: core::Proof::new(vec![ @@ -122,7 +121,6 @@ pub fn genesis_main() -> core::Block { tm_mday: 14, ..time::empty_tm() }, - difficulty: Difficulty::from_num(global::initial_block_difficulty()), total_difficulty: Difficulty::from_num(global::initial_block_difficulty()), nonce: global::get_genesis_nonce(), pow: core::Proof::zero(consensus::PROOFSIZE), diff --git a/doc/pow/pow.md b/doc/pow/pow.md index 3965b5801..9faaedf1f 100644 --- a/doc/pow/pow.md +++ b/doc/pow/pow.md @@ -149,8 +149,6 @@ the median timestamps at the beginning and the end of the window. If the timespa range, (adjusted with a dampening factor to allow for normal variation,) then the difficulty is raised or lowered to a value aiming for the target block solve time. -The minimum difficuly is 10, as defined in the consensus MINIMUM_DIFFICULTY value. - ### The Mining Loop All of these systems are put together in the mining loop, which attempts to create diff --git a/grin/src/miner.rs b/grin/src/miner.rs index 1acc99941..08d1f69e7 100644 --- a/grin/src/miner.rs +++ b/grin/src/miner.rs @@ -167,7 +167,7 @@ impl Miner { cuckoo_size, attempt_time_per_block, b.header.height, - b.header.difficulty + b.header.total_difficulty ); // look for a pow for at most attempt_time_per_block sec on the @@ -209,7 +209,8 @@ impl Miner { proof_diff.into_num(), difficulty.into_num() ); - if proof_diff >= b.header.difficulty { + if proof_diff > (b.header.total_difficulty.clone() - head.total_difficulty.clone()) + { sol = Some(proof); b.header.nonce = s.get_nonce_as_u64(); break; @@ -304,7 +305,7 @@ impl Miner { cuckoo_size, attempt_time_per_block, latest_hash, - b.header.difficulty + b.header.total_difficulty ); let mut iter_count = 0; @@ -327,9 +328,10 @@ impl Miner { "Found cuckoo solution for nonce {} of difficulty {} (difficulty target {})", b.header.nonce, proof_diff.into_num(), - b.header.difficulty.into_num() + b.header.total_difficulty.into_num() ); - if proof_diff >= b.header.difficulty { + if proof_diff > (b.header.total_difficulty.clone() - head.total_difficulty.clone()) + { sol = Some(proof); break; } @@ -420,7 +422,7 @@ impl Miner { cuckoo_size, attempt_time_per_block, latest_hash, - b.header.difficulty + b.header.total_difficulty ); let mut iter_count = 0; @@ -438,7 +440,8 @@ impl Miner { let pow_hash = b.hash(); if let Ok(proof) = miner.mine(&pow_hash[..]) { let proof_diff = proof.clone().to_difficulty(); - if proof_diff >= b.header.difficulty { + if proof_diff > (b.header.total_difficulty.clone() - head.total_difficulty.clone()) + { sol = Some(proof); break; } @@ -537,7 +540,8 @@ impl Miner { { let mut mining_stats = mining_stats.write().unwrap(); mining_stats.block_height = b.header.height; - mining_stats.network_difficulty = b.header.difficulty.into_num(); + mining_stats.network_difficulty = + (b.header.total_difficulty.clone() - head.total_difficulty.clone()).into_num(); } let mut sol = None; @@ -551,7 +555,7 @@ impl Miner { if use_async { sol = self.inner_loop_async( &mut p, - b.header.difficulty.clone(), + (b.header.total_difficulty.clone() - head.total_difficulty.clone()), &mut b, cuckoo_size, &head, @@ -668,7 +672,6 @@ impl Miner { let mut rng = rand::OsRng::new().unwrap(); b.header.nonce = rng.gen(); - b.header.difficulty = difficulty; b.header.timestamp = time::at_utc(time::Timespec::new(now_sec, 0)); let roots_result = self.chain.set_txhashset_roots(&mut b, false); diff --git a/pow/src/lib.rs b/pow/src/lib.rs index 68a1cde29..397006df4 100644 --- a/pow/src/lib.rs +++ b/pow/src/lib.rs @@ -77,31 +77,18 @@ pub trait MiningWorker { /// Validates the proof of work of a given header, and that the proof of work /// satisfies the requirements of the header. pub fn verify_size(bh: &BlockHeader, cuckoo_sz: u32) -> bool { - // make sure the pow hash shows a difficulty at least as large as the target - // difficulty - if bh.difficulty > bh.pow.clone().to_difficulty() { - return false; - } Cuckoo::new(&bh.hash()[..], cuckoo_sz).verify(bh.pow.clone(), consensus::EASINESS as u64) } -/// Uses the much easier Cuckoo20 (mostly for -/// tests). -pub fn pow20( - miner: &mut T, - bh: &mut BlockHeader, - diff: Difficulty, -) -> Result<(), Error> { - pow_size(miner, bh, diff, 20) -} - /// Mines a genesis block, using the config specified miner if specified. /// Otherwise, uses the internal miner pub fn mine_genesis_block( miner_config: Option, ) -> Result { let mut gen = genesis::genesis_testnet2(); - let diff = gen.header.difficulty.clone(); + + // total_difficulty on the genesis header *is* the difficulty of that block + let genesis_difficulty = gen.header.total_difficulty.clone(); let sz = global::sizeshift() as u32; let proof_size = global::proofsize(); @@ -116,7 +103,7 @@ pub fn mine_genesis_block( }, None => Box::new(cuckoo::Miner::new(consensus::EASINESS, sz, proof_size)), }; - pow_size(&mut *miner, &mut gen.header, diff, sz as u32).unwrap(); + pow_size(&mut *miner, &mut gen.header, genesis_difficulty, sz as u32).unwrap(); Ok(gen) }