Refactor memory database config enum

[thunderbiscuit]

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

// 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:

[Enum]
interface DatabaseConfig {
  Memory(string junk);
  Sled(SledDbConfiguration config);
};

According to the docs from uniffi-rs 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:

[Enum]
interface DatabaseConfig {
  Memory();
  Sled(SledDbConfiguration config);
};

On the Rust side we then have

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)

sealed class DatabaseConfig  {
    
    data class Memory(
        val junk: String 
        ) : DatabaseConfig()
    
    data class Sled(
        val config: SledDbConfiguration 
        ) : DatabaseConfig()

to

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:

    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.

[notmandatory]

If this works then I'm all for it! Could be something got fixed in uniffi-rs since we started bdk-ffi. We should also test this with all three languages to make sure they support it.

[thunderbiscuit]

Yes the issue of the 3 languages is something I was thinking about. This one should be fine, but it does bring up the fact that we need to make sure all our changes always work for all 3 libraries. I think by the design of the uniffi-rs library, if the udl file can be parsed by the bindgen then the output should work, but when looking at the docs you do find some small things that they mention is not supported in all languages (Python and Ruby being the languages that don't always have everything).

If ever we run into this, one solution is to have different udl files for each language. At that point it starts looking like a different bdk-ffi for each bindings library, but anyway. I don't think we have anything at the moment that would be impacted by this issue of different language bindings supporting not quite the same APIs.

As for this particular PR, I think it's certainly not pressing and a very small change overall. We could keep it open and see if any other small refactorings come up over the next few weeks and bundle them all in this one.

[thunderbiscuit]

FYI I tested on Python and it worked.

[notmandatory]

I think it's the kind of thing we only need to make sure that for each language the bindgen doesn't throw an error.

[thunderbiscuit]

If you're going to do a few refactors (bug fix for the broadcast + flush), might as well merge this one in in the same batch so we do less breaking changes in between releases.

[notmandatory]

I confirmed in Swift the generated code looks good too:

public enum DatabaseConfig {
    
    case memory
    case sled(config: SledDbConfiguration )
}

[notmandatory]

ACK bc10ae1

[notmandatory]

Oops, I can't merge because your commit isn't signed. I'm going to merge #116 now, and then when you rebase this PR on that make sure your git signing is setup.

[thunderbiscuit]

Rebased and signed. Ready to go.