Skip to content

Commit be89d64

Browse files
DanGouldDanGould
committed
Reduce JsonError types
Only some errors are actually recoverable and must return JSON. Fix #522.
1 parent 3225f27 commit be89d64

File tree

10 files changed

+226
-216
lines changed

10 files changed

+226
-216
lines changed

‎payjoin-cli/src/app/v1.rs

+35-23
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use hyper_util::rt::TokioIo;
1616
use payjoin::bitcoin::psbt::Psbt;
1717
use payjoin::bitcoin::{self, FeeRate};
1818
use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal};
19-
use payjoin::receive::Error;
19+
use payjoin::receive::ReplyableError;
2020
use payjoin::send::v1::SenderBuilder;
2121
use payjoin::{Uri, UriExt};
2222
use tokio::net::TcpListener;
@@ -225,7 +225,7 @@ impl App {
225225
.handle_payjoin_post(req)
226226
.await
227227
.map_err(|e| match e {
228-
Error::Validation(e) => {
228+
ReplyableError::V1(e) => {
229229
log::error!("Error handling request: {}", e);
230230
Response::builder().status(400).body(full(e.to_string())).unwrap()
231231
}
@@ -246,12 +246,12 @@ impl App {
246246
fn handle_get_bip21(
247247
&self,
248248
amount: Option<Amount>,
249-
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
249+
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
250250
let address = self
251251
.bitcoind()
252-
.map_err(|e| Error::Implementation(e.into()))?
252+
.map_err(|e| ReplyableError::Implementation(e.into()))?
253253
.get_new_address(None, None)
254-
.map_err(|e| Error::Implementation(e.into()))?
254+
.map_err(|e| ReplyableError::Implementation(e.into()))?
255255
.assume_checked();
256256
let uri_string = if let Some(amount) = amount {
257257
format!(
@@ -264,7 +264,7 @@ impl App {
264264
format!("{}?pj={}", address.to_qr_uri(), self.config.pj_endpoint)
265265
};
266266
let uri = Uri::try_from(uri_string.clone()).map_err(|_| {
267-
Error::Implementation(anyhow!("Could not parse payjoin URI string.").into())
267+
ReplyableError::Implementation(anyhow!("Could not parse payjoin URI string.").into())
268268
})?;
269269
let _ = uri.assume_checked(); // we just got it from bitcoind above
270270

@@ -274,12 +274,16 @@ impl App {
274274
async fn handle_payjoin_post(
275275
&self,
276276
req: Request<Incoming>,
277-
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, Error> {
277+
) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
278278
let (parts, body) = req.into_parts();
279279
let headers = Headers(&parts.headers);
280280
let query_string = parts.uri.query().unwrap_or("");
281-
let body =
282-
body.collect().await.map_err(|e| Error::Implementation(e.into()))?.aggregate().reader();
281+
let body = body
282+
.collect()
283+
.await
284+
.map_err(|e| ReplyableError::Implementation(e.into()))?
285+
.aggregate()
286+
.reader();
283287
let proposal = UncheckedProposal::from_request(body, query_string, headers)?;
284288

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

295-
fn process_v1_proposal(&self, proposal: UncheckedProposal) -> Result<PayjoinProposal, Error> {
296-
let bitcoind = self.bitcoind().map_err(|e| Error::Implementation(e.into()))?;
299+
fn process_v1_proposal(
300+
&self,
301+
proposal: UncheckedProposal,
302+
) -> Result<PayjoinProposal, ReplyableError> {
303+
let bitcoind = self.bitcoind().map_err(|e| ReplyableError::Implementation(e.into()))?;
297304

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

301308
// The network is used for checks later
302-
let network =
303-
bitcoind.get_blockchain_info().map_err(|e| Error::Implementation(e.into()))?.chain;
309+
let network = bitcoind
310+
.get_blockchain_info()
311+
.map_err(|e| ReplyableError::Implementation(e.into()))?
312+
.chain;
304313

305314
// Receive Check 1: Can Broadcast
306315
let proposal = proposal.check_broadcast_suitability(None, |tx| {
307316
let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx);
308317
let mempool_results = bitcoind
309318
.test_mempool_accept(&[raw_tx])
310-
.map_err(|e| Error::Implementation(e.into()))?;
319+
.map_err(|e| ReplyableError::Implementation(e.into()))?;
311320
match mempool_results.first() {
312321
Some(result) => Ok(result.allowed),
313-
None => Err(Error::Implementation(
322+
None => Err(ReplyableError::Implementation(
314323
anyhow!("No mempool results returned on broadcast check").into(),
315324
)),
316325
}
@@ -323,7 +332,7 @@ impl App {
323332
bitcoind
324333
.get_address_info(&address)
325334
.map(|info| info.is_mine.unwrap_or(false))
326-
.map_err(|e| Error::Implementation(e.into()))
335+
.map_err(|e| ReplyableError::Implementation(e.into()))
327336
} else {
328337
Ok(false)
329338
}
@@ -332,7 +341,9 @@ impl App {
332341

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

@@ -341,7 +352,7 @@ impl App {
341352
bitcoind
342353
.get_address_info(&address)
343354
.map(|info| info.is_mine.unwrap_or(false))
344-
.map_err(|e| Error::Implementation(e.into()))
355+
.map_err(|e| ReplyableError::Implementation(e.into()))
345356
} else {
346357
Ok(false)
347358
}
@@ -351,12 +362,12 @@ impl App {
351362
.substitute_receiver_script(
352363
&bitcoind
353364
.get_new_address(None, None)
354-
.map_err(|e| Error::Implementation(e.into()))?
365+
.map_err(|e| ReplyableError::Implementation(e.into()))?
355366
.require_network(network)
356-
.map_err(|e| Error::Implementation(e.into()))?
367+
.map_err(|e| ReplyableError::Implementation(e.into()))?
357368
.script_pubkey(),
358369
)
359-
.map_err(|e| Error::Implementation(e.into()))?
370+
.map_err(|e| ReplyableError::Implementation(e.into()))?
360371
.commit_outputs();
361372

362373
let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind)
@@ -370,9 +381,10 @@ impl App {
370381
bitcoind
371382
.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
372383
.map(|res| {
373-
Psbt::from_str(&res.psbt).map_err(|e| Error::Implementation(e.into()))
384+
Psbt::from_str(&res.psbt)
385+
.map_err(|e| ReplyableError::Implementation(e.into()))
374386
})
375-
.map_err(|e| Error::Implementation(e.into()))?
387+
.map_err(|e| ReplyableError::Implementation(e.into()))?
376388
},
377389
None,
378390
self.config.max_fee_rate,

‎payjoin-cli/src/app/v2.rs

+26-27
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use payjoin::bitcoin::psbt::Psbt;
88
use payjoin::bitcoin::{Amount, FeeRate};
99
use payjoin::receive::v2::{Receiver, UncheckedProposal};
1010
use payjoin::receive::Error;
11+
use payjoin::receive::ReplyableError::Implementation;
1112
use payjoin::send::v2::{Sender, SenderBuilder};
1213
use payjoin::{bitcoin, Uri};
1314
use tokio::signal;
@@ -252,23 +253,21 @@ impl App {
252253
&self,
253254
proposal: payjoin::receive::v2::UncheckedProposal,
254255
) -> Result<payjoin::receive::v2::PayjoinProposal, Error> {
255-
let bitcoind = self.bitcoind().map_err(|e| Error::Implementation(e.into()))?;
256+
let bitcoind = self.bitcoind().map_err(|e| Implementation(e.into()))?;
256257

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

260261
// The network is used for checks later
261-
let network =
262-
bitcoind.get_blockchain_info().map_err(|e| Error::Implementation(e.into()))?.chain;
262+
let network = bitcoind.get_blockchain_info().map_err(|e| Implementation(e.into()))?.chain;
263263
// Receive Check 1: Can Broadcast
264264
let proposal = proposal.check_broadcast_suitability(None, |tx| {
265265
let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx);
266-
let mempool_results = bitcoind
267-
.test_mempool_accept(&[raw_tx])
268-
.map_err(|e| Error::Implementation(e.into()))?;
266+
let mempool_results =
267+
bitcoind.test_mempool_accept(&[raw_tx]).map_err(|e| Implementation(e.into()))?;
269268
match mempool_results.first() {
270269
Some(result) => Ok(result.allowed),
271-
None => Err(Error::Implementation(
270+
None => Err(Implementation(
272271
anyhow!("No mempool results returned on broadcast check").into(),
273272
)),
274273
}
@@ -281,7 +280,7 @@ impl App {
281280
bitcoind
282281
.get_address_info(&address)
283282
.map(|info| info.is_mine.unwrap_or(false))
284-
.map_err(|e| Error::Implementation(e.into()))
283+
.map_err(|e| Implementation(e.into()))
285284
} else {
286285
Ok(false)
287286
}
@@ -290,7 +289,7 @@ impl App {
290289

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

@@ -300,7 +299,7 @@ impl App {
300299
bitcoind
301300
.get_address_info(&address)
302301
.map(|info| info.is_mine.unwrap_or(false))
303-
.map_err(|e| Error::Implementation(e.into()))
302+
.map_err(|e| Implementation(e.into()))
304303
} else {
305304
Ok(false)
306305
}
@@ -317,10 +316,8 @@ impl App {
317316
|psbt: &Psbt| {
318317
bitcoind
319318
.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
320-
.map(|res| {
321-
Psbt::from_str(&res.psbt).map_err(|e| Error::Implementation(e.into()))
322-
})
323-
.map_err(|e| Error::Implementation(e.into()))?
319+
.map(|res| Psbt::from_str(&res.psbt).map_err(|e| Implementation(e.into())))
320+
.map_err(|e| Implementation(e.into()))?
324321
},
325322
None,
326323
self.config.max_fee_rate,
@@ -337,25 +334,27 @@ async fn handle_request_error(
337334
mut receiver: UncheckedProposal,
338335
ohttp_relay: &payjoin::Url,
339336
) -> anyhow::Error {
340-
let (err_req, err_ctx) = match receiver.extract_err_req(&e, ohttp_relay) {
337+
if let Some((err_req, err_ctx)) = match receiver.extract_err_req(&e, ohttp_relay) {
341338
Ok(req_ctx) => req_ctx,
342339
Err(e) => return anyhow!("Failed to extract error request: {}", e),
343-
};
340+
} {
341+
let err_response = match post_request(err_req).await {
342+
Ok(response) => response,
343+
Err(e) => return anyhow!("Failed to post error request: {}", e),
344+
};
344345

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

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

355-
if let Err(e) = receiver.process_err_res(&err_bytes, err_ctx) {
356-
return anyhow!("Failed to process error response: {}", e);
355+
return e.into();
357356
}
358-
357+
log::error!("Failed to extract error request: {}", e);
359358
e.into()
360359
}
361360

0 commit comments

Comments
 (0)