Separate send version modules (part 1) (#453)

Expose descriptive, accurate send errors so that we can switch on
them in implementations and mark payjoin sessions in persistent
storage accurately. We only want to spawn sessions that are capable
of making forward progress and drop those that cannot.

Patch overview:
1. housekeeping
2. Introduce specific SenderBuilder errors
3. make CreateRequestError v2 only since v1 can only make have sender
builder errors
4. separate the modules to reduce feature flags

This separation is incomplete because I'm still uncertain what to do
with send::ValidationError's feature gated variants. Do I make a
send::ValidationError and a send::v2::ValidationError separate? How do
those get handled in ResponseError? Does ResponseError get split into
two versions, or do I just leave a feature gated variant? That's left
for the next PR predicated on this design being an appropriate one.

Note this pays back some tech debt but leaves some slop in payjoin-cli.
Rather than making this PR 10 commits to review I left combining FeeRate
parsing to a later PR (#452).

This error puts us on the path to #392 and #403

payjoin-cli/src/app/mod.rs

@@ -8,7 +8,6 @@ use bitcoincore_rpc::bitcoin::Amount;
 use bitcoincore_rpc::RpcApi;
 use payjoin::bitcoin::psbt::Psbt;
 use payjoin::receive::InputPair;
-use payjoin::send::Sender;
 use payjoin::{bitcoin, PjUri};
 
 pub mod config;
@@ -31,7 +30,7 @@ pub trait App {
     async fn send_payjoin(&self, bip21: &str, fee_rate: &f32) -> Result<()>;
     async fn receive_payjoin(self, amount_arg: &str) -> Result<()>;
 
-    fn create_pj_request(&self, uri: &PjUri, fee_rate: &f32) -> Result<Sender> {
+    fn create_original_psbt(&self, uri: &PjUri, fee_rate: &f32) -> Result<Psbt> {
         let amount = uri.amount.ok_or_else(|| anyhow!("please specify the amount in the Uri"))?;
 
         // wallet_create_funded_psbt requires a HashMap<address: String, Amount>
@@ -67,12 +66,7 @@ pub trait App {
             .psbt;
         let psbt = Psbt::from_str(&psbt).with_context(|| "Failed to load PSBT from base64")?;
         log::debug!("Original psbt: {:#?}", psbt);
-        let req_ctx = payjoin::send::SenderBuilder::from_psbt_and_uri(psbt, uri.clone())
-            .with_context(|| "Failed to build payjoin request")?
-            .build_recommended(fee_rate)
-            .with_context(|| "Failed to build payjoin request")?;
-
-        Ok(req_ctx)
+        Ok(psbt)
     }
 
     fn process_pj_response(&self, psbt: Psbt) -> Result<bitcoin::Txid> {

payjoin-cli/src/app/v1.rs

@@ -16,6 +16,7 @@ use hyper_util::rt::TokioIo;
 use payjoin::bitcoin::psbt::Psbt;
 use payjoin::bitcoin::{self, FeeRate};
 use payjoin::receive::{PayjoinProposal, UncheckedProposal};
+use payjoin::send::v1::SenderBuilder;
 use payjoin::{Error, PjUriBuilder, Uri, UriExt};
 use tokio::net::TcpListener;
 
@@ -72,7 +73,13 @@ impl AppTrait for App {
             Uri::try_from(bip21).map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))?;
         let uri = uri.assume_checked();
         let uri = uri.check_pj_supported().map_err(|_| anyhow!("URI does not support Payjoin"))?;
-        let (req, ctx) = self.create_pj_request(&uri, fee_rate)?.extract_v1()?;
+        let psbt = self.create_original_psbt(&uri, fee_rate)?;
+        let fee_rate_sat_per_kwu = fee_rate * 250.0_f32;
+        let fee_rate = FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64);
+        let (req, ctx) = SenderBuilder::new(psbt, uri.clone())
+            .build_recommended(fee_rate)
+            .with_context(|| "Failed to build payjoin request")?
+            .extract_v1()?;
         let http = http_agent()?;
         let body = String::from_utf8(req.body.clone()).unwrap();
         println!("Sending fallback request to {}", &req.url);

payjoin-cli/src/app/v2.rs

@@ -7,7 +7,7 @@ use payjoin::bitcoin::consensus::encode::serialize_hex;
 use payjoin::bitcoin::psbt::Psbt;
 use payjoin::bitcoin::{Amount, FeeRate};
 use payjoin::receive::v2::Receiver;
-use payjoin::send::Sender;
+use payjoin::send::v2::{Sender, SenderBuilder};
 use payjoin::{bitcoin, Error, Uri};
 use tokio::signal;
 use tokio::sync::watch;
@@ -65,7 +65,12 @@ impl AppTrait for App {
         let req_ctx = match self.db.get_send_session(url)? {
             Some(send_session) => send_session,
             None => {
-                let mut req_ctx = self.create_pj_request(&uri, fee_rate)?;
+                let psbt = self.create_original_psbt(&uri, fee_rate)?;
+                let fee_rate_sat_per_kwu = fee_rate * 250.0_f32;
+                let fee_rate = FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64);
+                let mut req_ctx = SenderBuilder::new(psbt, uri.clone())
+                    .build_recommended(fee_rate)
+                    .with_context(|| "Failed to build payjoin request")?;
                 self.db.insert_send_session(&mut req_ctx, url)?;
                 req_ctx
             }
@@ -192,7 +197,7 @@ impl App {
         Ok(())
     }
 
-    async fn long_poll_post(&self, req_ctx: &mut payjoin::send::Sender) -> Result<Psbt> {
+    async fn long_poll_post(&self, req_ctx: &mut Sender) -> Result<Psbt> {
         match req_ctx.extract_v2(self.config.ohttp_relay.clone()) {
             Ok((req, ctx)) => {
                 println!("Posting Original PSBT Payload request...");

payjoin-cli/src/db/v2.rs

@@ -1,6 +1,6 @@
 use bitcoincore_rpc::jsonrpc::serde_json;
 use payjoin::receive::v2::Receiver;
-use payjoin::send::Sender;
+use payjoin::send::v2::Sender;
 use sled::{IVec, Tree};
 use url::Url;
 

payjoin/src/send/error.rs

@@ -4,8 +4,82 @@ use bitcoin::locktime::absolute::LockTime;
 use bitcoin::transaction::Version;
 use bitcoin::{AddressType, Sequence};
 
-#[cfg(feature = "v2")]
-use crate::uri::url_ext::ParseReceiverPubkeyParamError;
+/// Error building a Sender from a SenderBuilder.
+///
+/// This error is unrecoverable.
+#[derive(Debug)]
+pub struct BuildSenderError(InternalBuildSenderError);
+
+#[derive(Debug)]
+pub(crate) enum InternalBuildSenderError {
+    InvalidOriginalInput(crate::psbt::PsbtInputsError),
+    InconsistentOriginalPsbt(crate::psbt::InconsistentPsbt),
+    NoInputs,
+    PayeeValueNotEqual,
+    NoOutputs,
+    MultiplePayeeOutputs,
+    MissingPayeeOutput,
+    FeeOutputValueLowerThanFeeContribution,
+    AmbiguousChangeOutput,
+    ChangeIndexOutOfBounds,
+    ChangeIndexPointsAtPayee,
+    InputWeight(crate::psbt::InputWeightError),
+    AddressType(crate::psbt::AddressTypeError),
+}
+
+impl From<InternalBuildSenderError> for BuildSenderError {
+    fn from(value: InternalBuildSenderError) -> Self { BuildSenderError(value) }
+}
+
+impl From<crate::psbt::AddressTypeError> for BuildSenderError {
+    fn from(value: crate::psbt::AddressTypeError) -> Self {
+        BuildSenderError(InternalBuildSenderError::AddressType(value))
+    }
+}
+
+impl fmt::Display for BuildSenderError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use InternalBuildSenderError::*;
+
+        match &self.0 {
+            InvalidOriginalInput(e) => write!(f, "an input in the original transaction is invalid: {:#?}", e),
+            InconsistentOriginalPsbt(e) => write!(f, "the original transaction is inconsistent: {:#?}", e),
+            NoInputs => write!(f, "the original transaction has no inputs"),
+            PayeeValueNotEqual => write!(f, "the value in original transaction doesn't equal value requested in the payment link"),
+            NoOutputs => write!(f, "the original transaction has no outputs"),
+            MultiplePayeeOutputs => write!(f, "the original transaction has more than one output belonging to the payee"),
+            MissingPayeeOutput => write!(f, "the output belonging to payee is missing from the original transaction"),
+            FeeOutputValueLowerThanFeeContribution => write!(f, "the value of fee output is lower than maximum allowed contribution"),
+            AmbiguousChangeOutput => write!(f, "can not determine which output is change because there's more than two outputs"),
+            ChangeIndexOutOfBounds => write!(f, "fee output index is points out of bounds"),
+            ChangeIndexPointsAtPayee => write!(f, "fee output index is points at output belonging to the payee"),
+            AddressType(e) => write!(f, "can not determine input address type: {}", e),
+            InputWeight(e) => write!(f, "can not determine expected input weight: {}", e),
+        }
+    }
+}
+
+impl std::error::Error for BuildSenderError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use InternalBuildSenderError::*;
+
+        match &self.0 {
+            InvalidOriginalInput(error) => Some(error),
+            InconsistentOriginalPsbt(error) => Some(error),
+            NoInputs => None,
+            PayeeValueNotEqual => None,
+            NoOutputs => None,
+            MultiplePayeeOutputs => None,
+            MissingPayeeOutput => None,
+            FeeOutputValueLowerThanFeeContribution => None,
+            AmbiguousChangeOutput => None,
+            ChangeIndexOutOfBounds => None,
+            ChangeIndexPointsAtPayee => None,
+            AddressType(error) => Some(error),
+            InputWeight(error) => Some(error),
+        }
+    }
+}
 
 /// Error that may occur when the response from receiver is malformed.
 ///
@@ -174,125 +248,6 @@ impl std::error::Error for ValidationError {
     }
 }
 
-/// Error returned when request could not be created.
-///
-/// This error can currently only happen due to programmer mistake.
-/// `unwrap()`ing it is thus considered OK in Rust but you may achieve nicer message by displaying
-/// it.
-#[derive(Debug)]
-pub struct CreateRequestError(InternalCreateRequestError);
-
-#[derive(Debug)]
-pub(crate) enum InternalCreateRequestError {
-    InvalidOriginalInput(crate::psbt::PsbtInputsError),
-    InconsistentOriginalPsbt(crate::psbt::InconsistentPsbt),
-    NoInputs,
-    PayeeValueNotEqual,
-    NoOutputs,
-    MultiplePayeeOutputs,
-    MissingPayeeOutput,
-    FeeOutputValueLowerThanFeeContribution,
-    AmbiguousChangeOutput,
-    ChangeIndexOutOfBounds,
-    ChangeIndexPointsAtPayee,
-    Url(url::ParseError),
-    AddressType(crate::psbt::AddressTypeError),
-    InputWeight(crate::psbt::InputWeightError),
-    #[cfg(feature = "v2")]
-    Hpke(crate::hpke::HpkeError),
-    #[cfg(feature = "v2")]
-    OhttpEncapsulation(crate::ohttp::OhttpEncapsulationError),
-    #[cfg(feature = "v2")]
-    ParseReceiverPubkey(ParseReceiverPubkeyParamError),
-    #[cfg(feature = "v2")]
-    MissingOhttpConfig,
-    #[cfg(feature = "v2")]
-    Expired(std::time::SystemTime),
-}
-
-impl fmt::Display for CreateRequestError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use InternalCreateRequestError::*;
-
-        match &self.0 {
-            InvalidOriginalInput(e) => write!(f, "an input in the original transaction is invalid: {:#?}", e),
-            InconsistentOriginalPsbt(e) => write!(f, "the original transaction is inconsistent: {:#?}", e),
-            NoInputs => write!(f, "the original transaction has no inputs"),
-            PayeeValueNotEqual => write!(f, "the value in original transaction doesn't equal value requested in the payment link"),
-            NoOutputs => write!(f, "the original transaction has no outputs"),
-            MultiplePayeeOutputs => write!(f, "the original transaction has more than one output belonging to the payee"),
-            MissingPayeeOutput => write!(f, "the output belonging to payee is missing from the original transaction"),
-            FeeOutputValueLowerThanFeeContribution => write!(f, "the value of fee output is lower than maximum allowed contribution"),
-            AmbiguousChangeOutput => write!(f, "can not determine which output is change because there's more than two outputs"),
-            ChangeIndexOutOfBounds => write!(f, "fee output index is points out of bounds"),
-            ChangeIndexPointsAtPayee => write!(f, "fee output index is points at output belonging to the payee"),
-            Url(e) => write!(f, "cannot parse url: {:#?}", e),
-            AddressType(e) => write!(f, "can not determine input address type: {}", e),
-            InputWeight(e) => write!(f, "can not determine expected input weight: {}", e),
-            #[cfg(feature = "v2")]
-            Hpke(e) => write!(f, "v2 error: {}", e),
-            #[cfg(feature = "v2")]
-            OhttpEncapsulation(e) => write!(f, "v2 error: {}", e),
-            #[cfg(feature = "v2")]
-            ParseReceiverPubkey(e) => write!(f, "cannot parse receiver public key: {}", e),
-            #[cfg(feature = "v2")]
-            MissingOhttpConfig => write!(f, "no ohttp configuration with which to make a v2 request available"),
-            #[cfg(feature = "v2")]
-            Expired(expiry) => write!(f, "session expired at {:?}", expiry),
-        }
-    }
-}
-
-impl std::error::Error for CreateRequestError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use InternalCreateRequestError::*;
-
-        match &self.0 {
-            InvalidOriginalInput(error) => Some(error),
-            InconsistentOriginalPsbt(error) => Some(error),
-            NoInputs => None,
-            PayeeValueNotEqual => None,
-            NoOutputs => None,
-            MultiplePayeeOutputs => None,
-            MissingPayeeOutput => None,
-            FeeOutputValueLowerThanFeeContribution => None,
-            AmbiguousChangeOutput => None,
-            ChangeIndexOutOfBounds => None,
-            ChangeIndexPointsAtPayee => None,
-            Url(error) => Some(error),
-            AddressType(error) => Some(error),
-            InputWeight(error) => Some(error),
-            #[cfg(feature = "v2")]
-            Hpke(error) => Some(error),
-            #[cfg(feature = "v2")]
-            OhttpEncapsulation(error) => Some(error),
-            #[cfg(feature = "v2")]
-            ParseReceiverPubkey(error) => Some(error),
-            #[cfg(feature = "v2")]
-            MissingOhttpConfig => None,
-            #[cfg(feature = "v2")]
-            Expired(_) => None,
-        }
-    }
-}
-
-impl From<InternalCreateRequestError> for CreateRequestError {
-    fn from(value: InternalCreateRequestError) -> Self { CreateRequestError(value) }
-}
-
-impl From<crate::psbt::AddressTypeError> for CreateRequestError {
-    fn from(value: crate::psbt::AddressTypeError) -> Self {
-        CreateRequestError(InternalCreateRequestError::AddressType(value))
-    }
-}
-
-#[cfg(feature = "v2")]
-impl From<ParseReceiverPubkeyParamError> for CreateRequestError {
-    fn from(value: ParseReceiverPubkeyParamError) -> Self {
-        CreateRequestError(InternalCreateRequestError::ParseReceiverPubkey(value))
-    }
-}
-
 /// Represent an error returned by Payjoin receiver.
 pub enum ResponseError {
     /// `WellKnown` Errors are defined in the [`BIP78::ReceiverWellKnownError`] spec.