Create a duplicate proposal check in ns1r

benalleng · benalleng · commit 587af2dfd85e · 2025-04-25T12:29:48.000-04:00
When building a multiparty proposal we did not have a catch to
make sure that the proposals were all unique and as such a multiparty
could be built by just adding the same proposal over and over.  This creates
a new error IdenticalProposals that checks to make sure the contexts are not
matching when adding to a multiparty.
diff --git a/payjoin-test-utils/src/lib.rs b/payjoin-test-utils/src/lib.rs
@@ -299,3 +299,7 @@ pub static PARSED_PAYJOIN_PROPOSAL: Lazy<Psbt> =
 pub static PARSED_PAYJOIN_PROPOSAL_WITH_SENDER_INFO: Lazy<Psbt> = Lazy::new(|| {
     Psbt::from_str(PAYJOIN_PROPOSAL_WITH_SENDER_INFO).expect("known psbt should parse")
 });
+
+pub const MULTIPARTY_ORIGINAL_PSBT_ONE: &str = "cHNidP8BAMMCAAAAA9cTcqYjSnJdteUn3Re12vFuVb5xPypYheHze74WBW8EAAAAAAD+////6JHS6JgYdav7tr+Q5bWk95C2aA4AudqZnjeRF0hx33gBAAAAAP7///+d/1mRjJ6snVe9o0EfMM8bIExdlwfaig1j/JnUupqIWwAAAAAA/v///wJO4wtUAgAAABYAFGAEc6cWf7z5a97Xxulg+S7JZyg/kvEFKgEAAAAWABQexWFsGEdXxYNTJLJ1qBCO0nXELwAAAAAAAQCaAgAAAAKa/zFrWjicRDNVCehGj/2jdXGKuZLMzLNUo+uj7PBW8wAAAAAA/v///4HkRlmU9FA3sHckkCQMjGrabNpgAw37XaqUItNFjR5rAAAAAAD+////AgDyBSoBAAAAFgAUGPI/E9xCHmaL6DIsjkaD8LX2mpLg6QUqAQAAABYAFH6MyygysrAmk/nunEFuGGDLf244aAAAAAEBHwDyBSoBAAAAFgAUGPI/E9xCHmaL6DIsjkaD8LX2mpIBCGsCRzBEAiBLJUhV7tja6FdELXDcB6Q3Gd+BMBpkjdHpj3DLnbL6AAIgJmFl3kpWBzkO8yDDl73BMMalSlAOQvfY+hqFRos5DhQBIQLfeq7CCftNEUHcRLZniJOqcgKZEqPc1A40BnEiZHj3FQABAJoCAAAAAp3/WZGMnqydV72jQR8wzxsgTF2XB9qKDWP8mdS6mohbAQAAAAD+////1xNypiNKcl215SfdF7Xa8W5VvnE/KliF4fN7vhYFbwQBAAAAAP7///8CoNkFKgEAAAAWABQchAe9uxzqT1EjLx4jgx9u1Mn6QADyBSoBAAAAFgAUye8yXWX0MvouXYhAFb06xX5kADpoAAAAAQEfAPIFKgEAAAAWABTJ7zJdZfQy+i5diEAVvTrFfmQAOgEIawJHMEQCIF7ihY/YtUPUTOaEdJbg0/HiwKunK398BI67/LknPGqMAiBHBXmL6gTP8PxEGeWswk6T0tCI2Gvwq1zh+wd7h8QCWAEhApM0w2WFlw0eg64Zp3PeyRmxl/7WGHUED8Ul/aX1FiTBAAAAAA==";
+
+pub const MULTIPARTY_ORIGINAL_PSBT_TWO: &str = "cHNidP8BAMMCAAAAA9cTcqYjSnJdteUn3Re12vFuVb5xPypYheHze74WBW8EAAAAAAD+////6JHS6JgYdav7tr+Q5bWk95C2aA4AudqZnjeRF0hx33gBAAAAAP7///+d/1mRjJ6snVe9o0EfMM8bIExdlwfaig1j/JnUupqIWwAAAAAA/v///wJO4wtUAgAAABYAFGAEc6cWf7z5a97Xxulg+S7JZyg/kvEFKgEAAAAWABQexWFsGEdXxYNTJLJ1qBCO0nXELwAAAAAAAAEAmgIAAAACnf9ZkYyerJ1XvaNBHzDPGyBMXZcH2ooNY/yZ1LqaiFsBAAAAAP7////XE3KmI0pyXbXlJ90XtdrxblW+cT8qWIXh83u+FgVvBAEAAAAA/v///wKg2QUqAQAAABYAFByEB727HOpPUSMvHiODH27UyfpAAPIFKgEAAAAWABTJ7zJdZfQy+i5diEAVvTrFfmQAOmgAAAABAR8A8gUqAQAAABYAFMnvMl1l9DL6Ll2IQBW9OsV+ZAA6AQhrAkcwRAIgXuKFj9i1Q9RM5oR0luDT8eLAq6crf3wEjrv8uSc8aowCIEcFeYvqBM/w/EQZ5azCTpPS0IjYa/CrXOH7B3uHxAJYASECkzTDZYWXDR6Drhmnc97JGbGX/tYYdQQPxSX9pfUWJMEAAQCaAgAAAAIKOB8lY4eoEupDnxviz0/nAuR2biNFKbdkvckiW5ioPAAAAAAA/v///w0g/mj67592vy29xhnZMGeVtEXN1jD4lU/SMZM8oStqAAAAAAD+////AgDyBSoBAAAAFgAUC3r4YzVSpsWp3knMxbgWIx2R36/g6QUqAQAAABYAFMGlw3hwcx1b+KQGWIfOUxzwrwWkaAAAAAEBHwDyBSoBAAAAFgAUC3r4YzVSpsWp3knMxbgWIx2R368BCGsCRzBEAiA//JH+jonzbzqnKI0Uti16iJcdsXI+6Zu0IAZKlOq6AwIgP0XawCCH73uXKilFqSXQQQrBvmi/Sx44D/A+/MQ/mJYBIQIekOyEpJKpFQd7eHuY6Vt4Qlf0+00Wp529I23hl/EpcQAAAA==";
diff --git a/payjoin/src/receive/multiparty/error.rs b/payjoin/src/receive/multiparty/error.rs
@@ -1,13 +1,17 @@
 use core::fmt;
 use std::error;
 
+use crate::uri::ShortId;
+
 #[derive(Debug)]
 pub struct MultipartyError(InternalMultipartyError);
 
 #[derive(Debug)]
 pub(crate) enum InternalMultipartyError {
     /// Not enough proposals
     NotEnoughProposals,
+    /// Duplicate proposals
+    IdenticalProposals(IdenticalProposalError),
     /// Proposal version not supported
     ProposalVersionNotSupported(usize),
     /// Optimistic merge not supported
@@ -20,6 +24,27 @@ pub(crate) enum InternalMultipartyError {
     FailedToCombinePsbts(bitcoin::psbt::Error),
 }
 
+#[derive(Debug)]
+pub enum IdenticalProposalError {
+    IdenticalPsbts(Box<bitcoin::Psbt>, Box<bitcoin::Psbt>),
+    IdenticalContexts(Box<ShortId>, Box<ShortId>),
+}
+
+impl std::fmt::Display for IdenticalProposalError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            IdenticalProposalError::IdenticalPsbts(current_psbt, incoming_psbt) => write!(
+                f,
+                "Two sender psbts are identical\n left psbt: {current_psbt}\n right psbt: {incoming_psbt}"
+            ),
+            IdenticalProposalError::IdenticalContexts(current_context, incoming_context) => write!(
+                f,
+                "Two sender contexts are identical\n left id: {current_context}\n right id: {incoming_context}"
+            ),
+        }
+    }
+}
+
 impl From<InternalMultipartyError> for MultipartyError {
     fn from(e: InternalMultipartyError) -> Self { MultipartyError(e) }
 }
@@ -28,6 +53,8 @@ impl fmt::Display for MultipartyError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match &self.0 {
             InternalMultipartyError::NotEnoughProposals => write!(f, "Not enough proposals"),
+            InternalMultipartyError::IdenticalProposals(e) =>
+                write!(f, "More than one identical participant: {e}"),
             InternalMultipartyError::ProposalVersionNotSupported(v) =>
                 write!(f, "Proposal version not supported: {v}"),
             InternalMultipartyError::OptimisticMergeNotSupported =>
@@ -46,6 +73,7 @@ impl error::Error for MultipartyError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match &self.0 {
             InternalMultipartyError::NotEnoughProposals => None,
+            InternalMultipartyError::IdenticalProposals(_) => None,
             InternalMultipartyError::ProposalVersionNotSupported(_) => None,
             InternalMultipartyError::OptimisticMergeNotSupported => None,
             InternalMultipartyError::BitcoinExtractTxError(e) => Some(e),
diff --git a/payjoin/src/receive/multiparty/mod.rs b/payjoin/src/receive/multiparty/mod.rs
@@ -1,4 +1,5 @@
 use bitcoin::{FeeRate, Psbt};
+use error::IdenticalProposalError;
 
 use super::error::InputContributionError;
 use super::{v1, v2, Error, ImplementationError, InputPair};
@@ -36,6 +37,30 @@ impl UncheckedProposalBuilder {
         if !params.optimistic_merge {
             return Err(InternalMultipartyError::OptimisticMergeNotSupported.into());
         }
+
+        if let Some(duplicate_context) =
+            self.proposals.iter().find(|c| c.context == proposal.context)
+        {
+            return Err(InternalMultipartyError::IdenticalProposals(
+                IdenticalProposalError::IdenticalContexts(
+                    Box::new(duplicate_context.id()),
+                    Box::new(proposal.id()),
+                ),
+            )
+            .into());
+        };
+
+        if let Some(duplicate_psbt) =
+            self.proposals.iter().find(|psbt| psbt.v1.psbt == proposal.v1.psbt)
+        {
+            return Err(InternalMultipartyError::IdenticalProposals(
+                IdenticalProposalError::IdenticalPsbts(
+                    Box::new(duplicate_psbt.v1.psbt.clone()),
+                    Box::new(proposal.v1.psbt.clone()),
+                ),
+            )
+            .into());
+        }
         Ok(())
     }
 
@@ -218,6 +243,29 @@ impl FinalizedProposal {
                 InternalMultipartyError::ProposalVersionNotSupported(proposal.v1.params.v).into()
             );
         }
+        if let Some(duplicate_context) =
+            self.v2_proposals.iter().find(|c| c.context == proposal.context)
+        {
+            return Err(InternalMultipartyError::IdenticalProposals(
+                IdenticalProposalError::IdenticalContexts(
+                    Box::new(duplicate_context.id()),
+                    Box::new(proposal.id()),
+                ),
+            )
+            .into());
+        };
+
+        if let Some(duplicate_psbt) =
+            self.v2_proposals.iter().find(|psbt| psbt.v1.psbt == proposal.v1.psbt)
+        {
+            return Err(InternalMultipartyError::IdenticalProposals(
+                IdenticalProposalError::IdenticalPsbts(
+                    Box::new(duplicate_psbt.v1.psbt.clone()),
+                    Box::new(proposal.v1.psbt.clone()),
+                ),
+            )
+            .into());
+        }
         Ok(())
     }
 
@@ -253,51 +301,120 @@ impl FinalizedProposal {
 mod test {
 
     use std::any::{Any, TypeId};
-
-    use payjoin_test_utils::{BoxError, PARSED_ORIGINAL_PSBT};
-
-    use super::{v1, v2, FinalizedProposal, UncheckedProposalBuilder, SUPPORTED_VERSIONS};
+    use std::str::FromStr;
+
+    use bitcoin::Psbt;
+    use payjoin_test_utils::{
+        BoxError, MULTIPARTY_ORIGINAL_PSBT_ONE, MULTIPARTY_ORIGINAL_PSBT_TWO,
+    };
+
+    use super::error::IdenticalProposalError;
+    use super::{
+        v1, v2, FinalizedProposal, InternalMultipartyError, MultipartyError,
+        UncheckedProposalBuilder, SUPPORTED_VERSIONS,
+    };
     use crate::receive::optional_parameters::Params;
-    use crate::receive::v2::test::SHARED_CONTEXT;
+    use crate::receive::v2::test::{SHARED_CONTEXT, SHARED_CONTEXT_TWO};
 
-    fn multiparty_proposal_from_test_vector() -> v1::UncheckedProposal {
+    fn multiparty_proposals() -> Vec<v1::UncheckedProposal> {
         let pairs = url::form_urlencoded::parse("v=2&optimisticmerge=true".as_bytes());
         let params = Params::from_query_pairs(pairs, SUPPORTED_VERSIONS)
             .expect("Could not parse from query pairs");
-        v1::UncheckedProposal { psbt: PARSED_ORIGINAL_PSBT.clone(), params }
+
+        [MULTIPARTY_ORIGINAL_PSBT_ONE, MULTIPARTY_ORIGINAL_PSBT_TWO]
+            .iter()
+            .map(|psbt_str| v1::UncheckedProposal {
+                psbt: Psbt::from_str(psbt_str).expect("known psbt should parse"),
+                params: params.clone(),
+            })
+            .collect()
     }
 
     #[test]
-    fn test_build_multiparty() -> Result<(), BoxError> {
+    fn test_single_context_multiparty() -> Result<(), BoxError> {
         let proposal_one = v2::UncheckedProposal {
-            v1: multiparty_proposal_from_test_vector(),
+            v1: multiparty_proposals()[0].clone(),
+            context: SHARED_CONTEXT.clone(),
+        };
+        let mut multiparty = UncheckedProposalBuilder::new();
+        multiparty.add(proposal_one)?;
+        match multiparty.build() {
+            Ok(_) => panic!("multiparty has two identical participants and should error"),
+            Err(e) => assert_eq!(
+                e.to_string(),
+                MultipartyError::from(InternalMultipartyError::NotEnoughProposals).to_string()
+            ),
+        }
+        Ok(())
+    }
+
+    #[test]
+    fn test_duplicate_context_multiparty() -> Result<(), BoxError> {
+        let proposal_one = v2::UncheckedProposal {
+            v1: multiparty_proposals()[0].clone(),
             context: SHARED_CONTEXT.clone(),
         };
         let proposal_two = v2::UncheckedProposal {
-            v1: multiparty_proposal_from_test_vector(),
+            v1: multiparty_proposals()[1].clone(),
             context: SHARED_CONTEXT.clone(),
         };
-        let mut multiparty = UncheckedProposalBuilder::new();
-        multiparty.add(proposal_one)?;
-        multiparty.add(proposal_two)?;
-        let unchecked_proposal = multiparty.build();
-        assert!(unchecked_proposal?.contexts.len() == 2);
+        let mut multiparty = UncheckedProposalBuilder::new().add(proposal_one.clone())?;
+        match multiparty.add(proposal_two.clone()) {
+            Ok(_) => panic!("multiparty has two identical contexts and should error"),
+            Err(e) => assert_eq!(
+                e.to_string(),
+                MultipartyError::from(InternalMultipartyError::IdenticalProposals(
+                    IdenticalProposalError::IdenticalContexts(
+                        Box::new(proposal_one.id()),
+                        Box::new(proposal_two.id())
+                    )
+                ))
+                .to_string()
+            ),
+        }
+        Ok(())
+    }
+
+    #[test]
+    fn test_duplicate_psbt_multiparty() -> Result<(), BoxError> {
+        let proposal_one = v2::UncheckedProposal {
+            v1: multiparty_proposals()[0].clone(),
+            context: SHARED_CONTEXT.clone(),
+        };
+        let proposal_two = v2::UncheckedProposal {
+            v1: multiparty_proposals()[0].clone(),
+            context: SHARED_CONTEXT_TWO.clone(),
+        };
+        let mut multiparty = UncheckedProposalBuilder::new().add(proposal_one.clone())?;
+        match multiparty.add(proposal_two.clone()) {
+            Ok(_) => panic!("multiparty has two identical psbts and should error"),
+            Err(e) => assert_eq!(
+                e.to_string(),
+                MultipartyError::from(InternalMultipartyError::IdenticalProposals(
+                    IdenticalProposalError::IdenticalPsbts(
+                        Box::new(proposal_one.v1.psbt),
+                        Box::new(proposal_two.v1.psbt)
+                    )
+                ))
+                .to_string()
+            ),
+        }
         Ok(())
     }
 
     #[test]
     fn finalize_multiparty() -> Result<(), BoxError> {
         use crate::psbt::PsbtExt;
         let proposal_one = v2::UncheckedProposal {
-            v1: multiparty_proposal_from_test_vector(),
+            v1: multiparty_proposals()[0].clone(),
             context: SHARED_CONTEXT.clone(),
         };
         let proposal_two = v2::UncheckedProposal {
-            v1: multiparty_proposal_from_test_vector(),
-            context: SHARED_CONTEXT.clone(),
+            v1: multiparty_proposals()[1].clone(),
+            context: SHARED_CONTEXT_TWO.clone(),
         };
         let mut finalized_multiparty = FinalizedProposal::new();
-        finalized_multiparty.add(proposal_one.clone())?;
+        finalized_multiparty.add(proposal_one)?;
         assert_eq!(finalized_multiparty.v2()[0].type_id(), TypeId::of::<v2::UncheckedProposal>());
 
         finalized_multiparty.add(proposal_two)?;
diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs
@@ -660,6 +660,20 @@ pub mod test {
         e: None,
     });
 
+    pub(crate) static SHARED_CONTEXT_TWO: Lazy<SessionContext> = Lazy::new(|| SessionContext {
+        address: Address::from_str("tb1qv7scm7gxs32qg3lnm9kf267kllc63yvdxyh72e")
+            .expect("valid address")
+            .assume_checked(),
+        directory: EXAMPLE_URL.clone(),
+        subdirectory: None,
+        ohttp_keys: OhttpKeys(
+            ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"),
+        ),
+        expiry: SystemTime::now() + Duration::from_secs(60),
+        s: HpkeKeyPair::gen_keypair(),
+        e: None,
+    });
+
     #[test]
     fn extract_err_req() -> Result<(), BoxError> {
         let mut proposal = UncheckedProposal {
