Skip to content

Commit

Permalink
Merge branch 'aleks/refactor' (#1717)
Browse files Browse the repository at this point in the history
* aleks/refactor:
  changelog: add #1717
  refactor: rename function
  refactor: rename method
  refactor: use immutable reference
  refactor: use immutable reference
  refactor: remove duplicated code
  • Loading branch information
tzemanovic committed Jul 14, 2023
2 parents 47e4fe1 + 37bbbdc commit d9809da
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 24 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/1717-storage-refactor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Refactored storage code to only use an immutable reference when reading and
writing to a batch. ([\#1717](https://github.com/anoma/namada/pull/1717))
26 changes: 9 additions & 17 deletions apps/src/lib/node/ledger/storage/rocksdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,6 @@ impl RocksDB {
.ok_or(Error::DBError("No {cf_name} column family".to_string()))
}

fn flush(&self, wait: bool) -> Result<()> {
let mut flush_opts = FlushOptions::default();
flush_opts.set_wait(wait);
self.0
.flush_opt(&flush_opts)
.map_err(|e| Error::DBError(e.into_string()))
}

/// Persist the diff of an account subspace key-val under the height where
/// it was changed.
fn write_subspace_diff(
Expand Down Expand Up @@ -512,7 +504,7 @@ impl DB for RocksDB {
.map_err(|e| Error::DBError(e.into_string()))
}

fn read_last_block(&mut self) -> Result<Option<BlockStateRead>> {
fn read_last_block(&self) -> Result<Option<BlockStateRead>> {
// Block height
let state_cf = self.get_column_family(STATE_CF)?;
let height: BlockHeight = match self
Expand Down Expand Up @@ -730,8 +722,8 @@ impl DB for RocksDB {
}
}

fn write_block(
&mut self,
fn add_block_to_batch(
&self,
state: BlockStateWrite,
batch: &mut Self::WriteBatch,
is_full_commit: bool,
Expand Down Expand Up @@ -1543,7 +1535,7 @@ mod test {
)
.unwrap();

write_block(&mut db, &mut batch, BlockHeight::default()).unwrap();
add_block_to_batch(&db, &mut batch, BlockHeight::default()).unwrap();
db.exec_batch(batch.0).unwrap();

let _state = db
Expand Down Expand Up @@ -1734,7 +1726,7 @@ mod test {
)
.unwrap();

write_block(&mut db, &mut batch, height_0).unwrap();
add_block_to_batch(&db, &mut batch, height_0).unwrap();
db.exec_batch(batch.0).unwrap();

// Write second block
Expand All @@ -1754,7 +1746,7 @@ mod test {
db.batch_delete_subspace_val(&mut batch, height_1, &delete_key)
.unwrap();

write_block(&mut db, &mut batch, height_1).unwrap();
add_block_to_batch(&db, &mut batch, height_1).unwrap();
db.exec_batch(batch.0).unwrap();

// Check that the values are as expected from second block
Expand All @@ -1778,8 +1770,8 @@ mod test {
}

/// A test helper to write a block
fn write_block(
db: &mut RocksDB,
fn add_block_to_batch(
db: &RocksDB,
batch: &mut RocksDBWriteBatch,
height: BlockHeight,
) -> Result<()> {
Expand Down Expand Up @@ -1814,6 +1806,6 @@ mod test {
eth_events_queue: &eth_events_queue,
};

db.write_block(block, batch, true)
db.add_block_to_batch(block, batch, true)
}
}
6 changes: 3 additions & 3 deletions core/src/ledger/storage/mockdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl DB for MockDB {
Ok(())
}

fn read_last_block(&mut self) -> Result<Option<BlockStateRead>> {
fn read_last_block(&self) -> Result<Option<BlockStateRead>> {
// Block height
let height: BlockHeight = match self.0.borrow().get("height") {
Some(bytes) => types::decode(bytes).map_err(Error::CodingError)?,
Expand Down Expand Up @@ -211,8 +211,8 @@ impl DB for MockDB {
}
}

fn write_block(
&mut self,
fn add_block_to_batch(
&self,
state: BlockStateWrite,
_batch: &mut Self::WriteBatch,
_is_full_commit: bool,
Expand Down
9 changes: 5 additions & 4 deletions core/src/ledger/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,12 @@ pub trait DB: std::fmt::Debug {
fn flush(&self, wait: bool) -> Result<()>;

/// Read the last committed block's metadata
fn read_last_block(&mut self) -> Result<Option<BlockStateRead>>;
fn read_last_block(&self) -> Result<Option<BlockStateRead>>;

/// Write block's metadata. Merkle tree sub-stores are committed only when
/// `is_full_commit` is `true` (typically on a beginning of a new epoch).
fn write_block(
&mut self,
fn add_block_to_batch(
&self,
state: BlockStateWrite,
batch: &mut Self::WriteBatch,
is_full_commit: bool,
Expand Down Expand Up @@ -532,7 +532,8 @@ where
ethereum_height: self.ethereum_height.as_ref(),
eth_events_queue: &self.eth_events_queue,
};
self.db.write_block(state, &mut batch, is_full_commit)?;
self.db
.add_block_to_batch(state, &mut batch, is_full_commit)?;
let header = self
.header
.take()
Expand Down

0 comments on commit d9809da

Please sign in to comment.