Merge bitcoindevkit/bdk-ffi#109: Refactor memory database config enum

cc3736809a2910d3a739b37a61f1613404fba572 Fix memory database configuration enum (thunderbiscuit)

Pull request description:

  The `DatabaseConfig.Memory` enum currently requires a "junk" string argument which is not used when creating the wallet:

  ```rust
  // lib.rs line 24
  pub enum DatabaseConfig {
      Memory { junk: String },
      Sled { config: SledDbConfiguration },
  }

  // lib.rs line 209
  impl Wallet {
      fn new(
          descriptor: String,
          change_descriptor: Option<String>,
          network: Network,
          database_config: DatabaseConfig,
          blockchain_config: BlockchainConfig,
      ) -> Result<Self, BdkError> {
          let any_database_config = match database_config {
              DatabaseConfig::Memory { .. } => AnyDatabaseConfig::Memory(()),
              DatabaseConfig::Sled { config } => AnyDatabaseConfig::Sled(config),
          };
  ```

  Which translates to the udl file like this:
  ```txt
  [Enum]
  interface DatabaseConfig {
    Memory(string junk);
    Sled(SledDbConfiguration config);
  };
  ```

  According to the [docs from uniffi-rs](https://mozilla.github.io/uniffi-rs/udl/enumerations.html) the `interface` here is required because the enums have named fields. But after testing I found that we can declare the udl file like so, and remove the requirement for the `junk` argument:
  ```txt
  [Enum]
  interface DatabaseConfig {
    Memory();
    Sled(SledDbConfiguration config);
  };
  ```

  On the Rust side we then have
  ```rust
  pub enum DatabaseConfig {
      Memory,
      Sled { config: SledDbConfiguration },
  }
  ```

  And the resulting bindings go from (note that the bindings transform the enum into a sealed class rather than a Kotlin enum)
  ```kotlin
  sealed class DatabaseConfig  {

      data class Memory(
          val junk: String
          ) : DatabaseConfig()

      data class Sled(
          val config: SledDbConfiguration
          ) : DatabaseConfig()
  ```
  to
  ```kotlin
  sealed class DatabaseConfig  {
      object Memory : DatabaseConfig()

      data class Sled(
          val config: SledDbConfiguration
          ) : DatabaseConfig()
  ```

  Which makes the API simpler to use, and removes the confusion created by having to provide an empty string (or not know what we're supposed to provide) to the `Memory()` enum.

  The final call-site looks like this:
  ```kotlin
      fun onlineWalletSyncGetBalance() {
          // val db = DatabaseConfig.Memory("")
          val db = DatabaseConfig.Memory
          val client = BlockchainConfig.Electrum(
              ElectrumConfig(
                  "ssl://electrum.blockstream.info:60002",
                  null,
                  5u,
                  null,
                  100u
              )
          )
          val wallet = Wallet(descriptor, null, Network.REGTEST, databaseConfig, blockchainConfig)
          wallet.sync(LogProgress(), null)
          val balance = wallet.getBalance()
          assertTrue(balance > 0u)
      }
  ```

  All tests run well on my side of things, but I'm opening this more as a discussion piece because I wasn't sure if there were other reasons for the choice of providing the argument to the `Memory` enum, or other design choices I'm not aware of. Any thoughts on this @artfuldev? I think you were the one who wrote the initial enum.

Top commit has no ACKs.

Tree-SHA512: 135e5943039a08522773f721a7cf6bbb93bd5bb9394bf42a30bab5f3e16fd35ce078056756e020a666d4f574d74080bc3404cc81809c0d7e0afe5c9471878425
This commit is contained in:
Steve Myers 2022-03-14 10:01:43 -05:00
commit f76f3234b4
No known key found for this signature in database
GPG Key ID: 8105A46B22C2D051
2 changed files with 3 additions and 3 deletions

View File

@ -67,7 +67,7 @@ dictionary SqliteDbConfiguration {
[Enum]
interface DatabaseConfig {
Memory(string junk);
Memory();
Sled(SledDbConfiguration config);
Sqlite(SqliteDbConfiguration config);
};

View File

@ -22,7 +22,7 @@ uniffi_macros::include_scaffolding!("bdk");
type BdkError = Error;
pub enum DatabaseConfig {
Memory { junk: String },
Memory,
Sled { config: SledDbConfiguration },
Sqlite { config: SqliteDbConfiguration },
}
@ -216,7 +216,7 @@ impl Wallet {
blockchain_config: BlockchainConfig,
) -> Result<Self, BdkError> {
let any_database_config = match database_config {
DatabaseConfig::Memory { .. } => AnyDatabaseConfig::Memory(()),
DatabaseConfig::Memory => AnyDatabaseConfig::Memory(()),
DatabaseConfig::Sled { config } => AnyDatabaseConfig::Sled(config),
DatabaseConfig::Sqlite { config } => AnyDatabaseConfig::Sqlite(config),
};