From ed6409f345832a6a1910313e44728601a347c8bf Mon Sep 17 00:00:00 2001 From: mdelle1 <108158289+mdelle1@users.noreply.github.com> Date: Thu, 22 Feb 2024 20:06:04 -0500 Subject: [PATCH 1/8] Updates authorized validator set --- node/bft/src/gateway.rs | 48 ++++++++++++++++++++++++++++++++++------- node/bft/src/primary.rs | 2 +- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index 06030e5d5d..21191860c5 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -14,7 +14,7 @@ use crate::{ events::{EventCodec, PrimaryPing}, - helpers::{assign_to_worker, Cache, PrimarySender, Resolver, SyncSender, WorkerSender}, + helpers::{assign_to_worker, Cache, PrimarySender, Resolver, SyncSender, WorkerSender, Storage}, spawn_blocking, Worker, CONTEXT, @@ -88,6 +88,9 @@ const MIN_CONNECTED_VALIDATORS: usize = 175; /// The maximum number of validators to send in a validators response event. const MAX_VALIDATORS_TO_SEND: usize = 200; +/// The maximum number of blocks tolerated before the primary is considered behind its peers. +pub const MAX_BLOCKS_BEHIND: u32 = 1; // blocks + /// Part of the Gateway API that deals with networking. /// This is a separate trait to allow for easier testing/mocking. #[async_trait] @@ -100,6 +103,8 @@ pub trait Transport: Send + Sync { pub struct Gateway { /// The account of the node. account: Account, + /// The storage. + storage: Storage, /// The ledger service. ledger: Arc>, /// The TCP stack. @@ -133,6 +138,7 @@ impl Gateway { /// Initializes a new gateway. pub fn new( account: Account, + storage: Storage, ledger: Arc>, ip: Option, trusted_validators: &[SocketAddr], @@ -149,6 +155,7 @@ impl Gateway { // Return the gateway. Ok(Self { account, + storage, ledger, tcp, cache: Default::default(), @@ -335,13 +342,38 @@ impl Gateway { // 1. New validators should be able to connect immediately once bonded as a committee member. // 2. Existing validators must remain connected until they are no longer bonded as a committee member. // (i.e. meaning they must stay online until the next block has been produced) - self.ledger - .get_committee_lookback_for_round(self.ledger.latest_round()) - .map_or(false, |committee| committee.is_committee_member(validator_address)) - || self - .ledger - .current_committee() - .map_or(false, |committee| committee.is_committee_member(validator_address)) + + // Retrieve the latest ledger block height. + let latest_ledger_height = self.ledger.latest_block_height(); + // Retrieve the earliest block height to consider from the sync range tolerance. + let earliest_sync_ledger_height = latest_ledger_height.saturating_sub(MAX_BLOCKS_BEHIND); + // Initialize a flag to check if the validator is in a previous committee with lookback. + let mut is_val_in_previous_committee_lookback = false; + + // Determine if the validator is in any of the previous committees with lookback up until the sync range tolerance. + if let Ok(block) = self.ledger.get_block(earliest_sync_ledger_height){ + // Retrieve the block round. + let block_round = block.header().metadata().round(); + for round in (block_round..self.storage.current_round()).step_by(2){ + if let Ok(previous_committee_lookback) = self.ledger.get_committee_lookback_for_round(round) { + // Determine if the validator is in the committee. + if previous_committee_lookback.is_committee_member(validator_address){ + // Set the tracker to 'true'. + is_val_in_previous_committee_lookback = true; + break; + } + } + } + } + // Determine if the validator is in the current committee with lookback. + let is_val_in_current_committee_lookback = match self.ledger.get_committee_lookback_for_round(self.storage.current_round()) { + Ok(current_committee_lookback) => current_committee_lookback.is_committee_member(validator_address), + Err(_) => false, + }; + // Determine if the validator is in the latest committee on the ledger. + let is_val_in_latest_committee = self.ledger.current_committee().map_or(false, |committee| committee.is_committee_member(validator_address)); + + return is_val_in_previous_committee_lookback || is_val_in_current_committee_lookback || is_val_in_latest_committee; } /// Returns the maximum number of connected peers. diff --git a/node/bft/src/primary.rs b/node/bft/src/primary.rs index 8dea9e33a2..aea25c4861 100644 --- a/node/bft/src/primary.rs +++ b/node/bft/src/primary.rs @@ -108,7 +108,7 @@ impl Primary { dev: Option, ) -> Result { // Initialize the gateway. - let gateway = Gateway::new(account, ledger.clone(), ip, trusted_validators, dev)?; + let gateway = Gateway::new(account, storage.clone(), ledger.clone(), ip, trusted_validators, dev)?; // Initialize the sync module. let sync = Sync::new(gateway.clone(), storage.clone(), ledger.clone()); // Initialize the primary instance. From b1c0ddc9a291430b29386e286df7776945041b14 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:53:16 -0800 Subject: [PATCH 2/8] Cleanup --- node/bft/src/gateway.rs | 81 +++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index 21191860c5..277b9ae53d 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -14,7 +14,7 @@ use crate::{ events::{EventCodec, PrimaryPing}, - helpers::{assign_to_worker, Cache, PrimarySender, Resolver, SyncSender, WorkerSender, Storage}, + helpers::{assign_to_worker, Cache, PrimarySender, Resolver, Storage, SyncSender, WorkerSender}, spawn_blocking, Worker, CONTEXT, @@ -39,7 +39,7 @@ use snarkos_node_bft_events::{ ValidatorsResponse, }; use snarkos_node_bft_ledger_service::LedgerService; -use snarkos_node_sync::communication_service::CommunicationService; +use snarkos_node_sync::{communication_service::CommunicationService, MAX_BLOCKS_BEHIND}; use snarkos_node_tcp::{ is_bogon_ip, is_unspecified_or_broadcast_ip, @@ -88,9 +88,6 @@ const MIN_CONNECTED_VALIDATORS: usize = 175; /// The maximum number of validators to send in a validators response event. const MAX_VALIDATORS_TO_SEND: usize = 200; -/// The maximum number of blocks tolerated before the primary is considered behind its peers. -pub const MAX_BLOCKS_BEHIND: u32 = 1; // blocks - /// Part of the Gateway API that deals with networking. /// This is a separate trait to allow for easier testing/mocking. #[async_trait] @@ -103,8 +100,8 @@ pub trait Transport: Send + Sync { pub struct Gateway { /// The account of the node. account: Account, - /// The storage. - storage: Storage, + /// The storage. + storage: Storage, /// The ledger service. ledger: Arc>, /// The TCP stack. @@ -337,43 +334,35 @@ impl Gateway { /// Returns `true` if the given address is an authorized validator. pub fn is_authorized_validator_address(&self, validator_address: Address) -> bool { - // Determine if the validator address is a member of the committee lookback or the current committee. + // Retrieve the previous block height to consider from the sync tolerance. + let previous_block_height = self.ledger.latest_block_height().saturating_sub(MAX_BLOCKS_BEHIND); + // Determine if the validator is in any of the previous committee lookbacks. + let exists_in_previous_committee_lookback = match self.ledger.get_block(previous_block_height) { + Ok(block) => (block.round()..self.storage.current_round()).step_by(2).any(|round| { + self.ledger + .get_committee_lookback_for_round(round) + .map_or(false, |committee| committee.is_committee_member(validator_address)) + }), + Err(_) => false, + }; + + // Determine if the validator is in the current committee with lookback. + let exists_in_current_committee_lookback = self + .ledger + .get_committee_lookback_for_round(self.storage.current_round()) + .map_or(false, |committee| committee.is_committee_member(validator_address)); + + // Determine if the validator is in the latest committee on the ledger. + let exists_in_latest_committee = + self.ledger.current_committee().map_or(false, |committee| committee.is_committee_member(validator_address)); + + // Determine if the validator address is a member of the previous committee lookbacks, + // the committee lookback, or the current committee. // We allow leniency in this validation check in order to accommodate these two scenarios: // 1. New validators should be able to connect immediately once bonded as a committee member. // 2. Existing validators must remain connected until they are no longer bonded as a committee member. // (i.e. meaning they must stay online until the next block has been produced) - - // Retrieve the latest ledger block height. - let latest_ledger_height = self.ledger.latest_block_height(); - // Retrieve the earliest block height to consider from the sync range tolerance. - let earliest_sync_ledger_height = latest_ledger_height.saturating_sub(MAX_BLOCKS_BEHIND); - // Initialize a flag to check if the validator is in a previous committee with lookback. - let mut is_val_in_previous_committee_lookback = false; - - // Determine if the validator is in any of the previous committees with lookback up until the sync range tolerance. - if let Ok(block) = self.ledger.get_block(earliest_sync_ledger_height){ - // Retrieve the block round. - let block_round = block.header().metadata().round(); - for round in (block_round..self.storage.current_round()).step_by(2){ - if let Ok(previous_committee_lookback) = self.ledger.get_committee_lookback_for_round(round) { - // Determine if the validator is in the committee. - if previous_committee_lookback.is_committee_member(validator_address){ - // Set the tracker to 'true'. - is_val_in_previous_committee_lookback = true; - break; - } - } - } - } - // Determine if the validator is in the current committee with lookback. - let is_val_in_current_committee_lookback = match self.ledger.get_committee_lookback_for_round(self.storage.current_round()) { - Ok(current_committee_lookback) => current_committee_lookback.is_committee_member(validator_address), - Err(_) => false, - }; - // Determine if the validator is in the latest committee on the ledger. - let is_val_in_latest_committee = self.ledger.current_committee().map_or(false, |committee| committee.is_committee_member(validator_address)); - - return is_val_in_previous_committee_lookback || is_val_in_current_committee_lookback || is_val_in_latest_committee; + exists_in_previous_committee_lookback || exists_in_current_committee_lookback || exists_in_latest_committee } /// Returns the maximum number of connected peers. @@ -1434,6 +1423,7 @@ mod prop_tests { .prop_map(|(storage, _, private_key, address)| { Gateway::new( Account::try_from(private_key).unwrap(), + storage.clone(), storage.ledger().clone(), address.ip(), &[], @@ -1482,7 +1472,9 @@ mod prop_tests { let (storage, _, private_key, dev) = input; let account = Account::try_from(private_key).unwrap(); - let gateway = Gateway::new(account.clone(), storage.ledger().clone(), dev.ip(), &[], dev.port()).unwrap(); + let gateway = + Gateway::new(account.clone(), storage.clone(), storage.ledger().clone(), dev.ip(), &[], dev.port()) + .unwrap(); let tcp_config = gateway.tcp().config(); assert_eq!(tcp_config.listener_ip, Some(IpAddr::V4(Ipv4Addr::LOCALHOST))); assert_eq!(tcp_config.desired_listening_port, Some(MEMORY_POOL_PORT + dev.port().unwrap())); @@ -1497,7 +1489,9 @@ mod prop_tests { let (storage, _, private_key, dev) = input; let account = Account::try_from(private_key).unwrap(); - let gateway = Gateway::new(account.clone(), storage.ledger().clone(), dev.ip(), &[], dev.port()).unwrap(); + let gateway = + Gateway::new(account.clone(), storage.clone(), storage.ledger().clone(), dev.ip(), &[], dev.port()) + .unwrap(); let tcp_config = gateway.tcp().config(); if let Some(socket_addr) = dev.ip() { assert_eq!(tcp_config.listener_ip, Some(socket_addr.ip())); @@ -1522,7 +1516,8 @@ mod prop_tests { let worker_storage = storage.clone(); let account = Account::try_from(private_key).unwrap(); - let gateway = Gateway::new(account, storage.ledger().clone(), dev.ip(), &[], dev.port()).unwrap(); + let gateway = + Gateway::new(account, storage.clone(), storage.ledger().clone(), dev.ip(), &[], dev.port()).unwrap(); let (primary_sender, _) = init_primary_channels(); From 9bd3df5221a17803334d8d0301517808accd2277 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:40:39 -0800 Subject: [PATCH 3/8] Fix mock ledger test with get_block_round --- node/bft/ledger-service/src/ledger.rs | 5 ++++ node/bft/ledger-service/src/mock.rs | 28 ++++++++++++++++++---- node/bft/ledger-service/src/prover.rs | 5 ++++ node/bft/ledger-service/src/traits.rs | 3 +++ node/bft/ledger-service/src/translucent.rs | 5 ++++ node/bft/src/gateway.rs | 4 ++-- node/bft/src/worker.rs | 1 + 7 files changed, 44 insertions(+), 7 deletions(-) diff --git a/node/bft/ledger-service/src/ledger.rs b/node/bft/ledger-service/src/ledger.rs index 1f42f909b0..4864534aa1 100644 --- a/node/bft/ledger-service/src/ledger.rs +++ b/node/bft/ledger-service/src/ledger.rs @@ -89,6 +89,11 @@ impl> LedgerService for CoreLedgerService< self.ledger.get_hash(height) } + /// Returns the block round for the given block height, if it exists. + fn get_block_round(&self, height: u32) -> Result { + self.ledger.get_block(height).map(|block| block.round()) + } + /// Returns the block for the given block height. fn get_block(&self, height: u32) -> Result> { self.ledger.get_block(height) diff --git a/node/bft/ledger-service/src/mock.rs b/node/bft/ledger-service/src/mock.rs index c36679a40e..26974f3061 100644 --- a/node/bft/ledger-service/src/mock.rs +++ b/node/bft/ledger-service/src/mock.rs @@ -32,7 +32,7 @@ use tracing::*; #[derive(Debug)] pub struct MockLedgerService { committee: Committee, - height_to_hash: Mutex>, + height_to_hash: Mutex>, } impl MockLedgerService { @@ -45,7 +45,7 @@ impl MockLedgerService { pub fn new_at_height(committee: Committee, height: u32) -> Self { let mut height_to_hash = BTreeMap::new(); for i in 0..=height { - height_to_hash.insert(i, (Field::::from_u32(i)).into()); + height_to_hash.insert(i, (i as u64 * 2, Field::::from_u32(i).into())); } Self { committee, height_to_hash: Mutex::new(height_to_hash) } } @@ -75,7 +75,12 @@ impl LedgerService for MockLedgerService { /// Returns the canonical block height for the given block hash, if it exists. fn get_block_height(&self, hash: &N::BlockHash) -> Result { - match self.height_to_hash.lock().iter().find_map(|(height, h)| if h == hash { Some(*height) } else { None }) { + match self + .height_to_hash + .lock() + .iter() + .find_map(|(height, (_, h))| if h == hash { Some(*height) } else { None }) + { Some(height) => Ok(height), None => bail!("Missing block {hash}"), } @@ -84,7 +89,20 @@ impl LedgerService for MockLedgerService { /// Returns the canonical block hash for the given block height, if it exists. fn get_block_hash(&self, height: u32) -> Result { match self.height_to_hash.lock().get(&height).cloned() { - Some(hash) => Ok(hash), + Some((_, hash)) => Ok(hash), + None => bail!("Missing block {height}"), + } + } + + /// Returns the block round for the given block height, if it exists. + fn get_block_round(&self, height: u32) -> Result { + match self + .height_to_hash + .lock() + .iter() + .find_map(|(h, (round, _))| if *h == height { Some(*round) } else { None }) + { + Some(round) => Ok(round), None => bail!("Missing block {height}"), } } @@ -197,7 +215,7 @@ impl LedgerService for MockLedgerService { block.height(), self.latest_block_height() ); - self.height_to_hash.lock().insert(block.height(), block.hash()); + self.height_to_hash.lock().insert(block.height(), (block.round(), block.hash())); Ok(()) } } diff --git a/node/bft/ledger-service/src/prover.rs b/node/bft/ledger-service/src/prover.rs index 6edfbafd4c..357e1f49cf 100644 --- a/node/bft/ledger-service/src/prover.rs +++ b/node/bft/ledger-service/src/prover.rs @@ -71,6 +71,11 @@ impl LedgerService for ProverLedgerService { bail!("Block {height} does not exist in prover") } + /// Returns the block round for the given block height, if it exists. + fn get_block_round(&self, height: u32) -> Result { + bail!("Block {height} does not exist in prover") + } + /// Returns the block for the given block height. fn get_block(&self, height: u32) -> Result> { bail!("Block {height} does not exist in prover") diff --git a/node/bft/ledger-service/src/traits.rs b/node/bft/ledger-service/src/traits.rs index a97fdeb373..c4700afa8f 100644 --- a/node/bft/ledger-service/src/traits.rs +++ b/node/bft/ledger-service/src/traits.rs @@ -45,6 +45,9 @@ pub trait LedgerService: Debug + Send + Sync { /// Returns the block hash for the given block height, if it exists. fn get_block_hash(&self, height: u32) -> Result; + /// Returns the block round for the given block height, if it exists. + fn get_block_round(&self, height: u32) -> Result; + /// Returns the block for the given block height. fn get_block(&self, height: u32) -> Result>; diff --git a/node/bft/ledger-service/src/translucent.rs b/node/bft/ledger-service/src/translucent.rs index 34afa15667..4077a4d751 100644 --- a/node/bft/ledger-service/src/translucent.rs +++ b/node/bft/ledger-service/src/translucent.rs @@ -82,6 +82,11 @@ impl> LedgerService for TranslucentLedgerS self.inner.get_block_hash(height) } + /// Returns the block round for the given block height, if it exists. + fn get_block_round(&self, height: u32) -> Result { + self.inner.get_block_round(height) + } + /// Returns the block for the given block height. fn get_block(&self, height: u32) -> Result> { self.inner.get_block(height) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index 277b9ae53d..c51184fece 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -337,8 +337,8 @@ impl Gateway { // Retrieve the previous block height to consider from the sync tolerance. let previous_block_height = self.ledger.latest_block_height().saturating_sub(MAX_BLOCKS_BEHIND); // Determine if the validator is in any of the previous committee lookbacks. - let exists_in_previous_committee_lookback = match self.ledger.get_block(previous_block_height) { - Ok(block) => (block.round()..self.storage.current_round()).step_by(2).any(|round| { + let exists_in_previous_committee_lookback = match self.ledger.get_block_round(previous_block_height) { + Ok(block_round) => (block_round..self.storage.current_round()).step_by(2).any(|round| { self.ledger .get_committee_lookback_for_round(round) .map_or(false, |committee| committee.is_committee_member(validator_address)) diff --git a/node/bft/src/worker.rs b/node/bft/src/worker.rs index 4e59a399e8..7b0d38480e 100644 --- a/node/bft/src/worker.rs +++ b/node/bft/src/worker.rs @@ -492,6 +492,7 @@ mod tests { fn contains_block_height(&self, height: u32) -> bool; fn get_block_height(&self, hash: &N::BlockHash) -> Result; fn get_block_hash(&self, height: u32) -> Result; + fn get_block_round(&self, height: u32) -> Result; fn get_block(&self, height: u32) -> Result>; fn get_blocks(&self, heights: Range) -> Result>>; fn get_solution(&self, solution_id: &PuzzleCommitment) -> Result>; From 1b89d09c3fc003d382549388514d5028eda200d4 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Tue, 27 Feb 2024 22:15:20 -0800 Subject: [PATCH 4/8] Clarify MockLedgerService variable name --- node/bft/ledger-service/src/mock.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/node/bft/ledger-service/src/mock.rs b/node/bft/ledger-service/src/mock.rs index 26974f3061..df9530b201 100644 --- a/node/bft/ledger-service/src/mock.rs +++ b/node/bft/ledger-service/src/mock.rs @@ -32,13 +32,13 @@ use tracing::*; #[derive(Debug)] pub struct MockLedgerService { committee: Committee, - height_to_hash: Mutex>, + height_to_round_and_hash: Mutex>, } impl MockLedgerService { /// Initializes a new mock ledger service. pub fn new(committee: Committee) -> Self { - Self { committee, height_to_hash: Default::default() } + Self { committee, height_to_round_and_hash: Default::default() } } /// Initializes a new mock ledger service at the specified height. @@ -47,7 +47,7 @@ impl MockLedgerService { for i in 0..=height { height_to_hash.insert(i, (i as u64 * 2, Field::::from_u32(i).into())); } - Self { committee, height_to_hash: Mutex::new(height_to_hash) } + Self { committee, height_to_round_and_hash: Mutex::new(height_to_hash) } } } @@ -55,12 +55,12 @@ impl MockLedgerService { impl LedgerService for MockLedgerService { /// Returns the latest round in the ledger. fn latest_round(&self) -> u64 { - *self.height_to_hash.lock().keys().last().unwrap_or(&0) as u64 + *self.height_to_round_and_hash.lock().keys().last().unwrap_or(&0) as u64 } /// Returns the latest block height in the canonical ledger. fn latest_block_height(&self) -> u32 { - self.height_to_hash.lock().last_key_value().map(|(height, _)| *height).unwrap_or(0) + self.height_to_round_and_hash.lock().last_key_value().map(|(height, _)| *height).unwrap_or(0) } /// Returns the latest block in the ledger. @@ -70,13 +70,13 @@ impl LedgerService for MockLedgerService { /// Returns `true` if the given block height exists in the canonical ledger. fn contains_block_height(&self, height: u32) -> bool { - self.height_to_hash.lock().contains_key(&height) + self.height_to_round_and_hash.lock().contains_key(&height) } /// Returns the canonical block height for the given block hash, if it exists. fn get_block_height(&self, hash: &N::BlockHash) -> Result { match self - .height_to_hash + .height_to_round_and_hash .lock() .iter() .find_map(|(height, (_, h))| if h == hash { Some(*height) } else { None }) @@ -88,7 +88,7 @@ impl LedgerService for MockLedgerService { /// Returns the canonical block hash for the given block height, if it exists. fn get_block_hash(&self, height: u32) -> Result { - match self.height_to_hash.lock().get(&height).cloned() { + match self.height_to_round_and_hash.lock().get(&height).cloned() { Some((_, hash)) => Ok(hash), None => bail!("Missing block {height}"), } @@ -97,7 +97,7 @@ impl LedgerService for MockLedgerService { /// Returns the block round for the given block height, if it exists. fn get_block_round(&self, height: u32) -> Result { match self - .height_to_hash + .height_to_round_and_hash .lock() .iter() .find_map(|(h, (round, _))| if *h == height { Some(*round) } else { None }) @@ -215,7 +215,7 @@ impl LedgerService for MockLedgerService { block.height(), self.latest_block_height() ); - self.height_to_hash.lock().insert(block.height(), (block.round(), block.hash())); + self.height_to_round_and_hash.lock().insert(block.height(), (block.round(), block.hash())); Ok(()) } } From 6b353a0e24518d0cc250be5363c2379a38f01178 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 7 Mar 2024 16:25:50 -0800 Subject: [PATCH 5/8] Clippy --- node/bft/tests/components/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/node/bft/tests/components/mod.rs b/node/bft/tests/components/mod.rs index 1a11df65a7..2864fe598d 100644 --- a/node/bft/tests/components/mod.rs +++ b/node/bft/tests/components/mod.rs @@ -59,12 +59,15 @@ pub fn sample_storage(ledger: Arc(ledger: Arc>>) -> Gateway { +pub fn sample_gateway( + storage: Storage, + ledger: Arc>>, +) -> Gateway { let num_nodes: u16 = ledger.current_committee().unwrap().num_members().try_into().unwrap(); let (accounts, _committee) = primary::new_test_committee(num_nodes); let account = Account::from_str(&accounts[0].private_key().to_string()).unwrap(); // Initialize the gateway. - Gateway::new(account, ledger, None, &[], None).unwrap() + Gateway::new(account, storage, ledger, None, &[], None).unwrap() } /// Samples a new worker with the given ledger. @@ -72,7 +75,7 @@ pub fn sample_worker(id: u8, ledger: Arc Date: Mon, 11 Mar 2024 10:20:49 -0700 Subject: [PATCH 6/8] Optimize is_authorized_validator_address --- node/bft/src/gateway.rs | 43 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index c51184fece..2da658a480 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -334,35 +334,38 @@ impl Gateway { /// Returns `true` if the given address is an authorized validator. pub fn is_authorized_validator_address(&self, validator_address: Address) -> bool { + // Determine if the validator address is a member of the committee lookback, + // the current committee, or the previous committee lookbacks. + // We allow leniency in this validation check in order to accommodate these two scenarios: + // 1. New validators should be able to connect immediately once bonded as a committee member. + // 2. Existing validators must remain connected until they are no longer bonded as a committee member. + // (i.e. meaning they must stay online until the next block has been produced) + + // Determine if the validator is in the current committee with lookback. + if self + .ledger + .get_committee_lookback_for_round(self.storage.current_round()) + .map_or(false, |committee| committee.is_committee_member(validator_address)) + { + return true; + } + + // Determine if the validator is in the latest committee on the ledger. + if self.ledger.current_committee().map_or(false, |committee| committee.is_committee_member(validator_address)) { + return true; + } + // Retrieve the previous block height to consider from the sync tolerance. let previous_block_height = self.ledger.latest_block_height().saturating_sub(MAX_BLOCKS_BEHIND); // Determine if the validator is in any of the previous committee lookbacks. - let exists_in_previous_committee_lookback = match self.ledger.get_block_round(previous_block_height) { + match self.ledger.get_block_round(previous_block_height) { Ok(block_round) => (block_round..self.storage.current_round()).step_by(2).any(|round| { self.ledger .get_committee_lookback_for_round(round) .map_or(false, |committee| committee.is_committee_member(validator_address)) }), Err(_) => false, - }; - - // Determine if the validator is in the current committee with lookback. - let exists_in_current_committee_lookback = self - .ledger - .get_committee_lookback_for_round(self.storage.current_round()) - .map_or(false, |committee| committee.is_committee_member(validator_address)); - - // Determine if the validator is in the latest committee on the ledger. - let exists_in_latest_committee = - self.ledger.current_committee().map_or(false, |committee| committee.is_committee_member(validator_address)); - - // Determine if the validator address is a member of the previous committee lookbacks, - // the committee lookback, or the current committee. - // We allow leniency in this validation check in order to accommodate these two scenarios: - // 1. New validators should be able to connect immediately once bonded as a committee member. - // 2. Existing validators must remain connected until they are no longer bonded as a committee member. - // (i.e. meaning they must stay online until the next block has been produced) - exists_in_previous_committee_lookback || exists_in_current_committee_lookback || exists_in_latest_committee + } } /// Returns the maximum number of connected peers. From 7d8d262c665928c21d199044b789c5bcccaac7fa Mon Sep 17 00:00:00 2001 From: mdelle1 <108158289+mdelle1@users.noreply.github.com> Date: Mon, 11 Mar 2024 18:29:30 -0400 Subject: [PATCH 7/8] Adds unit test for authorized validators --- node/bft/src/gateway.rs | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index 2da658a480..731335b7d8 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -1555,4 +1555,65 @@ mod prop_tests { ); assert_eq!(gateway.num_workers(), workers.len() as u8); } + #[proptest] + fn test_is_authorized_validator(#[strategy(any_valid_dev_gateway())] input: GatewayInput) { + use indexmap::IndexSet; + use crate::helpers::Storage; + use snarkos_account::Account; + use snarkos_node_bft_ledger_service::MockLedgerService; + use snarkos_node_bft_storage_service::BFTMemoryService; + use snarkvm::{ + ledger::{ + narwhal::batch_certificate::test_helpers::sample_batch_certificate_for_round, + committee::test_helpers::sample_committee_for_round_and_members, + }, + utilities::TestRng, + }; + let rng = &mut TestRng::default(); + + // Initialize the round parameters. + let current_round = 2; + let committee_size = 4; + let max_gc_rounds = snarkvm::ledger::narwhal::BatchHeader::::MAX_GC_ROUNDS as u64; + let (_, _, private_key, dev) = input; + let account = Account::try_from(private_key).unwrap(); + + // Sample the certificates. + let mut certificates = IndexSet::new(); + for _ in 0..committee_size{ + certificates.insert(sample_batch_certificate_for_round(current_round, rng)); + } + let addresses: Vec<_> = certificates.iter() + .map(|certificate| certificate.author()) + .collect(); + // Initialize the committee. + let committee = sample_committee_for_round_and_members( + current_round, + addresses, + rng, + ); + // Sample extra certificates from non-committee members. + for _ in 0..committee_size{ + certificates.insert(sample_batch_certificate_for_round(current_round, rng)); + } + // Initialize the ledger. + let ledger = Arc::new(MockLedgerService::new(committee.clone())); + // Initialize the storage. + let storage = Storage::new(ledger.clone(), Arc::new(BFTMemoryService::new()), max_gc_rounds); + // Initialize the gateway. + let gateway = Gateway::new(account.clone(), storage.clone(), ledger.clone(), dev.ip(), &[], dev.port()).unwrap(); + // Insert certificate to the storage. + for certificate in certificates.iter() { + storage.testing_only_insert_certificate_testing_only(certificate.clone()); + } + // Check that the current committee members are authorized validators. + for i in 0..certificates.clone().len(){ + let is_authorized = gateway.is_authorized_validator_address(certificates[i].author()); + if i < committee_size { + assert!(is_authorized); + } else { + assert!(!is_authorized); + } + } + } } From d0443cc93ea694a45fbc8ec0083fa59980d8376e Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:59:01 -0700 Subject: [PATCH 8/8] clippy --- node/bft/src/gateway.rs | 78 ++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 44 deletions(-) diff --git a/node/bft/src/gateway.rs b/node/bft/src/gateway.rs index 731335b7d8..82ba0d2b11 100644 --- a/node/bft/src/gateway.rs +++ b/node/bft/src/gateway.rs @@ -1365,16 +1365,22 @@ mod prop_tests { }; use snarkos_account::Account; use snarkos_node_bft_ledger_service::MockLedgerService; + use snarkos_node_bft_storage_service::BFTMemoryService; use snarkos_node_tcp::P2P; use snarkvm::{ - ledger::committee::{ - prop_tests::{CommitteeContext, ValidatorSet}, - Committee, + ledger::{ + committee::{ + prop_tests::{CommitteeContext, ValidatorSet}, + test_helpers::sample_committee_for_round_and_members, + Committee, + }, + narwhal::{batch_certificate::test_helpers::sample_batch_certificate_for_round, BatchHeader}, }, prelude::{MainnetV0, PrivateKey}, + utilities::TestRng, }; - use indexmap::IndexMap; + use indexmap::{IndexMap, IndexSet}; use proptest::{ prelude::{any, any_with, Arbitrary, BoxedStrategy, Just, Strategy}, sample::Selector, @@ -1555,64 +1561,48 @@ mod prop_tests { ); assert_eq!(gateway.num_workers(), workers.len() as u8); } + #[proptest] fn test_is_authorized_validator(#[strategy(any_valid_dev_gateway())] input: GatewayInput) { - use indexmap::IndexSet; - use crate::helpers::Storage; - use snarkos_account::Account; - use snarkos_node_bft_ledger_service::MockLedgerService; - use snarkos_node_bft_storage_service::BFTMemoryService; - use snarkvm::{ - ledger::{ - narwhal::batch_certificate::test_helpers::sample_batch_certificate_for_round, - committee::test_helpers::sample_committee_for_round_and_members, - }, - utilities::TestRng, - }; let rng = &mut TestRng::default(); - // Initialize the round parameters. - let current_round = 2; - let committee_size = 4; - let max_gc_rounds = snarkvm::ledger::narwhal::BatchHeader::::MAX_GC_ROUNDS as u64; + // Initialize the round parameters. + let current_round = 2; + let committee_size = 4; + let max_gc_rounds = BatchHeader::::MAX_GC_ROUNDS as u64; let (_, _, private_key, dev) = input; let account = Account::try_from(private_key).unwrap(); - // Sample the certificates. - let mut certificates = IndexSet::new(); - for _ in 0..committee_size{ - certificates.insert(sample_batch_certificate_for_round(current_round, rng)); + // Sample the certificates. + let mut certificates = IndexSet::new(); + for _ in 0..committee_size { + certificates.insert(sample_batch_certificate_for_round(current_round, rng)); } - let addresses: Vec<_> = certificates.iter() - .map(|certificate| certificate.author()) - .collect(); + let addresses: Vec<_> = certificates.iter().map(|certificate| certificate.author()).collect(); // Initialize the committee. - let committee = sample_committee_for_round_and_members( - current_round, - addresses, - rng, - ); - // Sample extra certificates from non-committee members. - for _ in 0..committee_size{ - certificates.insert(sample_batch_certificate_for_round(current_round, rng)); - } + let committee = sample_committee_for_round_and_members(current_round, addresses, rng); + // Sample extra certificates from non-committee members. + for _ in 0..committee_size { + certificates.insert(sample_batch_certificate_for_round(current_round, rng)); + } // Initialize the ledger. let ledger = Arc::new(MockLedgerService::new(committee.clone())); // Initialize the storage. let storage = Storage::new(ledger.clone(), Arc::new(BFTMemoryService::new()), max_gc_rounds); - // Initialize the gateway. - let gateway = Gateway::new(account.clone(), storage.clone(), ledger.clone(), dev.ip(), &[], dev.port()).unwrap(); + // Initialize the gateway. + let gateway = + Gateway::new(account.clone(), storage.clone(), ledger.clone(), dev.ip(), &[], dev.port()).unwrap(); // Insert certificate to the storage. for certificate in certificates.iter() { storage.testing_only_insert_certificate_testing_only(certificate.clone()); } // Check that the current committee members are authorized validators. - for i in 0..certificates.clone().len(){ - let is_authorized = gateway.is_authorized_validator_address(certificates[i].author()); - if i < committee_size { - assert!(is_authorized); - } else { - assert!(!is_authorized); + for i in 0..certificates.clone().len() { + let is_authorized = gateway.is_authorized_validator_address(certificates[i].author()); + if i < committee_size { + assert!(is_authorized); + } else { + assert!(!is_authorized); } } }