Skip to content

Duplicate proposals catch in ns1r #642

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.

Sign up for GitHub

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

Merged
arminsabouri merged 2 commits into from
Apr 28, 2025
Merged

Duplicate proposals catch in ns1r #642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a556161
Add new id method for UncheckedProposal
benalleng Apr 23, 2025
587af2d
Create a duplicate proposal check in ns1r
benalleng Apr 10, 2025
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
4 changes: 4 additions & 0 deletions payjoin-test-utils/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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==";
28 changes: 28 additions & 0 deletions payjoin/src/receive/multiparty/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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) }
}
Expand All @@ -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 =>
Expand All @@ -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),
Expand Down
155 changes: 136 additions & 19 deletions payjoin/src/receive/multiparty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use bitcoin::{FeeRate, Psbt};
use error::IdenticalProposalError;

use super::error::InputContributionError;
use super::{v1, v2, Error, ImplementationError, InputPair};
Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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)?;
Expand Down
17 changes: 17 additions & 0 deletions payjoin/src/receive/v2/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,9 @@ impl UncheckedProposal {
_ => Err(InternalSessionError::UnexpectedStatusCode(response.status()).into()),
}
}

/// The per-session identifier
pub fn id(&self) -> ShortId { id(&self.context.s) }
}

/// Typestate to validate that the Original PSBT has no receiver-owned inputs.
Expand Down Expand Up @@ -657,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 {
Expand Down