From 3da3cfccdb1310c29696ad052b52c22ee08f8662 Mon Sep 17 00:00:00 2001 From: David Dunn <26876072+doubledup@users.noreply.github.com> Date: Thu, 1 Dec 2022 07:27:19 +0200 Subject: [PATCH 1/5] Sort hashes when generating & verifying MMR proofs --- frame/beefy-mmr/primitives/src/lib.rs | 42 +++++++++++++++----------- frame/beefy-mmr/src/tests.rs | 10 +++--- frame/merkle-mountain-range/src/lib.rs | 3 +- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/frame/beefy-mmr/primitives/src/lib.rs b/frame/beefy-mmr/primitives/src/lib.rs index f88fb89acaaab..a41899fac2deb 100644 --- a/frame/beefy-mmr/primitives/src/lib.rs +++ b/frame/beefy-mmr/primitives/src/lib.rs @@ -27,7 +27,8 @@ //! Merkle Tree is constructed from arbitrary-length leaves, that are initially hashed using the //! same hasher as the inner nodes. //! Inner nodes are created by concatenating child hashes and hashing again. The implementation -//! does not perform any sorting of the input data (leaves) nor when inner nodes are created. +//! sorts each pair of hashes before every hash operation. This makes proof verification more +//! efficient by removing the need to track which side each intermediate hash is concatenated on. //! //! If the number of leaves is not even, last leaf (hash of) is promoted to the upper layer. @@ -45,7 +46,7 @@ use beefy_primitives::mmr::{BeefyAuthoritySet, BeefyNextAuthoritySet}; pub fn merkle_root(leaves: I) -> H::Output where H: HashT, - H::Output: Default + AsRef<[u8]>, + H::Output: Default + AsRef<[u8]> + PartialOrd, I: IntoIterator, I::Item: AsRef<[u8]>, { @@ -56,7 +57,7 @@ where fn merkelize(leaves: I, visitor: &mut V) -> H::Output where H: HashT, - H::Output: Default + AsRef<[u8]>, + H::Output: Default + AsRef<[u8]> + PartialOrd, V: Visitor, I: Iterator, { @@ -141,7 +142,7 @@ impl Visitor for () { pub fn merkle_proof(leaves: I, leaf_index: usize) -> MerkleProof where H: HashT, - H::Output: Default + Copy + AsRef<[u8]>, + H::Output: Default + Copy + AsRef<[u8]> + PartialOrd, I: IntoIterator, I::IntoIter: ExactSizeIterator, T: AsRef<[u8]>, @@ -241,7 +242,7 @@ pub fn verify_proof<'a, H, P, L>( ) -> bool where H: HashT, - H::Output: PartialEq + AsRef<[u8]>, + H::Output: PartialEq + AsRef<[u8]> + PartialOrd, P: IntoIterator, L: Into>, { @@ -259,12 +260,12 @@ where let mut position = leaf_index; let mut width = number_of_leaves; let computed = proof.into_iter().fold(leaf_hash, |a, b| { - if position % 2 == 1 || position + 1 == width { - combined[..hash_len].copy_from_slice(&b.as_ref()); - combined[hash_len..].copy_from_slice(&a.as_ref()); - } else { + if a < b { combined[..hash_len].copy_from_slice(&a.as_ref()); combined[hash_len..].copy_from_slice(&b.as_ref()); + } else { + combined[..hash_len].copy_from_slice(&b.as_ref()); + combined[hash_len..].copy_from_slice(&a.as_ref()); } let hash = ::hash(&combined); #[cfg(feature = "debug")] @@ -295,7 +296,7 @@ fn merkelize_row( ) -> Result> where H: HashT, - H::Output: AsRef<[u8]>, + H::Output: AsRef<[u8]> + PartialOrd, V: Visitor, I: Iterator, { @@ -321,8 +322,13 @@ where index += 2; match (a, b) { (Some(a), Some(b)) => { - combined[..hash_len].copy_from_slice(a.as_ref()); - combined[hash_len..].copy_from_slice(b.as_ref()); + if a < b { + combined[..hash_len].copy_from_slice(a.as_ref()); + combined[hash_len..].copy_from_slice(b.as_ref()); + } else { + combined[..hash_len].copy_from_slice(b.as_ref()); + combined[hash_len..].copy_from_slice(a.as_ref()); + } next.push(::hash(&combined)); }, @@ -428,12 +434,12 @@ mod tests { }; test( - "aff1208e69c9e8be9b584b07ebac4e48a1ee9d15ce3afe20b77a4d29e4175aa3", + "5842148bc6ebeb52af882a317c765fccd3ae80589b21a9b8cbf21abb630e46a7", vec!["a", "b", "c"], ); test( - "b8912f7269068901f231a965adfefbc10f0eedcfa61852b103efd54dac7db3d7", + "7b84bec68b13c39798c6c50e9e40a0b268e3c1634db8f4cb97314eb243d4c514", vec!["a", "b", "a"], ); @@ -443,7 +449,7 @@ mod tests { ); test( - "fb3b3be94be9e983ba5e094c9c51a7d96a4fa2e5d8e891df00ca89ba05bb1239", + "cc50382cfd3c9a617741e9a85efee8752b8feb95a2cbecd6365fb21366ce0c8c", vec!["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"], ); } @@ -761,7 +767,7 @@ mod tests { "0xc26B34D375533fFc4c5276282Fa5D660F3d8cbcB", ]; let root: H256 = array_bytes::hex2array_unchecked( - "72b0acd7c302a84f1f6b6cefe0ba7194b7398afb440e1b44a9dbbe270394ca53", + "7b2c6eebec6e85b2e272325a11c31af71df52bc0534d2d4f903e0ced191f022e", ) .into(); @@ -806,11 +812,11 @@ mod tests { ) .into(), array_bytes::hex2array_unchecked( - "d02609d2bbdb28aa25f58b85afec937d5a4c85d37925bce6d0cf802f9d76ba79" + "1fad92ed8d0504ef6c0231bbbeeda960a40693f297c64e87b582beb92ecfb00f" ) .into(), array_bytes::hex2array_unchecked( - "ae3f8991955ed884613b0a5f40295902eea0e0abe5858fc520b72959bc016d4e" + "0b84c852cbcf839d562d826fd935e1b37975ccaa419e1def8d219df4b83dcbf4" ) .into(), ], diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index 1826331f59e53..0f7804e544aa4 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -70,7 +70,7 @@ fn should_contain_mmr_digest() { ValidatorSet::new(vec![mock_beefy_id(1), mock_beefy_id(2)], 1).unwrap() )), beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "95803defe6ea9f41e7ec6afa497064f21bfded027d8812efacbdf984e630cbdc" + "200e73880940ac0b66735ffb560fa0a3989292463d262deac6ad61e78a3e46a4" ))) ] ); @@ -85,13 +85,13 @@ fn should_contain_mmr_digest() { ValidatorSet::new(vec![mock_beefy_id(1), mock_beefy_id(2)], 1).unwrap() )), beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "95803defe6ea9f41e7ec6afa497064f21bfded027d8812efacbdf984e630cbdc" + "200e73880940ac0b66735ffb560fa0a3989292463d262deac6ad61e78a3e46a4" ))), beefy_log(ConsensusLog::AuthoritiesChange( ValidatorSet::new(vec![mock_beefy_id(3), mock_beefy_id(4)], 2).unwrap() )), beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "a73271a0974f1e67d6e9b8dd58e506177a2e556519a330796721e98279a753e2" + "ba37d8d5d195ac8caec391da35472f9ecf1116ff1642409148b62e08896d3884" ))), ] ); @@ -124,7 +124,7 @@ fn should_contain_valid_leaf_data() { ) }, leaf_extra: array_bytes::hex2bytes_unchecked( - "55b8e9e1cc9f0db7776fac0ca66318ef8acfb8ec26db11e373120583e07ee648" + "5572d58c82bddf323f4fc7aecab8a8f0ad6ed2f06ab2bfb8ade36a77a45fcc68" ) } ); @@ -149,7 +149,7 @@ fn should_contain_valid_leaf_data() { ) }, leaf_extra: array_bytes::hex2bytes_unchecked( - "55b8e9e1cc9f0db7776fac0ca66318ef8acfb8ec26db11e373120583e07ee648" + "5572d58c82bddf323f4fc7aecab8a8f0ad6ed2f06ab2bfb8ade36a77a45fcc68" ) } ); diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 46af84d218247..170c1d724f3a2 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -164,7 +164,8 @@ pub mod pallet { + codec::Codec + codec::EncodeLike + scale_info::TypeInfo - + MaxEncodedLen; + + MaxEncodedLen + + PartialOrd; /// Data stored in the leaf nodes. /// From e5fa257a1ec18e122996b60fc14a9d89380b1795 Mon Sep 17 00:00:00 2001 From: David Dunn <26876072+doubledup@users.noreply.github.com> Date: Wed, 7 Dec 2022 04:45:13 +0200 Subject: [PATCH 2/5] Remove unused variables --- frame/beefy-mmr/primitives/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/beefy-mmr/primitives/src/lib.rs b/frame/beefy-mmr/primitives/src/lib.rs index a41899fac2deb..e6f8acefb039a 100644 --- a/frame/beefy-mmr/primitives/src/lib.rs +++ b/frame/beefy-mmr/primitives/src/lib.rs @@ -257,8 +257,6 @@ where let hash_len = ::LENGTH; let mut combined = vec![0_u8; hash_len * 2]; - let mut position = leaf_index; - let mut width = number_of_leaves; let computed = proof.into_iter().fold(leaf_hash, |a, b| { if a < b { combined[..hash_len].copy_from_slice(&a.as_ref()); @@ -276,8 +274,6 @@ where array_bytes::bytes2hex("", &hash.as_ref()), array_bytes::bytes2hex("", &combined.as_ref()) ); - position /= 2; - width = ((width - 1) / 2) + 1; hash }); From f788f5ed85e575a866639316aaef29dffbcd1c8c Mon Sep 17 00:00:00 2001 From: David Dunn <26876072+doubledup@users.noreply.github.com> Date: Mon, 9 Jan 2023 17:07:15 +0400 Subject: [PATCH 3/5] Double-hash leaves in beefy-merkle-tree --- frame/beefy-mmr/primitives/src/lib.rs | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/frame/beefy-mmr/primitives/src/lib.rs b/frame/beefy-mmr/primitives/src/lib.rs index e6f8acefb039a..0adb58b01453c 100644 --- a/frame/beefy-mmr/primitives/src/lib.rs +++ b/frame/beefy-mmr/primitives/src/lib.rs @@ -50,7 +50,7 @@ where I: IntoIterator, I::Item: AsRef<[u8]>, { - let iter = leaves.into_iter().map(|l| ::hash(l.as_ref())); + let iter = leaves.into_iter().map(|l| ::hash(::hash(l.as_ref()).as_ref())); merkelize::(iter, &mut ()).into() } @@ -149,7 +149,7 @@ where { let mut leaf = None; let iter = leaves.into_iter().enumerate().map(|(idx, l)| { - let hash = ::hash(l.as_ref()); + let hash = ::hash(::hash(l.as_ref()).as_ref()); if idx == leaf_index { leaf = Some(l); } @@ -251,7 +251,7 @@ where } let leaf_hash = match leaf.into() { - Leaf::Value(content) => ::hash(content), + Leaf::Value(content) => ::hash(::hash(content).as_ref()), Leaf::Hash(hash) => hash, }; @@ -396,7 +396,7 @@ mod tests { // then assert_eq!( array_bytes::bytes2hex("", out.as_ref()), - "aeb47a269393297f4b0a3c9c9cfd00c7a4195255274cf39d83dabc2fcc9ff3d7" + "3c137409c513c2a370698ff7e042154c00ce1d1a2f98dc630366637a798bbb76" ); } @@ -415,7 +415,7 @@ mod tests { // then assert_eq!( array_bytes::bytes2hex("", out.as_ref()), - "697ea2a8fe5b03468548a7a413424a6292ab44a82a6f5cc594c3fa7dda7ce402" + "4f54979717a78e07c0730c45f96ce77a3d9f4d55ab3bcdbbd8d8daf58932daa8" ); } @@ -430,22 +430,22 @@ mod tests { }; test( - "5842148bc6ebeb52af882a317c765fccd3ae80589b21a9b8cbf21abb630e46a7", + "ec5728bad90d3185989eddaae1e83be5f3e03ad9f48bd743df58dad4b7625505", vec!["a", "b", "c"], ); test( - "7b84bec68b13c39798c6c50e9e40a0b268e3c1634db8f4cb97314eb243d4c514", + "9e0a460a09c8bec16bd24a3a88e7c4a0c63a1a0e83146d0b1c261b5197095bfc", vec!["a", "b", "a"], ); test( - "dc8e73fe6903148ff5079baecc043983625c23b39f31537e322cd0deee09fa9c", + "0eddd6bcda73a09e07b81108e219f13ed34ab0a9bee6f9477dc99b0e0f760e07", vec!["a", "b", "a", "b"], ); test( - "cc50382cfd3c9a617741e9a85efee8752b8feb95a2cbecd6365fb21366ce0c8c", + "65db6ad4efda46af046d7ecfe41dc4f29012e2968cbdac705e07b7fb1e14bd12", vec!["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"], ); } @@ -763,7 +763,7 @@ mod tests { "0xc26B34D375533fFc4c5276282Fa5D660F3d8cbcB", ]; let root: H256 = array_bytes::hex2array_unchecked( - "7b2c6eebec6e85b2e272325a11c31af71df52bc0534d2d4f903e0ced191f022e", + "9ecb057963d989c798e96adbc3279e639d26fed5e2d85d36222d4902ca38dceb", ) .into(); @@ -800,19 +800,19 @@ mod tests { root, proof: vec![ array_bytes::hex2array_unchecked( - "340bcb1d49b2d82802ddbcf5b85043edb3427b65d09d7f758fbc76932ad2da2f" + "4b30a34ddaf2c978393cfaaff86b63a70dd528a1206720a72cc4f1e7c4b868ba" ) .into(), array_bytes::hex2array_unchecked( - "ba0580e5bd530bc93d61276df7969fb5b4ae8f1864b4a28c280249575198ff1f" + "841ab2af60ad43be1d28693431012681cba4ac997900d67c7cb228da59a9a91a" ) .into(), array_bytes::hex2array_unchecked( - "1fad92ed8d0504ef6c0231bbbeeda960a40693f297c64e87b582beb92ecfb00f" + "b1316e36d2b5bda678ad074255653bcc29b68273dc4d0693c922533fd71fe09d" ) .into(), array_bytes::hex2array_unchecked( - "0b84c852cbcf839d562d826fd935e1b37975ccaa419e1def8d219df4b83dcbf4" + "8e5deae2101af5e99a1e1e447e66afa51a0a71605b4112897962b5e66720e648" ) .into(), ], From 27aafb6c5deb487eddb7b65c18f55b903a65b6c1 Mon Sep 17 00:00:00 2001 From: David Dunn <26876072+doubledup@users.noreply.github.com> Date: Mon, 9 Jan 2023 17:45:40 +0400 Subject: [PATCH 4/5] Revert "Double-hash leaves in beefy-merkle-tree" This reverts commit f788f5ed85e575a866639316aaef29dffbcd1c8c. --- frame/beefy-mmr/primitives/src/lib.rs | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/frame/beefy-mmr/primitives/src/lib.rs b/frame/beefy-mmr/primitives/src/lib.rs index 0adb58b01453c..e6f8acefb039a 100644 --- a/frame/beefy-mmr/primitives/src/lib.rs +++ b/frame/beefy-mmr/primitives/src/lib.rs @@ -50,7 +50,7 @@ where I: IntoIterator, I::Item: AsRef<[u8]>, { - let iter = leaves.into_iter().map(|l| ::hash(::hash(l.as_ref()).as_ref())); + let iter = leaves.into_iter().map(|l| ::hash(l.as_ref())); merkelize::(iter, &mut ()).into() } @@ -149,7 +149,7 @@ where { let mut leaf = None; let iter = leaves.into_iter().enumerate().map(|(idx, l)| { - let hash = ::hash(::hash(l.as_ref()).as_ref()); + let hash = ::hash(l.as_ref()); if idx == leaf_index { leaf = Some(l); } @@ -251,7 +251,7 @@ where } let leaf_hash = match leaf.into() { - Leaf::Value(content) => ::hash(::hash(content).as_ref()), + Leaf::Value(content) => ::hash(content), Leaf::Hash(hash) => hash, }; @@ -396,7 +396,7 @@ mod tests { // then assert_eq!( array_bytes::bytes2hex("", out.as_ref()), - "3c137409c513c2a370698ff7e042154c00ce1d1a2f98dc630366637a798bbb76" + "aeb47a269393297f4b0a3c9c9cfd00c7a4195255274cf39d83dabc2fcc9ff3d7" ); } @@ -415,7 +415,7 @@ mod tests { // then assert_eq!( array_bytes::bytes2hex("", out.as_ref()), - "4f54979717a78e07c0730c45f96ce77a3d9f4d55ab3bcdbbd8d8daf58932daa8" + "697ea2a8fe5b03468548a7a413424a6292ab44a82a6f5cc594c3fa7dda7ce402" ); } @@ -430,22 +430,22 @@ mod tests { }; test( - "ec5728bad90d3185989eddaae1e83be5f3e03ad9f48bd743df58dad4b7625505", + "5842148bc6ebeb52af882a317c765fccd3ae80589b21a9b8cbf21abb630e46a7", vec!["a", "b", "c"], ); test( - "9e0a460a09c8bec16bd24a3a88e7c4a0c63a1a0e83146d0b1c261b5197095bfc", + "7b84bec68b13c39798c6c50e9e40a0b268e3c1634db8f4cb97314eb243d4c514", vec!["a", "b", "a"], ); test( - "0eddd6bcda73a09e07b81108e219f13ed34ab0a9bee6f9477dc99b0e0f760e07", + "dc8e73fe6903148ff5079baecc043983625c23b39f31537e322cd0deee09fa9c", vec!["a", "b", "a", "b"], ); test( - "65db6ad4efda46af046d7ecfe41dc4f29012e2968cbdac705e07b7fb1e14bd12", + "cc50382cfd3c9a617741e9a85efee8752b8feb95a2cbecd6365fb21366ce0c8c", vec!["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"], ); } @@ -763,7 +763,7 @@ mod tests { "0xc26B34D375533fFc4c5276282Fa5D660F3d8cbcB", ]; let root: H256 = array_bytes::hex2array_unchecked( - "9ecb057963d989c798e96adbc3279e639d26fed5e2d85d36222d4902ca38dceb", + "7b2c6eebec6e85b2e272325a11c31af71df52bc0534d2d4f903e0ced191f022e", ) .into(); @@ -800,19 +800,19 @@ mod tests { root, proof: vec![ array_bytes::hex2array_unchecked( - "4b30a34ddaf2c978393cfaaff86b63a70dd528a1206720a72cc4f1e7c4b868ba" + "340bcb1d49b2d82802ddbcf5b85043edb3427b65d09d7f758fbc76932ad2da2f" ) .into(), array_bytes::hex2array_unchecked( - "841ab2af60ad43be1d28693431012681cba4ac997900d67c7cb228da59a9a91a" + "ba0580e5bd530bc93d61276df7969fb5b4ae8f1864b4a28c280249575198ff1f" ) .into(), array_bytes::hex2array_unchecked( - "b1316e36d2b5bda678ad074255653bcc29b68273dc4d0693c922533fd71fe09d" + "1fad92ed8d0504ef6c0231bbbeeda960a40693f297c64e87b582beb92ecfb00f" ) .into(), array_bytes::hex2array_unchecked( - "8e5deae2101af5e99a1e1e447e66afa51a0a71605b4112897962b5e66720e648" + "0b84c852cbcf839d562d826fd935e1b37975ccaa419e1def8d219df4b83dcbf4" ) .into(), ], From 43d79576629a6aae1e1eb509a0226b3cb190a820 Mon Sep 17 00:00:00 2001 From: David Dunn <26876072+doubledup@users.noreply.github.com> Date: Tue, 10 Jan 2023 13:26:56 +0400 Subject: [PATCH 5/5] Retry Polkadot companion CI jobs