Skip to content
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.

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

Allow P2PKH and P2SH scripts in coinjoin outputs #12190

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

lontivero
Copy link
Collaborator

Allow to register more scriptpubkyes to enable more use cases for coinjoins

@MaxHillebrand
Copy link
Collaborator

MaxHillebrand commented Jan 4, 2024

Nice, how about segwit v0 script P2WSH?

@lontivero
Copy link
Collaborator Author

Nice, how about segwit v0 script P2WSH?

One step at a time.

@kristapsk
Copy link
Collaborator

Concept ACK. With pay in coinjoins combined with big coinjoins of WabiSabi this makes sense.

Copy link
Collaborator

@turbolay turbolay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cACK, code LGTM

Are we planning to allow a way to do this in the RPC? How would it look like?


[DefaultValue(false)]
[JsonProperty(PropertyName = "AllowP2shOutputs", DefaultValueHandling = DefaultValueHandling.Populate)]
public bool AllowP2shOutputs { get; set; } = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why false by default? Is there some cases where it would be annoying for a coordinator to have this? I can only see upsides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. Next PR will switch them to true.

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cACK

@MaxHillebrand
Copy link
Collaborator

MaxHillebrand commented Jan 5, 2024

Partially closes #12189

Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lontivero
Copy link
Collaborator Author

@are we planning to allow a way to do this in the RPC? How would it look like?

With payincoinjoin you to use whatever script type is allowed.

@lontivero lontivero merged commit a4347fa into WalletWasabi:master Jan 5, 2024
7 checks passed
lontivero added a commit that referenced this pull request Jan 5, 2024
lontivero added a commit that referenced this pull request Jan 5, 2024
@lontivero lontivero mentioned this pull request Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants