-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: use structured concurrency for tss processes #160
Conversation
Go Test coverage is 66.6 %\ ✨ ✨ ✨ |
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.
Hey all, I have looked at the structured concurrency changes and I think think this is on a good path. My main comments relate to the way the goroutines are started around the TSS code (they do not only apply to the keygen code but resharing.go and signing.go as well). Also there still is an error channel being passed around, which could probably be removed by just returning the error.
I left some more comments in other places that I think would further improve the robustness of the concurrent code.
Go Test coverage is 66.7 %\ ✨ ✨ ✨ |
Go Test coverage is 66.9 %\ ✨ ✨ ✨ |
Hey, thanks for the comments. |
Hey @MakMuftic @keks is unavailable for the time being, however he passed along that the PR looks good to approve. If you want to add me @bashirabuamr I can approve it on his behalf. We will mark it resolved in the Final Audit Report. |
Go Test coverage is 67.0 %\ ✨ ✨ ✨ |
Go Test coverage is 67.0 %\ ✨ ✨ ✨ |
This PR implements:
Sorry for the big change, but there were some problems with memory leaks before so wanted to make sure everything is properly cleaned up now when running singing, keygen and refresh.
@keks
The long running routines in the tss process are now running under the same context pool of routines and when 1 errors out or cancel is called manually they are all closed.
ExecuteProposals
function now waits until coordinatorExecute
is over which waits until tss process is finished.Related Issue Or Context
Closes: #159
How Has This Been Tested? Testing details.
Types of changes