[Merged by Bors] - Use dedicated PoST service for Proof generation #5061

fasmat · 2023-09-21T18:41:42Z

Motivation

Part of #5042
Merge after #5091

Changes

Generating poofs is now done via the GRPC API
- Foundation laid in [Merged by Bors] - Add PoST service to node #5091
- Upon connection PostClient is passed to services via the postConnectionListener interface. At the moment these are: activation::Builder and activation::NIPostBuilder who use it to generate a proof
- Instead of starting the PostSupervisor when PostServiceCmd is set it is started when StartSmeshing == true
- The connection does not require authentication (yet) - will be addressed in gRPC: add mTLS authentication to post service #5131
- Connection cannot handle multiple post services (yet)
NiPoSTBuilder doesn't verify PoST proofs any more
- PoST Service does this already before providing the proof
- additionally when publishing the ATX it goes through the ATX handler that validates the ATX again before broadcasting it.
Refactored tests in activation package to use new API
- Integration tests that do not use mocks for generating PoST proofs have been moved to activation/e2e to allow the use of the api/grpcserver package in them
- e2e tests spin up a post service using the post supervisor and query a proof from there
Slimmed down postSetupProvider interface:
- it is used by the activation::Builder and implemented by activation::PostSetupManager
- some of its functionality has been moved into PostClient (GenerateProof -> Proof)
Replaced go-spacemesh/log with zap in a few components in the activation package.

Test Plan

All existing tests involving proof generation have been migrated to use the new PoST service
New tests added to test the connection specifically.
Test added for custom types in config.

TODO

Explain motivation or link existing issue(s)
Test changes and document test plan
Update documentation as needed
Update changelog as needed

codecov · 2023-09-24T20:29:03Z

Codecov Report

Merging #5061 (584315d) into develop (fa44c7d) will increase coverage by 0.1%.
The diff coverage is 81.9%.

@@            Coverage Diff            @@
##           develop   #5061     +/-   ##
=========================================
+ Coverage     77.0%   77.2%   +0.1%     
=========================================
  Files          260     260             
  Lines        30575   30599     +24     
=========================================
+ Hits         23562   23636     +74     
+ Misses        5482    5423     -59     
- Partials      1531    1540      +9

Files	Coverage Δ
activation/post.go	`85.9% <100.0%> (-1.9%)`	⬇️
activation/post_types.go	`100.0% <100.0%> (ø)`
activation/post_verifier.go	`87.9% <100.0%> (-2.5%)`	⬇️
activation/validation.go	`86.7% <100.0%> (ø)`
common/types/nodeid.go	`100.0% <ø> (ø)`
config/config.go	`100.0% <100.0%> (+2.4%)`	⬆️
config/mainnet.go	`96.3% <100.0%> (+<0.1%)`	⬆️
config/presets/fastnet.go	`100.0% <100.0%> (ø)`
config/presets/standalone.go	`100.0% <100.0%> (ø)`
api/grpcserver/post_client.go	`77.9% <78.5%> (+0.5%)`	⬆️
... and 6 more

... and 7 files with indirect coverage changes

## Motivation Closes #4171 Occasionally the test will fail with an incorrect assertion of an error. The stream receive can fail either because the deadline was exceeded or due to bad timing for another (random) reason. Since it doesn't matter which return code is received, just that the stream ended I changed the assertion to also allow `code.Unkown`. ## Changes - change assertion to allow `codes.DeadlineExceeded` and `codes.Unkown` instead of just the former. - Additionally get rid of the global GRPC config. This will allow to run GRPC service tests in parallel (which will be used by #5061) ## Test Plan  ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed - [ ] Update [changelog](../CHANGELOG.md) as needed

## Motivation Closes #4171 Occasionally the test will fail with an incorrect assertion of an error. The stream receive can fail either because the deadline was exceeded or due to bad timing for another (random) reason. Since it doesn't matter which return code is received, just that the stream ended I changed the assertion to also allow `code.Unkown`. ## Changes - change assertion to allow `codes.DeadlineExceeded` and `codes.Unkown` instead of just the former. - Additionally get rid of the global GRPC config. This will allow to run GRPC service tests in parallel (which will be used by #5061) ## Test Plan  ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed - [x] Update [changelog](../CHANGELOG.md) as needed

fasmat · 2023-10-05T16:24:46Z

bors try

bors · 2023-10-05T17:34:44Z

try

Build failed:

systest-status

config/config.go

activation/post_supervisor_test.go

activation/post_supervisor_win.go

…t-api

fasmat · 2023-10-11T13:37:19Z

bors try

poszu

Approving as we agreed to continue refactoring in a follow-up.

bors · 2023-10-11T14:29:33Z

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

fasmat · 2023-10-11T14:38:13Z

bors merge

## Motivation Part of #5042 Merge after #5091 ## Changes - Generating poofs is now done via the GRPC API - Foundation laid in #5091 - Upon connection `PostClient` is passed to services via the `postConnectionListener` interface. At the moment these are: `activation::Builder` and `activation::NIPostBuilder` who use it to generate a proof - Instead of starting the `PostSupervisor` when `PostServiceCmd` is set it is started when `StartSmeshing == true` - The connection does not require authentication (yet) - will be addressed in #5131 - Connection cannot handle multiple post services (yet) - NiPoSTBuilder doesn't verify PoST proofs any more - PoST Service does this already before providing the proof - additionally when publishing the ATX it goes through the ATX handler that validates the ATX again before broadcasting it. - Refactored tests in `activation` package to use new API - Integration tests that do not use mocks for generating PoST proofs have been moved to `activation/e2e` to allow the use of the `api/grpcserver` package in them - `e2e` tests spin up a post service using the post supervisor and query a proof from there - Slimmed down `postSetupProvider` interface: - it is used by the `activation::Builder` and implemented by `activation::PostSetupManager` - some of its functionality has been moved into `PostClient` (`GenerateProof` -> `Proof`) - Replaced `go-spacemesh/log` with `zap` in a few components in the `activation` package. ## Test Plan - All existing tests involving proof generation have been migrated to use the new PoST service - New tests added to test the connection specifically. - Test added for custom types in config. ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed - [x] Update [changelog](../CHANGELOG.md) as needed

bors · 2023-10-11T15:26:29Z

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

## Motivation Follow up on #5061 (comment) This changes the `activation.Builder` and `activation.NIPostBuilder` to use `grpcservice.PostService` directly instead of the later signalling new and dropped connections to them. ## Changes - Inverse dependency between the three components as discussed in previous PR - During startup the `grpcservice.PostService` is now always initialized along the other node services, but only registered in the `grpcserver` when it is configured for at least one of the available listeners. ## Test Plan n/a ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed - [x] Update [changelog](../CHANGELOG.md) as needed

fasmat self-assigned this Sep 21, 2023

fasmat force-pushed the 5042-integrate-post-api branch 2 times, most recently from 4c86676 to 558aaa8 Compare September 22, 2023 17:41

fasmat force-pushed the 5042-integrate-post-api branch 3 times, most recently from fc378b4 to 249db8f Compare September 25, 2023 08:02

fasmat mentioned this pull request Sep 25, 2023

[Merged by Bors] - Fix flaky TestAccountMeshDataStream_comprehensive #5075

Closed

4 tasks

fasmat force-pushed the 5042-integrate-post-api branch from 1618557 to ca7ba55 Compare September 25, 2023 16:20

fasmat force-pushed the 5042-integrate-post-api branch 4 times, most recently from cf42665 to b1822e2 Compare September 26, 2023 09:20

fasmat mentioned this pull request Sep 26, 2023

[Merged by Bors] - Add PoST service to node #5091

Closed

4 tasks

fasmat force-pushed the 5042-integrate-post-api branch from f86d6c8 to b19e447 Compare October 4, 2023 17:51

fasmat changed the base branch from develop to 5042-add-post-service October 4, 2023 17:52

fasmat force-pushed the 5042-integrate-post-api branch from b19e447 to 32d95b0 Compare October 4, 2023 18:59

bors bot added a commit that referenced this pull request Oct 5, 2023

Try #5061:

ada8b8c

fasmat commented Oct 5, 2023

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

fasmat force-pushed the 5042-add-post-service branch from c0c5967 to 906f08a Compare October 6, 2023 12:50

fasmat force-pushed the 5042-integrate-post-api branch from 27a3497 to 97d882b Compare October 6, 2023 12:55

bors bot changed the base branch from 5042-add-post-service to develop October 6, 2023 15:28

fasmat force-pushed the 5042-integrate-post-api branch from 490ad72 to ff05d06 Compare October 6, 2023 16:17

fasmat changed the title ~~WiP: use dedicated PoST service for Proof generation~~ Use dedicated PoST service for Proof generation Oct 6, 2023

fasmat marked this pull request as ready for review October 6, 2023 17:00

fasmat requested a review from countvonzero as a code owner October 6, 2023 17:00

fasmat added 10 commits October 11, 2023 08:33

Replace Flags in proving options with RandomX mode

341fbbf

Deduplicate config, improve parsing and add tests

1650a90

Remove obsolete parameter

980711f

Update post service

d33e258

Remove scrypt options

a906dec

Fix supervisor for windows and make restartable

0a1805b

Fix tests

90305db

revert to alpha2

b049226

Fix test on windows

6904bf8

Start and Stop smeshing via GRPC uses supervisor

21064a7

fasmat force-pushed the 5042-integrate-post-api branch from fb39c0b to 21064a7 Compare October 11, 2023 08:34

fasmat requested a review from poszu October 11, 2023 08:46

poszu reviewed Oct 11, 2023

View reviewed changes

activation/post_supervisor_test.go Show resolved Hide resolved

activation/post_supervisor_win.go Outdated Show resolved Hide resolved

fasmat added 2 commits October 11, 2023 13:26

Merge remote-tracking branch 'origin/develop' into 5042-integrate-pos…

0677787

…t-api

Update review feedback

584315d

fasmat force-pushed the 5042-integrate-post-api branch from dd91ee8 to 584315d Compare October 11, 2023 13:29

bors bot added a commit that referenced this pull request Oct 11, 2023

Try #5061:

cbc92b5

poszu approved these changes Oct 11, 2023

View reviewed changes

bors bot changed the title ~~Use dedicated PoST service for Proof generation~~ [Merged by Bors] - Use dedicated PoST service for Proof generation Oct 11, 2023

bors bot closed this Oct 11, 2023

bors bot deleted the 5042-integrate-post-api branch October 11, 2023 15:26

fasmat mentioned this pull request Oct 11, 2023

[Merged by Bors] - Refactor gRPC PostService to be injected into services #5145

Closed

4 tasks

fasmat mentioned this pull request Oct 13, 2023

activation: support multiple smeshers #5149

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Use dedicated PoST service for Proof generation #5061

[Merged by Bors] - Use dedicated PoST service for Proof generation #5061

fasmat commented Sep 21, 2023 •

edited

Loading

codecov bot commented Sep 24, 2023 •

edited

Loading

fasmat commented Oct 5, 2023

bors bot commented Oct 5, 2023

fasmat commented Oct 11, 2023

poszu left a comment

bors bot commented Oct 11, 2023

fasmat commented Oct 11, 2023

bors bot commented Oct 11, 2023

[Merged by Bors] - Use dedicated PoST service for Proof generation #5061

[Merged by Bors] - Use dedicated PoST service for Proof generation #5061

Conversation

fasmat commented Sep 21, 2023 • edited Loading

Motivation

Changes

Test Plan

TODO

codecov bot commented Sep 24, 2023 • edited Loading

Codecov Report

fasmat commented Oct 5, 2023

bors bot commented Oct 5, 2023

try

fasmat commented Oct 11, 2023

poszu left a comment

Choose a reason for hiding this comment

bors bot commented Oct 11, 2023

try

fasmat commented Oct 11, 2023

bors bot commented Oct 11, 2023

fasmat commented Sep 21, 2023 •

edited

Loading

codecov bot commented Sep 24, 2023 •

edited

Loading