Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit aeb3a2e

Browse files
mergify[bot]AshwinSekar
authored andcommitted
v1.18: blockstore: only consume duplicate proofs from root_slot + 1 on startup (backport of #1971) (#2113)
* blockstore: only consume duplicate proofs from root_slot + 1 on startup (#1971) * blockstore: only consume duplicate proofs from root_slot + 1 on startup * pr feedback: update test comments * pr feedback: add pub behind dcou for test fns (cherry picked from commit 2a48564) * fix conflicts --------- Co-authored-by: Ashwin Sekar <[email protected]> Co-authored-by: Ashwin Sekar <[email protected]>
1 parent b6e708a commit aeb3a2e

File tree

6 files changed

+140
-2
lines changed

6 files changed

+140
-2
lines changed

Diff for: Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: core/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ serde_json = { workspace = true }
9090
serial_test = { workspace = true }
9191
# See order-crates-for-publishing.py for using this unusual `path = "."`
9292
solana-core = { path = ".", features = ["dev-context-only-utils"] }
93+
solana-ledger = { workspace = true, features = ["dev-context-only-utils"] }
9394
solana-logger = { workspace = true }
9495
solana-poh = { workspace = true, features = ["dev-context-only-utils"] }
9596
solana-program-runtime = { workspace = true }

Diff for: core/src/replay_stage.rs

+132-2
Original file line numberDiff line numberDiff line change
@@ -1285,8 +1285,14 @@ impl ReplayStage {
12851285
) -> (ProgressMap, HeaviestSubtreeForkChoice) {
12861286
let (root_bank, frozen_banks, duplicate_slot_hashes) = {
12871287
let bank_forks = bank_forks.read().unwrap();
1288+
let root_bank = bank_forks.root_bank();
12881289
let duplicate_slots = blockstore
1289-
.duplicate_slots_iterator(bank_forks.root_bank().slot())
1290+
// It is important that the root bank is not marked as duplicate on initialization.
1291+
// Although this bank could contain a duplicate proof, the fact that it was rooted
1292+
// either during a previous run or artificially means that we should ignore any
1293+
// duplicate proofs for the root slot, thus we start consuming duplicate proofs
1294+
// from the root slot + 1
1295+
.duplicate_slots_iterator(root_bank.slot().saturating_add(1))
12901296
.unwrap();
12911297
let duplicate_slot_hashes = duplicate_slots.filter_map(|slot| {
12921298
let bank = bank_forks.get(slot)?;
@@ -1295,7 +1301,7 @@ impl ReplayStage {
12951301
.then_some((slot, bank.hash()))
12961302
});
12971303
(
1298-
bank_forks.root_bank(),
1304+
root_bank,
12991305
bank_forks.frozen_banks().values().cloned().collect(),
13001306
duplicate_slot_hashes.collect::<Vec<(Slot, Hash)>>(),
13011307
)
@@ -4310,6 +4316,9 @@ pub(crate) mod tests {
43104316
replay_stage::ReplayStage,
43114317
vote_simulator::{self, VoteSimulator},
43124318
},
4319+
blockstore_processor::{
4320+
confirm_full_slot, fill_blockstore_slot_with_ticks, process_bank_0, ProcessOptions,
4321+
},
43134322
crossbeam_channel::unbounded,
43144323
itertools::Itertools,
43154324
solana_entry::entry::{self, Entry},
@@ -8963,4 +8972,125 @@ pub(crate) mod tests {
89638972
&mut PurgeRepairSlotCounter::default(),
89648973
);
89658974
}
8975+
8976+
#[test]
8977+
fn test_initialize_progress_and_fork_choice_with_duplicates() {
8978+
solana_logger::setup();
8979+
let GenesisConfigInfo {
8980+
mut genesis_config, ..
8981+
} = create_genesis_config(123);
8982+
8983+
let ticks_per_slot = 1;
8984+
genesis_config.ticks_per_slot = ticks_per_slot;
8985+
let (ledger_path, blockhash) =
8986+
solana_ledger::create_new_tmp_ledger_auto_delete!(&genesis_config);
8987+
let blockstore = Blockstore::open(ledger_path.path()).unwrap();
8988+
8989+
/*
8990+
Bank forks with:
8991+
slot 0
8992+
|
8993+
slot 1 -> Duplicate before restart, the restart slot
8994+
|
8995+
slot 2
8996+
|
8997+
slot 3 -> Duplicate before restart, artificially rooted
8998+
|
8999+
slot 4 -> Duplicate before restart, artificially rooted
9000+
|
9001+
slot 5 -> Duplicate before restart
9002+
|
9003+
slot 6
9004+
*/
9005+
9006+
let mut last_hash = blockhash;
9007+
for i in 0..6 {
9008+
last_hash =
9009+
fill_blockstore_slot_with_ticks(&blockstore, ticks_per_slot, i + 1, i, last_hash);
9010+
}
9011+
// Artifically root 3 and 4
9012+
blockstore.set_roots([3, 4].iter()).unwrap();
9013+
9014+
// Set up bank0
9015+
let bank_forks = BankForks::new_rw_arc(Bank::new_for_tests(&genesis_config));
9016+
let bank0 = bank_forks.read().unwrap().get_with_scheduler(0).unwrap();
9017+
let recyclers = VerifyRecyclers::default();
9018+
9019+
process_bank_0(
9020+
&bank0,
9021+
&blockstore,
9022+
&ProcessOptions::default(),
9023+
&recyclers,
9024+
None,
9025+
None,
9026+
);
9027+
9028+
// Mark block 1, 3, 4, 5 as duplicate
9029+
blockstore.store_duplicate_slot(1, vec![], vec![]).unwrap();
9030+
blockstore.store_duplicate_slot(3, vec![], vec![]).unwrap();
9031+
blockstore.store_duplicate_slot(4, vec![], vec![]).unwrap();
9032+
blockstore.store_duplicate_slot(5, vec![], vec![]).unwrap();
9033+
9034+
let bank1 = bank_forks.write().unwrap().insert(Bank::new_from_parent(
9035+
bank0.clone_without_scheduler(),
9036+
&Pubkey::default(),
9037+
1,
9038+
));
9039+
confirm_full_slot(
9040+
&blockstore,
9041+
&bank1,
9042+
&ProcessOptions::default(),
9043+
&recyclers,
9044+
&mut ConfirmationProgress::new(bank0.last_blockhash()),
9045+
None,
9046+
None,
9047+
None,
9048+
&mut ExecuteTimings::default(),
9049+
)
9050+
.unwrap();
9051+
9052+
bank_forks.write().unwrap().set_root(
9053+
1,
9054+
&solana_runtime::accounts_background_service::AbsRequestSender::default(),
9055+
None,
9056+
);
9057+
9058+
let leader_schedule_cache = LeaderScheduleCache::new_from_bank(&bank1);
9059+
9060+
// process_blockstore_from_root() from slot 1 onwards
9061+
blockstore_processor::process_blockstore_from_root(
9062+
&blockstore,
9063+
&bank_forks,
9064+
&leader_schedule_cache,
9065+
&ProcessOptions::default(),
9066+
None,
9067+
None,
9068+
None,
9069+
&AbsRequestSender::default(),
9070+
)
9071+
.unwrap();
9072+
9073+
assert_eq!(bank_forks.read().unwrap().root(), 4);
9074+
9075+
// Verify that fork choice can be initialized and that the root is not marked duplicate
9076+
let (_progress, fork_choice) =
9077+
ReplayStage::initialize_progress_and_fork_choice_with_locked_bank_forks(
9078+
&bank_forks,
9079+
&Pubkey::new_unique(),
9080+
&Pubkey::new_unique(),
9081+
&blockstore,
9082+
);
9083+
9084+
let bank_forks = bank_forks.read().unwrap();
9085+
// 4 (the artificial root) is the tree root and no longer duplicate
9086+
assert_eq!(fork_choice.tree_root().0, 4);
9087+
assert!(fork_choice
9088+
.is_candidate(&(4, bank_forks.bank_hash(4).unwrap()))
9089+
.unwrap());
9090+
9091+
// 5 is still considered duplicate, so it is not a valid fork choice candidate
9092+
assert!(!fork_choice
9093+
.is_candidate(&(5, bank_forks.bank_hash(5).unwrap()))
9094+
.unwrap());
9095+
}
89669096
}

Diff for: ledger/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ mockall = { workspace = true }
2929
num_cpus = { workspace = true }
3030
num_enum = { workspace = true }
3131
prost = { workspace = true }
32+
qualifier_attr = { workspace = true }
3233
rand = { workspace = true }
3334
rand_chacha = { workspace = true }
3435
rayon = { workspace = true }

Diff for: ledger/src/blockstore_processor.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#[cfg(feature = "dev-context-only-utils")]
2+
use qualifier_attr::qualifiers;
13
use {
24
crate::{
35
block_error::BlockError,
@@ -974,6 +976,7 @@ fn verify_ticks(
974976
Ok(())
975977
}
976978

979+
#[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
977980
fn confirm_full_slot(
978981
blockstore: &Blockstore,
979982
bank: &BankWithScheduler,
@@ -1378,6 +1381,7 @@ fn confirm_slot_entries(
13781381
}
13791382

13801383
// Special handling required for processing the entries in slot 0
1384+
#[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
13811385
fn process_bank_0(
13821386
bank0: &BankWithScheduler,
13831387
blockstore: &Blockstore,

Diff for: programs/sbf/Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)