From 1d0ba0d1f2ab45441229ec532d8ee769beb1bd3d Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 11 Dec 2019 15:06:54 -0700 Subject: [PATCH] Add special handling for snapshot root slot in get_confirmed_block (#7430) * Add special handling for snapshot root slot * Improve test --- Cargo.lock | 1 + client/src/rpc_request.rs | 2 +- ledger/Cargo.toml | 1 + ledger/src/blocktree.rs | 97 ++++++++++++++++++++++++++++----------- 4 files changed, 73 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eaebfafe4..01023a5aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3566,6 +3566,7 @@ dependencies = [ name = "solana-ledger" version = "0.22.0" dependencies = [ + "assert_matches 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "bzip2 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/client/src/rpc_request.rs b/client/src/rpc_request.rs index 1605491bd..be96d70d9 100644 --- a/client/src/rpc_request.rs +++ b/client/src/rpc_request.rs @@ -31,7 +31,7 @@ pub struct RpcConfirmedBlock { pub transactions: Vec<(Transaction, Option)>, } -#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct RpcTransactionStatus { pub status: Result<()>, pub fee: u64, diff --git a/ledger/Cargo.toml b/ledger/Cargo.toml index aeae227e9..35039d08f 100644 --- a/ledger/Cargo.toml +++ b/ledger/Cargo.toml @@ -55,6 +55,7 @@ default-features = false features = ["lz4"] [dev-dependencies] +assert_matches = "1.3.0" matches = "0.1.6" solana-budget-program = { path = "../programs/budget", version = "0.22.0" } diff --git a/ledger/src/blocktree.rs b/ledger/src/blocktree.rs index 3cdbd7258..170457881 100644 --- a/ledger/src/blocktree.rs +++ b/ledger/src/blocktree.rs @@ -1182,24 +1182,30 @@ impl Blocktree { .expect("Rooted slot must exist in SlotMeta"); let slot_entries = self.get_slot_entries(slot, 0, None)?; - let slot_transaction_iterator = slot_entries - .iter() - .cloned() - .flat_map(|entry| entry.transactions); - let parent_slot_entries = self.get_slot_entries(slot_meta.parent_slot, 0, None)?; + if !slot_entries.is_empty() { + let slot_transaction_iterator = slot_entries + .iter() + .cloned() + .flat_map(|entry| entry.transactions); + let parent_slot_entries = self.get_slot_entries(slot_meta.parent_slot, 0, None)?; + let previous_blockhash = if !parent_slot_entries.is_empty() { + get_last_hash(parent_slot_entries.iter()).unwrap() + } else { + Hash::default() + }; - let block = RpcConfirmedBlock { - previous_blockhash: get_last_hash(parent_slot_entries.iter()) - .expect("Rooted parent slot must have blockhash"), - blockhash: get_last_hash(slot_entries.iter()) - .expect("Rooted slot must have blockhash"), - parent_slot: slot_meta.parent_slot, - transactions: self.map_transactions_to_statuses(slot, slot_transaction_iterator), - }; - Ok(block) - } else { - Err(BlocktreeError::SlotNotRooted) + let block = RpcConfirmedBlock { + previous_blockhash, + blockhash: get_last_hash(slot_entries.iter()) + .unwrap_or_else(|| panic!("Rooted slot {:?} must have blockhash", slot)), + parent_slot: slot_meta.parent_slot, + transactions: self + .map_transactions_to_statuses(slot, slot_transaction_iterator), + }; + return Ok(block); + } } + Err(BlocktreeError::SlotNotRooted) } fn map_transactions_to_statuses<'a>( @@ -2137,11 +2143,13 @@ pub mod tests { leader_schedule::{FixedSchedule, LeaderSchedule}, shred::{max_ticks_per_n_shreds, DataShredHeader}, }; + use assert_matches::assert_matches; + use bincode::serialize; use itertools::Itertools; use rand::{seq::SliceRandom, thread_rng}; use solana_runtime::bank::Bank; use solana_sdk::{ - hash::{self, Hash}, + hash::{self, hash, Hash}, instruction::CompiledInstruction, packet::PACKET_DATA_SIZE, pubkey::Pubkey, @@ -2153,7 +2161,7 @@ pub mod tests { // used for tests only fn make_slot_entries_with_transactions(num_entries: u64) -> Vec { let mut entries: Vec = Vec::new(); - for _ in 0..num_entries { + for x in 0..num_entries { let transaction = Transaction::new_with_compiled_instructions( &[&Keypair::new()], &[Pubkey::new_rand()], @@ -2162,7 +2170,7 @@ pub mod tests { vec![CompiledInstruction::new(1, &(), vec![0])], ); entries.push(next_entry_mut(&mut Hash::default(), 0, vec![transaction])); - let mut tick = create_ticks(1, 0, Hash::default()); + let mut tick = create_ticks(1, 0, hash(&serialize(&x).unwrap())); entries.append(&mut tick); } entries @@ -4208,13 +4216,22 @@ pub mod tests { #[test] fn test_get_confirmed_block() { - let slot = 0; + let slot = 10; let entries = make_slot_entries_with_transactions(100); - let shreds = entries_to_test_shreds(entries.clone(), slot, 0, true, 0); + let blockhash = get_last_hash(entries.iter()).unwrap(); + let shreds = entries_to_test_shreds(entries.clone(), slot, slot - 1, true, 0); + let more_shreds = entries_to_test_shreds(entries.clone(), slot + 1, slot, true, 0); let ledger_path = get_tmp_ledger_path!(); let ledger = Blocktree::open(&ledger_path).unwrap(); ledger.insert_shreds(shreds, None, false).unwrap(); - ledger.set_roots(&[0]).unwrap(); + ledger.insert_shreds(more_shreds, None, false).unwrap(); + ledger.set_roots(&[slot - 1, slot, slot + 1]).unwrap(); + + let mut parent_meta = SlotMeta::default(); + parent_meta.parent_slot = std::u64::MAX; + ledger + .put_meta_bytes(slot - 1, &serialize(&parent_meta).unwrap()) + .unwrap(); let expected_transactions: Vec<(Transaction, Option)> = entries .iter() @@ -4233,6 +4250,16 @@ pub mod tests { }, ) .unwrap(); + ledger + .transaction_status_cf + .put( + (slot + 1, signature), + &RpcTransactionStatus { + status: Ok(()), + fee: 42, + }, + ) + .unwrap(); ( transaction, Some(RpcTransactionStatus { @@ -4243,17 +4270,33 @@ pub mod tests { }) .collect(); - let confirmed_block = ledger.get_confirmed_block(0).unwrap(); + // Even if marked as root, a slot that is empty of entries should return an error + let confirmed_block_err = ledger.get_confirmed_block(slot - 1).unwrap_err(); + assert_matches!(confirmed_block_err, BlocktreeError::SlotNotRooted); + + let confirmed_block = ledger.get_confirmed_block(slot).unwrap(); + assert_eq!(confirmed_block.transactions.len(), 100); + + let mut expected_block = RpcConfirmedBlock::default(); + expected_block.transactions = expected_transactions.clone(); + expected_block.parent_slot = slot - 1; + expected_block.blockhash = blockhash; + // The previous_blockhash of `expected_block` is default because its parent slot is a + // root, but empty of entries. This is special handling for snapshot root slots. + assert_eq!(confirmed_block, expected_block); + + let confirmed_block = ledger.get_confirmed_block(slot + 1).unwrap(); assert_eq!(confirmed_block.transactions.len(), 100); let mut expected_block = RpcConfirmedBlock::default(); expected_block.transactions = expected_transactions; - // The blockhash and previous_blockhash of `expected_block` are default only because - // `make_slot_entries_with_transactions` sets all entry hashes to default + expected_block.parent_slot = slot; + expected_block.previous_blockhash = blockhash; + expected_block.blockhash = blockhash; assert_eq!(confirmed_block, expected_block); - let not_root = ledger.get_confirmed_block(1); - assert!(not_root.is_err()); + let not_root = ledger.get_confirmed_block(slot + 2).unwrap_err(); + assert_matches!(not_root, BlocktreeError::SlotNotRooted); drop(ledger); Blocktree::destroy(&ledger_path).expect("Expected successful database destruction");