Merge #575: Remove database flush

5ff8320 add private function ivcec_to_u32 in keyvalue (KaFai Choi)
e68d3b9 remove Database::flush (KaFai Choi)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  This PR is to remove Database::flush. See this issue for detail #567

  ### Notes to the reviewers
  The 2nd commit is a small refactoring of adding a new private ivec_to_u32 to avoid too much code duplication. Please let me know if it's ok to include this in this PR or I should make it into a separate PR

  Currently existing test cases are shared across for all Databaes implementation so I am not sure if we should add  specific test cases for keyvalue(Tree) for this auto-flush behaviour?(and I feel like it's more a implementation detail). Please let me know how should I proceed for test case in this PR

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [ ] I've added docs for the new feature
  * [x] I've updated `CHANGELOG.md`

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [ ] I'm linking the issue being fixed by this PR

ACKs for top commit:
  danielabrozzoni:
    re-ACK 5ff8320

Tree-SHA512: eb37de8217efeb89d3ae346da36d0fb55aa67554d591b4759500f793bcf6aa7601c3d717fd473136c88e76aa72dbb6008ecf62b1d4ccf5ba3cbd1598f758522a

Author: notmandatory

CHANGELOG.md

@@ -53,6 +53,7 @@ To decouple the `Wallet` from the `Blockchain` we've made major changes:
 - Stop making a request for the block height when calling `Wallet:new`.
 - Added `SyncOptions` to capture extra (future) arguments to `Wallet::sync`.
 - Removed `max_addresses` sync parameter which determined how many addresses to cache before syncing since this can just be done with `ensure_addresses_cached`.
+- remove `flush` method from the `Database` trait.
 
 ## [v0.16.1] - [v0.16.0]
 

src/database/any.rs

@@ -255,10 +255,6 @@ impl Database for AnyDatabase {
     fn increment_last_index(&mut self, keychain: KeychainKind) -> Result<u32, Error> {
         impl_inner_method!(AnyDatabase, self, increment_last_index, keychain)
     }
-
-    fn flush(&mut self) -> Result<(), Error> {
-        impl_inner_method!(AnyDatabase, self, flush)
-    }
 }
 
 impl BatchOperations for AnyBatch {

src/database/keyvalue.rs

@@ -166,16 +166,9 @@ macro_rules! impl_batch_operations {
         fn del_last_index(&mut self, keychain: KeychainKind) -> Result<Option<u32>, Error> {
             let key = MapKey::LastIndex(keychain).as_map_key();
             let res = self.remove(key);
-            let res = $process_delete!(res);
-
-            match res {
-                None => Ok(None),
-                Some(b) => {
-                    let array: [u8; 4] = b.as_ref().try_into().map_err(|_| Error::InvalidU32Bytes(b.to_vec()))?;
-                    let val = u32::from_be_bytes(array);
-                    Ok(Some(val))
-                }
-            }
+            $process_delete!(res)
+            .map(ivec_to_u32)
+            .transpose()
         }
 
         fn del_sync_time(&mut self) -> Result<Option<SyncTime>, Error> {
@@ -357,16 +350,7 @@ impl Database for Tree {
 
     fn get_last_index(&self, keychain: KeychainKind) -> Result<Option<u32>, Error> {
         let key = MapKey::LastIndex(keychain).as_map_key();
-        self.get(key)?
-            .map(|b| -> Result<_, Error> {
-                let array: [u8; 4] = b
-                    .as_ref()
-                    .try_into()
-                    .map_err(|_| Error::InvalidU32Bytes(b.to_vec()))?;
-                let val = u32::from_be_bytes(array);
-                Ok(val)
-            })
-            .transpose()
+        self.get(key)?.map(ivec_to_u32).transpose()
     }
 
     fn get_sync_time(&self) -> Result<Option<SyncTime>, Error> {
@@ -393,19 +377,17 @@ impl Database for Tree {
 
             Some(new.to_be_bytes().to_vec())
         })?
-        .map_or(Ok(0), |b| -> Result<_, Error> {
-            let array: [u8; 4] = b
-                .as_ref()
-                .try_into()
-                .map_err(|_| Error::InvalidU32Bytes(b.to_vec()))?;
-            let val = u32::from_be_bytes(array);
-            Ok(val)
-        })
+        .map_or(Ok(0), ivec_to_u32)
     }
+}
 
-    fn flush(&mut self) -> Result<(), Error> {
-        Ok(Tree::flush(self).map(|_| ())?)
-    }
+fn ivec_to_u32(b: sled::IVec) -> Result<u32, Error> {
+    let array: [u8; 4] = b
+        .as_ref()
+        .try_into()
+        .map_err(|_| Error::InvalidU32Bytes(b.to_vec()))?;
+    let val = u32::from_be_bytes(array);
+    Ok(val)
 }
 
 impl BatchDatabase for Tree {

src/database/memory.rs

@@ -449,10 +449,6 @@ impl Database for MemoryDatabase {
 
         Ok(*value)
     }
-
-    fn flush(&mut self) -> Result<(), Error> {
-        Ok(())
-    }
 }
 
 impl BatchDatabase for MemoryDatabase {

src/database/mod.rs

@@ -158,13 +158,6 @@ pub trait Database: BatchOperations {
     ///
     /// It should insert and return `0` if not present in the database
     fn increment_last_index(&mut self, keychain: KeychainKind) -> Result<u32, Error>;
-
-    #[deprecated(
-        since = "0.18.0",
-        note = "The flush function is only needed for the sled database on mobile, instead for mobile use the sqlite database."
-    )]
-    /// Force changes to be written to disk
-    fn flush(&mut self) -> Result<(), Error>;
 }
 
 /// Trait for a database that supports batch operations

src/database/sqlite.rs

@@ -891,10 +891,6 @@ impl Database for SqliteDatabase {
             }
         }
     }
-
-    fn flush(&mut self) -> Result<(), Error> {
-        Ok(())
-    }
 }
 
 impl BatchDatabase for SqliteDatabase {