From be5c5aebef38164a7e306df43f67ac313822a9cd Mon Sep 17 00:00:00 2001 From: chavic Date: Fri, 20 Mar 2026 13:33:23 +0200 Subject: [PATCH] Expose Receiver Modification Errors Add stable kind accessors for receiver output substitution and input contribution failures and preserve the duplicate input outpoint across the FFI boundary. Bindings currently only see opaque error wrappers for these paths, which forces callers to branch on display strings even though core already distinguishes actionable failure classes. This keeps the internal enums private while making receiver integrations able to react to invalid drain scripts, disabled output substitutions, duplicate inputs, and value-too-low contributions. --- payjoin-ffi/src/receive/error.rs | 267 +++++++++++++++++++++++++++++- payjoin/src/core/receive/error.rs | 86 +++++++++- payjoin/src/core/receive/mod.rs | 4 +- 3 files changed, 347 insertions(+), 10 deletions(-) diff --git a/payjoin-ffi/src/receive/error.rs b/payjoin-ffi/src/receive/error.rs index 2ceb53b97..79791936f 100644 --- a/payjoin-ffi/src/receive/error.rs +++ b/payjoin-ffi/src/receive/error.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use payjoin::receive; +use super::PlainOutPoint; use crate::error::{FfiValidationError, ImplementationError}; use crate::uri::error::IntoUrlError; @@ -168,10 +169,66 @@ impl From for JsonReply { #[error(transparent)] pub struct SessionError(#[from] receive::v2::SessionError); +#[derive(Clone, Copy, Debug, PartialEq, Eq, uniffi::Enum)] +pub enum OutputSubstitutionErrorKind { + DecreasedValueWhenDisabled, + ScriptPubKeyChangedWhenDisabled, + NotEnoughOutputs, + InvalidDrainScript, + Other, +} + +impl From for OutputSubstitutionErrorKind { + fn from(value: receive::OutputSubstitutionErrorKind) -> Self { + match value { + receive::OutputSubstitutionErrorKind::DecreasedValueWhenDisabled => + Self::DecreasedValueWhenDisabled, + receive::OutputSubstitutionErrorKind::ScriptPubKeyChangedWhenDisabled => + Self::ScriptPubKeyChangedWhenDisabled, + receive::OutputSubstitutionErrorKind::NotEnoughOutputs => Self::NotEnoughOutputs, + receive::OutputSubstitutionErrorKind::InvalidDrainScript => Self::InvalidDrainScript, + _ => Self::Other, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, uniffi::Enum)] +pub enum InputContributionErrorKind { + ValueTooLow, + DuplicateInput, + Other, +} + +impl From for InputContributionErrorKind { + fn from(value: receive::InputContributionErrorKind) -> Self { + match value { + receive::InputContributionErrorKind::ValueTooLow => Self::ValueTooLow, + receive::InputContributionErrorKind::DuplicateInput => Self::DuplicateInput, + _ => Self::Other, + } + } +} + /// Protocol error raised during output substitution. #[derive(Debug, thiserror::Error, uniffi::Object)] -#[error(transparent)] -pub struct OutputSubstitutionProtocolError(#[from] receive::OutputSubstitutionError); +#[error("{message}")] +pub struct OutputSubstitutionProtocolError { + kind: OutputSubstitutionErrorKind, + message: String, +} + +impl From for OutputSubstitutionProtocolError { + fn from(value: receive::OutputSubstitutionError) -> Self { + Self { kind: value.kind().into(), message: value.to_string() } + } +} + +#[uniffi::export] +impl OutputSubstitutionProtocolError { + pub fn kind(&self) -> OutputSubstitutionErrorKind { self.kind } + + pub fn message(&self) -> String { self.message.clone() } +} /// Error that may occur when output substitution fails. #[derive(Debug, thiserror::Error, uniffi::Error)] @@ -199,8 +256,33 @@ pub struct SelectionError(#[from] receive::SelectionError); /// Error that may occur when input contribution fails. #[derive(Debug, thiserror::Error, uniffi::Object)] -#[error(transparent)] -pub struct InputContributionError(#[from] receive::InputContributionError); +#[error("{message}")] +pub struct InputContributionError { + kind: InputContributionErrorKind, + message: String, + duplicate_input_outpoint: Option, +} + +impl From for InputContributionError { + fn from(value: receive::InputContributionError) -> Self { + Self { + kind: value.kind().into(), + message: value.to_string(), + duplicate_input_outpoint: value.duplicate_input_outpoint().map(Into::into), + } + } +} + +#[uniffi::export] +impl InputContributionError { + pub fn kind(&self) -> InputContributionErrorKind { self.kind } + + pub fn message(&self) -> String { self.message.clone() } + + pub fn duplicate_input_outpoint(&self) -> Option { + self.duplicate_input_outpoint.clone() + } +} /// Error validating a PSBT Input #[derive(Debug, thiserror::Error, uniffi::Object)] @@ -237,3 +319,180 @@ impl From for InputPairError { pub struct ReceiverReplayError( #[from] payjoin::error::ReplayError, ); + +#[cfg(all(test, feature = "_test-utils"))] +mod tests { + use std::str::FromStr; + + use payjoin::bitcoin::{Address, Amount, Network, Psbt, ScriptBuf, TxOut}; + use payjoin::receive::v1::{Headers, UncheckedOriginalPayload}; + use payjoin_test_utils::{ORIGINAL_PSBT, QUERY_PARAMS, RECEIVER_INPUT_CONTRIBUTION}; + + use super::*; + + struct TestHeaders { + content_type: Option<&'static str>, + content_length: String, + } + + impl Headers for TestHeaders { + fn get_header(&self, key: &str) -> Option<&str> { + match key { + "content-type" => self.content_type, + "content-length" => Some(self.content_length.as_str()), + _ => None, + } + } + } + + fn wants_outputs_from_test_vector() -> payjoin::receive::v1::WantsOutputs { + let body = ORIGINAL_PSBT.as_bytes(); + let headers = TestHeaders { + content_type: Some("text/plain"), + content_length: body.len().to_string(), + }; + let receiver_address = Address::from_str("3CZZi7aWFugaCdUCS15dgrUUViupmB8bVM") + .expect("known address should parse") + .require_network(Network::Bitcoin) + .expect("known address should match network"); + + UncheckedOriginalPayload::from_request(body, QUERY_PARAMS, headers) + .expect("test vector should parse") + .assume_interactive_receiver() + .check_inputs_not_owned(&mut |_| Ok(false)) + .expect("proposal should not spend receiver inputs") + .check_no_inputs_seen_before(&mut |_| Ok(false)) + .expect("proposal should not contain seen inputs") + .identify_receiver_outputs(&mut |script| { + Ok(Address::from_script(script, Network::Bitcoin) + .expect("known script should decode") + == receiver_address) + }) + .expect("receiver output should be identified") + } + + fn wants_inputs_from_test_vector() -> payjoin::receive::v1::WantsInputs { + wants_outputs_from_test_vector().commit_outputs() + } + + fn receiver_output_from_test_vector() -> TxOut { + let receiver_script = Address::from_str("3CZZi7aWFugaCdUCS15dgrUUViupmB8bVM") + .expect("known address should parse") + .require_network(Network::Bitcoin) + .expect("known address should match network") + .script_pubkey(); + let original = Psbt::from_str(ORIGINAL_PSBT).expect("known PSBT should parse"); + + original + .unsigned_tx + .output + .iter() + .find(|output| output.script_pubkey == receiver_script) + .cloned() + .expect("test vector should pay the receiver") + } + + fn receiver_input_pair() -> payjoin::receive::InputPair { + let proposal_psbt = + Psbt::from_str(RECEIVER_INPUT_CONTRIBUTION).expect("known PSBT should parse"); + payjoin::receive::InputPair::new( + proposal_psbt.unsigned_tx.input[1].clone(), + proposal_psbt.inputs[1].clone(), + None, + ) + .expect("test vector input should be valid") + } + + fn receiver_input_outpoint() -> PlainOutPoint { + let proposal_psbt = + Psbt::from_str(RECEIVER_INPUT_CONTRIBUTION).expect("known PSBT should parse"); + PlainOutPoint::from(proposal_psbt.unsigned_tx.input[1].previous_output) + } + + fn wants_inputs_with_minimum_contribution( + required_delta: Amount, + ) -> payjoin::receive::v1::WantsInputs { + let mut receiver_output = receiver_output_from_test_vector(); + let drain_script = receiver_output.script_pubkey.clone(); + receiver_output.value += required_delta; + + wants_outputs_from_test_vector() + .replace_receiver_outputs(vec![receiver_output], &drain_script) + .expect("higher receiver output should be accepted") + .commit_outputs() + } + + fn low_value_input_pair() -> payjoin::receive::InputPair { + let proposal_psbt = + Psbt::from_str(RECEIVER_INPUT_CONTRIBUTION).expect("known PSBT should parse"); + let mut psbt_input = proposal_psbt.inputs[1].clone(); + let mut witness_utxo = + psbt_input.witness_utxo.clone().expect("test vector input should include witness UTXO"); + witness_utxo.value = Amount::from_sat(123); + psbt_input.witness_utxo = Some(witness_utxo); + + payjoin::receive::InputPair::new( + proposal_psbt.unsigned_tx.input[1].clone(), + psbt_input, + None, + ) + .expect("low-value test input should remain structurally valid") + } + + #[test] + fn test_output_substitution_error_exposes_kind() { + let receiver_output = receiver_output_from_test_vector(); + let missing_drain_script = ScriptBuf::new(); + let error = wants_outputs_from_test_vector() + .replace_receiver_outputs(vec![receiver_output], &missing_drain_script) + .expect_err("missing drain script should fail"); + let OutputSubstitutionError::Protocol(protocol) = OutputSubstitutionError::from(error) + else { + panic!("expected protocol substitution error"); + }; + + assert_eq!(protocol.kind(), OutputSubstitutionErrorKind::InvalidDrainScript); + assert_eq!( + protocol.message(), + "The provided drain script could not be identified in the provided replacement outputs" + ); + } + + #[test] + fn test_input_contribution_error_exposes_duplicate_outpoint() { + let input = receiver_input_pair(); + let contributed = wants_inputs_from_test_vector() + .contribute_inputs(vec![input.clone()]) + .expect("first contribution should succeed"); + let error = contributed + .contribute_inputs(vec![input]) + .expect_err("duplicate contribution should fail"); + let error = InputContributionError::from(error); + let expected_outpoint = receiver_input_outpoint(); + + assert_eq!(error.kind(), InputContributionErrorKind::DuplicateInput); + let outpoint = + error.duplicate_input_outpoint().expect("duplicate outpoint should be present"); + assert_eq!(outpoint.txid, expected_outpoint.txid); + assert_eq!(outpoint.vout, expected_outpoint.vout); + assert_eq!( + error.message(), + format!("Duplicate input detected: {}:{}", outpoint.txid, outpoint.vout) + ); + } + + #[test] + fn test_input_contribution_error_exposes_value_too_low_kind() { + let error = wants_inputs_with_minimum_contribution(Amount::from_sat(1_000)) + .contribute_inputs(vec![low_value_input_pair()]) + .expect_err("low value contribution should fail"); + let error = InputContributionError::from(error); + + assert_eq!(error.kind(), InputContributionErrorKind::ValueTooLow); + assert!(error.duplicate_input_outpoint().is_none()); + assert_eq!( + error.message(), + "Total input value is not enough to cover additional output value" + ); + } +} diff --git a/payjoin/src/core/receive/error.rs b/payjoin/src/core/receive/error.rs index 5d9b02083..056a682c1 100644 --- a/payjoin/src/core/receive/error.rs +++ b/payjoin/src/core/receive/error.rs @@ -299,11 +299,41 @@ impl std::error::Error for PayloadError { /// 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. +/// This type keeps its internal variants private, but exposes a stable +/// classification via [`OutputSubstitutionError::kind`]. #[derive(Debug, PartialEq)] pub struct OutputSubstitutionError(InternalOutputSubstitutionError); +/// A stable classification for output substitution failures. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum OutputSubstitutionErrorKind { + /// Output substitution is disabled and the receiver output value decreased. + DecreasedValueWhenDisabled, + /// Output substitution is disabled and the receiver output script changed. + ScriptPubKeyChangedWhenDisabled, + /// Fewer replacement outputs were provided than receiver-owned outputs. + NotEnoughOutputs, + /// The designated drain script was not present in the replacement outputs. + InvalidDrainScript, +} + +impl OutputSubstitutionError { + /// Returns the stable classification of the substitution failure. + pub fn kind(&self) -> OutputSubstitutionErrorKind { + match &self.0 { + InternalOutputSubstitutionError::DecreasedValueWhenDisabled => + OutputSubstitutionErrorKind::DecreasedValueWhenDisabled, + InternalOutputSubstitutionError::ScriptPubKeyChangedWhenDisabled => + OutputSubstitutionErrorKind::ScriptPubKeyChangedWhenDisabled, + InternalOutputSubstitutionError::NotEnoughOutputs => + OutputSubstitutionErrorKind::NotEnoughOutputs, + InternalOutputSubstitutionError::InvalidDrainScript => + OutputSubstitutionErrorKind::InvalidDrainScript, + } + } +} + #[derive(Debug, PartialEq, Eq)] pub(crate) enum InternalOutputSubstitutionError { /// Output substitution is disabled and output value was decreased @@ -394,11 +424,40 @@ impl From for SelectionError { /// 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. +/// This type keeps its internal variants private, but exposes a stable +/// classification via [`InputContributionError::kind`]. #[derive(Debug, PartialEq, Eq)] pub struct InputContributionError(InternalInputContributionError); +/// A stable classification for input contribution failures. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum InputContributionErrorKind { + /// The contributed inputs did not cover the receiver's additional output value. + ValueTooLow, + /// The contributed inputs reused an outpoint already present in the transaction. + DuplicateInput, +} + +impl InputContributionError { + /// Returns the stable classification of the contribution failure. + pub fn kind(&self) -> InputContributionErrorKind { + match &self.0 { + InternalInputContributionError::ValueTooLow => InputContributionErrorKind::ValueTooLow, + InternalInputContributionError::DuplicateInput(_) => + InputContributionErrorKind::DuplicateInput, + } + } + + /// Returns the duplicated outpoint when the failure was caused by reusing an input. + pub fn duplicate_input_outpoint(&self) -> Option { + match &self.0 { + InternalInputContributionError::DuplicateInput(outpoint) => Some(*outpoint), + InternalInputContributionError::ValueTooLow => None, + } + } +} + #[derive(Debug, PartialEq, Eq)] pub(crate) enum InternalInputContributionError { /// Total input value is not enough to cover additional output value @@ -504,4 +563,23 @@ mod tests { assert_eq!(json["errorCode"], "original-psbt-rejected"); assert_eq!(json["message"], "Missing payment."); } + + #[test] + fn test_output_substitution_error_kind_accessor() { + let error = OutputSubstitutionError(InternalOutputSubstitutionError::InvalidDrainScript); + + assert_eq!(error.kind(), OutputSubstitutionErrorKind::InvalidDrainScript); + } + + #[test] + fn test_input_contribution_error_duplicate_input_accessor() { + let outpoint = "0000000000000000000000000000000000000000000000000000000000000000:3" + .parse() + .expect("known outpoint should parse"); + let error = + InputContributionError(InternalInputContributionError::DuplicateInput(outpoint)); + + assert_eq!(error.kind(), InputContributionErrorKind::DuplicateInput); + assert_eq!(error.duplicate_input_outpoint(), Some(outpoint)); + } } diff --git a/payjoin/src/core/receive/mod.rs b/payjoin/src/core/receive/mod.rs index a2be3bc0e..868489c44 100644 --- a/payjoin/src/core/receive/mod.rs +++ b/payjoin/src/core/receive/mod.rs @@ -18,8 +18,8 @@ use bitcoin::{ }; pub(crate) use error::InternalPayloadError; pub use error::{ - Error, InputContributionError, JsonReply, OutputSubstitutionError, PayloadError, ProtocolError, - SelectionError, + Error, InputContributionError, InputContributionErrorKind, JsonReply, OutputSubstitutionError, + OutputSubstitutionErrorKind, PayloadError, ProtocolError, SelectionError, }; use optional_parameters::Params; use serde::{Deserialize, Serialize};