-
Notifications
You must be signed in to change notification settings - Fork 492
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
Publish witnesses only after signing phase is completed [issue 11342] #11498
Conversation
I think this doesn't worth the effort and extra complexity. I think at this point it is perfectly okay to remove the witnesses completely. Clarification: I am not against this PR btw. |
Any update? |
Hi @lontivero. The pull-request is still in a draft state. Could you please clarify to me - is this solution ok, or do you want to not publish the witnesses at all? |
This is okay. However I think it would be simpler and cleaner to simply remove the witnesses completely. |
Ok, sorry, I think this could be confusing for you so, let me clarify it once and forever: this approach (PR) is okay. Continue in this way. |
Ok, this PR is thus ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be good to have a test for this. It could be also okay to modify an existing one.
WalletWasabi/WabiSabi/Models/MultipartyTransaction/SigningState.cs
Outdated
Show resolved
Hide resolved
WalletWasabi/WabiSabi/Models/MultipartyTransaction/SigningState.cs
Outdated
Show resolved
Hide resolved
WalletWasabi/WabiSabi/Models/MultipartyTransaction/SigningState.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congratulations for your first PR @M1nd3r. Nice test.
Close #11342
Fix - Publish witnesses only after signing phase is completed