Skip to content

Improve receive error heirarchy #1031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DanGould merged 7 commits into from
Sep 2, 2025
Merged

Improve receive error heirarchy #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8e854a7
Remove FinalizeProposalError
DanGould Aug 29, 2025
46010ee
Return Weight, not Result additional_input_weight
DanGould Aug 29, 2025
50b824e
Narrow fee_range application error
DanGould Aug 29, 2025
b57fea7
Prefer infallible `mailbox_endpoint` to ImplementationError
DanGould Aug 31, 2025
cbfb043
Return json from v1
DanGould Aug 31, 2025
ae18325
Move Implementation to top-level receive::Error
DanGould Sep 1, 2025
59dad2e
Refactor check_broadcast_suitability legibility
DanGould Sep 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 22 additions & 26 deletions payjoin-cli/src/app/v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use hyper_util::rt::TokioIo;
use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::{Amount, FeeRate};
use payjoin::receive::v1::{PayjoinProposal, UncheckedOriginalPayload};
use payjoin::receive::ReplyableError::{self, Implementation, V1};
use payjoin::receive::Error;
use payjoin::send::v1::SenderBuilder;
use payjoin::{ImplementationError, IntoUrl, Uri, UriExt};
use tokio::net::TcpListener;
Expand Down Expand Up @@ -238,15 +238,13 @@ impl App {
(&Method::POST, _) => self
.handle_payjoin_post(req)
.await
.map_err(|e| match e {
V1(e) => {
tracing::error!("Error handling request: {e}");
Response::builder().status(400).body(full(e.to_string())).unwrap()
}
e => {
tracing::error!("Error handling request: {e}");
Response::builder().status(500).body(full(e.to_string())).unwrap()
}
.map_err(|e| {
let json = payjoin::receive::JsonReply::from(&e);
tracing::error!("Error handling request: {e}");
Response::builder()
.status(json.status_code())
.body(full(json.to_json().to_string()))
.unwrap()
})
.unwrap_or_else(|err_resp| err_resp),
_ => Response::builder().status(StatusCode::NOT_FOUND).body(full("Not found")).unwrap(),
Expand All @@ -260,15 +258,13 @@ impl App {
fn handle_get_bip21(
&self,
amount: Option<Amount>,
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
let v1_config = self
.config
.v1()
.map_err(|e| Implementation(ImplementationError::from(e.into_boxed_dyn_error())))?;
let address = self
.wallet
.get_new_address()
.map_err(|e| Implementation(ImplementationError::from(e.into_boxed_dyn_error())))?;
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
let v1_config = self.config.v1().map_err(|e| {
Error::Implementation(ImplementationError::from(e.into_boxed_dyn_error()))
})?;
let address = self.wallet.get_new_address().map_err(|e| {
Error::Implementation(ImplementationError::from(e.into_boxed_dyn_error()))
})?;
let uri_string = if let Some(amount) = amount {
format!(
"{}?amount={}&pj={}",
Expand All @@ -280,7 +276,7 @@ impl App {
format!("{}?pj={}", address.to_qr_uri(), v1_config.pj_endpoint)
};
let uri = Uri::try_from(uri_string.clone()).map_err(|_| {
Implementation(ImplementationError::from(
Error::Implementation(ImplementationError::from(
anyhow!("Could not parse payjoin URI string.").into_boxed_dyn_error(),
))
})?;
Expand All @@ -292,14 +288,14 @@ impl App {
async fn handle_payjoin_post(
&self,
req: Request<Incoming>,
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
let (parts, body) = req.into_parts();
let headers = Headers(&parts.headers);
let query_string = parts.uri.query().unwrap_or("");
let body = body
.collect()
.await
.map_err(|e| Implementation(ImplementationError::new(e)))?
.map_err(|e| Error::Implementation(ImplementationError::new(e)))?
.to_bytes();
let proposal = UncheckedOriginalPayload::from_request(&body, query_string, headers)?;

Expand All @@ -316,7 +312,7 @@ impl App {
fn process_v1_proposal(
&self,
proposal: UncheckedOriginalPayload,
) -> Result<PayjoinProposal, ReplyableError> {
) -> Result<PayjoinProposal, Error> {
let wallet = self.wallet();

// Receive Check 1: Can Broadcast
Expand Down Expand Up @@ -354,15 +350,15 @@ impl App {
.wallet
.get_new_address()
.map_err(|e| {
Implementation(ImplementationError::from(e.into_boxed_dyn_error()))
Error::Implementation(ImplementationError::from(e.into_boxed_dyn_error()))
})?
.script_pubkey(),
)
.map_err(|e| Implementation(ImplementationError::new(e)))?
.map_err(|e| Error::Implementation(ImplementationError::new(e)))?
.commit_outputs();

let wants_fee_range = try_contributing_inputs(payjoin.clone(), &self.wallet)
.map_err(ReplyableError::Implementation)?;
.map_err(Error::Implementation)?;
let provisional_payjoin =
wants_fee_range.apply_fee_range(None, self.config.max_fee_rate)?;

Expand Down
33 changes: 16 additions & 17 deletions payjoin-ffi/src/receive/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ use crate::uri::error::IntoUrlError;
#[derive(Debug, thiserror::Error, uniffi::Error)]
#[non_exhaustive]
pub enum ReceiverError {
/// Errors that can be replied to the sender
#[error("Replyable error: {0}")]
ReplyToSender(Arc<ReplyableError>),
/// V2-specific errors that are infeasable to reply to the sender
#[error("Unreplyable error: {0}")]
V2(Arc<SessionError>),
/// Error in underlying protocol function
#[error("Protocol error: {0}")]
Protocol(Arc<ProtocolError>),
/// Error arising due to the specific receiver implementation
///
/// e.g. database errors, network failures, wallet errors
#[error("Implementation error: {0}")]
Implementation(Arc<ImplementationError>),
/// Error that may occur when converting a some type to a URL
#[error("IntoUrl error: {0}")]
IntoUrl(Arc<IntoUrlError>),
Expand All @@ -28,8 +30,9 @@ impl From<receive::Error> for ReceiverError {
use ReceiverError::*;

match value {
receive::Error::ReplyToSender(e) => ReplyToSender(Arc::new(ReplyableError(e))),
receive::Error::V2(e) => V2(Arc::new(SessionError(e))),
receive::Error::Protocol(e) => Protocol(Arc::new(ProtocolError(e))),
receive::Error::Implementation(e) =>
Implementation(Arc::new(ImplementationError::from(e))),
_ => Unexpected,
}
}
Expand Down Expand Up @@ -75,12 +78,8 @@ macro_rules! impl_persisted_error_from {
};
}

impl_persisted_error_from!(receive::ReplyableError, |api_err: receive::ReplyableError| {
ReceiverError::ReplyToSender(Arc::new(api_err.into()))
});

impl_persisted_error_from!(receive::v2::SessionError, |api_err: receive::v2::SessionError| {
ReceiverError::V2(Arc::new(api_err.into()))
impl_persisted_error_from!(receive::ProtocolError, |api_err: receive::ProtocolError| {
ReceiverError::Protocol(Arc::new(api_err.into()))
});

impl_persisted_error_from!(receive::Error, |api_err: receive::Error| api_err.into());
Expand All @@ -100,7 +99,7 @@ impl_persisted_error_from!(payjoin::IntoUrlError, |api_err: payjoin::IntoUrlErro
/// after conversion into [`JsonReply`]
#[derive(Debug, thiserror::Error, uniffi::Object)]
#[error(transparent)]
pub struct ReplyableError(#[from] receive::ReplyableError);
pub struct ProtocolError(#[from] receive::ProtocolError);

/// The standard format for errors that can be replied as JSON.
///
Expand All @@ -122,8 +121,8 @@ impl From<receive::JsonReply> for JsonReply {
fn from(value: receive::JsonReply) -> Self { Self(value) }
}

impl From<ReplyableError> for JsonReply {
fn from(value: ReplyableError) -> Self { Self((&value.0).into()) }
impl From<ProtocolError> for JsonReply {
fn from(value: ProtocolError) -> Self { Self((&value.0).into()) }
}

/// Error that may occur during a v2 session typestate change
Expand Down
16 changes: 8 additions & 8 deletions payjoin-ffi/src/receive/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::sync::{Arc, RwLock};
use std::time::Duration;

pub use error::{
InputContributionError, JsonReply, OutputSubstitutionError, PsbtInputError, ReceiverError,
ReplyableError, SelectionError, SessionError,
InputContributionError, JsonReply, OutputSubstitutionError, ProtocolError, PsbtInputError,
ReceiverError, SelectionError, SessionError,
};
use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::{Amount, FeeRate};
Expand Down Expand Up @@ -443,7 +443,7 @@ pub struct UncheckedOriginalPayloadTransition(
MaybeFatalTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::MaybeInputsOwned>,
payjoin::receive::ReplyableError,
payjoin::receive::Error,
>,
>,
>,
Expand Down Expand Up @@ -531,7 +531,7 @@ pub struct MaybeInputsOwnedTransition(
MaybeFatalTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::MaybeInputsSeen>,
payjoin::receive::ReplyableError,
payjoin::receive::Error,
>,
>,
>,
Expand Down Expand Up @@ -587,7 +587,7 @@ pub struct MaybeInputsSeenTransition(
MaybeFatalTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::OutputsUnknown>,
payjoin::receive::ReplyableError,
payjoin::receive::Error,
>,
>,
>,
Expand Down Expand Up @@ -639,7 +639,7 @@ pub struct OutputsUnknownTransition(
MaybeFatalTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::WantsOutputs>,
payjoin::receive::ReplyableError,
payjoin::receive::Error,
>,
>,
>,
Expand Down Expand Up @@ -830,7 +830,7 @@ pub struct WantsFeeRangeTransition(
payjoin::persist::MaybeFatalTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::ProvisionalProposal>,
payjoin::receive::ReplyableError,
payjoin::receive::ProtocolError,
>,
>,
>,
Expand Down Expand Up @@ -906,7 +906,7 @@ pub struct ProvisionalProposalTransition(
payjoin::persist::MaybeTransientTransition<
payjoin::receive::v2::SessionEvent,
payjoin::receive::v2::Receiver<payjoin::receive::v2::PayjoinProposal>,
payjoin::receive::v2::SessionError,
payjoin::ImplementationError,
>,
>,
>,
Expand Down
4 changes: 2 additions & 2 deletions payjoin/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ exclude = ["tests"]
[features]
default = ["v2"]
#[doc = "Core features for payjoin state machines"]
_core = ["bitcoin/rand-std", "serde_json", "url/serde", "bitcoin_uri", "serde", "bitcoin/serde"]
_core = ["bitcoin/rand-std", "dep:http", "serde_json", "url/serde", "bitcoin_uri", "serde", "bitcoin/serde"]
directory = []
v1 = ["_core"]
v2 = ["_core", "hpke", "dep:http", "bhttp", "ohttp", "directory"]
v2 = ["_core", "hpke", "bhttp", "ohttp", "directory"]
#[doc = "Functions to fetch OHTTP keys via CONNECT proxy using reqwest. Enables `v2` since only `v2` uses OHTTP."]
io = ["v2", "reqwest/rustls-tls"]
_manual-tls = ["reqwest/rustls-tls", "rustls"]
Expand Down
32 changes: 9 additions & 23 deletions payjoin/src/core/receive/common/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use super::error::{
InternalSelectionError,
};
use super::optional_parameters::Params;
use super::{InputPair, OutputSubstitutionError, ReplyableError, SelectionError};
use super::{InputPair, OutputSubstitutionError, SelectionError};
use crate::output_substitution::OutputSubstitution;
use crate::psbt::PsbtExt;
use crate::receive::{InternalPayloadError, OriginalPayload, PsbtContext};
Expand Down Expand Up @@ -389,7 +389,7 @@ impl WantsFeeRange {
// If the sender specified a fee contribution, the receiver is allowed to decrease the
// sender's fee output to pay for additional input fees. Any fees in excess of
// `max_additional_fee_contribution` must be covered by the receiver.
let input_contribution_weight = self.additional_input_weight()?;
let input_contribution_weight = self.additional_input_weight();
let additional_fee = input_contribution_weight * min_fee_rate;
tracing::trace!("additional_fee: {additional_fee}");
let mut receiver_additional_fee = additional_fee;
Expand Down Expand Up @@ -444,8 +444,8 @@ impl WantsFeeRange {
}

/// Calculate the additional input weight contributed by the receiver.
fn additional_input_weight(&self) -> Result<Weight, InternalPayloadError> {
Ok(self.receiver_inputs.iter().map(|input_pair| input_pair.expected_weight).sum())
fn additional_input_weight(&self) -> Weight {
self.receiver_inputs.iter().map(|input_pair| input_pair.expected_weight).sum()
}

/// Calculate the additional output weight contributed by the receiver.
Expand Down Expand Up @@ -474,7 +474,7 @@ impl WantsFeeRange {
self,
min_fee_rate: Option<FeeRate>,
max_effective_fee_rate: Option<FeeRate>,
) -> Result<PsbtContext, ReplyableError> {
) -> Result<PsbtContext, InternalPayloadError> {
let payjoin_psbt =
self.calculate_psbt_with_fee_range(min_fee_rate, max_effective_fee_rate)?;
Ok(PsbtContext { original_psbt: self.original_psbt, payjoin_psbt })
Expand Down Expand Up @@ -729,10 +729,7 @@ mod tests {
}, None)
.unwrap()],
};
assert_eq!(
p2pkh_proposal.additional_input_weight().expect("should calculate input weight"),
Weight::from_wu(592)
);
assert_eq!(p2pkh_proposal.additional_input_weight(), Weight::from_wu(592));

// Input weight for a single nested P2WPKH (nested segwit) receiver input
let nested_p2wpkh_proposal = WantsFeeRange {
Expand All @@ -756,12 +753,7 @@ mod tests {
}, None)
.unwrap()],
};
assert_eq!(
nested_p2wpkh_proposal
.additional_input_weight()
.expect("should calculate input weight"),
Weight::from_wu(364)
);
assert_eq!(nested_p2wpkh_proposal.additional_input_weight(), Weight::from_wu(364));

// Input weight for a single P2WPKH (native segwit) receiver input
let p2wpkh_proposal = WantsFeeRange {
Expand All @@ -783,10 +775,7 @@ mod tests {
}, None)
.unwrap()],
};
assert_eq!(
p2wpkh_proposal.additional_input_weight().expect("should calculate input weight"),
Weight::from_wu(272)
);
assert_eq!(p2wpkh_proposal.additional_input_weight(), Weight::from_wu(272));

// Input weight for a single P2TR (taproot) receiver input
let p2tr_proposal = WantsFeeRange {
Expand All @@ -808,10 +797,7 @@ mod tests {
}, None)
.unwrap()],
};
assert_eq!(
p2tr_proposal.additional_input_weight().expect("should calculate input weight"),
Weight::from_wu(230)
);
assert_eq!(p2tr_proposal.additional_input_weight(), Weight::from_wu(230));
}

#[test]
Expand Down
Loading
Loading