From 7419d8173949abc419d95301aa9c701ee665593e Mon Sep 17 00:00:00 2001 From: spacebear Date: Thu, 22 Aug 2024 18:09:56 -0400 Subject: [PATCH] Add custom error types for output and input contributions Partially addresses https://github.com/payjoin/rust-payjoin/issues/312 --- payjoin-cli/src/app/v1.rs | 4 ++- payjoin-cli/src/app/v2.rs | 4 ++- payjoin/src/receive/error.rs | 61 +++++++++++++++++++++++++++++++++++ payjoin/src/receive/mod.rs | 42 ++++++++++++++++-------- payjoin/src/receive/v2/mod.rs | 18 +++++++---- payjoin/tests/integration.rs | 3 +- 6 files changed, 109 insertions(+), 23 deletions(-) diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index d3495376..afb087ea 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -348,7 +348,8 @@ impl App { .require_network(network) .map_err(|e| Error::Server(e.into()))? .script_pubkey(), - )? + ) + .map_err(|e| Error::Server(anyhow!(e.to_string()).into()))? .freeze_outputs(); let provisional_payjoin = @@ -394,6 +395,7 @@ fn try_contributing_inputs( Ok(payjoin .contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)]) + .expect("This shouldn't happen. Failed to contribute inputs.") .freeze_inputs()) } diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index 4b4d31a2..7e43decd 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -326,7 +326,8 @@ impl App { } else { Ok(false) } - })? + }) + .map_err(|e| Error::Server(anyhow!(e.to_string()).into()))? .freeze_outputs(); let provisional_payjoin = @@ -374,6 +375,7 @@ fn try_contributing_inputs( Ok(payjoin .contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)]) + .expect("This shouldn't happen. Failed to contribute inputs.") .freeze_inputs()) } diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 6c7d21ea..ea021931 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -196,6 +196,41 @@ 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 for OutputSubstitutionError { + fn from(value: InternalOutputSubstitutionError) -> Self { OutputSubstitutionError(value) } +} + /// Error that may occur when coin selection fails. /// /// This is currently opaque type because we aren't sure which variants will stay. @@ -230,3 +265,29 @@ impl fmt::Display for SelectionError { impl From 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 for InputContributionError { + fn from(value: InternalInputContributionError) -> Self { InputContributionError(value) } +} diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 5c4ca730..2605d6ab 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -38,8 +38,11 @@ mod optional_parameters; pub mod v2; use bitcoin::secp256k1::rand::{self, Rng}; -pub use error::{Error, RequestError, SelectionError}; -use error::{InternalRequestError, InternalSelectionError}; +pub use error::{Error, OutputSubstitutionError, RequestError, SelectionError}; +use error::{ + InputContributionError, InternalInputContributionError, InternalOutputSubstitutionError, + InternalRequestError, InternalSelectionError, +}; use optional_parameters::Params; use crate::input_type::InputType; @@ -359,7 +362,10 @@ impl WantsOutputs { } /// Substitute the receiver output script with the provided script. - pub fn substitute_receiver_script(self, output_script: &Script) -> Result { + pub fn substitute_receiver_script( + self, + output_script: &Script, + ) -> Result { let output_value = self.original_psbt.unsigned_tx.output[self.change_vout].value; let outputs = vec![TxOut { value: output_value, script_pubkey: output_script.into() }]; self.replace_receiver_outputs(outputs, &output_script) @@ -374,7 +380,7 @@ impl WantsOutputs { self, replacement_outputs: Vec, drain_script: &Script, - ) -> Result { + ) -> Result { let mut payjoin_psbt = self.original_psbt.clone(); let mut change_vout: Option = None; if self.params.disable_output_substitution { @@ -387,13 +393,17 @@ impl WantsOutputs { .find(|txo| txo.script_pubkey == original_output.script_pubkey) { Some(txo) if txo.value < original_output.value => { - return Err(Error::Server( - "Decreasing the receiver output value is not allowed".into(), + return Err(OutputSubstitutionError::from( + InternalOutputSubstitutionError::OutputSubstitutionDisabled( + "Decreasing the receiver output value is not allowed", + ), )); } None => - return Err(Error::Server( - "Changing the receiver output script pubkey is not allowed".into(), + return Err(OutputSubstitutionError::from( + InternalOutputSubstitutionError::OutputSubstitutionDisabled( + "Changing the receiver output script pubkey is not allowed", + ), )), _ => log::info!("Receiver is augmenting outputs"), } @@ -406,7 +416,9 @@ impl WantsOutputs { if self.owned_vouts.contains(&i) { // Receiver output: substitute with a provided output picked randomly if replacement_outputs.is_empty() { - return Err(Error::Server("Not enough outputs".into())); + return Err(OutputSubstitutionError::from( + InternalOutputSubstitutionError::NotEnoughOutputs, + )); } let index = rng.gen_range(0..replacement_outputs.len()); let txo = replacement_outputs.swap_remove(index); @@ -435,7 +447,9 @@ impl WantsOutputs { original_psbt: self.original_psbt, payjoin_psbt, params: self.params, - change_vout: change_vout.ok_or(Error::Server("Invalid drain script".into()))?, + change_vout: change_vout.ok_or(OutputSubstitutionError::from( + InternalOutputSubstitutionError::InvalidDrainScript, + ))?, owned_vouts: self.owned_vouts, }) } @@ -549,7 +563,7 @@ impl WantsInputs { pub fn contribute_witness_inputs( self, inputs: impl IntoIterator, - ) -> WantsInputs { + ) -> Result { let mut payjoin_psbt = self.payjoin_psbt.clone(); // The payjoin proposal must not introduce mixed input sequence numbers @@ -587,15 +601,15 @@ impl WantsInputs { let change_amount = inputs_amount - min_input_amount; payjoin_psbt.unsigned_tx.output[self.change_vout].value += change_amount; } else { - todo!("Input amount is not enough to cover additional output value"); + return Err(InternalInputContributionError::ValueTooLow.into()); } - WantsInputs { + Ok(WantsInputs { original_psbt: self.original_psbt, payjoin_psbt, params: self.params, change_vout: self.change_vout, - } + }) } // Compute the minimum amount that the receiver must contribute to the transaction as input diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 819fdf64..d7b0907b 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -15,7 +15,10 @@ use serde::{Deserialize, Serialize, Serializer}; use url::Url; use super::v2::error::{InternalSessionError, SessionError}; -use super::{Error, InternalRequestError, RequestError, SelectionError}; +use super::{ + Error, InputContributionError, InternalRequestError, OutputSubstitutionError, RequestError, + SelectionError, +}; use crate::psbt::PsbtExt; use crate::receive::optional_parameters::Params; use crate::v2::OhttpEncapsulationError; @@ -402,7 +405,10 @@ impl WantsOutputs { } /// Substitute the receiver output script with the provided script. - pub fn substitute_receiver_script(self, output_script: &Script) -> Result { + pub fn substitute_receiver_script( + self, + output_script: &Script, + ) -> Result { let inner = self.inner.substitute_receiver_script(output_script)?; Ok(WantsOutputs { inner, context: self.context }) } @@ -416,7 +422,7 @@ impl WantsOutputs { self, replacement_outputs: Vec, drain_script: &Script, - ) -> Result { + ) -> Result { let inner = self.inner.replace_receiver_outputs(replacement_outputs, drain_script)?; Ok(WantsOutputs { inner, context: self.context }) } @@ -460,9 +466,9 @@ impl WantsInputs { pub fn contribute_witness_inputs( self, inputs: impl IntoIterator, - ) -> WantsInputs { - let inner = self.inner.contribute_witness_inputs(inputs); - WantsInputs { inner, context: self.context } + ) -> Result { + let inner = self.inner.contribute_witness_inputs(inputs)?; + Ok(WantsInputs { inner, context: self.context }) } /// Proceed to the proposal finalization step. diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 390d2372..a597b7f1 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -644,6 +644,7 @@ mod integration { let payjoin = payjoin .contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)]) + .unwrap() .freeze_inputs(); // Sign and finalize the proposal PSBT @@ -1103,7 +1104,7 @@ mod integration { } }; - let payjoin = payjoin.contribute_witness_inputs(inputs).freeze_inputs(); + let payjoin = payjoin.contribute_witness_inputs(inputs).unwrap().freeze_inputs(); let payjoin_proposal = payjoin .finalize_proposal(