[Merged by Bors] - Support multiple signers in beacon #5158

poszu · 2023-10-15T13:10:11Z

Motivation

Part of spacemeshos/pm#261
Closes #5088

Changes

beacon protocol driver supports registering new keys to participate in the protocol
when a new epoch starts, the protocol selects active signers that will participate
state is changed to contain the set of active protocol participants. A participant session consists of:
- signer
- VRF nonce
weakcoin supports multiple participants. Each one publishes the same proposal.

Test Plan

Existing unit-tests were extended to run beacon and weakcoin with multiple signers per node.

codecov · 2023-10-15T13:19:04Z

Codecov Report

Merging #5158 (8ee47e8) into develop (bc34962) will increase coverage by 0.0%.
Report is 3 commits behind head on develop.
The diff coverage is 80.5%.

@@           Coverage Diff           @@
##           develop   #5158   +/-   ##
=======================================
  Coverage     77.7%   77.7%           
=======================================
  Files          259     259           
  Lines        30750   30781   +31     
=======================================
+ Hits         23898   23945   +47     
+ Misses        5361    5347   -14     
+ Partials      1491    1489    -2

Files	Coverage Δ
beacon/proposal_set.go	`28.5% <100.0%> (ø)`
beacon/state.go	`100.0% <100.0%> (ø)`
node/node.go	`67.3% <100.0%> (ø)`
signing/vrf.go	`100.0% <ø> (+13.3%)`	⬆️
beacon/handlers.go	`87.1% <75.0%> (+<0.1%)`	⬆️
beacon/weakcoin/weak_coin.go	`80.7% <93.7%> (+0.3%)`	⬆️
beacon/beacon.go	`82.4% <78.7%> (+1.6%)`	⬆️

... and 8 files with indirect coverage changes

beacon/beacon.go

beacon/handlers.go

fasmat · 2023-10-16T11:13:31Z

beacon/beacon.go

+func (s *participant) Id() signerID {
+	return signerID(s.signer.NodeID())
+}
+
+type signerID types.NodeID
+
+func (id signerID) Field() log.Field {
+	return log.ShortStringer("id", types.NodeID(id))
+}


I'm not sure about introducing a new type here just to change the logging behavour of NodeID only for beacon. We should consider (NodeID) Field() to return a log.ShortStringer and have that behaviour everywhere.

What's wrong with having a private type here? Btw, it changed the key as well (NodeID logs "node_id", not "id").

The problem isn't the private type, it's that beacon now logs NodeID differently from all other components, which makes it harder to read them imo.

IMHO, it doesn't make sense to log messages per-signer with a "node_id" field, it has nothing to do with "node" anymore (it's not a node ID in this context, there are many IDs governed by a single node).

Perhaps instead, the NodeID type should be dropped entirely in favor of a SignerID or something similar? But that's a bigger refactor.

beacon/beacon.go

Avoid infallibile error checks.

Closes #5156

… 0.12.0 (#5159) Bumps github.com/multiformats/go-multiaddr from 0.11.0 to 0.12.0.

this threshold used to determine if peers are seen as equally responsive within certain threshold. parametrizing it to run some experiments

still able to reproduce the bug below. > 2023-10-02T15:28:14.002+0200 WARN fd68b.sync mesh failed to process layer from sync {"node_id": "fd68b9397572556c2f329f3e5af2faf23aef85dbbbb7e38447fae2f4ef38899f", "module": "sync", "sessionId": "29422935-68d6-47d1-87a8-02293aa181f3", "layer_id": 23104, "errmsg": "requested layer 8063 is before evicted 13102", "name": "sync"} in the change i removed Results method from tortoise, and instead keeping Updates until mesh applies block and notifies tortoise that the block was applied, only then tortoise is free to evict results from memory. OnApplied has layer and opinion in the signature. Opinion is a unique representation of consensus state. If it changed while blocks were applied it means that history changed and we should not evict pending results.

closes: #5087 data is separated into shared data (beacon and active set) and signer specific data. both beacon and activeset are used from shared data, until smeshers generated a reference ballot. once any smesher generated a ballot it will be using data recorded in the reference ballot. build method now loops over all signers (copied at the start of the layer). there are parts that have to be run once for all signers and parts that makes sense to run in parallel. serial parts: - loading share data (beacon and active set) - deciding on mesh hash - tally votes & encode votes tortoise calls parallel parts: - loading data (it can be also run serially, but it was convenient to run it in parallel) - computing eligibilities (this is done once per node startup) - selecting txs - publishing proposal. this is the most important to avoid blocking serially in Publish while it runs validation worker pool (errgroup) is limited by number of cores as there is no network requests during parallel work.

## Motivation Not specifying a version leads the jobs to download the latest version on every build. Since a new version of gcloud is released on average once per week and those installations take up ~550 MB on our runners and are not removed automatically this eventually fills up the disks of the runners. ## Changes - Specify a gcloud version in `systests` and `release` jobs. This way only that version is downloaded and re-used on every subsequent run. - We can manually update gcloud to a newer version if needed. ## 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

## Motivation Instead of trying to access the local PoET via `0.0.0.0:10010` access it via `127.0.0.1:10010`. The first address seems to cause problems on some Windows systems ## Changes Updated standalone config to use 127.0.0.1 for local addresses instead of 0.0.0.0 ## 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

…t-multiple-ids

poszu · 2023-10-18T07:39:36Z

bors try

beacon/weakcoin/weak_coin.go

bors · 2023-10-18T08:42:19Z

try

Build failed:

systest-status

poszu · 2023-10-18T16:44:22Z

bors try

bors · 2023-10-18T17:10:52Z

try

Build failed:

ci-status

poszu · 2023-10-18T19:07:37Z

bors merge

## Motivation Part of spacemeshos/pm#261 Closes #5088 ## Changes - beacon protocol driver supports registering new keys to participate in the protocol - when a new epoch starts, the protocol selects active signers that will participate - state is changed to contain the set of active protocol participants. A participant session consists of: * signer * VRF nonce - weakcoin supports multiple participants. Each one publishes the same proposal. ## Test Plan Existing unit-tests were extended to run beacon and weakcoin with multiple signers per node. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dmitry Shulyak <[email protected]> Co-authored-by: Matthias Fasching <[email protected]>

bors · 2023-10-18T19:21:39Z

Build failed:

ci-status

poszu · 2023-10-18T19:48:36Z

Failure in unrelated UT

bors merge

## Motivation Part of spacemeshos/pm#261 Closes #5088 ## Changes - beacon protocol driver supports registering new keys to participate in the protocol - when a new epoch starts, the protocol selects active signers that will participate - state is changed to contain the set of active protocol participants. A participant session consists of: * signer * VRF nonce - weakcoin supports multiple participants. Each one publishes the same proposal. ## Test Plan Existing unit-tests were extended to run beacon and weakcoin with multiple signers per node. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dmitry Shulyak <[email protected]> Co-authored-by: Matthias Fasching <[email protected]>

bors · 2023-10-18T20:35:17Z

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.

poszu requested review from countvonzero, dshulyak and fasmat as code owners October 15, 2023 13:10

Support multiple signers in beacon

8917cae

poszu force-pushed the 5088-beacon-support-multiple-ids branch from 9912352 to 8917cae Compare October 15, 2023 13:39

Satisfy linter

926d3e2

dshulyak reviewed Oct 16, 2023

View reviewed changes

beacon/beacon.go Outdated Show resolved Hide resolved

beacon/beacon.go Outdated Show resolved Hide resolved

beacon/handlers.go Outdated Show resolved Hide resolved

beacon/handlers.go Outdated Show resolved Hide resolved

beacon/handlers.go Outdated Show resolved Hide resolved

fasmat reviewed Oct 16, 2023

View reviewed changes

poszu and others added 11 commits October 17, 2023 12:33

Reuse picked epoch state where it makes sense

0c0492a

Avoid infallibile error checks.

Don't publish proposals on redundant goroutine

2165ac7

Fix TestPrune unit test (#5157)

9dc721b

Closes #5156

build(deps): Bump github.com/multiformats/go-multiaddr from 0.11.0 to…

aafbd5d

… 0.12.0 (#5159) Bumps github.com/multiformats/go-multiaddr from 0.11.0 to 0.12.0.

fetch: parametrize success rate threshold (#5161)

3c2ebdb

this threshold used to determine if peers are seen as equally responsive within certain threshold. parametrizing it to run some experiments

Support multiple signers in weakcoin

7821cd7

Removed LittleEndian() from vrfSigner interface

cf8ad8b

poszu requested review from dshulyak and fasmat October 17, 2023 17:53

Merge remote-tracking branch 'origin/develop' into 5088-beacon-suppor…

7227342

…t-multiple-ids

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

Try #5158:

93751c7

dshulyak approved these changes Oct 18, 2023

View reviewed changes

beacon/weakcoin/weak_coin.go Outdated Show resolved Hide resolved

Fix beacon regression in systests

7e58d7d

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

Try #5158:

e417eb3

Remove usages of log.FieldNamed in beacon pkg

8ee47e8

bors bot changed the title ~~Support multiple signers in beacon~~ [Merged by Bors] - Support multiple signers in beacon Oct 18, 2023

bors bot closed this Oct 18, 2023

bors bot deleted the 5088-beacon-support-multiple-ids branch October 18, 2023 20:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Support multiple signers in beacon #5158

[Merged by Bors] - Support multiple signers in beacon #5158

poszu commented Oct 15, 2023 •

edited

Loading

codecov bot commented Oct 15, 2023 •

edited

Loading

fasmat Oct 16, 2023

poszu Oct 17, 2023

fasmat Oct 17, 2023

poszu Oct 17, 2023

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

[Merged by Bors] - Support multiple signers in beacon #5158

[Merged by Bors] - Support multiple signers in beacon #5158

Conversation

poszu commented Oct 15, 2023 • edited Loading

Motivation

Changes

Test Plan

codecov bot commented Oct 15, 2023 • edited Loading

Codecov Report

fasmat Oct 16, 2023

Choose a reason for hiding this comment

poszu Oct 17, 2023

Choose a reason for hiding this comment

fasmat Oct 17, 2023

Choose a reason for hiding this comment

poszu Oct 17, 2023

Choose a reason for hiding this comment

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

try

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

try

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

poszu commented Oct 18, 2023

bors bot commented Oct 18, 2023

poszu commented Oct 15, 2023 •

edited

Loading

codecov bot commented Oct 15, 2023 •

edited

Loading