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

Use context instead of an atomic variable #3599

Conversation

tsachiherman
Copy link
Contributor

Summary

Switch the usage of a atomic variable with a context.

Test Plan

The updated tests seems to be failing due to unrelated issues, and these are fixed now.

@tsachiherman tsachiherman requested review from winder and id-ms February 8, 2022 23:55
@tsachiherman tsachiherman self-assigned this Feb 8, 2022
@tsachiherman tsachiherman requested a review from cce February 9, 2022 00:02
@@ -240,7 +241,7 @@ func TestSigning(t *testing.T) {

err = signer.GetVerifier().Verify(start+5, hashable, sig)
a.Error(err)
a.Contains(err.Error(), ErrSignatureSchemeVerificationFailed)
a.True(errors.Is(err, ErrSignatureSchemeVerificationFailed))
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2022

Codecov Report

Merging #3599 (7ab2c45) into feature/dilithium-scheme-integration (de20e30) will increase coverage by 18.85%.
The diff coverage is 33.33%.

Impacted file tree graph

@@                            Coverage Diff                            @@
##           feature/dilithium-scheme-integration    #3599       +/-   ##
=========================================================================
+ Coverage                                 29.04%   47.90%   +18.85%     
=========================================================================
  Files                                       381      381               
  Lines                                     61878    61879        +1     
=========================================================================
+ Hits                                      17975    29643    +11668     
+ Misses                                    41451    28807    -12644     
- Partials                                   2452     3429      +977     
Impacted Files Coverage Δ
crypto/merklesignature/keysBuilder.go 71.05% <33.33%> (+6.18%) ⬆️
network/wsPeer.go 68.61% <0.00%> (+0.55%) ⬆️
daemon/algod/api/server/v1/handlers/handlers.go 0.62% <0.00%> (+0.62%) ⬆️
nodecontrol/kmdControl.go 0.80% <0.00%> (+0.80%) ⬆️
data/committee/credential.go 39.70% <0.00%> (+1.47%) ⬆️
cmd/algofix/main.go 1.50% <0.00%> (+1.50%) ⬆️
agreement/proposalTracker.go 95.16% <0.00%> (+1.61%) ⬆️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
netdeploy/remote/deployedNetwork.go 19.00% <0.00%> (+2.11%) ⬆️
rpcs/txService.go 54.25% <0.00%> (+2.12%) ⬆️
... and 219 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de20e30...7ab2c45. Read the comment docs.

@Aharonee Aharonee merged commit 8c625b7 into algorand:feature/dilithium-scheme-integration Feb 9, 2022
PhearZero pushed a commit to PhearNet/crypto that referenced this pull request Jan 17, 2025
Use context instead of an atomic variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants