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

Implemented New REST interfaces #3099

Merged
merged 4 commits into from
Oct 21, 2021
Merged

Implemented New REST interfaces #3099

merged 4 commits into from
Oct 21, 2021

Conversation

AlgoStephenAkiki
Copy link
Contributor

Resolves #2656

Implemented Handlers and e2e test for Participation ID REST:

GET (Both specific and list of all installed)
POST (Aka install)
DELETE

Summary

Test Plan

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2021

Codecov Report

Merging #3099 (6013baa) into feature/partkey (ee51a98) will decrease coverage by 0.05%.
The diff coverage is 0.79%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           feature/partkey    #3099      +/-   ##
===================================================
- Coverage            44.01%   43.95%   -0.06%     
===================================================
  Files                  394      394              
  Lines                87341    87463     +122     
===================================================
+ Hits                 38443    38445       +2     
- Misses               42812    42930     +118     
- Partials              6086     6088       +2     
Impacted Files Coverage Δ
daemon/algod/api/server/v2/handlers.go 0.00% <0.00%> (ø)
daemon/algod/api/server/v2/test/helpers.go 74.81% <0.00%> (-2.29%) ⬇️
node/node.go 24.33% <0.00%> (-2.61%) ⬇️
data/account/participationRegistry.go 78.04% <11.11%> (-1.84%) ⬇️
ledger/blockqueue.go 83.90% <0.00%> (-1.15%) ⬇️
data/transactions/verify/txn.go 43.42% <0.00%> (-0.88%) ⬇️
catchup/service.go 69.35% <0.00%> (ø)
network/wsPeer.go 74.35% <0.00%> (+0.77%) ⬆️
cmd/algoh/blockWatcher.go 80.95% <0.00%> (+3.17%) ⬆️

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 ee51a98...6013baa. Read the comment docs.

Resolves #2656

Implemented Handlers and e2e test for Participation ID REST:

GET (Both specific and list of all installed)
POST (Aka install)
DELETE
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.

Looks good, left some initial feedback

// TODO add this
"VoteID": crypto.OneTimeSignatureVerifier{},
// TODO add this
"SelectionID": crypto.VRFVerifier{},
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,49 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Other than a few nits, this test looks really nice. This is going to be a nice template for when the storage changes and we get new error cases to check for.

@@ -894,6 +920,11 @@ func (node *AlgorandFullNode) InstallParticipationKey(partKeyBinary *[]byte) (ac
// Tell the AccountManager about the Participation (dupes don't matter) so we ignore the return value
_ = node.accountManager.AddParticipation(partkey)
Copy link
Contributor

Choose a reason for hiding this comment

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

On closer inspection, AddParticipation returns a bool not an error.:

// The return value indicates if the key has been added (true) or    
// if this is a duplicate key (false).     

The code would currently overwrite the duplicate key, I think that's probably fine. In the future when we stop writing the file to begin with, it would just be a no-op.

# $5 - expected HTTP status code to check
# $6 - match result
# $7... - substring(s) that should be in the response
function verify {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@AlgoStephenAkiki AlgoStephenAkiki marked this pull request as ready for review October 21, 2021 11:55
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.

Looks great, thanks for the changes

@winder winder merged commit f86207e into algorand:feature/partkey Oct 21, 2021
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.

Implement New Participation Key REST endpoints
3 participants