diff --git a/node/bft/ledger-service/src/ledger.rs b/node/bft/ledger-service/src/ledger.rs index d278719005..3044ed2e10 100644 --- a/node/bft/ledger-service/src/ledger.rs +++ b/node/bft/ledger-service/src/ledger.rs @@ -108,6 +108,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 9681076dc0..9d04f74dfa 100644 --- a/node/bft/ledger-service/src/mock.rs +++ b/node/bft/ledger-service/src/mock.rs @@ -32,22 +32,22 @@ 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. 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) } + 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. @@ -78,12 +78,17 @@ 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.lock().iter().find_map(|(height, h)| if h == hash { Some(*height) } else { None }) { + match self + .height_to_round_and_hash + .lock() + .iter() + .find_map(|(height, (_, h))| if h == hash { Some(*height) } else { None }) + { Some(height) => Ok(height), None => bail!("Missing block {hash}"), } @@ -91,8 +96,21 @@ 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), + match self.height_to_round_and_hash.lock().get(&height).cloned() { + 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_round_and_hash + .lock() + .iter() + .find_map(|(h, (round, _))| if *h == height { Some(*round) } else { None }) + { + Some(round) => Ok(round), None => bail!("Missing block {height}"), } } @@ -205,7 +223,7 @@ impl LedgerService for MockLedgerService { block.height(), self.latest_block_height() ); - self.height_to_hash.lock().insert(block.height(), block.hash()); + self.height_to_round_and_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 cebb2a00b0..57e62c216f 100644 --- a/node/bft/ledger-service/src/prover.rs +++ b/node/bft/ledger-service/src/prover.rs @@ -81,6 +81,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 af2c20d6b0..b419d32958 100644 --- a/node/bft/ledger-service/src/traits.rs +++ b/node/bft/ledger-service/src/traits.rs @@ -51,6 +51,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 ff2c326523..6107d5120c 100644 --- a/node/bft/ledger-service/src/translucent.rs +++ b/node/bft/ledger-service/src/translucent.rs @@ -92,6 +92,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 06030e5d5d..82ba0d2b11 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, 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, @@ -100,6 +100,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 +135,7 @@ impl Gateway { /// Initializes a new gateway. pub fn new( account: Account, + storage: Storage, ledger: Arc>, ip: Option, trusted_validators: &[SocketAddr], @@ -149,6 +152,7 @@ impl Gateway { // Return the gateway. Ok(Self { account, + storage, ledger, tcp, cache: Default::default(), @@ -330,18 +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 or the current committee. + // 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) - self.ledger - .get_committee_lookback_for_round(self.ledger.latest_round()) + + // 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)) - || self - .ledger - .current_committee() - .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. + 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, + } } /// Returns the maximum number of connected peers. @@ -1341,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, @@ -1402,6 +1432,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(), &[], @@ -1450,7 +1481,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())); @@ -1465,7 +1498,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())); @@ -1490,7 +1525,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(); @@ -1525,4 +1561,49 @@ 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) { + let rng = &mut TestRng::default(); + + // 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)); + } + 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); + } + } + } } diff --git a/node/bft/src/primary.rs b/node/bft/src/primary.rs index 5e438e8f9c..f91cd52bf4 100644 --- a/node/bft/src/primary.rs +++ b/node/bft/src/primary.rs @@ -109,7 +109,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. diff --git a/node/bft/src/worker.rs b/node/bft/src/worker.rs index c2fe485ad9..5baa8a0aac 100644 --- a/node/bft/src/worker.rs +++ b/node/bft/src/worker.rs @@ -511,6 +511,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>; 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