Refactor: Unify intrusive pruning point update and apply proof db writes #807
Conversation
| // Update virtual state based to the new pruning point | ||
| // Updating of the utxoset is done separately as it requires downloading the new utxoset in its entirety. | ||
|
|
||
| // IMPORTANT: This must be active; we need the object! |
There was a problem hiding this comment.
It seems this comment is a personal one directed at you. Without your context it is not meaningful.
|
|
||
| use crate::model::stores::selected_chain::SelectedChainStoreReader; | ||
|
|
||
| // --- IMPORTS --- |
There was a problem hiding this comment.
There is no need for something like this
| virtual_state::VirtualStores, | ||
| }; | ||
| use parking_lot::RwLock; | ||
| // ----------------------------- |
There was a problem hiding this comment.
same regarding the separator above
| self.virtual_stores.write().state.set_batch(&mut batch, virtual_state).unwrap(); | ||
| // Remove old body tips and insert pruning point as the current tip | ||
|
|
||
| // Remove old body tips (specific to intrusive update) |
There was a problem hiding this comment.
likewise, this comment contains info regarding the coding process itself.
| // Update selected_chain | ||
| self.selected_chain_store.write().init_with_pruning_point(&mut batch, new_pruning_point).unwrap(); | ||
|
|
||
| // Call the new standalone helper to do the common DB updates |
There was a problem hiding this comment.
What is new today won't be new in 2 years. Comments should not have temporal aspects unless well justified.
| self.body_tips_store.write().init_batch(&mut batch, &virtual_parents).unwrap(); | ||
|
|
||
| // REFACTOR: Use the common helper to apply standard pruning point DB updates | ||
| // Wir nutzen .unwrap(), weil ein DB-Fehler hier fatal ist (genau wie im Original-Code) |
There was a problem hiding this comment.
Some german has survived the previous culling :)
| let mut batch = WriteBatch::default(); | ||
| self.body_tips_store.write().init_batch(&mut batch, &virtual_parents).unwrap(); | ||
|
|
||
| // REFACTOR: Use the common helper to apply standard pruning point DB updates |
There was a problem hiding this comment.
likewise, this comment contains info regarding the coding process itself.
| relations::StagingRelationsStore, | ||
| selected_chain::SelectedChainStore, | ||
| virtual_state::{VirtualState, VirtualStateStore}, | ||
| //selected_chain::SelectedChainStore, |
There was a problem hiding this comment.
if not necessary remove, don't comment out
| let (_pruning_point, pruning_index) = self.pruning_point_store.read().pruning_point_and_index().unwrap(); | ||
| (0..=pruning_index).rev().take(n).map(|ind| self.past_pruning_points_store.get(ind).unwrap()).collect_vec() | ||
| } | ||
| pub fn write_common_pruning_point_db_updates( |
There was a problem hiding this comment.
If you look above you will something unique in this function from the entire file - this is likely the only function that doesn't have self as an argument. in other words this is a utility function, and could have been near anywhere. Utility functions are meant to be rather general, here however you have many many arguments, of very specific types. Since we are refactoring, we want the good to have good form.
What we would want is for this to be a method of some object oriented class. Unfortunately, no current class is a perfect fit. What I would want the refactor to really do, is to make sense and redivide of some of the existing classes, e.g. consensus, pruning point manager, pruning proof manager, and pruning processor.
You can see the conversation here for some more context https://github.com/kaspanet/rusty-kaspa/pull/702/changes#r2379996647
| &self.selected_chain_store, | ||
| &mut batch, | ||
| new_pruning_point, | ||
| virtual_state, // Wir übergeben das oben berechnete Objekt |
2 participants
Description
This PR addresses issue #758 by unifying the database update logic for pruning points.
Changes
write_common_pruning_point_db_updatesinconsensus/mod.rsto handle common DB writes (Virtual State, Body Tips, Selected Chain).intrusive_pruning_point_store_writesinconsensus/mod.rsto use this new helper.apply_proofinapply.rsto use this new helper, removing duplicated code.unwrapconsistent with previous logic for fatal DB errors).Testing
cargo check(passed)cargo test -p kaspa-consensus(passed)