Uniformly use primitive types in FFI boundary, with runtime validation

[spacebear21]

On one hand strong types make it harder to misue the API.

on the other hand, they require conversion on the implementers side. It could get even trickier if there are different versions of bdk-ffi and payjoin-ffi that want to work together but use different bitcoin-ffi versions.

Perhaps to compose these, we need a bdk-ffi that depends on payjoin-ffi with a feature gate so that the same bitcoin-ffi dependency can be used without duplication. @thunderbiscuit Have you given this any thought before?

_Originally posted by @DanGould in LtbLightning/payjoin-ffi#70 (comment)

[spacebear21]

Adding to 1.0 milestone not necessarily because I think we should use strong types, but we should make a decision for 1.0 either way.

[arminsabouri]

Re-assigning this to @chavic

[chavic]

Earlier, I raised a concern about some of the BTC types generating here. I performed a smoke test, and the Bitcoin types work just fine here...

Here are my thoughts on the issue: It may be possible to maintain the public FFI surface for simple primitives (PSBT strings, Satoshi u64, plain URLs) in the 1.0 cycle and compensate with stricter runtime validation. That avoids binding ourselves to a specific Bitcoin-FFI release that we don’t control and keeps BDK-FFI integrators from juggling multiple wrapper hierarchies.

If we want to help adopters use strong types, expose them via optional helpers: e.g., add a small compat module that accepts/returns bitcoin_ffi::Address/Amount behind a feature flag. This keeps the core API stable while giving teams who manage their dependency graph tightly the choice to opt in. Which looks something like this:

#[cfg(feature = "typed-helpers")]
pub mod typed {
    use crate::{uri::PjUri, bitcoin_ffi::{Address, Amount}};
    use std::sync::Arc;

    pub fn parse_sender_inputs(psbt: &str, uri: &PjUri) -> Result<(payjoin::bitcoin::Psbt, Address), String> {
        let psbt = payjoin::bitcoin::psbt::Psbt::from_str(psbt).map_err(|e| e.to_string())?;
        let address = Address::new(uri.address(), bitcoin_ffi::Network::Bitcoin)
            .map_err(|e| e.to_string())?;
        Ok((psbt, address))
    }

    pub fn amount_from_sats(sats: u64) -> Arc<Amount> {
        Arc::new(Amount::from_sat(sats))
    }
}

As opposed to:

#[uniffi::constructor]
pub fn new(psbt: Arc<Psbt>, uri: Arc<PjUri>) -> Result<Self, BuildSenderError> { ... }

pub fn build_with_additional_fee(
    &self,
    max_fee_contribution: Arc<Amount>,
    change_index: Option<u8>,
    min_fee_rate: Arc<FeeRate>,
    clamp_fee_contribution: bool,
) -> Result<InitialSendTransition, BuildSenderError> { ... }

The default API stays on plain strings/u64, so anyone who ignores the helper feature can treat bitcoin-ffi as an internal implementation detail and avoid synchronizing versions. Only users who opt in to the helper module (and therefore choose to work with bitcoin_ffi::Address, Amount, etc.) would take on the dependency-alignment responsibility; it becomes a conscious trade-off rather than a requirement for everyone.

I'd say I'm biased towards a stricter, but I'm making the assumption that library versions won't be in sync much of the time

[spacebear21]

It may be possible to maintain the public FFI surface for simple primitives (PSBT strings, Satoshi u64, plain URLs) in the 1.0 cycle and compensate with stricter runtime validation. That avoids binding ourselves to a specific Bitcoin-FFI release that we don’t control and keeps BDK-FFI integrators from juggling multiple wrapper hierarchies.

I generally agree with this. I think our current approach is basically a mix of both, which is the worst of both worlds. Given the interop requirements with bdk-ffi it seems cleaner to use primitives for all boundary crossings, with runtime validation.

We can keep exporting bitcoin_ffi alongside payjoin_ffi as a convenience for implementers (for example converting callback inputs for processing), but remove the bitcoin_ffi types from the API.

@DanGould / @nothingmuch any objections?

[DanGould]

sounds good to me. I hadn't considered the bitcoin-ffi release cadence. Crossing boundaries with primitives and validating at runtime sounds appropriate for the usecases in our medium-term roadmap. Perhaps bitcoin-ffi will. get up a more predictable release cadence sometime in the future at which point we can reconsider.