Skip to content

Commit

Permalink
Allow database begin_batch functions to fail
Browse files Browse the repository at this point in the history
When dealing with local or remote databases than beginning a transaction
can fail. Not allowing implementations to return a result and resorting
to `unwrap()` is a non-starter for reliable production code as this will
skip Rust's own drop handler in many places and crashes the application.

Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
  • Loading branch information
Mic92 committed Feb 1, 2023
1 parent 97f8fe3 commit c5b995c
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/blockchain/compact_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl CompactFiltersBlockchain {
internal_max_deriv: &mut Option<u32>,
external_max_deriv: &mut Option<u32>,
) -> Result<(), Error> {
let mut updates = database.begin_batch();
let mut updates = database.begin_batch()?;

let mut incoming: u64 = 0;
let mut outgoing: u64 = 0;
Expand Down Expand Up @@ -410,7 +410,7 @@ impl WalletSync for CompactFiltersBlockchain {
"Dropping transactions newer than `last_synced_block` = {}",
last_synced_block
);
let mut updates = database.begin_batch();
let mut updates = database.begin_batch()?;
for details in database.iter_txs(false)? {
match details.confirmation_time {
Some(c) if (c.height as usize) < last_synced_block => continue,
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ impl<'a, D: BatchDatabase> DbState<'a, D> {

/// Prepare db batch operations.
fn as_db_batch(&self) -> Result<D::Batch, Error> {
let mut batch = self.db.begin_batch();
let mut batch = self.db.begin_batch()?;
let mut del_txs = 0_u32;

// delete stale (not retained) txs from db
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain/script_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ impl<'a, D: BatchDatabase> State<'a, D> {
})
.collect::<Result<Vec<(KeychainKind, u32)>, _>>()?;

let mut batch = self.db.begin_batch();
let mut batch = self.db.begin_batch()?;

// Delete old txs that no longer exist
for txid in txids_to_delete {
Expand Down
8 changes: 4 additions & 4 deletions src/database/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,13 @@ impl BatchOperations for AnyBatch {
impl BatchDatabase for AnyDatabase {
type Batch = AnyBatch;

fn begin_batch(&self) -> Self::Batch {
fn begin_batch(&self) -> Result<Self::Batch, Error> {
match self {
AnyDatabase::Memory(inner) => inner.begin_batch().into(),
AnyDatabase::Memory(inner) => inner.begin_batch().map(Into::into),
#[cfg(feature = "key-value-db")]
AnyDatabase::Sled(inner) => inner.begin_batch().into(),
AnyDatabase::Sled(inner) => inner.begin_batch().map(Into::into),
#[cfg(feature = "sqlite")]
AnyDatabase::Sqlite(inner) => inner.begin_batch().into(),
AnyDatabase::Sqlite(inner) => inner.begin_batch().map(Into::into),
}
}
fn commit_batch(&mut self, batch: Self::Batch) -> Result<(), Error> {
Expand Down
4 changes: 2 additions & 2 deletions src/database/keyvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,8 @@ fn ivec_to_u32(b: sled::IVec) -> Result<u32, Error> {
impl BatchDatabase for Tree {
type Batch = sled::Batch;

fn begin_batch(&self) -> Self::Batch {
sled::Batch::default()
fn begin_batch(&self) -> Result<Self::Batch, Error> {
Ok(sled::Batch::default())
}

fn commit_batch(&mut self, batch: Self::Batch) -> Result<(), Error> {
Expand Down
4 changes: 2 additions & 2 deletions src/database/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,8 @@ impl Database for MemoryDatabase {
impl BatchDatabase for MemoryDatabase {
type Batch = Self;

fn begin_batch(&self) -> Self::Batch {
MemoryDatabase::new()
fn begin_batch(&self) -> Result<Self::Batch, Error> {
Ok(MemoryDatabase::new())
}

fn commit_batch(&mut self, mut batch: Self::Batch) -> Result<(), Error> {
Expand Down
4 changes: 2 additions & 2 deletions src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub trait BatchDatabase: Database {
type Batch: BatchOperations;

/// Create a new batch container
fn begin_batch(&self) -> Self::Batch;
fn begin_batch(&self) -> Result<Self::Batch, Error>;
/// Consume and apply a batch of operations
fn commit_batch(&mut self, batch: Self::Batch) -> Result<(), Error>;
}
Expand Down Expand Up @@ -243,7 +243,7 @@ pub mod test {
}

pub fn test_batch_script_pubkey<D: BatchDatabase>(mut db: D) {
let mut batch = db.begin_batch();
let mut batch = db.begin_batch().unwrap();

let script = Script::from(
Vec::<u8>::from_hex("76a91402306a7c23f3e8010de41e9e591348bb83f11daa88ac").unwrap(),
Expand Down
6 changes: 3 additions & 3 deletions src/database/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,10 +918,10 @@ impl Database for SqliteDatabase {
impl BatchDatabase for SqliteDatabase {
type Batch = SqliteDatabase;

fn begin_batch(&self) -> Self::Batch {
fn begin_batch(&self) -> Result<Self::Batch, Error> {
let db = SqliteDatabase::new(self.path.clone());
db.connection.execute("BEGIN TRANSACTION", []).unwrap();
db
db.connection.execute("BEGIN TRANSACTION", [])?;
Ok(db)
}

fn commit_batch(&mut self, batch: Self::Batch) -> Result<(), Error> {
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ where
count = 1;
}

let mut address_batch = self.database.borrow().begin_batch();
let mut address_batch = self.database.borrow().begin_batch()?;

let start_time = time::Instant::new();
for i in from..(from + count) {
Expand Down

0 comments on commit c5b995c

Please sign in to comment.