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

activation: support multiple smeshers #5149

Closed
1 task done
Tracked by #259 ...
fasmat opened this issue Oct 12, 2023 · 3 comments
Closed
1 task done
Tracked by #259 ...

activation: support multiple smeshers #5149

fasmat opened this issue Oct 12, 2023 · 3 comments

Comments

@fasmat
Copy link
Member

fasmat commented Oct 12, 2023

Description

To fully utilise the separation of PoST from go-spacemesh a node needs to be able to handle multiple post services simultaneously.

Acceptance criteria

  • Allow multiple PoST services to connect via the grpc interface.
  • Code in the activation package needs to be updated to handle multiple identities correctly.
    • The activation package builds an ATX for every identity that is set up for the node and has a post service connected
    • Only one post service per identity can connect to a node
  • Connections from PoST services over public interfaces have to be authenticated with mTLS
    • Assume that both sides have each others certificate

Implementation hints

  • The general design of the setup is described here: Post as a service pm#259
  • Consider storing the nipost builder state, - challenge, and initial post in the DB instead of on disk (post.bin, nipost_challenge.bin and nipost_builder_state.bin).
    • There will be at least one initial post and at least one nipost challenge for every identity managed by the node
    • Also consider splitting the builder state into one state for each identiy & poet combination, so they can be tracked individually.
    • Add second database for local state #5206
@fasmat fasmat self-assigned this Oct 12, 2023
@fasmat fasmat moved this to 🔖 Next in Dev team kanban Oct 12, 2023
@fasmat fasmat changed the title Allow multiple PoST services to connect to a spacemesh node activation: support multiple smeshers Oct 12, 2023
@dshulyak
Copy link
Contributor

ideally we also should skip post validation for self-published atxs

@fasmat
Copy link
Member Author

fasmat commented Oct 13, 2023

ideally we also should skip post validation for self-published atxs

validation in the NiPost builder was already removed in #5061. The activation package trusts the post service to deliver a valid proof and creates an ATX with it. That ATX however will still be validated (including proof validation) before/as part of broadcasting it, I think.

@fasmat fasmat moved this from 🔖 Next to 🏗 Doing in Dev team kanban Oct 19, 2023
bors bot pushed a commit that referenced this issue Oct 23, 2023
## Motivation
Part of #5149 

This extends the `PostClient` to be able to fetch metadata about the PoST from the service

## Changes
- Add method to fetch metadata from PoST service

## Test Plan
- Tests were added for new functionality with and without TLS

## TODO
<!-- This section should be removed when all items are complete -->
- [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 bot pushed a commit that referenced this issue Oct 25, 2023
## Motivation
Part of #5149  merge after #5186 & #5189

This moves the initialization code into the PoST supervisor. The ATX builder should not concern itself with initialization:
- A remote PoST service that connects is expected to already be fully initialized 
- In supervised mode the supervisor takes care of initialization

This allows to simplify the ATX builder, which has partially already been done in this PR and will continue in future PRs.

## Changes
- Removed dependency on `postSetupManager` from `atxBuilder`
- Simplified `atxBuilder`
  - Initial Proofs are not verified any more (PoST service is trusted)
  - Information about the PoST is fetched from the PoST service rather than accessed via the `postSetupManager`
- Simplified `postSetupManager`
  - Since meta information is now fetched via the client everything besides `StartSession`, `StopSession` and `Reset` has been removed from the interface
  - `postSetupManager` isn't used by the gRPC `SmeshingService` any more, instead it's now used by `PostSupervisor` to init a supervised node before starting it

For next PR: 
- change `atxBuilder` from `Start`/`Stop` to `Run`
  - a post service connecting triggers the builder loop (one per connected service)
  - simplify `atxBuilder` further and consider merging `nipostBuilder` into it.

## Test Plan
- added new tests to PoST supervisor to cover new functionality
- updated existing tests

## TODO
<!-- This section should be removed when all items are complete -->
- [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 bot pushed a commit that referenced this issue Oct 25, 2023
## Motivation
Part of #5149  merge after #5186 & #5189

This moves the initialization code into the PoST supervisor. The ATX builder should not concern itself with initialization:
- A remote PoST service that connects is expected to already be fully initialized 
- In supervised mode the supervisor takes care of initialization

This allows to simplify the ATX builder, which has partially already been done in this PR and will continue in future PRs.

## Changes
- Removed dependency on `postSetupManager` from `atxBuilder`
- Simplified `atxBuilder`
  - Initial Proofs are not verified any more (PoST service is trusted)
  - Information about the PoST is fetched from the PoST service rather than accessed via the `postSetupManager`
- Simplified `postSetupManager`
  - Since meta information is now fetched via the client everything besides `StartSession`, `StopSession` and `Reset` has been removed from the interface
  - `postSetupManager` isn't used by the gRPC `SmeshingService` any more, instead it's now used by `PostSupervisor` to init a supervised node before starting it

For next PR: 
- change `atxBuilder` from `Start`/`Stop` to `Run`
  - a post service connecting triggers the builder loop (one per connected service)
  - simplify `atxBuilder` further and consider merging `nipostBuilder` into it.

## Test Plan
- added new tests to PoST supervisor to cover new functionality
- updated existing tests

## TODO
<!-- This section should be removed when all items are complete -->
- [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
Copy link
Member Author

fasmat commented Jan 23, 2024

Removing the dedicated validation of the NIPostBuilder lead to some (rare) cases where publishing of an ATX might result in weird behaviour:

  • if the activation.Builder successfully publishes an ATX but fails to persist that it did, it might publish a second ATX causing the node to create a malfeasance proof against itself
    • this should be changed to instead fail the ATX verification but without creating the malfeasance proof against oneself
  • there were cases reported where the broadcast of the ATX failed (due to a falling validation by the activation handler). The activation.Builder however seems to still consider it a successful Publication and continues with the next ATX.
    • this won't lead to any adverse effects for the node (besides missing rewards - but the ATX would have been invalid anyway).
    • This is more a nuisance as the logs won't report what happened correctly: the NIPostBuilder and activation.Builder both report that they were successful and only the activation.Handler reports that it failed to validate an ATX (which appears to be an incoming ATX, but with she SmesherID being the same as the Nodes ID)
    • here the logging should be improved to more easily identify when this happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
2 participants