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.

Sign up for GitHub

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

Jump to bottom

PKI: 3924 handle stateproof in rest api goal #3950

Merged
merged 8 commits into from
May 17, 2022
Merged

PKI: 3924 handle stateproof in rest api goal #3950

merged 8 commits into from
May 17, 2022

Conversation

AlgoStephenAkiki
Copy link
Contributor

@AlgoStephenAkiki AlgoStephenAkiki commented May 4, 2022 •
edited
Loading

Remove Node Key Monitoring Function

Resolves #2596

Removes function which calls loadParticipationKeys() every 60 seconds
since MakeFull() for the FullNode does the same thing

Install State Proof Keys

Resolves #3924

Installs state proof keys inside of algod after getting the binary blob

Removes temp file generation

Resolves #3106

Removes the ability to create temporary files from the REST interface.

@AlgoStephenAkiki AlgoStephenAkiki requested a review from winder May 4, 2022 17:55
@AlgoStephenAkiki AlgoStephenAkiki self-assigned this May 4, 2022
@AlgoStephenAkiki AlgoStephenAkiki linked an issue May 4, 2022 that may be closed by this pull request
Handle StateProof in REST API / goal #3924
Closed
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the 3924-handle-stateproof-in-rest-api-goal branch 6 times, most recently from 46f9745 to 691c485 Compare May 10, 2022 17:34
@AlgoStephenAkiki AlgoStephenAkiki linked an issue May 10, 2022 that may be closed by this pull request
Remove temporary REST partkey file management #3106
Closed
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2022 •
edited by codecov bot
Loading

Codecov Report

Merging #3950 (cdaf817) into sakiki/integration/restful-goal (2dfe250) will increase coverage by 0.01%.
The diff coverage is 20.00%.

@@                         Coverage Diff                         @@
##           sakiki/integration/restful-goal    #3950      +/-   ##
===================================================================
+ Coverage                            49.78%   49.80%   +0.01%     
===================================================================
  Files                                  409      409              
  Lines                                69168    69153      -15     
===================================================================
+ Hits                                 34437    34439       +2     
+ Misses                               31011    30996      -15     
+ Partials                              3720     3718       -2
Impacted Files Coverage Δ
libgoal/libgoal.go 2.74% <ø> (ø)
node/node.go 22.50% <20.00%> (-0.56%) ⬇️
data/transactions/verify/txn.go 44.15% <0.00%> (-0.87%) ⬇️
catchup/service.go 69.62% <0.00%> (ø)
network/wsPeer.go 68.88% <0.00%> (+0.83%) ⬆️
data/abi/abi_type.go 88.62% <0.00%> (+0.94%) ⬆️
ledger/tracker.go 74.45% <0.00%> (+1.29%) ⬆️
ledger/roundlru.go 96.22% <0.00%> (+5.66%) ⬆️

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 2dfe250...cdaf817. Read the comment docs.

winder
winder reviewed May 11, 2022
View reviewed changes
node/node.go Outdated
// Periodically check for new participation keys
go node.checkForParticipationKeys()

node.monitoringRoutinesWaitGroup.Add(2)
go node.txPoolGaugeThread()
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you still need to rebase the integration branch, there is a new ctx.Done argument here.

@AlgoStephenAkiki
Copy link
Contributor Author

@winder At this location: https://github.com/algorand/go-algorand/pull/3950/files#diff-375d57e386f20eaa5f09f02bb9d28bfc48ac3dca18d0325f59492208219e5618R896

Should it be account.RestoreParticipation() or account.RestoreParticipationWithSecrets()?

@winder
Copy link
Contributor

winder commented May 11, 2022

@winder At this location: https://github.com/algorand/go-algorand/pull/3950/files#diff-375d57e386f20eaa5f09f02bb9d28bfc48ac3dca18d0325f59492208219e5618R896

Should it be account.RestoreParticipation() or account.RestoreParticipationWithSecrets()?

The link isn't working for me, which file/line?

@AlgoStephenAkiki
Copy link
Contributor Author

@winder At this location: https://github.com/algorand/go-algorand/pull/3950/files#diff-375d57e386f20eaa5f09f02bb9d28bfc48ac3dca18d0325f59492208219e5618R896
Should it be account.RestoreParticipation() or account.RestoreParticipationWithSecrets()?

The link isn't working for me, which file/line?

node/node.go Line 896. in loadParticipationkeys() it uses the withSecrets() version of the function. Wondering if we should do that in node.go as well.

@AlgoStephenAkiki AlgoStephenAkiki requested a review from winder May 11, 2022 16:28
winder
winder approved these changes May 11, 2022
View reviewed changes
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

@AlgoStephenAkiki AlgoStephenAkiki force-pushed the sakiki/integration/restful-goal branch from 6439fff to de3c582 Compare May 12, 2022 16:31
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the 3924-handle-stateproof-in-rest-api-goal branch 2 times, most recently from f8bcfd8 to c78bb0c Compare May 12, 2022 16:53
@cce cce changed the title Enhancement: 3924 handle stateproof in rest api goal PKI: 3924 handle stateproof in rest api goal May 16, 2022
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the sakiki/integration/restful-goal branch from de3c582 to 2dfe250 Compare May 16, 2022 14:31
@AlgoStephenAkiki
Install State Proof Keys
25a2eef 
Resolves #3924

Installs state proof keys inside of algod after getting the binary blob
@AlgoStephenAkiki
Remove Node Key Monitoring Function
9b49a92 
Resolves #2596

Removes function which calls loadParticipationKeys() every 60 seconds
since MakeFull() for the FullNode does the same thing
@AlgoStephenAkiki
Testing
6e2cf9e
@AlgoStephenAkiki
PR comments
3f13cea
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the 3924-handle-stateproof-in-rest-api-goal branch from 06ed2a2 to 831086d Compare May 16, 2022 14:53
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the 3924-handle-stateproof-in-rest-api-goal branch from 831086d to 94f3036 Compare May 17, 2022 15:09
@AlgoStephenAkiki AlgoStephenAkiki merged commit 1eed267 into sakiki/integration/restful-goal May 17, 2022
@AlgoStephenAkiki AlgoStephenAkiki deleted the 3924-handle-stateproof-in-rest-api-goal branch May 17, 2022 21:08
algorandskiy pushed a commit that referenced this pull request Jun 6, 2022
@AlgoStephenAkiki @winder
PKI: Enable full usage of REST API to install participation keys (#4002)
febeb1e 
* Migrate goal account commands to REST API (#3916)
* PKI: 3924 handle stateproof in rest api goal (#3950)

Some additional refactoring:
* Remove Node Key Monitoring Function
* Removes function which calls loadParticipationKeys() every 60 seconds
  since MakeFull() for the FullNode does the same thing

Co-authored-by: Will Winder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winder winder winder approved these changes

Assignees

@AlgoStephenAkiki AlgoStephenAkiki

Labels
Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle StateProof in REST API / goal Remove temporary REST partkey file management
3 participants
@AlgoStephenAkiki @codecov-commenter @winder