Skip to content

Commit 5fe5db1

Browse files
Abstract common sender functionality, not heirarchy (#884)
`{v1,v2,multiparty}::Sender{,Builder}` types have been built off of a heirarchy where multiparty:: has a v2:: and v2:: has a v1::, which was done for quick convenience and not because that relationship actually makes functional sense. This PR is the first in a few which I believe are necessary to rectify this distinction. It does so by drawing the separation between the sender's `PsbtContext` checks / response processing and the version-specific serialization for networked messaging. However, I don't think it goes far enough. For one, it only really rectifies this issue between v1 and v2. Multiparty is left abstract over v2. Second, There are still distinct SenderBuilders that can't tell whether or not they're handling a v1 or v2 URIs. Since the information necessary to distinguish between a v1/v2 URI is in the URI itself, it seems that ought to be the first order of business for the sender to do even before calling `SenderBuilder::new`. The lack of this distinction leads to a [problem](bitcoindevkit/bdk-cli#200 (comment)) with a hacky [solution](bitcoindevkit/bdk-cli#200 (comment)) where downstream users need to wait all the way until they attempt to create a v2 request and handle an error there in order to figure out the version. The `SenderBuilder` also ought to behave differently for each version, and I'm not sure our current fix of #847 does this completely (Does a v2 SenderBuilder sending to v1 URI honor pjos? it should). In order to do so I reckon we could create an actual `PjUri` type, rather than an alias, that enumerates over the versions when `check_pj_supported` or its replacement is called. In order to do *that* effectively by making sure the correct parameters are there and we're not just switching on the presence of a fragment, I think `UrlExt` also needs to check for the parameter presence and validity. The other issue with v1::Sender flow is that it doesn't use the generic typestate machine pattern to match v2, which would be nice as well but out of the scope of this PR. re: #809
2 parents fbaaa8d + 3e616b3 commit 5fe5db1

File tree

7 files changed

+466
-364
lines changed

7 files changed

+466
-364
lines changed

payjoin-ffi/src/send/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ impl InitInputsTransition {
9393
///
9494
///These parameters define how client wants to handle Payjoin.
9595
#[derive(Clone)]
96-
pub struct SenderBuilder(payjoin::send::v2::SenderBuilder<'static>);
96+
pub struct SenderBuilder(payjoin::send::v2::SenderBuilder);
9797

98-
impl From<payjoin::send::v2::SenderBuilder<'static>> for SenderBuilder {
99-
fn from(value: payjoin::send::v2::SenderBuilder<'static>) -> Self { Self(value) }
98+
impl From<payjoin::send::v2::SenderBuilder> for SenderBuilder {
99+
fn from(value: payjoin::send::v2::SenderBuilder) -> Self { Self(value) }
100100
}
101101

102102
impl SenderBuilder {
@@ -259,16 +259,16 @@ impl WithReplyKey {
259259
/// Data required for validation of response.
260260
/// This type is used to process the response. Get it from SenderBuilder's build methods. Then you only need to call .process_response() on it to continue BIP78 flow.
261261
#[derive(Clone)]
262-
pub struct V1Context(Arc<payjoin::send::v1::V1Context>);
263-
impl From<payjoin::send::v1::V1Context> for V1Context {
264-
fn from(value: payjoin::send::v1::V1Context) -> Self { Self(Arc::new(value)) }
262+
pub struct V1Context(Arc<payjoin::send::V1Context>);
263+
impl From<payjoin::send::V1Context> for V1Context {
264+
fn from(value: payjoin::send::V1Context) -> Self { Self(Arc::new(value)) }
265265
}
266266

267267
impl V1Context {
268268
///Decodes and validates the response.
269269
/// Call this method with response from receiver to continue BIP78 flow. If the response is valid you will get appropriate PSBT that you should sign and broadcast.
270270
pub fn process_response(&self, response: &[u8]) -> Result<String, ResponseError> {
271-
<payjoin::send::v1::V1Context as Clone>::clone(&self.0.clone())
271+
<payjoin::send::V1Context as Clone>::clone(&self.0.clone())
272272
.process_response(response)
273273
.map(|e| e.to_string())
274274
.map_err(Into::into)

payjoin/src/core/output_substitution.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,3 @@ pub enum OutputSubstitution {
44
Enabled,
55
Disabled,
66
}
7-
8-
impl OutputSubstitution {
9-
/// Combine two output substitution flags.
10-
///
11-
/// If both are enabled, the result is enabled.
12-
/// If one is disabled, the result is disabled.
13-
pub(crate) fn combine(self, other: Self) -> Self {
14-
match (self, other) {
15-
(Self::Enabled, Self::Enabled) => Self::Enabled,
16-
_ => Self::Disabled,
17-
}
18-
}
19-
}

0 commit comments

Comments
 (0)