Skip to content
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.

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

Tx cut-through #332

Merged
merged 9 commits into from
Sep 12, 2024
11 changes: 10 additions & 1 deletion payjoin-cli/src/app/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub struct AppConfig {
pub bitcoind_rpcuser: String,
pub bitcoind_rpcpassword: String,
pub db_path: PathBuf,
// receive-only
pub max_fee_rate: Option<u64>,

// v2 only
#[cfg(feature = "v2")]
Expand Down Expand Up @@ -113,7 +115,14 @@ impl AppConfig {
)?
};

builder
let max_fee_rate = matches
.get_one::<String>("max_fee_rate")
.map(|max_fee_rate| max_fee_rate.parse::<u64>())
.transpose()
.map_err(|_| {
ConfigError::Message("\"max_fee_rate\" must be a valid amount".to_string())
})?;
builder.set_override_option("max_fee_rate", max_fee_rate)?
}
#[cfg(feature = "v2")]
Some(("resume", _)) => builder,
Expand Down
32 changes: 0 additions & 32 deletions payjoin-cli/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,38 +94,6 @@ pub trait App {
}
}

fn try_contributing_inputs(
payjoin: &mut payjoin::receive::ProvisionalProposal,
bitcoind: &bitcoincore_rpc::Client,
) -> Result<()> {
spacebear21 marked this conversation as resolved.
Show resolved Hide resolved
use bitcoin::OutPoint;

let available_inputs = bitcoind
.list_unspent(None, None, None, None, None)
.context("Failed to list unspent from bitcoind")?;
let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
.iter()
.map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
.collect();

let selected_outpoint = payjoin.try_preserving_privacy(candidate_inputs).expect("gg");
let selected_utxo = available_inputs
.iter()
.find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
.context("This shouldn't happen. Failed to retrieve the privacy preserving utxo from those we provided to the seclector.")?;
log::debug!("selected utxo: {:#?}", selected_utxo);

// calculate receiver payjoin outputs given receiver payjoin inputs and original_psbt,
let txo_to_contribute = bitcoin::TxOut {
value: selected_utxo.amount,
script_pubkey: selected_utxo.script_pub_key.clone(),
};
let outpoint_to_contribute =
bitcoin::OutPoint { txid: selected_utxo.txid, vout: selected_utxo.vout };
payjoin.contribute_witness_input(txo_to_contribute, outpoint_to_contribute);
Ok(())
}

#[cfg(feature = "danger-local-https")]
fn http_agent() -> Result<reqwest::Client> { Ok(http_agent_builder()?.build()?) }

Expand Down
62 changes: 50 additions & 12 deletions payjoin-cli/src/app/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ use hyper::service::service_fn;
use hyper::{Method, Request, Response, StatusCode};
use hyper_util::rt::TokioIo;
use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::{self};
use payjoin::bitcoin::{self, FeeRate};
use payjoin::receive::{PayjoinProposal, UncheckedProposal};
use payjoin::{Error, PjUriBuilder, Uri, UriExt};
use tokio::net::TcpListener;

use super::config::AppConfig;
use super::App as AppTrait;
use crate::app::{http_agent, try_contributing_inputs};
use crate::app::http_agent;
use crate::db::Database;
#[cfg(feature = "danger-local-https")]
pub const LOCAL_CERT_FILE: &str = "localhost.der";
Expand Down Expand Up @@ -329,7 +329,7 @@ impl App {
})?;
log::trace!("check4");

let mut provisional_payjoin = payjoin.identify_receiver_outputs(|output_script| {
let payjoin = payjoin.identify_receiver_outputs(|output_script| {
if let Ok(address) = bitcoin::Address::from_script(output_script, network) {
bitcoind
.get_address_info(&address)
Expand All @@ -340,19 +340,23 @@ impl App {
}
})?;

_ = try_contributing_inputs(&mut provisional_payjoin, &bitcoind)
.map_err(|e| log::warn!("Failed to contribute inputs: {}", e));

_ = provisional_payjoin
.try_substitute_receiver_output(|| {
Ok(bitcoind
let payjoin = payjoin
.substitute_receiver_script(
&bitcoind
.get_new_address(None, None)
.map_err(|e| Error::Server(e.into()))?
.require_network(network)
.map_err(|e| Error::Server(e.into()))?
.script_pubkey())
})
.map_err(|e| log::warn!("Failed to substitute output: {}", e));
.script_pubkey(),
)
.map_err(|e| Error::Server(e.into()))?
.commit_outputs();

let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind)
.unwrap_or_else(|e| {
log::warn!("Failed to contribute inputs: {}", e);
payjoin.commit_inputs()
});

let payjoin_proposal = provisional_payjoin.finalize_proposal(
|psbt: &Psbt| {
Expand All @@ -362,11 +366,45 @@ impl App {
.map_err(|e| Error::Server(e.into()))?
},
Some(bitcoin::FeeRate::MIN),
self.config.max_fee_rate.map_or(Ok(FeeRate::ZERO), |fee_rate| {
FeeRate::from_sat_per_vb(fee_rate).ok_or(Error::Server("Invalid fee rate".into()))
})?,
)?;
Ok(payjoin_proposal)
}
}

fn try_contributing_inputs(
payjoin: payjoin::receive::WantsInputs,
bitcoind: &bitcoincore_rpc::Client,
) -> Result<payjoin::receive::ProvisionalProposal> {
use bitcoin::OutPoint;

let available_inputs = bitcoind
.list_unspent(None, None, None, None, None)
.context("Failed to list unspent from bitcoind")?;
let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
.iter()
.map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
.collect();

let selected_outpoint = payjoin.try_preserving_privacy(candidate_inputs).expect("gg");
let selected_utxo = available_inputs
.iter()
.find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
.context("This shouldn't happen. Failed to retrieve the privacy preserving utxo from those we provided to the seclector.")?;
log::debug!("selected utxo: {:#?}", selected_utxo);
let txo_to_contribute = bitcoin::TxOut {
value: selected_utxo.amount,
script_pubkey: selected_utxo.script_pub_key.clone(),
};

Ok(payjoin
.contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)])
.expect("This shouldn't happen. Failed to contribute inputs.")
.commit_inputs())
}

fn full<T: Into<Bytes>>(chunk: T) -> BoxBody<Bytes, hyper::Error> {
Full::new(chunk.into()).map_err(|never| match never {}).boxed()
}
68 changes: 53 additions & 15 deletions payjoin-cli/src/app/v2.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::HashMap;
use std::str::FromStr;
use std::sync::Arc;

use anyhow::{anyhow, Context, Result};
use bitcoincore_rpc::RpcApi;
use payjoin::bitcoin::consensus::encode::serialize_hex;
use payjoin::bitcoin::psbt::Psbt;
use payjoin::bitcoin::Amount;
use payjoin::bitcoin::{Amount, FeeRate};
use payjoin::receive::v2::ActiveSession;
use payjoin::send::RequestContext;
use payjoin::{bitcoin, Error, Uri};
Expand Down Expand Up @@ -272,8 +273,6 @@ impl App {
&self,
proposal: payjoin::receive::v2::UncheckedProposal,
) -> Result<payjoin::receive::v2::PayjoinProposal, Error> {
use crate::app::try_contributing_inputs;

let bitcoind = self.bitcoind().map_err(|e| Error::Server(e.into()))?;

// in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx
Expand Down Expand Up @@ -317,19 +316,24 @@ impl App {
})?;
log::trace!("check4");

let mut provisional_payjoin = payjoin.identify_receiver_outputs(|output_script| {
if let Ok(address) = bitcoin::Address::from_script(output_script, network) {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Server(e.into()))
} else {
Ok(false)
}
})?;
let payjoin = payjoin
.identify_receiver_outputs(|output_script| {
if let Ok(address) = bitcoin::Address::from_script(output_script, network) {
bitcoind
.get_address_info(&address)
.map(|info| info.is_mine.unwrap_or(false))
.map_err(|e| Error::Server(e.into()))
} else {
Ok(false)
}
})?
.commit_outputs();

_ = try_contributing_inputs(&mut provisional_payjoin.inner, &bitcoind)
.map_err(|e| log::warn!("Failed to contribute inputs: {}", e));
let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind)
.unwrap_or_else(|e| {
log::warn!("Failed to contribute inputs: {}", e);
payjoin.commit_inputs()
});

let payjoin_proposal = provisional_payjoin.finalize_proposal(
|psbt: &Psbt| {
Expand All @@ -339,13 +343,47 @@ impl App {
.map_err(|e| Error::Server(e.into()))?
},
Some(bitcoin::FeeRate::MIN),
self.config.max_fee_rate.map_or(Ok(FeeRate::ZERO), |fee_rate| {
FeeRate::from_sat_per_vb(fee_rate).ok_or(Error::Server("Invalid fee rate".into()))
})?,
Comment on lines +346 to +348
Copy link
Contributor

Choose a reason for hiding this comment

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

Raather than fail here with a FeeRate::ZERO max_fee_rate, I think the config could make max_fee_rate compulsory so that this validation issue would crop up right away and not only after receiving a proposal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It only fails if from_sat_per_vb fails (from an integer overflow). If unspecified it defaults to FeeRate::ZERO which is accepted by apply_fee and would only fail if the receiver needs to contribute an additional fee. In most cases that's not an issue because the sender's additional_fee_contribution the receiver input fee for regular payjoins.

fwiw i added a docstring to that effect in apply_fee: "A max_feerate of zero indicates that the receiver is not willing to pay any additional fees."

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I misunderstood (apply_fee is sooo uncomplicated /s) But I guess that was default behavior from before so it makes sense as a default

)?;
let payjoin_proposal_psbt = payjoin_proposal.psbt();
log::debug!("Receiver's Payjoin proposal PSBT Rsponse: {:#?}", payjoin_proposal_psbt);
Ok(payjoin_proposal)
}
}

fn try_contributing_inputs(
payjoin: payjoin::receive::v2::WantsInputs,
bitcoind: &bitcoincore_rpc::Client,
) -> Result<payjoin::receive::v2::ProvisionalProposal> {
use bitcoin::OutPoint;

let available_inputs = bitcoind
.list_unspent(None, None, None, None, None)
.context("Failed to list unspent from bitcoind")?;
let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
.iter()
.map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
.collect();

let selected_outpoint = payjoin.try_preserving_privacy(candidate_inputs).expect("gg");
let selected_utxo = available_inputs
.iter()
.find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
.context("This shouldn't happen. Failed to retrieve the privacy preserving utxo from those we provided to the seclector.")?;
log::debug!("selected utxo: {:#?}", selected_utxo);
let txo_to_contribute = bitcoin::TxOut {
value: selected_utxo.amount,
script_pubkey: selected_utxo.script_pub_key.clone(),
};

Ok(payjoin
.contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)])
.expect("This shouldn't happen. Failed to contribute inputs.")
.commit_inputs())
}

async fn unwrap_ohttp_keys_or_else_fetch(config: &AppConfig) -> Result<payjoin::OhttpKeys> {
if let Some(keys) = config.ohttp_keys.clone() {
println!("Using OHTTP Keys from config");
Expand Down
6 changes: 6 additions & 0 deletions payjoin-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ fn cli() -> ArgMatches {
let mut cmd = cmd.subcommand(Command::new("resume"));

// Conditional arguments based on features for the receive subcommand
receive_cmd = receive_cmd.arg(
Arg::new("max_fee_rate")
.long("max-fee-rate")
.num_args(1)
.help("The maximum effective fee rate the receiver is willing to pay (in sat/vB)"),
);
#[cfg(not(feature = "v2"))]
{
receive_cmd = receive_cmd.arg(
Expand Down
81 changes: 81 additions & 0 deletions payjoin/src/receive/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ pub(crate) enum InternalRequestError {
///
/// Second argument is the minimum fee rate optionaly set by the receiver.
PsbtBelowFeeRate(bitcoin::FeeRate, bitcoin::FeeRate),
/// Effective receiver feerate exceeds maximum allowed feerate
FeeTooHigh(bitcoin::FeeRate, bitcoin::FeeRate),
}

impl From<InternalRequestError> for RequestError {
Expand Down Expand Up @@ -172,6 +174,14 @@ impl fmt::Display for RequestError {
original_psbt_fee_rate, receiver_min_fee_rate
),
),
InternalRequestError::FeeTooHigh(proposed_feerate, max_feerate) => write_error(
f,
"original-psbt-rejected",
&format!(
"Effective receiver feerate exceeds maximum allowed feerate: {} > {}",
proposed_feerate, max_feerate
),
),
}
}
}
Expand All @@ -196,6 +206,51 @@ impl std::error::Error for RequestError {
}
}

/// Error that may occur when output substitution fails.
///
/// This is currently opaque type because we aren't sure which variants will stay.
/// You can only display it.
#[derive(Debug)]
pub struct OutputSubstitutionError(InternalOutputSubstitutionError);

#[derive(Debug)]
pub(crate) enum InternalOutputSubstitutionError {
/// Output substitution is disabled
OutputSubstitutionDisabled(&'static str),
/// Current output substitution implementation doesn't support reducing the number of outputs
NotEnoughOutputs,
/// The provided drain script could not be identified in the provided replacement outputs
InvalidDrainScript,
}

impl fmt::Display for OutputSubstitutionError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match &self.0 {
InternalOutputSubstitutionError::OutputSubstitutionDisabled(reason) => write!(f, "{}", &format!("Output substitution is disabled: {}", reason)),
InternalOutputSubstitutionError::NotEnoughOutputs => write!(
f,
"Current output substitution implementation doesn't support reducing the number of outputs"
),
InternalOutputSubstitutionError::InvalidDrainScript =>
write!(f, "The provided drain script could not be identified in the provided replacement outputs"),
}
}
}

impl From<InternalOutputSubstitutionError> for OutputSubstitutionError {
fn from(value: InternalOutputSubstitutionError) -> Self { OutputSubstitutionError(value) }
}

impl std::error::Error for OutputSubstitutionError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self.0 {
InternalOutputSubstitutionError::OutputSubstitutionDisabled(_) => None,
InternalOutputSubstitutionError::NotEnoughOutputs => None,
InternalOutputSubstitutionError::InvalidDrainScript => None,
}
}
}

/// Error that may occur when coin selection fails.
///
/// This is currently opaque type because we aren't sure which variants will stay.
Expand Down Expand Up @@ -230,3 +285,29 @@ impl fmt::Display for SelectionError {
impl From<InternalSelectionError> for SelectionError {
fn from(value: InternalSelectionError) -> Self { SelectionError(value) }
}

/// Error that may occur when input contribution fails.
///
/// This is currently opaque type because we aren't sure which variants will stay.
/// You can only display it.
#[derive(Debug)]
pub struct InputContributionError(InternalInputContributionError);

#[derive(Debug)]
pub(crate) enum InternalInputContributionError {
/// Total input value is not enough to cover additional output value
ValueTooLow,
}

impl fmt::Display for InputContributionError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match &self.0 {
InternalInputContributionError::ValueTooLow =>
write!(f, "Total input value is not enough to cover additional output value"),
}
}
}

impl From<InternalInputContributionError> for InputContributionError {
fn from(value: InternalInputContributionError) -> Self { InputContributionError(value) }
}
Loading
Loading