diff --git a/api/src/types.rs b/api/src/types.rs index 59dc7b8a7..9b51c0b73 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -20,6 +20,7 @@ use core::core::SwitchCommitHash; use chain; use p2p; use util; +use util::LOGGER; use util::secp::pedersen; use util::secp::constants::MAX_PROOF_SIZE; use serde; diff --git a/core/src/core/block.rs b/core/src/core/block.rs index a07b997fc..9423efb94 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -235,13 +235,13 @@ pub struct CompactBlock { /// Implementation of Writeable for a compact block, defines how to write the block to a /// binary writer. Differentiates between writing the block for the purpose of /// full serialization and the one of just extracting a hash. -/// Note: compact block hash uses both the header *and* the nonce. impl Writeable for CompactBlock { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { try!(self.header.write(writer)); - try!(writer.write_u64(self.nonce)); if writer.serialization_mode() != ser::SerializationMode::Hash { + try!(writer.write_u64(self.nonce)); + ser_multiwrite!( writer, [write_u64, self.out_full.len() as u64], @@ -454,9 +454,6 @@ impl Block { let header = self.header.clone(); let nonce = thread_rng().next_u64(); - // concatenate the nonce with our block_header to build the hash - let hash = (self, nonce).hash(); - let mut out_full = self.outputs .iter() .filter(|x| x.features.contains(OutputFeatures::COINBASE_OUTPUT)) @@ -470,7 +467,7 @@ impl Block { if k.features.contains(KernelFeatures::COINBASE_KERNEL) { kern_full.push(k.clone()); } else { - kern_ids.push(k.short_id(&hash)); + kern_ids.push(k.short_id(&header.hash(), nonce)); } } @@ -1166,16 +1163,18 @@ mod test { let cb1 = b.as_compact_block(); let cb2 = b.as_compact_block(); - // random nonce included in hash each time we generate a compact_block - // so the hash will always be unique (we use this to generate unique short_ids) - assert!(cb1.hash() != cb2.hash()); + // random nonce will not affect the hash of the compact block itself + // hash is based on header only + assert!(cb1.nonce != cb2.nonce); + assert_eq!(b.hash(), cb1.hash()); + assert_eq!(cb1.hash(), cb2.hash()); assert!(cb1.kern_ids[0] != cb2.kern_ids[0]); // check we can identify the specified kernel from the short_id - // in either of the compact_blocks - assert_eq!(cb1.kern_ids[0], tx.kernels[0].short_id(&cb1.hash())); - assert_eq!(cb2.kern_ids[0], tx.kernels[0].short_id(&cb2.hash())); + // correctly in both of the compact_blocks + assert_eq!(cb1.kern_ids[0], tx.kernels[0].short_id(&cb1.hash(), cb1.nonce)); + assert_eq!(cb2.kern_ids[0], tx.kernels[0].short_id(&cb2.hash(), cb2.nonce)); } #[test] @@ -1195,7 +1194,7 @@ mod test { .iter() .find(|x| !x.features.contains(KernelFeatures::COINBASE_KERNEL)) .unwrap() - .short_id(&cb.hash()) + .short_id(&cb.hash(), cb.nonce) ); } diff --git a/core/src/core/id.rs b/core/src/core/id.rs index 98ab637e0..c7e72928a 100644 --- a/core/src/core/id.rs +++ b/core/src/core/id.rs @@ -32,25 +32,28 @@ pub const SHORT_ID_SIZE: usize = 6; pub trait ShortIdentifiable { /// The short_id of a kernel uses a hash built from the block_header *and* a /// connection specific nonce to minimize the effect of collisions. - fn short_id(&self, hash: &Hash) -> ShortId; + fn short_id(&self, hash: &Hash, nonce: u64) -> ShortId; } impl ShortIdentifiable for H { /// Generate a short_id via the following - /// - /// * extract k0/k1 from block_hash (first two u64 values) + /// * extract k0/k1 from block_hash hashed with the nonce (first two u64 values) /// * initialize a siphasher24 with k0/k1 /// * self.hash() passing in the siphasher24 instance /// * drop the 2 most significant bytes (to return a 6 byte short_id) /// - fn short_id(&self, hash: &Hash) -> ShortId { + fn short_id(&self, hash: &Hash, nonce: u64) -> ShortId { + // take the block hash and the nonce and hash them together + let hash_with_nonce = (hash, nonce).hash(); + // we "use" core::hash::Hash in the outer namespace // so doing this here in the fn to minimize collateral damage/confusion use std::hash::Hasher; // extract k0/k1 from the block_hash - let k0 = LittleEndian::read_u64(&hash.0[0..8]); - let k1 = LittleEndian::read_u64(&hash.0[8..16]); + let k0 = LittleEndian::read_u64(&hash_with_nonce.0[0..8]); + let k1 = LittleEndian::read_u64(&hash_with_nonce.0[8..16]); // initialize a siphasher24 with k0/k1 let mut sip_hasher = SipHasher24::new_with_keys(k0, k1); @@ -142,14 +145,14 @@ mod test { } let foo = Foo(0); + let expected_hash = Hash::from_hex( "81e47a19e6b29b0a65b9591762ce5143ed30d0261e5d24a3201752506b20f15c", ).unwrap(); assert_eq!(foo.hash(), expected_hash); let other_hash = Hash::zero(); - println!("{:?}", foo.short_id(&other_hash)); - assert_eq!(foo.short_id(&other_hash), ShortId::from_hex("e973960ba690").unwrap()); + assert_eq!(foo.short_id(&other_hash, foo.0), ShortId::from_hex("4cc808b62476").unwrap()); let foo = Foo(5); let expected_hash = Hash::from_hex( @@ -158,8 +161,7 @@ mod test { assert_eq!(foo.hash(), expected_hash); let other_hash = Hash::zero(); - println!("{:?}", foo.short_id(&other_hash)); - assert_eq!(foo.short_id(&other_hash), ShortId::from_hex("f0c06e838e59").unwrap()); + assert_eq!(foo.short_id(&other_hash, foo.0), ShortId::from_hex("02955a094534").unwrap()); let foo = Foo(5); let expected_hash = Hash::from_hex( @@ -170,7 +172,6 @@ mod test { let other_hash = Hash::from_hex( "81e47a19e6b29b0a65b9591762ce5143ed30d0261e5d24a3201752506b20f15c", ).unwrap(); - println!("{:?}", foo.short_id(&other_hash)); - assert_eq!(foo.short_id(&other_hash), ShortId::from_hex("95bf0ca12d5b").unwrap()); + assert_eq!(foo.short_id(&other_hash, foo.0), ShortId::from_hex("3e9cde72a687").unwrap()); } } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 836d44080..90ab22525 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -717,7 +717,7 @@ impl Writeable for Output { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { writer.write_u8(self.features.bits())?; writer.write_fixed_bytes(&self.commit)?; - // Hash of an output doesn't cover the switch commit, it should + // Hash of an output doesn't cover the switch commit, it should // be wound into the range proof separately if writer.serialization_mode() != ser::SerializationMode::Hash { writer.write_fixed_bytes(&self.switch_commit_hash)?; @@ -921,13 +921,13 @@ impl Readable for OutputStoreable { features: features, }) } -} +} /// A structure which contains fields that are to be commited to within -/// an Output's range (bullet) proof. +/// an Output's range (bullet) proof. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] pub struct ProofMessageElements { - /// The amount, stored to allow for wallet reconstruction as + /// The amount, stored to allow for wallet reconstruction as /// rewinding isn't supported in bulletproofs just yet pub value: u64, } @@ -1120,8 +1120,10 @@ fn input_short_id() { "3a42e66e46dd7633b57d1f921780a1ac715e6b93c19ee52ab714178eb3a9f673", ).unwrap(); - let short_id = input.short_id(&block_hash); - assert_eq!(short_id, ShortId::from_hex("3e1262905b7a").unwrap()); + let nonce = 0; + + let short_id = input.short_id(&block_hash, nonce); + assert_eq!(short_id, ShortId::from_hex("28fea5a693af").unwrap()); // now generate the short_id for a *very* similar output (single feature flag different) // and check it generates a different short_id @@ -1135,7 +1137,7 @@ fn input_short_id() { "3a42e66e46dd7633b57d1f921780a1ac715e6b93c19ee52ab714178eb3a9f673", ).unwrap(); - let short_id = input.short_id(&block_hash); - assert_eq!(short_id, ShortId::from_hex("90653c1c870a").unwrap()); + let short_id = input.short_id(&block_hash, nonce); + assert_eq!(short_id, ShortId::from_hex("c8af83b54e46").unwrap()); } } diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 014b63b11..fbbd22893 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -84,7 +84,7 @@ where for tx in self.transactions.values() { for kernel in &tx.kernels { // rehash each kernel to calculate the block specific short_id - let short_id = kernel.short_id(&cb.hash()); + let short_id = kernel.short_id(&cb.hash(), cb.nonce); // if any kernel matches then keep the tx for later if cb.kern_ids.contains(&short_id) {