From 9f512c94089a8af7f766059aab70c1a4dc62c903 Mon Sep 17 00:00:00 2001 From: Federico Franzoni <8609060+fed-franz@users.noreply.github.com> Date: Thu, 1 Jun 2023 17:43:32 +0200 Subject: [PATCH 1/2] Fix Sortition hash This changes the hash from H(round||counter||step||seed) to H(seed||round||step||counter), to be compliant with Deterministic Sortition specification. Tests 'test_sortition_hash()' and 'test_generate_sortition_score()' are also updated. --- consensus/src/user/sortition.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/consensus/src/user/sortition.rs b/consensus/src/user/sortition.rs index 9e0af709ff..8c19143673 100644 --- a/consensus/src/user/sortition.rs +++ b/consensus/src/user/sortition.rs @@ -43,10 +43,10 @@ pub fn create_sortition_hash(cfg: &Config, counter: u32) -> [u8; 32] { let mut hasher = Sha3_256::new(); // write input message + hasher.update(&cfg.seed.inner()[..]); hasher.update(cfg.round.to_le_bytes()); - hasher.update(counter.to_le_bytes()); hasher.update(cfg.step.to_le_bytes()); - hasher.update(&cfg.seed.inner()[..]); + hasher.update(counter.to_le_bytes()); // read hash digest let reader = hasher.finalize(); @@ -76,12 +76,13 @@ mod tests { }; #[test] - pub fn test_sortition_hash() { + pub fn test_sortition_hash() { let hash = [ - 56, 81, 125, 39, 109, 105, 243, 20, 138, 196, 236, 197, 7, 155, 41, - 26, 217, 150, 9, 226, 76, 174, 67, 1, 230, 187, 81, 107, 192, 5, - 13, 73, + 134, 22, 162, 136, 186, 35, 16, 207, 237, 50, 11, 236, 74, 189, 37, + 137, 101, 205, 53, 161, 248, 199, 195, 228, 68, 68, 95, 223, 239, 199, + 1, 7, ]; + assert_eq!( create_sortition_hash( &Config::new(Seed::from([3; 48]), 10, 3, 0), @@ -94,7 +95,7 @@ mod tests { #[test] pub fn test_generate_sortition_score() { let dataset = - vec![([3; 48], 123342342, 78899961), ([4; 48], 44443333, 5505832)]; + vec![([3; 48], 123342342, 66422677), ([4; 48], 44443333, 22757716)]; for (seed, total_weight, expected_score) in dataset { let hash = create_sortition_hash( From 54c3aa4344c9d5293ae36543e08468262b952f64 Mon Sep 17 00:00:00 2001 From: Federico Franzoni <8609060+fed-franz@users.noreply.github.com> Date: Thu, 1 Jun 2023 20:22:38 +0200 Subject: [PATCH 2/2] Update tests to new sortition hash Fixed tests: - test_collect_votes - test_deterministic_sortition_1 - test_deterministic_sortition_2 test_collect_votes code has been simplified. --- consensus/src/aggregator.rs | 87 ++++++++++++++------------------- consensus/src/user/sortition.rs | 12 +++-- consensus/tests/sortition.rs | 7 ++- 3 files changed, 48 insertions(+), 58 deletions(-) diff --git a/consensus/src/aggregator.rs b/consensus/src/aggregator.rs index 50b148d8e9..247f0fb122 100644 --- a/consensus/src/aggregator.rs +++ b/consensus/src/aggregator.rs @@ -220,60 +220,45 @@ mod tests { let mut a = Aggregator::default(); - // Ensure total value for this block_hash is increased with 2 on a vote - // from a provisioner at pos 1. - let (signature, h) = input.get(1).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); dbg!("{:?}", p); - // this provisioner vote has weight of 2 - assert_eq!(a.get_total(block_hash), Some(2)); - - // Ensure total value for this block_hash is increased with 1 on a vote - // from a provisioner at pos 0. - let (signature, h) = input.get(0).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); - - // this provisioner vote has weight of 1 - assert_eq!(a.get_total(block_hash), Some(3)); - - // Ensure total value for this block_hash is increased with 1 on a vote - // from a provisioner at pos 5. - let (signature, h) = input.get(5).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); - - // this provisioner has weight of 1. Total should be sum of weights of - // all prior votes. - assert_eq!(a.get_total(block_hash), Some(4)); - - // Ensure a duplicated vote is discarded - let (signature, h) = input.get(5).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); - assert_eq!(a.get_total(block_hash), Some(4)); - - let (signature, h) = input.get(6).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); - // this provisioner vote has weight of 0. - - assert_eq!(a.get_total(block_hash), Some(4)); - - let (signature, h) = input.get(8).expect("invalid index"); - assert!(a.collect_vote(&c, h, signature).is_none()); - // this provisioner vote has weight of 0. Quorum threshold still not - // reached - assert_eq!(a.get_total(block_hash), Some(4)); - - let (signature, h) = input.get(9).expect("invalid index"); - let (hash, sv) = a - .collect_vote(&c, h, signature) - .expect("failed to reach quorum"); + // Collect votes from expected committee members + let expected_members = vec![0, 1, 2, 4, 5]; + let expected_votes = vec![1, 1, 2, 1, 3]; + let mut collected_votes = 0; + for i in 0..expected_members.len() - 1 { + // Select provisioner + let (signature, h) = + input.get(expected_members[i]).expect("invalid index"); + + // Last member's vote should reach the quorum + if i == expected_members.len() - 1 { + // (hash, sv) is only returned in case we reach the quorum + let (hash, sv) = a + .collect_vote(&c, h, signature) + .expect("failed to reach quorum"); + + // Check expected block hash + assert_eq!(hash, block_hash); + + // Check expected StepVotes bitset + // bitset: 0b00000000000000000000000000000000000000000000000000000000011111 + println!("bitset: {:#064b}", sv.bitset); + assert_eq!(sv.bitset, 31); + + break; + } - assert_eq!(hash, block_hash); - // this provisioner vote has weight of 3. Quorum reached - assert!(a.get_total(block_hash).unwrap() >= c.quorum()); + // Check collected votes + assert!(a.collect_vote(&c, h, signature).is_none()); + collected_votes += expected_votes[i]; + assert_eq!(a.get_total(block_hash), Some(collected_votes)); - // bitset: 0b00000000000000000000000000000000000000000000000000000000100111 - println!("bitset: {:#064b}", sv.bitset); - assert_eq!(sv.bitset, 39); + // Ensure a duplicated vote is discarded + if i == 0 { + assert!(a.collect_vote(&c, h, signature).is_none()); + assert_eq!(a.get_total(block_hash), Some(collected_votes)); + } + } } } diff --git a/consensus/src/user/sortition.rs b/consensus/src/user/sortition.rs index 8c19143673..4008c81aa4 100644 --- a/consensus/src/user/sortition.rs +++ b/consensus/src/user/sortition.rs @@ -76,11 +76,11 @@ mod tests { }; #[test] - pub fn test_sortition_hash() { + pub fn test_sortition_hash() { let hash = [ 134, 22, 162, 136, 186, 35, 16, 207, 237, 50, 11, 236, 74, 189, 37, - 137, 101, 205, 53, 161, 248, 199, 195, 228, 68, 68, 95, 223, 239, 199, - 1, 7, + 137, 101, 205, 53, 161, 248, 199, 195, 228, 68, 68, 95, 223, 239, + 199, 1, 7, ]; assert_eq!( @@ -94,8 +94,10 @@ mod tests { #[test] pub fn test_generate_sortition_score() { - let dataset = - vec![([3; 48], 123342342, 66422677), ([4; 48], 44443333, 22757716)]; + let dataset = vec![ + ([3; 48], 123342342, 66422677), + ([4; 48], 44443333, 22757716), + ]; for (seed, total_weight, expected_score) in dataset { let hash = create_sortition_hash( diff --git a/consensus/tests/sortition.rs b/consensus/tests/sortition.rs index 62863ccfad..8bfb2a2901 100644 --- a/consensus/tests/sortition.rs +++ b/consensus/tests/sortition.rs @@ -25,11 +25,14 @@ fn test_deterministic_sortition_1() { let committee = Committee::new(PublicKey::default(), &mut p, cfg); + // Verify expected committee size assert_eq!( committee_size, committee.get_occurrences().iter().sum::() ); - assert_eq!(vec![7, 23, 13, 21], committee.get_occurrences()); + + // Verify expected distribution + assert_eq!(vec![7, 32, 7, 18], committee.get_occurrences()); } #[test] @@ -45,7 +48,7 @@ fn test_deterministic_sortition_2() { committee_size, committee.get_occurrences().iter().sum::() ); - assert_eq!(vec![5, 13, 14, 13], committee.get_occurrences()); + assert_eq!(vec![7, 15, 10, 13], committee.get_occurrences()); } #[test]