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

Make the random beacon suck less #3180

Merged
merged 3 commits into from
Feb 1, 2021
Merged

Make the random beacon suck less #3180

merged 3 commits into from
Feb 1, 2021

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Aug 10, 2020

  • Implement PVSS
  • Rewrite all the consensus bits
    • Switch to unified epochtime/beacon
    • epochtime
    • beacon
  • Write the new PVSS node worker
  • Alter the tooling to support the new parameters
    • oasis-node genesis
    • oasis-test-runner
  • Test
  • Backfill protocol error handling
    • Slashing
    • Worker should retry submitting failed tx(es)
    • The beacon tx(es) should cost gas
    • Disable new node eligibility for next election on end of commit phase
    • Round failure should disable runtimes
  • Review/pending changes
    • Unify Threshold/PVSSThreshold
    • Refactoring required to support the transition

To be done later (file issues after merge):

I'm not the most thrilled by the performance from the PVSS scheme when used without pairing based crypto, but it has several nice properties:

  • It doesn't chew up ridiculous amounts of consensus state, even with a relatively large number of nodes.
  • All protocol messages can be independently verified for correctness, on-chain.
  • The final entropy generation can be pushed on chain.
  • The number of broadcast messages (transactions) is not massive.
  • Someone else implemented dleq based PVSS so it's easy to implement.

@Yawning Yawning force-pushed the yawning/feature/scrape branch 2 times, most recently from 5f073ab to a6a3422 Compare August 10, 2020 13:57
@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #3180 (fc3cc5f) into master (5b261e5) will decrease coverage by 0.58%.
The diff coverage is 59.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3180      +/-   ##
==========================================
- Coverage   67.38%   66.80%   -0.59%     
==========================================
  Files         390      391       +1     
  Lines       36687    38122    +1435     
==========================================
+ Hits        24723    25468     +745     
- Misses       8511     9000     +489     
- Partials     3453     3654     +201     
Impacted Files Coverage Δ
go/common/node/node.go 65.48% <ø> (ø)
go/consensus/api/api.go 14.28% <ø> (ø)
go/consensus/tendermint/abci/mux_mock.go 0.00% <ø> (ø)
go/consensus/tendermint/api/state.go 60.81% <ø> (ø)
...onsensus/tendermint/apps/governance/state/state.go 73.23% <ø> (ø)
...consensus/tendermint/apps/keymanager/keymanager.go 66.25% <ø> (ø)
go/consensus/tendermint/apps/staking/query.go 76.92% <ø> (+3.84%) ⬆️
...nsensus/tendermint/apps/staking/signing_rewards.go 20.00% <ø> (ø)
go/consensus/tendermint/apps/staking/staking.go 51.70% <ø> (-1.37%) ⬇️
...nsus/tendermint/apps/supplementarysanity/checks.go 42.78% <0.00%> (+1.06%) ⬆️
... and 113 more

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 5b261e5...fc3cc5f. Read the comment docs.

@Yawning Yawning force-pushed the yawning/feature/scrape branch 13 times, most recently from 00d7fa0 to ccec498 Compare August 14, 2020 08:55
@Yawning Yawning force-pushed the yawning/feature/scrape branch 7 times, most recently from 7e68e40 to c89c478 Compare August 26, 2020 11:07
@Yawning Yawning force-pushed the yawning/feature/scrape branch 6 times, most recently from 34a1283 to ce2e057 Compare August 31, 2020 11:22
@Yawning Yawning force-pushed the yawning/feature/scrape branch 8 times, most recently from a8a0610 to 5eb6231 Compare January 19, 2021 10:59
@Yawning Yawning force-pushed the yawning/feature/scrape branch 5 times, most recently from 895d4d3 to 59a1542 Compare January 25, 2021 12:20
go/beacon/api/scrape.go Outdated Show resolved Hide resolved
go/consensus/api/api.go Show resolved Hide resolved
go/consensus/tendermint/apps/beacon/slashing.go Outdated Show resolved Hide resolved
@Yawning Yawning force-pushed the yawning/feature/scrape branch 4 times, most recently from d631aef to ddefca4 Compare January 28, 2021 11:27
@@ -71,8 +72,9 @@ type Node struct { // nolint: maligned
// Beacon contains information for this node's participation
// in the random beacon protocol.
//
// NOTE: This is reserved for future use.
Beacon cbor.RawMessage `json:"beacon,omitempty"`
// TODO: This is optional for now, make mandatory once enough
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this required in master? And optional in the backported version.

Copy link
Contributor Author

@Yawning Yawning Jan 28, 2021

Choose a reason for hiding this comment

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

Depends on how the transition will happen. We could save a bunch of backporting effort if the transition is just "Deploy the new code with the old backend enabled, then switch backends at a later date". If that is the option taken, then this needs to remain optional.

While well intentioned, the 1 epoch wait after the halt epoch does
not make sense with the new consensus driven timekeeping method, and
will not work at all due to timekeeping services relying on consensus
operations that get disabled via the halt process.
@Yawning Yawning marked this pull request as ready for review February 1, 2021 12:28
 * Unify the beacon and epochtime components under beacon
 * Add support for multiple beacon backends
 * Add the initial PVSS backend
@Yawning Yawning merged commit fc59330 into master Feb 1, 2021
@Yawning Yawning deleted the yawning/feature/scrape branch February 1, 2021 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/cfg Category: breaks configuration c:breaking/consensus Category: breaking consensus changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants