feat: add testnet4 to network type

[reez]

Description

Add Testnet4 to Network type

Notes to Reviewers

The first tag I see rust-bitcoin Network added Testnet4 is 0.32.4 so I bumped our dependency version on bitcoin

[reez]

@thunderbiscuit this is ready to go now I think, and rebased on master

[reez]

rebased on master

[thunderbiscuit]

Using the new [NonExhaustive] keyword in the UDL I think you can remove the From trait implementations. Those were there because you could not previously compile with non-exhaustive enums otherwise I think?

[reez]

Using the new [NonExhaustive] keyword in the UDL I think you can remove the From trait implementations. Those were there because you could not previously compile with non-exhaustive enums otherwise I think?

ah yes, I think we still need From<Network> for bitcoin::Network because we need to convert our FFI Network enum into rust-bitcoin Network type for direct library calls (like require_network and is_valid_for_network), but I can remove From<bitcoin::Network> for Network trait because we never need to convert in the opposite direction in our codebase.

updated the PR to reflect this now, thanks!

[thunderbiscuit]

I think you can now replace the UDL with what we have in bdk-ffi:

[NonExhaustive, Remote]
enum Network {
  "Bitcoin",
  "Testnet",
  "Signet",
  "Regtest",
  "Testnet4",
};

And remove the type from the Rust side entirely (it will simply pull what it needs from rust bitcoin and since you use it "directly", there is no conversion necessary.

# Remove this

#[derive(Clone, Default, uniffi::Enum)]
#[non_exhaustive]
pub enum Network {
    #[default]
    Bitcoin,
    Testnet,
    Signet,
    Regtest,
}

impl From<Network> for bitcoin::Network {
    fn from(network: Network) -> Self {
        match network {
            Network::Bitcoin => bitcoin::Network::Bitcoin,
            Network::Testnet => bitcoin::Network::Testnet,
            Network::Signet => bitcoin::Network::Signet,
            Network::Regtest => bitcoin::Network::Regtest,
        }
    }
}

impl From<bitcoin::Network> for Network {
    fn from(network: bitcoin::Network) -> Self {
        match network {
            bitcoin::Network::Bitcoin => Network::Bitcoin,
            bitcoin::Network::Testnet => Network::Testnet,
            bitcoin::Network::Signet => Network::Signet,
            bitcoin::Network::Regtest => Network::Regtest,
            _ => unreachable!(),
        }
    }
}

[thunderbiscuit]

I got the above working locally.

From there your next step (once you're updated to 0.29.0) is probably a new commit that removes the type entirely from the UDL using the docs here: https://mozilla.github.io/uniffi-rs/latest/types/remote_ext_types.html#proc-macros.

[thunderbiscuit]

None of this is required for inclusion, if you just need a quick merge to push 1.3 I don't mean to block that.

Maybe @DanGould can comment on whether this is something he'd want before the 1.3 release? I don't have a dog in this fight so happy to go either way.

[reez]

@thunderbiscuit I dont think a quick merge to push 0.1.3 is urgent or anything, I just figure we are about ready for a release since it would be nice to do one now that we have procmacros and we may have users of bitcoin-ffi that would enjoy a new release for everything new.

I dont believe I can do your suggestion here until we upgrade to 0.29.0 (correct me if I'm wrong). I'd suggest we:

• merge this PR if it works well with 0.28.2, keeping it encapulated to purely adding testnet4 to the network type
• follow up with a PR updating everything to 0.29.0, incorporating your suggestion, and any other things we happen to find/want that are related purely to updating to 0.29.0 in that follow up PR

How does that sound?

[reez]

Merged, following up with #33

[DanGould]

Maybe @DanGould can comment on whether this is something he'd want before the 1.3 release? I don't have a dog in this fight so happy to go either way.

I feel like I'm missing some context on this ask. Payjoin lets the downstream implementations define Network, so we don't
need Testnet4 explicitly as far as I can tell.