From 2af119824e3b21294c4545b18b2fb6a86bb96ea4 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Thu, 10 Aug 2023 12:57:35 +0200 Subject: [PATCH] fix!: limit monero hashes and force coinbase to be tx 0 (#5602) Description --- This limits the merkle proof hashes from monero blocks to be all used stopping miners from adding junk data at the end that is not used. Also brings the merkle root construction inline with monero rule by forcing the coinbase to be tx 0. Motivation and Context --- If the hashes are not limited, miners can add endless data to the header. This causes TARI-006 and TARI-008, TARI-009, TARI-011 and TARI-012 How Has This Been Tested? --- Unit tests Audit Finding Number --- TARI-006 TARI-008 TARI-009 TARI-011 TARI-012 --------- Co-authored-by: Stan Bondi --- .../src/proof_of_work/monero_rx/helpers.rs | 18 +- .../proof_of_work/monero_rx/merkle_tree.rs | 156 +++++------------- .../core/tests/tests/block_validation.rs | 2 +- 3 files changed, 49 insertions(+), 127 deletions(-) diff --git a/base_layer/core/src/proof_of_work/monero_rx/helpers.rs b/base_layer/core/src/proof_of_work/monero_rx/helpers.rs index 9c23836b2f..7a0e129898 100644 --- a/base_layer/core/src/proof_of_work/monero_rx/helpers.rs +++ b/base_layer/core/src/proof_of_work/monero_rx/helpers.rs @@ -139,7 +139,7 @@ pub fn serialize_monero_block_to_hex(obj: &monero::Block) -> Result Result { let hashes = create_ordered_transaction_hashes_from_block(&block); let root = tree_hash(&hashes)?; - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).ok_or_else(|| { + let coinbase_merkle_proof = create_merkle_proof(&hashes).ok_or_else(|| { MergeMineError::ValidationError( "create_merkle_proof returned None because the block had no coinbase (which is impossible because the \ Block type does not allow that)" @@ -339,7 +339,7 @@ mod test { let hashes = create_ordered_transaction_hashes_from_block(&block); assert_eq!(hashes.len(), block.tx_hashes.len() + 1); let root = tree_hash(&hashes).unwrap(); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, @@ -401,7 +401,7 @@ mod test { } let root = tree_hash(&hashes).unwrap(); assert_eq!(root, hashes[0]); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), @@ -449,7 +449,7 @@ mod test { hashes.push(item); } let root = tree_hash(&hashes).unwrap(); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), @@ -505,7 +505,7 @@ mod test { proof.push(item); } let root = tree_hash(&hashes).unwrap(); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), @@ -563,7 +563,7 @@ mod test { } let root = tree_hash(&hashes).unwrap(); assert_eq!(root, hashes[0]); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), @@ -618,7 +618,7 @@ mod test { proof.push(item); } let root = tree_hash(&hashes).unwrap(); - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), @@ -662,7 +662,7 @@ mod test { randomx_key: FixedByteArray::default(), transaction_count: 1, merkle_root: Default::default(), - coinbase_merkle_proof: create_merkle_proof(&[Hash::null()], &Hash::null()).unwrap(), + coinbase_merkle_proof: create_merkle_proof(&[Hash::null()]).unwrap(), coinbase_tx: Default::default(), }; let mut serialized = Vec::new(); @@ -711,7 +711,7 @@ mod test { proof.push(item); } - let coinbase_merkle_proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = create_merkle_proof(&hashes).unwrap(); let monero_data = MoneroPowData { header: block.header, randomx_key: FixedByteArray::from_bytes(&from_hex(&seed_hash).unwrap()).unwrap(), diff --git a/base_layer/core/src/proof_of_work/monero_rx/merkle_tree.rs b/base_layer/core/src/proof_of_work/monero_rx/merkle_tree.rs index ee8a4d269a..ec016cc4ea 100644 --- a/base_layer/core/src/proof_of_work/monero_rx/merkle_tree.rs +++ b/base_layer/core/src/proof_of_work/monero_rx/merkle_tree.rs @@ -126,8 +126,6 @@ pub fn tree_hash(hashes: &[Hash]) -> Result { #[cfg_attr(test, derive(PartialEq))] pub struct MerkleProof { branch: Vec, - depth: u16, - path_bitmap: u32, } impl BorshSerialize for MerkleProof { @@ -136,8 +134,6 @@ impl BorshSerialize for MerkleProof { for hash in &self.branch { hash.consensus_encode(writer)?; } - BorshSerialize::serialize(&self.depth, writer)?; - BorshSerialize::serialize(&self.path_bitmap, writer)?; Ok(()) } } @@ -152,27 +148,13 @@ impl BorshDeserialize for MerkleProof { .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err.to_string()))?, ); } - let depth = BorshDeserialize::deserialize(buf)?; - let path_bitmap = BorshDeserialize::deserialize(buf)?; - Ok(Self { - branch, - depth, - path_bitmap, - }) + Ok(Self { branch }) } } impl MerkleProof { - fn try_construct(branch: Vec, depth: u16, path_bitmap: u32) -> Option { - if branch.is_empty() { - return None; - } - - Some(Self { - branch, - depth, - path_bitmap, - }) + fn try_construct(branch: Vec) -> Option { + Some(Self { branch }) } /// Returns the merkle proof branch as a list of Monero hashes @@ -183,17 +165,13 @@ impl MerkleProof { /// Calculates the merkle root hash from the provide Monero hash pub fn calculate_root(&self, hash: &Hash) -> Hash { - if self.depth == 0 { - return self.branch[0]; + if self.branch.is_empty() { + return *hash; } let mut root = *hash; - for d in 0..self.depth { - if (self.path_bitmap >> (self.depth - d - 1)) & 1 > 0 { - root = cn_fast_hash2(&self.branch[d as usize], &root); - } else { - root = cn_fast_hash2(&root, &self.branch[d as usize]); - } + for hash in &self.branch { + root = cn_fast_hash2(&root, hash); } root @@ -204,33 +182,21 @@ impl Default for MerkleProof { fn default() -> Self { Self { branch: vec![Hash::null()], - depth: 0, - path_bitmap: 0, } } } /// Creates a merkle proof for the given hash within the set of hashes. This function returns None if the hash is not in -/// hashes. This is a port of Monero's tree_branch function +/// hashes. #[allow(clippy::cognitive_complexity)] -pub fn create_merkle_proof(hashes: &[Hash], hash: &Hash) -> Option { +pub fn create_merkle_proof(hashes: &[Hash]) -> Option { + // Monero coinbase rules specify that the coinbase should be hash[0] match hashes.len() { 0 => None, - 1 => { - if hashes[0] != *hash { - return None; - } - MerkleProof::try_construct(vec![hashes[0]], 0, 0) - }, - 2 => hashes.iter().enumerate().find_map(|(pos, h)| { - if h != hash { - return None; - } - let i = usize::from(pos == 0); - MerkleProof::try_construct(vec![hashes[i]], 1, u32::from(pos != 0)) - }), + 1 => MerkleProof::try_construct(vec![]), + 2 => MerkleProof::try_construct(vec![hashes[1]]), len => { - let mut idx = hashes.iter().position(|node| node == hash)?; + let mut idx = 0; let mut count = tree_hash_count(len).ok()?; let mut ints = vec![Hash::null(); count]; @@ -239,16 +205,12 @@ pub fn create_merkle_proof(hashes: &[Hash], hash: &Hash) -> Option ints[..c].copy_from_slice(&hashes[..c]); let mut branch = Vec::new(); - let mut depth = 0u16; - let mut path = 0u32; let mut i = c; for (j, val) in ints.iter_mut().enumerate().take(count).skip(c) { // Left or right if idx == i || idx == i + 1 { let ii = if idx == i { i + 1 } else { i }; branch.push(hashes[ii]); - depth += 1; - path = (path << 1) | u32::from(idx != i); idx = j; } *val = cn_fast_hash2(&hashes[i], &hashes[i + 1]); @@ -264,8 +226,6 @@ pub fn create_merkle_proof(hashes: &[Hash], hash: &Hash) -> Option if idx == i || idx == i + 1 { let ii = if idx == i { i + 1 } else { i }; branch.push(ints[ii]); - depth += 1; - path = (path << 1) | u32::from(idx != i); idx = j; } ints[j] = cn_fast_hash2(&ints[i], &ints[i + 1]); @@ -276,11 +236,9 @@ pub fn create_merkle_proof(hashes: &[Hash], hash: &Hash) -> Option if idx == 0 || idx == 1 { let ii = usize::from(idx == 0); branch.push(ints[ii]); - depth += 1; - path = (path << 1) | u32::from(idx != 0); } - MerkleProof::try_construct(branch, depth, path) + MerkleProof::try_construct(branch) }, } } @@ -475,19 +433,16 @@ mod test { #[test] fn empty_hashset_has_no_proof() { - assert!(create_merkle_proof(&[], &Hash::null()).is_none()); + assert!(create_merkle_proof(&[]).is_none()); } #[test] fn single_hash_is_its_own_proof() { let tx_hashes = &[Hash::from_str("fa58575f7d1d377709f1621fac98c758860ca6dc5f2262be9ce5fd131c370d1a").unwrap()]; - let proof = create_merkle_proof(&tx_hashes[..], &tx_hashes[0]).unwrap(); - assert_eq!(proof.depth, 0); + let proof = create_merkle_proof(&tx_hashes[..]).unwrap(); + assert_eq!(proof.branch.len(), 0); assert_eq!(proof.calculate_root(&tx_hashes[0]), tx_hashes[0]); - assert_eq!(proof.branch(), tx_hashes); - - assert!(create_merkle_proof(&tx_hashes[..], &Hash::null()).is_none()); } #[test] @@ -501,62 +456,35 @@ mod test { .collect::>(); let expected_root = cn_fast_hash2(&tx_hashes[0], &tx_hashes[1]); - let proof = create_merkle_proof(tx_hashes, &tx_hashes[0]).unwrap(); - assert_eq!(proof.branch()[0], tx_hashes[1]); + let proof = create_merkle_proof(tx_hashes).unwrap(); assert_eq!(proof.calculate_root(&tx_hashes[0]), expected_root); - - let proof = create_merkle_proof(tx_hashes, &tx_hashes[1]).unwrap(); - assert_eq!(proof.branch()[0], tx_hashes[0]); - assert_eq!(proof.calculate_root(&tx_hashes[1]), expected_root); - - assert!(create_merkle_proof(tx_hashes, &Hash::null()).is_none()); } #[test] fn simple_proof_construction() { - // { root } - // / \ - // h01 h2345 - // / \ / \ - // h0 h1 h23 h45 - // / \ / \ - // h2 h3 h4 h5 - - let hashes = (1..=6).map(|i| Hash::from([i; 32])).collect::>(); + // { root } + // / \ + // h0123 h4567 + // / \ / \ + // h01 h23 h45 h67 + // / \ / \ / \ / \ + // h0 h1 h2 h3 h4 h5 h6 h7 + let hashes = (1..=8).map(|i| Hash::from([i; 32])).collect::>(); let h23 = cn_fast_hash2(&hashes[2], &hashes[3]); let h45 = cn_fast_hash2(&hashes[4], &hashes[5]); + let h67 = cn_fast_hash2(&hashes[6], &hashes[7]); let h01 = cn_fast_hash2(&hashes[0], &hashes[1]); - let h2345 = cn_fast_hash2(&h23, &h45); - let expected_root = cn_fast_hash2(&h01, &h2345); + let h0123 = cn_fast_hash2(&h01, &h23); + let h4567 = cn_fast_hash2(&h45, &h67); + let expected_root = cn_fast_hash2(&h0123, &h4567); // Proof for h0 - let proof = create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let proof = create_merkle_proof(&hashes).unwrap(); assert_eq!(proof.calculate_root(&hashes[0]), expected_root); - assert_eq!(proof.depth, 2); - assert_eq!(proof.branch().len(), 2); + assert_eq!(proof.branch().len(), 3); assert_eq!(proof.branch()[0], hashes[1]); - assert_eq!(proof.branch()[1], h2345); - assert_eq!(proof.path_bitmap, 0b00000000); - - // Proof for h2 - let proof = create_merkle_proof(&hashes, &hashes[2]).unwrap(); - assert_eq!(proof.calculate_root(&hashes[2]), expected_root); - assert_eq!(proof.path_bitmap, 0b00000001); - let branch = proof.branch(); - assert_eq!(branch[0], hashes[3]); - assert_eq!(branch[1], h45); - assert_eq!(branch[2], h01); - assert_eq!(branch.len(), 3); - - // Proof for h5 - let proof = create_merkle_proof(&hashes, &hashes[5]).unwrap(); - assert_eq!(proof.calculate_root(&hashes[5]), expected_root); - assert_eq!(proof.path_bitmap, 0b00000111); - let branch = proof.branch(); - assert_eq!(branch[0], hashes[4]); - assert_eq!(branch[1], h23); - assert_eq!(branch[2], h01); - assert_eq!(branch.len(), 3); + assert_eq!(proof.branch()[1], h23); + assert_eq!(proof.branch()[2], h4567) } #[test] @@ -582,11 +510,8 @@ mod test { let expected_root = tree_hash(tx_hashes).unwrap(); - let hash = Hash::from_str("fa58575f7d1d377709f1621fac98c758860ca6dc5f2262be9ce5fd131c370d1a").unwrap(); - let proof = create_merkle_proof(tx_hashes, &hash).unwrap(); - - assert_eq!(proof.depth, 4); - assert_eq!(proof.path_bitmap, 0b00001111); + let hash = Hash::from_str("d96756959949db23764592fea0bfe88c790e1fd131dabb676948b343aa9ecc24").unwrap(); + let proof = create_merkle_proof(tx_hashes).unwrap(); assert_eq!(proof.calculate_root(&hash), expected_root); @@ -610,11 +535,8 @@ mod test { let expected_root = tree_hash(&tx_hashes).unwrap(); - let hash = tx_hashes.last().unwrap(); - let proof = create_merkle_proof(&tx_hashes, hash).unwrap(); - - assert_eq!(proof.depth, 16); - assert_eq!(proof.path_bitmap, 0b1111_1111_1111_1111); + let hash = tx_hashes.first().unwrap(); + let proof = create_merkle_proof(&tx_hashes).unwrap(); assert_eq!(proof.calculate_root(hash), expected_root); @@ -626,7 +548,7 @@ mod test { fn test_borsh_de_serialization() { let tx_hashes = &[Hash::from_str("fa58575f7d1d377709f1621fac98c758860ca6dc5f2262be9ce5fd131c370d1a").unwrap()]; - let proof = create_merkle_proof(&tx_hashes[..], &tx_hashes[0]).unwrap(); + let proof = create_merkle_proof(&tx_hashes[..]).unwrap(); let mut buf = Vec::new(); proof.serialize(&mut buf).unwrap(); buf.extend_from_slice(&[1, 2, 3]); diff --git a/base_layer/core/tests/tests/block_validation.rs b/base_layer/core/tests/tests/block_validation.rs index cd5db651e8..e274c79c6a 100644 --- a/base_layer/core/tests/tests/block_validation.rs +++ b/base_layer/core/tests/tests/block_validation.rs @@ -198,7 +198,7 @@ fn add_monero_data(tblock: &mut Block, seed_key: &str) { monero_rx::append_merge_mining_tag(&mut mblock, hash).unwrap(); let hashes = monero_rx::create_ordered_transaction_hashes_from_block(&mblock); let merkle_root = monero_rx::tree_hash(&hashes).unwrap(); - let coinbase_merkle_proof = monero_rx::create_merkle_proof(&hashes, &hashes[0]).unwrap(); + let coinbase_merkle_proof = monero_rx::create_merkle_proof(&hashes).unwrap(); #[allow(clippy::cast_possible_truncation)] let monero_data = MoneroPowData { header: mblock.header,