Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions payjoin-cli/src/app/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ impl AppTrait for App {
.send()
.await
.with_context(|| "HTTP request failed")?;
let response_str = response.text().await.with_context(|| "Failed to read response")?;

let fallback_tx = Psbt::from_str(&body)
.map_err(|e| anyhow!("Failed to load PSBT from base64: {}", e))?
.extract_tx()?;
Expand All @@ -97,12 +99,10 @@ impl AppTrait for App {
"Sent fallback transaction hex: {:#}",
payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx)
);
let psbt = ctx.process_response(&mut response.bytes().await?.to_vec().as_slice()).map_err(
|e| {
log::debug!("Error processing response: {:?}", e);
anyhow!("Failed to process response {}", e)
},
)?;
let psbt = ctx.process_response(&response_str).map_err(|e| {
log::debug!("Error processing response: {:?}", e);
anyhow!("Failed to process response {}", e)
})?;

self.process_pj_response(psbt)?;
Ok(())
Expand Down
22 changes: 8 additions & 14 deletions payjoin/src/hpke.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::ops::Deref;
use std::{error, fmt};

use bitcoin::key::constants::{ELLSWIFT_ENCODING_SIZE, PUBLIC_KEY_SIZE};
use bitcoin::key::constants::{
ELLSWIFT_ENCODING_SIZE, PUBLIC_KEY_SIZE, UNCOMPRESSED_PUBLIC_KEY_SIZE,
};
use bitcoin::secp256k1;
use bitcoin::secp256k1::ellswift::ElligatorSwift;
use hpke::aead::ChaCha20Poly1305;
Expand Down Expand Up @@ -170,7 +172,7 @@ impl<'de> serde::Deserialize<'de> for HpkePublicKey {

/// Message A is sent from the sender to the receiver containing an Original PSBT payload
pub fn encrypt_message_a(
body: Vec<u8>,
body: &[u8],
reply_pk: &HpkePublicKey,
receiver_pk: &HpkePublicKey,
) -> Result<Vec<u8>, HpkeError> {
Expand All @@ -193,7 +195,7 @@ pub fn encrypt_message_a(

pub fn decrypt_message_a(
message_a: &[u8],
receiver_sk: HpkeSecretKey,
receiver_sk: &HpkeSecretKey,
) -> Result<(Vec<u8>, HpkePublicKey), HpkeError> {
use std::io::{Cursor, Read};

Expand Down Expand Up @@ -222,7 +224,7 @@ pub fn decrypt_message_a(

/// Message B is sent from the receiver to the sender containing a Payjoin PSBT payload or an error
pub fn encrypt_message_b(
mut plaintext: Vec<u8>,
mut plaintext: &[u8],
receiver_keypair: &HpkeKeyPair,
sender_pk: &HpkePublicKey,
) -> Result<Vec<u8>, HpkeError> {
Expand All @@ -245,8 +247,8 @@ pub fn encrypt_message_b(

pub fn decrypt_message_b(
message_b: &[u8],
receiver_pk: HpkePublicKey,
sender_sk: HpkeSecretKey,
receiver_pk: &HpkePublicKey,
sender_sk: &HpkeSecretKey,
) -> Result<Vec<u8>, HpkeError> {
let enc = message_b.get(..ELLSWIFT_ENCODING_SIZE).ok_or(HpkeError::PayloadTooShort)?;
let enc = encapped_key_from_ellswift_bytes(enc)?;
Expand All @@ -260,14 +262,6 @@ pub fn decrypt_message_b(
Ok(plaintext)
}

fn pad_plaintext(msg: &mut Vec<u8>, padded_length: usize) -> Result<&[u8], HpkeError> {
if msg.len() > padded_length {
return Err(HpkeError::PayloadTooLarge { actual: msg.len(), max: padded_length });
}
msg.resize(padded_length, 0);
Ok(msg)
}

/// Error from de/encrypting a v2 Hybrid Public Key Encryption payload.
#[derive(Debug, PartialEq)]
pub enum HpkeError {
Expand Down
11 changes: 6 additions & 5 deletions payjoin/src/ohttp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const OHTTP_REQ_HEADER_BYTES: usize = 7;
pub const PADDED_BHTTP_REQ_BYTES: usize =
ENCAPSULATED_MESSAGE_BYTES - (N_ENC + N_T + OHTTP_REQ_HEADER_BYTES);

pub fn ohttp_encapsulate(
pub(crate) fn ohttp_encapsulate(
ohttp_keys: &mut ohttp::KeyConfig,
method: &str,
target_resource: &str,
Expand Down Expand Up @@ -50,7 +50,7 @@ pub fn ohttp_encapsulate(
}

/// decapsulate ohttp, bhttp response and return http response body and status code
pub fn ohttp_decapsulate(
pub(crate) fn ohttp_decapsulate(
res_ctx: ohttp::ClientResponse,
ohttp_body: &[u8; ENCAPSULATED_MESSAGE_BYTES],
) -> Result<http::Response<Vec<u8>>, OhttpEncapsulationError> {
Expand All @@ -69,7 +69,7 @@ pub fn ohttp_decapsulate(

/// Error from de/encapsulating an Oblivious HTTP request or response.
#[derive(Debug)]
pub enum OhttpEncapsulationError {
pub(crate) enum OhttpEncapsulationError {
Http(http::Error),
Ohttp(ohttp::Error),
Bhttp(bhttp::Error),
Expand Down Expand Up @@ -249,8 +249,9 @@ impl std::fmt::Display for ParseOhttpKeysError {
ParseOhttpKeysError::InvalidFormat => write!(f, "Invalid format"),
ParseOhttpKeysError::InvalidPublicKey => write!(f, "Invalid public key"),
ParseOhttpKeysError::DecodeBech32(e) => write!(f, "Failed to decode base64: {}", e),
ParseOhttpKeysError::DecodeKeyConfig(e) =>
write!(f, "Failed to decode KeyConfig: {}", e),
ParseOhttpKeysError::DecodeKeyConfig(e) => {
write!(f, "Failed to decode KeyConfig: {}", e)
}
}
}
}
Expand Down
16 changes: 5 additions & 11 deletions payjoin/src/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub(crate) trait PsbtExt: Sized {
// guarantees that length of psbt input matches that of unsigned_tx inputs and same
/// thing for outputs.
fn validate(self) -> Result<Self, InconsistentPsbt>;
fn validate_input_utxos(&self, treat_missing_as_error: bool) -> Result<(), PsbtInputsError>;
fn validate_input_utxos(&self) -> Result<(), PsbtInputsError>;
}

impl PsbtExt for Psbt {
Expand Down Expand Up @@ -83,11 +83,9 @@ impl PsbtExt for Psbt {
}
}

fn validate_input_utxos(&self, treat_missing_as_error: bool) -> Result<(), PsbtInputsError> {
fn validate_input_utxos(&self) -> Result<(), PsbtInputsError> {
self.input_pairs().enumerate().try_for_each(|(index, input)| {
input
.validate_utxo(treat_missing_as_error)
.map_err(|error| PsbtInputsError { index, error })
input.validate_utxo().map_err(|error| PsbtInputsError { index, error })
})
}
}
Expand Down Expand Up @@ -124,14 +122,10 @@ impl InternalInputPair<'_> {
}
}

pub fn validate_utxo(
&self,
treat_missing_as_error: bool,
) -> Result<(), InternalPsbtInputError> {
pub fn validate_utxo(&self) -> Result<(), InternalPsbtInputError> {
match (&self.psbtin.non_witness_utxo, &self.psbtin.witness_utxo) {
(None, None) if treat_missing_as_error =>
(None, None) =>
Err(InternalPsbtInputError::PrevTxOut(PrevTxOutError::MissingUtxoInformation)),
(None, None) => Ok(()),
(Some(tx), None) if tx.compute_txid() == self.txin.previous_output.txid => tx
.output
.get::<usize>(self.txin.previous_output.vout.try_into().map_err(|_| {
Expand Down
2 changes: 1 addition & 1 deletion payjoin/src/receive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl InputPair {
pub fn new(txin: TxIn, psbtin: psbt::Input) -> Result<Self, PsbtInputError> {
let input_pair = Self { txin, psbtin };
let raw = InternalInputPair::from(&input_pair);
raw.validate_utxo(true)?;
raw.validate_utxo()?;
let address_type = raw.address_type().map_err(InternalPsbtInputError::AddressType)?;
if address_type == AddressType::P2sh && input_pair.psbtin.redeem_script.is_none() {
return Err(InternalPsbtInputError::NoRedeemScript.into());
Expand Down
8 changes: 4 additions & 4 deletions payjoin/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ pub struct Request {

impl Request {
/// Construct a new v1 request.
pub(crate) fn new_v1(url: Url, body: Vec<u8>) -> Self {
Self { url, content_type: V1_REQ_CONTENT_TYPE, body }
pub(crate) fn new_v1(url: &Url, body: &[u8]) -> Self {
Self { url: url.clone(), content_type: V1_REQ_CONTENT_TYPE, body: body.to_vec() }
}

/// Construct a new v2 request.
#[cfg(feature = "v2")]
pub(crate) fn new_v2(url: Url, body: [u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES]) -> Self {
Self { url, content_type: V2_REQ_CONTENT_TYPE, body: body.to_vec() }
pub(crate) fn new_v2(url: &Url, body: [u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES]) -> Self {
Self { url: url.clone(), content_type: V2_REQ_CONTENT_TYPE, body: body.to_vec() }
}
}
3 changes: 0 additions & 3 deletions payjoin/src/send/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ pub struct ValidationError(InternalValidationError);
#[derive(Debug)]
pub(crate) enum InternalValidationError {
Parse,
Io(std::io::Error),
Proposal(InternalProposalError),
#[cfg(feature = "v2")]
V2Encapsulation(crate::send::v2::EncapsulationError),
Expand All @@ -115,7 +114,6 @@ impl fmt::Display for ValidationError {

match &self.0 {
Parse => write!(f, "couldn't decode as PSBT or JSON",),
Io(e) => write!(f, "couldn't read PSBT: {}", e),
Proposal(e) => write!(f, "proposal PSBT error: {}", e),
#[cfg(feature = "v2")]
V2Encapsulation(e) => write!(f, "v2 encapsulation error: {}", e),
Expand All @@ -129,7 +127,6 @@ impl std::error::Error for ValidationError {

match &self.0 {
Parse => None,
Io(error) => Some(error),
Proposal(e) => Some(e),
#[cfg(feature = "v2")]
V2Encapsulation(e) => Some(e),
Expand Down
6 changes: 3 additions & 3 deletions payjoin/src/send/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::str::FromStr;
use bitcoin::psbt::Psbt;
use bitcoin::{Amount, FeeRate, Script, ScriptBuf, TxOut, Weight};
pub use error::{BuildSenderError, ResponseError, ValidationError};
pub(crate) use error::{InternalBuildSenderError, InternalProposalError, InternalValidationError};
pub(crate) use error::{InternalBuildSenderError, InternalProposalError};
Comment thread
benalleng marked this conversation as resolved.
use url::Url;

use crate::psbt::PsbtExt;
Expand Down Expand Up @@ -400,13 +400,13 @@ fn determine_fee_contribution(
}

fn serialize_url(
endpoint: Url,
endpoint: &Url,
disable_output_substitution: bool,
fee_contribution: Option<(bitcoin::Amount, usize)>,
min_fee_rate: FeeRate,
version: &str,
) -> Result<Url, url::ParseError> {
let mut url = endpoint;
let mut url = endpoint.clone();
url.query_pairs_mut().append_pair("v", version);
if disable_output_substitution {
url.query_pairs_mut().append_pair("disableoutputsubstitution", "true");
Expand Down
19 changes: 7 additions & 12 deletions payjoin/src/send/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl<'a> SenderBuilder<'a> {
fn build(self) -> Result<Sender, BuildSenderError> {
let mut psbt =
self.psbt.validate().map_err(InternalBuildSenderError::InconsistentOriginalPsbt)?;
psbt.validate_input_utxos(true).map_err(InternalBuildSenderError::InvalidOriginalInput)?;
psbt.validate_input_utxos().map_err(InternalBuildSenderError::InvalidOriginalInput)?;
let endpoint = self.uri.extras.endpoint.clone();
let disable_output_substitution =
self.uri.extras.disable_output_substitution || self.disable_output_substitution;
Expand Down Expand Up @@ -229,15 +229,15 @@ impl Sender {
/// Extract serialized V1 Request and Context from a Payjoin Proposal
pub fn extract_v1(&self) -> Result<(Request, V1Context), url::ParseError> {
let url = serialize_url(
self.endpoint.clone(),
&self.endpoint,
self.disable_output_substitution,
self.fee_contribution,
self.min_fee_rate,
"1", // payjoin version
)?;
let body = self.psbt.to_string().as_bytes().to_vec();
Ok((
Request::new_v1(url, body),
Request::new_v1(&url, &body),
V1Context {
psbt_context: PsbtContext {
original_psbt: self.psbt.clone(),
Expand Down Expand Up @@ -269,13 +269,8 @@ impl V1Context {
/// Call this method with response from receiver to continue BIP78 flow. If the response is
/// valid you will get appropriate PSBT that you should sign and broadcast.
#[inline]
pub fn process_response(
self,
response: &mut impl std::io::Read,
) -> Result<Psbt, ResponseError> {
let mut res_str = String::new();
response.read_to_string(&mut res_str).map_err(InternalValidationError::Io)?;
let proposal = Psbt::from_str(&res_str).map_err(|_| ResponseError::parse(&res_str))?;
pub fn process_response(self, response: &str) -> Result<Psbt, ResponseError> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The note in #405 says response could be a &[u8], but I agree using &str seems more straightforward.

@DanGould is there a reason we should use bytes instead of a string?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems @nothingmuch addressed this same question in his review and I missed it when writing this.

let proposal = Psbt::from_str(response).map_err(|_| ResponseError::parse(response))?;
self.psbt_context.process_proposal(proposal).map_err(Into::into)
}
}
Expand All @@ -296,7 +291,7 @@ mod test {
"message": "This version of payjoin is not supported."
})
.to_string();
match ctx.process_response(&mut known_json_error.as_bytes()) {
match ctx.process_response(&known_json_error) {
Err(ResponseError::WellKnown(WellKnownError::VersionUnsupported { .. })) => (),
_ => panic!("Expected WellKnownError"),
}
Expand All @@ -307,7 +302,7 @@ mod test {
"message": "This version of payjoin is not supported."
})
.to_string();
match ctx.process_response(&mut invalid_json_error.as_bytes()) {
match ctx.process_response(&invalid_json_error) {
Err(ResponseError::Validation(_)) => (),
_ => panic!("Expected unrecognized JSON error"),
}
Expand Down
6 changes: 3 additions & 3 deletions payjoin/src/send/v2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl Sender {
/// and has no fallback to v1.
pub fn extract_v2(
&self,
ohttp_relay: Url,
ohttp_relay: &Url,
) -> Result<(Request, V2PostContext), CreateRequestError> {
use crate::hpke::encrypt_message_a;
use crate::ohttp::ohttp_encapsulate;
Expand Down Expand Up @@ -255,7 +255,7 @@ pub struct V2GetContext {
impl V2GetContext {
pub fn extract_req(
&self,
ohttp_relay: Url,
ohttp_relay: &Url,
) -> Result<(Request, ohttp::ClientResponse), CreateRequestError> {
use crate::uri::UrlExt;
let base_url = self.endpoint.clone();
Expand Down Expand Up @@ -317,7 +317,7 @@ struct HpkeContext {

#[cfg(feature = "v2")]
impl HpkeContext {
pub fn new(receiver: HpkePublicKey, reply_key: &HpkeSecretKey) -> Self {
fn new(receiver: HpkePublicKey, reply_key: &HpkeSecretKey) -> Self {
Self { receiver, reply_pair: HpkeKeyPair::from_secret_key(reply_key) }
}
}
Expand Down
6 changes: 3 additions & 3 deletions payjoin/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ mod integration {
// **********************
// Inside the Sender:
// Sender checks, signs, finalizes, extracts, and broadcasts
let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?;
let checked_payjoin_proposal_psbt = ctx.process_response(&response)?;
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;

Expand Down Expand Up @@ -1045,7 +1045,7 @@ mod integration {
// **********************
// Inside the Sender:
// Sender checks, signs, finalizes, extracts, and broadcasts
let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?;
let checked_payjoin_proposal_psbt = ctx.process_response(&response)?;
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;

Expand Down Expand Up @@ -1130,7 +1130,7 @@ mod integration {
// **********************
// Inside the Sender:
// Sender checks, signs, finalizes, extracts, and broadcasts
let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?;
let checked_payjoin_proposal_psbt = ctx.process_response(&response)?;
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;

Expand Down