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

Send Signer results over a channel #3923

Merged
merged 7 commits into from
Sep 20, 2023

Conversation

xoloki
Copy link
Collaborator

@xoloki xoloki commented Sep 14, 2023

Description

Right now the only way to find out what a Signer has done is to stop it and get the cached results.

Add another channel which lets Signer send results as they come in.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #3923 (32e8dab) into feat/stacks-signer (99c1218) will decrease coverage by 0.01%.
Report is 1 commits behind head on feat/stacks-signer.
The diff coverage is 0.00%.

@@                  Coverage Diff                   @@
##           feat/stacks-signer    #3923      +/-   ##
======================================================
- Coverage                0.16%    0.16%   -0.01%     
======================================================
  Files                     339      339              
  Lines                  290170   290260      +90     
======================================================
  Hits                      469      469              
- Misses                 289701   289791      +90     
Files Changed Coverage Δ
libsigner/src/runloop.rs 0.00% <0.00%> (ø)
libsigner/src/tests/mod.rs 0.00% <0.00%> (ø)
stacks-signer/src/crypto/frost.rs 0.00% <0.00%> (ø)
stacks-signer/src/crypto/mod.rs 0.00% <ø> (ø)
stacks-signer/src/main.rs 0.00% <0.00%> (ø)
stacks-signer/src/runloop.rs 0.00% <0.00%> (ø)
testnet/stacks-node/src/tests/signer.rs 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

libsigner/src/session.rs Outdated Show resolved Hide resolved
stacks-signer/src/main.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@jferrant jferrant left a comment

Choose a reason for hiding this comment

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

I updated the state machine substantially and fixed the test to actually tear down the signers at the end. My initial check on the meaty portion (following node and signer setup), it takes ~15 seconds to run DKG and Sign a message.

@jferrant jferrant force-pushed the chore/signer-sends-results branch 3 times, most recently from 040bb55 to 17e8615 Compare September 15, 2023 23:17
xoloki and others added 3 commits September 18, 2023 16:49
…s interested; remove sleep calls in test since we know when it's done
Signed-off-by: Jacinta Ferrant <[email protected]>
@xoloki
Copy link
Collaborator Author

xoloki commented Sep 19, 2023

I fixed the build so we don't expect any additional types to impl Clone, and rebased on the merged signer-first PR. This PR should be ready to go now.

stacks-signer/src/main.rs Outdated Show resolved Hide resolved
Signed-off-by: Jacinta Ferrant <[email protected]>
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for addressing my comments!

@jferrant jferrant merged commit 0189324 into feat/stacks-signer Sep 20, 2023
1 check passed
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants