Skip to content

Commit

Permalink
Fix history store range function
Browse files Browse the repository at this point in the history
The history store range function now uses a database transaction
Added unittest for total_len_at_epoch in the light history store
  • Loading branch information
viquezclaudio committed May 15, 2024
1 parent 3fc2977 commit 6eadd57
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 9 deletions.
14 changes: 11 additions & 3 deletions blockchain/src/history/history_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1257,9 +1257,17 @@ impl HistoryInterface for HistoryStore {
Some(total_size)
}

fn history_store_range(&self) -> (u32, u32) {
let read_txn = self.db.read_transaction();
let mut cursor = read_txn.cursor(&self.last_leaf_table);
fn history_store_range(&self, txn_option: Option<&TransactionProxy>) -> (u32, u32) {
let read_txn: TransactionProxy;
let txn = match txn_option {
Some(txn) => txn,
None => {
read_txn = self.db.read_transaction();
&read_txn
}
};

let mut cursor = txn.cursor(&self.last_leaf_table);

let first = if let Some((key, _)) = cursor.first::<u32, u32>() {
key
Expand Down
2 changes: 1 addition & 1 deletion blockchain/src/history/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub trait HistoryInterface {
-> usize;

/// Returns the first and last block number stored in the history store
fn history_store_range(&self) -> (u32, u32);
fn history_store_range(&self, txn_option: Option<&TransactionProxy>) -> (u32, u32);

/// Add a list of historic transactions to an existing history tree. It returns the root of the
/// resulting tree and the total size of the transactions added.
Expand Down
52 changes: 48 additions & 4 deletions blockchain/src/history/light_history_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,17 @@ impl HistoryInterface for LightHistoryStore {
tree.len() as u32
}

fn history_store_range(&self) -> (u32, u32) {
let read_txn = self.db.read_transaction();
fn history_store_range(&self, txn_option: Option<&TransactionProxy>) -> (u32, u32) {
let read_txn: TransactionProxy;
let txn = match txn_option {
Some(txn) => txn,
None => {
read_txn = self.db.read_transaction();
&read_txn
}
};

get_range(&self.hist_tree_table, &read_txn)
get_range(&self.hist_tree_table, &txn)

Check warning on line 190 in blockchain/src/history/light_history_store.rs

View workflow job for this annotation

GitHub Actions / Clippy Report

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> blockchain/src/history/light_history_store.rs:190:42 | 190 | get_range(&self.hist_tree_table, &txn) | ^^^^ help: change this to: `txn` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow = note: `#[warn(clippy::needless_borrow)]` on by default
}

fn total_len_at_epoch(
Expand All @@ -197,7 +204,7 @@ impl HistoryInterface for LightHistoryStore {
}
};

let (first_bn, last_bn) = self.history_store_range();
let (first_bn, last_bn) = self.history_store_range(Some(txn));

let length = if Policy::epoch_at(last_bn) == epoch_number {
// Get peaks mmr of the latest block stored
Expand Down Expand Up @@ -472,6 +479,43 @@ mod tests {
assert_eq!(len_3, 15);
}

#[test]
fn total_len_at_epoch() {
// Initialize History Store.
let env = VolatileDatabase::new(20).unwrap();
let history_store = LightHistoryStore::new(env.clone(), NetworkId::UnitAlbatross);

// Create historic transactions.
let ext_0 = create_transaction(Policy::genesis_block_number() + 1, 0);
let ext_1 = create_transaction(Policy::genesis_block_number() + 1, 1);
let ext_2 = create_transaction(Policy::genesis_block_number() + 2, 2);
let ext_3 = create_transaction(Policy::genesis_block_number() + 3, 3);
let ext_4 = create_transaction(Policy::genesis_block_number() + 3, 4);
let ext_5 = create_transaction(Policy::genesis_block_number() + 3, 5);
let ext_6 = create_transaction(Policy::genesis_block_number() + 3, 6);
let ext_7 = create_transaction(Policy::genesis_block_number() + 3, 7);

let hist_txs = vec![ext_0, ext_1];

// Add historic transactions to History Store.
let mut txn = env.write_transaction();
history_store.add_to_history(&mut txn, Policy::genesis_block_number() + 1, &hist_txs);

let hist_txs = vec![ext_2];
history_store.add_to_history(&mut txn, Policy::genesis_block_number() + 2, &hist_txs);

let hist_txs = vec![ext_3, ext_4, ext_5, ext_6, ext_7];
history_store.add_to_history(&mut txn, Policy::genesis_block_number() + 3, &hist_txs);

let total_length = history_store.total_len_at_epoch(
Policy::epoch_at(Policy::genesis_block_number() + 1),
Some(&txn),
);

// Lengths are cumulative because they are block number based
assert_eq!(total_length, 8);
}

#[test]
fn it_can_remove_a_block() {
// Initialize History Store.
Expand Down
2 changes: 1 addition & 1 deletion consensus/src/sync/light/validity_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
BlockchainProxy::Full(blockchain) => {
let blockchain_wr = blockchain.read();

let (first_bn, last_bn) = blockchain_wr.history_store.history_store_range();
let (first_bn, last_bn) = blockchain_wr.history_store.history_store_range(None);

// This means we already know the first epoch so we need to request the missing items from the current epoch
if last_bn >= verifier_block_number {
Expand Down

0 comments on commit 6eadd57

Please sign in to comment.