Skip to content

Commit

Permalink
Only allow BankForks creation with single bank (solana-labs#34449)
Browse files Browse the repository at this point in the history 
BankForks has two constructors; one that takes a single Bank (the root)
and one that can take an arbitrary number of Banks plus the root slot.
However, the constructor that accepts multiple banks is unnecessary; it
isn't used in production and is only used in several tests.

So, remove the multi-bank constructor and update unit tests.
  • Loading branch information
steviez authored Dec 14, 2023
1 parent 0a3a016 commit ae6a4c0
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 94 deletions.
3 changes: 1 addition & 2 deletions core/src/replay_stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4914,8 +4914,7 @@ pub(crate) mod tests {
bank0.register_default_tick_for_test();
}
bank0.freeze();
let arc_bank0 = Arc::new(bank0);
let bank_forks = BankForks::new_from_banks(&[arc_bank0], 0);
let bank_forks = BankForks::new_rw_arc(bank0);

let exit = Arc::new(AtomicBool::new(false));
let block_commitment_cache = Arc::new(RwLock::new(BlockCommitmentCache::default()));
Expand Down
48 changes: 26 additions & 22 deletions rpc/src/rpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,13 @@ impl JsonRpcRequestProcessor {

// Useful for unit testing
pub fn new_from_bank(
bank: Arc<Bank>,
bank: Bank,
socket_addr_space: SocketAddrSpace,
connection_cache: Arc<ConnectionCache>,
) -> Self {
let genesis_hash = bank.hash();
let bank_forks = BankForks::new_from_banks(&[bank.clone()], bank.slot());
let bank_forks = BankForks::new_rw_arc(bank);
let bank = bank_forks.read().unwrap().root_bank();
let blockstore = Arc::new(Blockstore::open(&get_tmp_ledger_path!()).unwrap());
let exit = Arc::new(AtomicBool::new(false));
let cluster_info = Arc::new({
Expand Down Expand Up @@ -5057,18 +5058,20 @@ pub mod tests {
fn test_rpc_request_processor_new() {
let bob_pubkey = solana_sdk::pubkey::new_rand();
let genesis = create_genesis_config(100);
let bank = Bank::new_with_bank_forks_for_tests(&genesis.genesis_config).0;
bank.transfer(20, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
let bank = Bank::new_for_tests(&genesis.genesis_config);
let connection_cache = Arc::new(ConnectionCache::new("connection_cache_test"));
let request_processor = JsonRpcRequestProcessor::new_from_bank(
let meta = JsonRpcRequestProcessor::new_from_bank(
bank,
SocketAddrSpace::Unspecified,
connection_cache,
);

let bank = meta.bank_forks.read().unwrap().root_bank();
bank.transfer(20, &genesis.mint_keypair, &bob_pubkey)
.unwrap();

assert_eq!(
request_processor
.get_transaction_count(RpcContextConfig::default())
meta.get_transaction_count(RpcContextConfig::default())
.unwrap(),
1
);
Expand All @@ -5078,7 +5081,7 @@ pub mod tests {
fn test_rpc_get_balance() {
let genesis = create_genesis_config(20);
let mint_pubkey = genesis.mint_keypair.pubkey();
let bank = Arc::new(Bank::new_for_tests(&genesis.genesis_config));
let bank = Bank::new_for_tests(&genesis.genesis_config);
let connection_cache = Arc::new(ConnectionCache::new("connection_cache_test"));
let meta = JsonRpcRequestProcessor::new_from_bank(
bank,
Expand Down Expand Up @@ -5110,7 +5113,7 @@ pub mod tests {
fn test_rpc_get_balance_via_client() {
let genesis = create_genesis_config(20);
let mint_pubkey = genesis.mint_keypair.pubkey();
let bank = Arc::new(Bank::new_for_tests(&genesis.genesis_config));
let bank = Bank::new_for_tests(&genesis.genesis_config);
let connection_cache = Arc::new(ConnectionCache::new("connection_cache_test"));
let meta = JsonRpcRequestProcessor::new_from_bank(
bank,
Expand Down Expand Up @@ -5227,17 +5230,7 @@ pub mod tests {
fn test_rpc_get_tx_count() {
let bob_pubkey = solana_sdk::pubkey::new_rand();
let genesis = create_genesis_config(10);
let bank = Bank::new_with_bank_forks_for_tests(&genesis.genesis_config).0;
// Add 4 transactions
bank.transfer(1, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(2, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(3, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(4, &genesis.mint_keypair, &bob_pubkey)
.unwrap();

let bank = Bank::new_for_tests(&genesis.genesis_config);
let connection_cache = Arc::new(ConnectionCache::new("connection_cache_test"));
let meta = JsonRpcRequestProcessor::new_from_bank(
bank,
Expand All @@ -5248,6 +5241,17 @@ pub mod tests {
let mut io = MetaIoHandler::default();
io.extend_with(rpc_minimal::MinimalImpl.to_delegate());

// Add 4 transactions
let bank = meta.bank_forks.read().unwrap().root_bank();
bank.transfer(1, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(2, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(3, &genesis.mint_keypair, &bob_pubkey)
.unwrap();
bank.transfer(4, &genesis.mint_keypair, &bob_pubkey)
.unwrap();

let req = r#"{"jsonrpc":"2.0","id":1,"method":"getTransactionCount"}"#;
let res = io.handle_request_sync(req, meta);
let expected = r#"{"jsonrpc":"2.0","result":4,"id":1}"#;
Expand Down Expand Up @@ -6383,7 +6387,7 @@ pub mod tests {
#[test]
fn test_rpc_send_bad_tx() {
let genesis = create_genesis_config(100);
let bank = Arc::new(Bank::new_for_tests(&genesis.genesis_config));
let bank = Bank::new_for_tests(&genesis.genesis_config);
let connection_cache = Arc::new(ConnectionCache::new("connection_cache_test"));
let meta = JsonRpcRequestProcessor::new_from_bank(
bank,
Expand Down
119 changes: 49 additions & 70 deletions runtime/src/bank_forks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,55 @@ impl Index<u64> for BankForks {
}

impl BankForks {
pub fn new_rw_arc(bank: Bank) -> Arc<RwLock<Self>> {
let root = bank.slot();
Self::new_from_banks(&[Arc::new(bank)], root)
pub fn new_rw_arc(root_bank: Bank) -> Arc<RwLock<Self>> {
let root_bank = Arc::new(root_bank);
let root_slot = root_bank.slot();

let mut banks = HashMap::new();
banks.insert(
root_slot,
BankWithScheduler::new_without_scheduler(root_bank.clone()),
);

let parents = root_bank.parents();
for parent in parents {
if banks
.insert(
parent.slot(),
BankWithScheduler::new_without_scheduler(parent.clone()),
)
.is_some()
{
// All ancestors have already been inserted by another fork
break;
}
}

let mut descendants = HashMap::<_, HashSet<_>>::new();
descendants.entry(root_slot).or_default();
for parent in root_bank.proper_ancestors() {
descendants.entry(parent).or_default().insert(root_slot);
}

let bank_forks = Arc::new(RwLock::new(Self {
root: Arc::new(AtomicSlot::new(root_slot)),
banks,
descendants,
snapshot_config: None,
accounts_hash_interval_slots: std::u64::MAX,
last_accounts_hash_slot: root_slot,
in_vote_only_mode: Arc::new(AtomicBool::new(false)),
highest_slot_at_startup: 0,
scheduler_pool: None,
}));

root_bank
.loaded_programs_cache
.write()
.unwrap()
.set_fork_graph(bank_forks.clone());

bank_forks
}

pub fn banks(&self) -> &HashMap<Slot, BankWithScheduler> {
Expand Down Expand Up @@ -166,58 +212,6 @@ impl BankForks {
self[self.root()].clone()
}

pub fn new_from_banks(initial_forks: &[Arc<Bank>], root: Slot) -> Arc<RwLock<Self>> {
let mut banks = HashMap::new();

// Iterate through the heads of all the different forks
for bank in initial_forks {
banks.insert(
bank.slot(),
BankWithScheduler::new_without_scheduler(bank.clone()),
);
let parents = bank.parents();
for parent in parents {
if banks
.insert(
parent.slot(),
BankWithScheduler::new_without_scheduler(parent.clone()),
)
.is_some()
{
// All ancestors have already been inserted by another fork
break;
}
}
}
let mut descendants = HashMap::<_, HashSet<_>>::new();
for (slot, bank) in &banks {
descendants.entry(*slot).or_default();
for parent in bank.proper_ancestors() {
descendants.entry(parent).or_default().insert(*slot);
}
}
let bank_forks = Arc::new(RwLock::new(Self {
root: Arc::new(AtomicSlot::new(root)),
banks,
descendants,
snapshot_config: None,
accounts_hash_interval_slots: std::u64::MAX,
last_accounts_hash_slot: root,
in_vote_only_mode: Arc::new(AtomicBool::new(false)),
highest_slot_at_startup: 0,
scheduler_pool: None,
}));

for bank in bank_forks.read().unwrap().banks.values() {
bank.loaded_programs_cache
.write()
.unwrap()
.set_fork_graph(bank_forks.clone());
}

bank_forks
}

pub fn install_scheduler_pool(&mut self, pool: InstalledSchedulerPoolArc) {
info!("Installed new scheduler_pool into bank_forks: {:?}", pool);
assert!(
Expand Down Expand Up @@ -753,21 +747,6 @@ mod tests {
assert_eq!(bank_forks.working_bank().tick_height(), 1);
}

#[test]
fn test_bank_forks_new_from_banks() {
let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
let bank = Arc::new(Bank::new_for_tests(&genesis_config));
let child_bank = Arc::new(Bank::new_from_parent(bank.clone(), &Pubkey::default(), 1));

let bank_forks = BankForks::new_from_banks(&[bank.clone(), child_bank.clone()], 0);
assert_eq!(bank_forks.read().unwrap().root(), 0);
assert_eq!(bank_forks.read().unwrap().working_bank().slot(), 1);

let bank_forks = BankForks::new_from_banks(&[child_bank, bank], 0);
assert_eq!(bank_forks.read().unwrap().root(), 0);
assert_eq!(bank_forks.read().unwrap().working_bank().slot(), 1);
}

#[test]
fn test_bank_forks_descendants() {
let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
Expand Down

0 comments on commit ae6a4c0

Please sign in to comment.