Skip to content

Commit

Permalink
Reduce JsonError types
Browse files Browse the repository at this point in the history 
Only some errors are actually recoverable and must return JSON. Fix #522.
  • Loading branch information
@DanGould
DanGould committed Feb 4, 2025
1 parent 3225f27 commit 553a644
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 200 deletions.
58 changes: 35 additions & 23 deletions payjoin-cli/src/app/v1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use hyper_util::rt::TokioIo;
use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::{self, FeeRate};
use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal};
use payjoin::receive::Error;
use payjoin::receive::ReplyableError;
use payjoin::send::v1::SenderBuilder;
use payjoin::{Uri, UriExt};
use tokio::net::TcpListener;
Expand Down Expand Up @@ -225,7 +225,7 @@ impl App {
.handle_payjoin_post(req)
.await
.map_err(|e| match e {
Error::Validation(e) => {
ReplyableError::V1(e) => {
log::error!("Error handling request: {}", e);
Response::builder().status(400).body(full(e.to_string())).unwrap()
}
Expand All @@ -246,12 +246,12 @@ impl App {
fn handle_get_bip21(
&self,
amount: Option<Amount>,
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
let address = self
.bitcoind()
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
.get_new_address(None, None)
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
.assume_checked();
let uri_string = if let Some(amount) = amount {
format!(
Expand All @@ -264,7 +264,7 @@ impl App {
format!("{}?pj={}", address.to_qr_uri(), self.config.pj_endpoint)
};
let uri = Uri::try_from(uri_string.clone()).map_err(|_| {
Error::Implementation(anyhow!("Could not parse payjoin URI string.").into())
ReplyableError::Implementation(anyhow!("Could not parse payjoin URI string.").into())
})?;
let _ = uri.assume_checked(); // we just got it from bitcoind above

Expand All @@ -274,12 +274,16 @@ impl App {
async fn handle_payjoin_post(
&self,
req: Request<Incoming>,
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
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| Error::Implementation(e.into()))?.aggregate().reader();
let body = body
.collect()
.await
.map_err(|e| ReplyableError::Implementation(e.into()))?
.aggregate()
.reader();
let proposal = UncheckedProposal::from_request(body, query_string, headers)?;

let payjoin_proposal = self.process_v1_proposal(proposal)?;
Expand All @@ -292,25 +296,30 @@ impl App {
Ok(Response::new(full(body)))
}

fn process_v1_proposal(&self, proposal: UncheckedProposal) -> Result<PayjoinProposal, Error> {
let bitcoind = self.bitcoind().map_err(|e| Error::Implementation(e.into()))?;
fn process_v1_proposal(
&self,
proposal: UncheckedProposal,
) -> Result<PayjoinProposal, ReplyableError> {
let bitcoind = self.bitcoind().map_err(|e| ReplyableError::Implementation(e.into()))?;

// in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx
let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast();

// The network is used for checks later
let network =
bitcoind.get_blockchain_info().map_err(|e| Error::Implementation(e.into()))?.chain;
let network = bitcoind
.get_blockchain_info()
.map_err(|e| ReplyableError::Implementation(e.into()))?
.chain;

// Receive Check 1: Can Broadcast
let proposal = proposal.check_broadcast_suitability(None, |tx| {
let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx);
let mempool_results = bitcoind
.test_mempool_accept(&[raw_tx])
.map_err(|e| Error::Implementation(e.into()))?;
.map_err(|e| ReplyableError::Implementation(e.into()))?;
match mempool_results.first() {
Some(result) => Ok(result.allowed),
None => Err(Error::Implementation(
None => Err(ReplyableError::Implementation(
anyhow!("No mempool results returned on broadcast check").into(),
)),
}
Expand All @@ -323,7 +332,7 @@ impl App {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Implementation(e.into()))
.map_err(|e| ReplyableError::Implementation(e.into()))
} else {
Ok(false)
}
Expand All @@ -332,7 +341,9 @@ impl App {

// Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
let payjoin = proposal.check_no_inputs_seen_before(|input| {
self.db.insert_input_seen_before(*input).map_err(|e| Error::Implementation(e.into()))
self.db
.insert_input_seen_before(*input)
.map_err(|e| ReplyableError::Implementation(e.into()))
})?;
log::trace!("check3");

Expand All @@ -341,7 +352,7 @@ impl App {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Implementation(e.into()))
.map_err(|e| ReplyableError::Implementation(e.into()))
} else {
Ok(false)
}
Expand All @@ -351,12 +362,12 @@ impl App {
.substitute_receiver_script(
&bitcoind
.get_new_address(None, None)
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
.require_network(network)
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
.script_pubkey(),
)
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
.commit_outputs();

let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind)
Expand All @@ -370,9 +381,10 @@ impl App {
bitcoind
.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
.map(|res| {
Psbt::from_str(&res.psbt).map_err(|e| Error::Implementation(e.into()))
Psbt::from_str(&res.psbt)
.map_err(|e| ReplyableError::Implementation(e.into()))
})
.map_err(|e| Error::Implementation(e.into()))?
.map_err(|e| ReplyableError::Implementation(e.into()))?
},
None,
self.config.max_fee_rate,
Expand Down
53 changes: 26 additions & 27 deletions payjoin-cli/src/app/v2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::{Amount, FeeRate};
use payjoin::receive::v2::{Receiver, UncheckedProposal};
use payjoin::receive::Error;
use payjoin::receive::ReplyableError::Implementation;
use payjoin::send::v2::{Sender, SenderBuilder};
use payjoin::{bitcoin, Uri};
use tokio::signal;
Expand Down Expand Up @@ -252,23 +253,21 @@ impl App {
&self,
proposal: payjoin::receive::v2::UncheckedProposal,
) -> Result<payjoin::receive::v2::PayjoinProposal, Error> {
let bitcoind = self.bitcoind().map_err(|e| Error::Implementation(e.into()))?;
let bitcoind = self.bitcoind().map_err(|e| Implementation(e.into()))?;

// in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx
let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast();

// The network is used for checks later
let network =
bitcoind.get_blockchain_info().map_err(|e| Error::Implementation(e.into()))?.chain;
let network = bitcoind.get_blockchain_info().map_err(|e| Implementation(e.into()))?.chain;
// Receive Check 1: Can Broadcast
let proposal = proposal.check_broadcast_suitability(None, |tx| {
let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx);
let mempool_results = bitcoind
.test_mempool_accept(&[raw_tx])
.map_err(|e| Error::Implementation(e.into()))?;
let mempool_results =
bitcoind.test_mempool_accept(&[raw_tx]).map_err(|e| Implementation(e.into()))?;
match mempool_results.first() {
Some(result) => Ok(result.allowed),
None => Err(Error::Implementation(
None => Err(Implementation(
anyhow!("No mempool results returned on broadcast check").into(),
)),
}
Expand All @@ -281,7 +280,7 @@ impl App {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Implementation(e.into()))
.map_err(|e| Implementation(e.into()))
} else {
Ok(false)
}
Expand All @@ -290,7 +289,7 @@ impl App {

// Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
let payjoin = proposal.check_no_inputs_seen_before(|input| {
self.db.insert_input_seen_before(*input).map_err(|e| Error::Implementation(e.into()))
self.db.insert_input_seen_before(*input).map_err(|e| Implementation(e.into()))
})?;
log::trace!("check3");

Expand All @@ -300,7 +299,7 @@ impl App {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Implementation(e.into()))
.map_err(|e| Implementation(e.into()))
} else {
Ok(false)
}
Expand All @@ -317,10 +316,8 @@ impl App {
|psbt: &Psbt| {
bitcoind
.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
.map(|res| {
Psbt::from_str(&res.psbt).map_err(|e| Error::Implementation(e.into()))
})
.map_err(|e| Error::Implementation(e.into()))?
.map(|res| Psbt::from_str(&res.psbt).map_err(|e| Implementation(e.into())))
.map_err(|e| Implementation(e.into()))?
},
None,
self.config.max_fee_rate,
Expand All @@ -337,25 +334,27 @@ async fn handle_request_error(
mut receiver: UncheckedProposal,
ohttp_relay: &payjoin::Url,
) -> anyhow::Error {
let (err_req, err_ctx) = match receiver.extract_err_req(&e, ohttp_relay) {
if let Some((err_req, err_ctx)) = match receiver.extract_err_req(&e, ohttp_relay) {
Ok(req_ctx) => req_ctx,
Err(e) => return anyhow!("Failed to extract error request: {}", e),
};
} {
let err_response = match post_request(err_req).await {
Ok(response) => response,
Err(e) => return anyhow!("Failed to post error request: {}", e),
};

let err_response = match post_request(err_req).await {
Ok(response) => response,
Err(e) => return anyhow!("Failed to post error request: {}", e),
};
let err_bytes = match err_response.bytes().await {
Ok(bytes) => bytes,
Err(e) => return anyhow!("Failed to get error response bytes: {}", e),
};

let err_bytes = match err_response.bytes().await {
Ok(bytes) => bytes,
Err(e) => return anyhow!("Failed to get error response bytes: {}", e),
};
if let Err(e) = receiver.process_err_res(&err_bytes, err_ctx) {
return anyhow!("Failed to process error response: {}", e);
}

if let Err(e) = receiver.process_err_res(&err_bytes, err_ctx) {
return anyhow!("Failed to process error response: {}", e);
return e.into();
}

log::error!("Failed to extract error request: {}", e);
e.into()
}

Expand Down
Loading

0 comments on commit 553a644

Please sign in to comment.