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

[Bug]: Race condition during DKG #929

Open
3 tasks
djordon opened this issue Nov 23, 2024 · 4 comments · May be fixed by #927
Open
3 tasks

[Bug]: Race condition during DKG #929

djordon opened this issue Nov 23, 2024 · 4 comments · May be fixed by #927
Assignees
Labels
bug Something isn't working signer communication Communication across sBTC bootstrap signers. signer coordination The actions executed by the signer coordinator.

Comments

@djordon
Copy link
Contributor

djordon commented Nov 23, 2024

Bug - Race condition during DKG

1. Description

During distributed key generation (DKG) there is a possibility of a race condition where some signers successfully finish DKG while others do not. This can happen in some network topologies when some signers are much further from the coordinator.

1.1 Context & Purpose

We've been assuming that in scenarios where DKG doesn't finish successfully, the signers would just try again when the next bitcoin block arrived. Unfortunately, there seems to be cases where DKG finishes successfully for some but now for everyone.

2. Technical Details:

One proposed fix, suggested by @xoloki, was to coordinate DKG in the same way that signing rounds are handled. During signing rounds, such race conditions are not possible because of the signing protocol structure. So, structuring DKG similarly could solve this issue.

2.1 Acceptance Criteria:

  • Eliminate the possibility of race conditions during DKG
  • Generate a protocol diagram for DKG in WSTS
  • Generate a protocol diagram for signing rounds using WSTS

3. Related Issues and Pull Requests (optional):

There are two draft PRs for fixing this, one in the signers and another in WSTS. They are:

4. Addendum

My main "concern" between the current WSTS flow and the one proposed in the above draft PRs is the unknowns. I've looked through the WSTS code, but I do not have full picture of how the pieces fit together where I know what will happen if some assumption isn't upheld.

Also, @xoloki pointed out that some of the new message sizes are quite large and grow quickly. Their size is O(n * k) where n is the number of signers and k is the number of keys. But k is generally linear in n so this can be quite large.

After looking through the proposed changes, they seem simple enough. I'd summarize them as "have the coordinator re-distribute everything after receiving it from everyone else". The coordinator was already a point source of failure before, so those changes don't change that assumption. I'm still a little nervous so at the very least I'd like to see some protocol diagrams for how things are supposed to work going forward. Let's use excalidraw for creating these diagrams. I believe that we should have these anyway for such a critical part of the sBTC protocol.


We briefly spoke about some alternatives to the draft PRs, let's flesh some of them out as well. It doesn't need to be in code, but we should at least have a description of them.

@djordon djordon added bug Something isn't working signer coordination The actions executed by the signer coordinator. signer communication Communication across sBTC bootstrap signers. labels Nov 23, 2024
@djordon djordon added this to the sBTC Release Ready milestone Nov 23, 2024
@djordon djordon added this to sBTC Nov 23, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in sBTC Nov 23, 2024
@aldur
Copy link
Collaborator

aldur commented Nov 23, 2024

Thank you for the detailed description! Just a nit: isn't this a fix rather than a feature?

@djordon djordon changed the title [Feature]: Race condition during DKG [Bug]: Race condition during DKG Nov 23, 2024
@djordon
Copy link
Contributor Author

djordon commented Nov 23, 2024

Oh yeah, updated!

@cylewitruk
Copy link
Member

If we can structure DKG in the same way as tx signing then I think I would prefer that for two reasons:

  1. We know it works in our testnet env
  2. We can desribe both flows more-or-less the same

@djordon djordon moved this from Needs Triage to Todo in sBTC Nov 26, 2024
@xoloki xoloki linked a pull request Dec 3, 2024 that will close this issue
4 tasks
@djordon djordon moved this from Todo to In Review in sBTC Dec 9, 2024
@xoloki
Copy link
Contributor

xoloki commented Dec 19, 2024

depends on Trust-Machines/wsts#98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working signer communication Communication across sBTC bootstrap signers. signer coordination The actions executed by the signer coordinator.
Projects
Status: In Review
Development

Successfully merging a pull request may close this issue.

4 participants