From 15621e5211d231874f0fe0e9fc5dd3f9c1625ea5 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 21 May 2024 15:49:38 +0100 Subject: [PATCH] BFT-463: Change LeaderSelectionMode::Sticky to allow multiple keys --- node/libs/roles/src/proto/validator.proto | 2 +- node/libs/roles/src/validator/conv.rs | 13 ++++++++---- .../roles/src/validator/messages/consensus.rs | 19 ++++++++++------- .../roles/src/validator/messages/tests.rs | 21 ++++++++++++------- node/libs/roles/src/validator/testonly.rs | 5 ++++- node/libs/roles/src/validator/tests.rs | 2 +- node/tools/src/tests.rs | 12 +++++++---- 7 files changed, 49 insertions(+), 25 deletions(-) diff --git a/node/libs/roles/src/proto/validator.proto b/node/libs/roles/src/proto/validator.proto index 87600b30..1ee0ef29 100644 --- a/node/libs/roles/src/proto/validator.proto +++ b/node/libs/roles/src/proto/validator.proto @@ -28,7 +28,7 @@ message LeaderSelectionMode { } message RoundRobin{} message Sticky{ - optional PublicKey key = 1; // required + repeated PublicKey keys = 1; // required } message Weighted{} } diff --git a/node/libs/roles/src/validator/conv.rs b/node/libs/roles/src/validator/conv.rs index 0cef59da..811afbe6 100644 --- a/node/libs/roles/src/validator/conv.rs +++ b/node/libs/roles/src/validator/conv.rs @@ -459,8 +459,13 @@ impl ProtoFmt for LeaderSelectionMode { Ok(LeaderSelectionMode::RoundRobin) } proto::leader_selection_mode::Mode::Sticky(inner) => { - let key = required(&inner.key).context("key")?; - Ok(LeaderSelectionMode::Sticky(PublicKey::read(key)?)) + let _ = required(&inner.keys.first()).context("key")?; + let pks = inner + .keys + .iter() + .map(PublicKey::read) + .collect::, _>>()?; + Ok(LeaderSelectionMode::Sticky(pks)) } proto::leader_selection_mode::Mode::Weighted(_) => Ok(LeaderSelectionMode::Weighted), } @@ -472,10 +477,10 @@ impl ProtoFmt for LeaderSelectionMode { proto::leader_selection_mode::RoundRobin {}, )), }, - LeaderSelectionMode::Sticky(pk) => proto::LeaderSelectionMode { + LeaderSelectionMode::Sticky(pks) => proto::LeaderSelectionMode { mode: Some(proto::leader_selection_mode::Mode::Sticky( proto::leader_selection_mode::Sticky { - key: Some(pk.build()), + keys: pks.iter().map(|pk| pk.build()).collect(), }, )), }, diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index 882bdcee..84347a0e 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -59,8 +59,8 @@ pub enum LeaderSelectionMode { /// Select in a round-robin fashion, based on validators' index within the set. RoundRobin, - /// Select based on a sticky assignment to a specific validator. - Sticky(validator::PublicKey), + /// Select based on a sticky assignment to a non-empty list of specific validators. + Sticky(Vec), /// Select pseudo-randomly, based on validators' weights. Weighted, @@ -174,8 +174,9 @@ impl Committee { } unreachable!() } - LeaderSelectionMode::Sticky(pk) => { - let index = self.index(pk).unwrap(); + LeaderSelectionMode::Sticky(pks) => { + let index = view_number.0 as usize % pks.len(); + let index = self.index(&pks[index]).unwrap(); self.get(index).unwrap().key.clone() } } @@ -309,9 +310,13 @@ impl fmt::Debug for Genesis { impl Genesis { /// Verifies correctness. pub fn verify(&self) -> anyhow::Result<()> { - if let LeaderSelectionMode::Sticky(pk) = &self.leader_selection { - if self.validators.index(pk).is_none() { - anyhow::bail!("leader_selection sticky mode public key is not in committee"); + if let LeaderSelectionMode::Sticky(pks) = &self.leader_selection { + for pk in pks { + if self.validators.index(pk).is_none() { + anyhow::bail!( + "leader_selection sticky mode public key is not in committee: {pk:?}" + ); + } } } diff --git a/node/libs/roles/src/validator/messages/tests.rs b/node/libs/roles/src/validator/messages/tests.rs index 32ac4d98..079c7189 100644 --- a/node/libs/roles/src/validator/messages/tests.rs +++ b/node/libs/roles/src/validator/messages/tests.rs @@ -92,14 +92,21 @@ fn test_sticky() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); let committee = validator_committee(); - let want = committee - .get(rng.gen_range(0..committee.len())) - .unwrap() - .key - .clone(); + let mut want = Vec::new(); + for _ in 0..3 { + want.push( + committee + .get(rng.gen_range(0..committee.len())) + .unwrap() + .key + .clone(), + ); + } let sticky = LeaderSelectionMode::Sticky(want.clone()); for _ in 0..100 { - assert_eq!(want, committee.view_leader(rng.gen(), &sticky)); + let vn: ViewNumber = rng.gen(); + let pk = &want[vn.0 as usize % want.len()]; + assert_eq!(*pk, committee.view_leader(vn, &sticky)); } } @@ -203,7 +210,7 @@ mod version1 { fn genesis_verify_leader_pubkey_not_in_committee() { let mut rng = StdRng::seed_from_u64(29483920); let mut genesis = rng.gen::(); - genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); + genesis.leader_selection = LeaderSelectionMode::Sticky(vec![rng.gen()]); let genesis = genesis.with_hash(); assert!(genesis.verify().is_err()) } diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index df7c09c6..f4c612b1 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/node/libs/roles/src/validator/testonly.rs @@ -269,7 +269,10 @@ impl Distribution for Standard { fn sample(&self, rng: &mut R) -> LeaderSelectionMode { match rng.gen_range(0..=2) { 0 => LeaderSelectionMode::RoundRobin, - 1 => LeaderSelectionMode::Sticky(rng.gen()), + 1 => LeaderSelectionMode::Sticky({ + let n = rng.gen_range(1..=3); + rng.sample_iter(Standard).take(n).collect() + }), _ => LeaderSelectionMode::Weighted, } } diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index 2f0136f0..1615102c 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -104,7 +104,7 @@ fn test_genesis_verify() { assert!(Genesis::read(&genesis.build()).is_ok()); let mut genesis = (*genesis).clone(); - genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); + genesis.leader_selection = LeaderSelectionMode::Sticky(vec![rng.gen()]); let genesis = genesis.with_hash(); assert!(genesis.verify().is_err()); assert!(Genesis::read(&genesis.build()).is_err()) diff --git a/node/tools/src/tests.rs b/node/tools/src/tests.rs index 79f6a56c..7db35fd3 100644 --- a/node/tools/src/tests.rs +++ b/node/tools/src/tests.rs @@ -11,10 +11,14 @@ impl Distribution for EncodeDist { fn sample(&self, rng: &mut R) -> AppConfig { let mut genesis: validator::GenesisRaw = rng.gen(); // In order for the genesis to be valid, the sticky leader needs to be in the validator committee. - if let LeaderSelectionMode::Sticky(_) = genesis.leader_selection { - let i = rng.gen_range(0..genesis.validators.len()); - genesis.leader_selection = - LeaderSelectionMode::Sticky(genesis.validators.get(i).unwrap().key.clone()); + if let LeaderSelectionMode::Sticky(pks) = genesis.leader_selection { + let n = pks.len(); + let i = rng.gen_range(0..genesis.committee.len()); + let mut pks = Vec::new(); + for _ in 0..n { + pks.push(genesis.validators.get(i).unwrap().key.clone()); + } + genesis.leader_selection = LeaderSelectionMode::Sticky(pks); } AppConfig { server_addr: self.sample(rng),