Remove Shards/Crosslinks from Phase 0 #1428

djrtwo · 2019-10-12T04:29:09Z

In light of discussions around the alternate phase 1 proposal, this PR removes the notion of Shard and Crosslink entirely from Phase 0 to allow for (1) a clean slate to design phase 1 on top of and (2) to enable client implementers to keep moving forward on phase 0 testnets.

Removed all Shard and Crosslink mechanics (Crosslink, AttestationData.crosslink, BeaconState.start_shard, process_crosslinks, get_crosslink_deltas, etc)
Added notion of CrosslinkIndex, AttestationData.index, and MAX_COMMITTEES_PER_SLOT. This indexes committees on a per-slot basis. The notion of Shard will map on top of this in phase 1.
Add slot to AttestationData. Required in this new context because with the removal shard, we can't reverse engineer the slot. Also we've been planning on doing this anyway.
Renamed "crosslink committee" to "beacon committee"
Temporarily disabled phase 1 tests from CI to isolate phase 0 for discussion before fixing/removing things from phase 1
constants changes
- SECONDS_PER_SLOT from 6 to 12 -- more events will occur per slot in these modified proposals so 12 to be conservation
- SLOTS_PER_EPOCH from 64 to 32 -- reduction of 64 to 32 here keeps epochs at 6 minutes and also help with crosslink committee sizes (wrt TARGET_COMMITTEE_SIZE)
- the combination of the prior two changes keep the same number of epochs per year so the same reward rate
- BASE_REWARDS_PER_EPOCH from 5 to 4 -- adjustment due to the removal of crosslinks. keeps rewards constant
- MAX_VALIDATORS_PER_COMMITTEE to 2048 -- TOTAL_ETH_SUPPLY // ETH_PER_VALIDATOR // SLOTS_PER_EPOCH // MAX_COMMITTEES_PER_SLOT -- 2**27 // 32 // 32 // 64

terencechain

Looks good so far. Crosschecked by implementing the changes in this Prysm branch. Able to build beacon node & validator client binaries. Haven't tested run time yet

specs/core/0_beacon-chain.md

terencechain · 2019-10-14T18:33:41Z

what's the timeline for this? do we plan to roll this out for the next release? : )

djrtwo · 2019-10-16T09:57:59Z

@terencechain, Made a few minor changes today. Still check out on your end?

what's the timeline for this? do we plan to roll this out for the next release? : )

Discussing this now. Want to get this out asap, but also want to keep things moving forward smoothly on the client/testnet side of things

JustinDrake · 2019-10-17T03:52:51Z

I'm loving the cleaner separation between phase 0 and phase 1 👍

Minor comments:

The Shard custom type can go
Transfers can go
Consider renaming get_committees_per_slot to get_committee_count_at_slot ("at slot" is consistent with get_block_root_at_slot; "committee_count" is more correct than "committees")
index should go immediately after slot in AttestationData (remove "Committee Index" comment (with inconsistent capitalisation 😂))
Consider the shorter "a committee index at a slot" over "an index for a committee within a slot"
MAX_COMMITTEES_PER_SLOT => MAX_COMMITTEE_COUNT_PER_SLOT
Should MAX_VALIDATORS_PER_COMMITTEE now be 2**11?
The "Shuffling" section comment in BeaconState is now more accurately "Randomness"
Rename compute_epoch_of_slot to compute_epoch_at_slot (consistent with get_block_root_at_slot and get_committee_count_at_slot)
If we remove the notion of persistent committee, do we need compute_committee?

protolambda

The phase 1 documents reference crosslinks still. What is the plan there, leave it as-is till we have "packages" (or a new form of crosslink) for phase 1? In that case we should add a note to top of the documents, and maybe refer to an issue/PR for open discussion.

specs/core/1_shard-data-chains.md

specs/core/0_beacon-chain.md

specs/core/0_fork-choice.md

specs/validator/0_beacon-chain-validator.md

specs/core/0_beacon-chain.md

specs/core/1_beacon-chain-misc.md

specs/core/0_beacon-chain.md

configs/minimal.yaml

djrtwo · 2019-10-17T08:32:23Z

@JustinDrake

Shard type -- Will move to phase 1 (to be majorly reworked but the type being there keeps build fine for now
Transfers -- will open a separate PR to make this more digestible for implementers
get_committees_per_slot -> get_committee_count_at_slot -- will do
index should go immediately after slot -- Was planning on it, but forgot. thanks!
Consider the shorter "a committee index at a slot" over "an index for a committee within a slot" -- affirmative
MAX_COMMITTEE_COUNT_PER_SLOT -- EDIT: I think it is in line with previous naming -- similar to MAX_VALIDATORS_PER_COMMITTEE -- and it is not quite as long which is good
MAX_VALIDATORS_PER_COMMITTEE now be 2**11 -- will be different for sure. will be defined in relation to the number of committees per slot but also the number of slots per epoch which is up for debate. I'll make the number work in relation to the current constants
"Randomness" -- willl do
compute_epoch_of_slot to compute_epoch_at_slot -- would rather reduce random cleanup changes, but.... okay
Think we should keep compute_committee. We are likely going to have a persistent committee upon which shard block proposers are selected and which serve as the p2p backbone for the shard

specs/core/0_beacon-chain.md

djrtwo · 2019-10-18T03:56:38Z

@JustinDrake @protolambda @hwwhww @terencechain
Thank you for the feedback! Comments addressed

configs/mainnet.yaml

Co-Authored-By: Cayman <caymannava@gmail.com>

JustinDrake · 2019-10-20T07:23:28Z

specs/core/0_beacon-chain.md

@@ -878,61 +855,37 @@ def get_seed(state: BeaconState, epoch: Epoch, domain_type: DomainType) -> Hash:
    return hash(domain_type + int_to_bytes(epoch, length=8) + mix)
 ```

-#### `get_committee_count`
+#### `get_committee_count_at_slot`


This function is not in the ToC

ah, thanks. need to re-run ToC generator

JustinDrake · 2019-10-20T07:24:31Z

specs/core/0_beacon-chain.md

@@ -69,7 +68,7 @@
            - [`compute_shuffled_index`](#compute_shuffled_index)
            - [`compute_proposer_index`](#compute_proposer_index)
            - [`compute_committee`](#compute_committee)
-            - [`compute_epoch_of_slot`](#compute_epoch_of_slot)
+            - [`compute_epoch_at_slot`](#compute_epoch_at_slot)
            - [`compute_start_slot_of_epoch`](#compute_start_slot_of_epoch)


should be compute_start_slot_at_epoch for consistency with compute_epoch_at_slot, get_block_root_at_slot and get_committee_count_at_slot.

terencechain · 2019-10-22T00:02:40Z

CustodyChunkChallenge still uses crosslink from attestation. I'm guessing It'll be a separate effort to revamp the custody game spec?

djrtwo · 2019-10-22T06:24:31Z

@terencechain, yes. I've currently disabled phase 1 CI as most of it needs to be revamped. Prioritizing getting the phase 0 things out and then we'll circle back to get the phase 1 spec in place

terencechain · 2019-10-22T14:42:38Z

@terencechain, yes. I've currently disabled phase 1 CI as most of it needs to be revamped. Prioritizing getting the phase 0 things out and then we'll circle back to get the phase 1 spec in place

Great. Thank you

…to phase0-simplify

djrtwo added 6 commits October 12, 2019 11:48

initial removal and cleanup of shard/crosslink from phase 0

dfdf3ab

working through phase 0 tests after crosslink/shard removal

b3b9b43

fix get crosslink committee and finalitytests

d98cabf

temporarily disable phase 1 tests

667bf67

remove some legacy code

b3a0a03

remove get_committee_count

5ccac7c

djrtwo requested review from protolambda and JustinDrake and removed request for protolambda October 12, 2019 04:39

djrtwo added 2 commits October 12, 2019 13:54

fix config files

1fbf7f8

fix max epochs var

c392db3

terencechain mentioned this pull request Oct 13, 2019

WIP: Remove crosslink and shards across beacon node prysmaticlabs/prysm#3760

Closed

terencechain reviewed Oct 13, 2019

View reviewed changes

specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved

djrtwo added 2 commits October 13, 2019 13:53

add start index back in

d8431f8

get crosslink committees by slot

6208e74

djrtwo added 2 commits October 16, 2019 18:47

simplify index

bd1c71b

add CommitteeIndex type

219084a

djrtwo added 3 commits October 17, 2019 10:45

crosslink committee -> beacon committee

c239ce0

remove refs to crosslinks

7af2c23

remove refs to crosslinks

283a8cb

protolambda reviewed Oct 17, 2019

View reviewed changes

hwwhww reviewed Oct 17, 2019

View reviewed changes

hwwhww mentioned this pull request Oct 17, 2019

Phase 0 Simplification checklist #1435

Closed

3 tasks

cleanup wrt Justin's comments

437a65d

terencechain reviewed Oct 18, 2019

View reviewed changes

specs/core/0_beacon-chain.md Show resolved Hide resolved

djrtwo added 3 commits October 18, 2019 12:05

fix comment re: proto

fbcc2a4

proto pr feedback

58c2834

minor comments resolved from hww

d5a2535

djrtwo force-pushed the phase0-simplify branch from 37bdc1e to d5a2535 Compare October 18, 2019 03:19

update constants for phase 0 simplification

a11b012

djrtwo force-pushed the phase0-simplify branch from 94a9868 to a11b012 Compare October 20, 2019 04:53

wemeetagain reviewed Oct 20, 2019

View reviewed changes

configs/mainnet.yaml Outdated Show resolved Hide resolved

minor fix to mainnet config comments

e86ff1e

Co-Authored-By: Cayman <caymannava@gmail.com>

JustinDrake approved these changes Oct 20, 2019

View reviewed changes

djrtwo and others added 3 commits October 23, 2019 09:37

minor pr review

f809b21

Merge branch 'phase0-simplify' of github.com:ethereum/eth2.0-specs in…

934008e

…to phase0-simplify

Update ToCs

13c3d9c

djrtwo mentioned this pull request Oct 24, 2019

Simple aggregation strategy #1440

Merged

tersec mentioned this pull request Oct 24, 2019

Crosslink issue status-im/nimbus-eth2#482

Closed

djrtwo mentioned this pull request Oct 24, 2019

Remove transfers from phase 0 #1443

Merged

zah mentioned this pull request Oct 24, 2019

Implement the phase 0 spec simplifications status-im/nimbus-eth2#499

Closed

djrtwo merged commit 364781b into dev Oct 28, 2019

djrtwo deleted the phase0-simplify branch October 28, 2019 06:33

terencechain mentioned this pull request Oct 28, 2019

Align to spec v0.9 prysmaticlabs/prysm#3865

Closed

21 tasks

hwwhww mentioned this pull request Oct 29, 2019

Spec v0.9.0 updates - eth2 part ethereum/trinity#1240

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Shards/Crosslinks from Phase 0 #1428

Remove Shards/Crosslinks from Phase 0 #1428

djrtwo commented Oct 12, 2019 •

edited

Loading

terencechain left a comment

terencechain commented Oct 14, 2019

djrtwo commented Oct 16, 2019

JustinDrake commented Oct 17, 2019

protolambda left a comment

djrtwo commented Oct 17, 2019 •

edited

Loading

djrtwo commented Oct 18, 2019

JustinDrake Oct 20, 2019

djrtwo Oct 20, 2019

JustinDrake Oct 20, 2019

terencechain commented Oct 22, 2019

djrtwo commented Oct 22, 2019

terencechain commented Oct 22, 2019

Remove Shards/Crosslinks from Phase 0 #1428

Remove Shards/Crosslinks from Phase 0 #1428

Conversation

djrtwo commented Oct 12, 2019 • edited Loading

terencechain left a comment

Choose a reason for hiding this comment

terencechain commented Oct 14, 2019

djrtwo commented Oct 16, 2019

JustinDrake commented Oct 17, 2019

protolambda left a comment

Choose a reason for hiding this comment

djrtwo commented Oct 17, 2019 • edited Loading

djrtwo commented Oct 18, 2019

JustinDrake Oct 20, 2019

Choose a reason for hiding this comment

djrtwo Oct 20, 2019

Choose a reason for hiding this comment

JustinDrake Oct 20, 2019

Choose a reason for hiding this comment

terencechain commented Oct 22, 2019

djrtwo commented Oct 22, 2019

terencechain commented Oct 22, 2019

djrtwo commented Oct 12, 2019 •

edited

Loading

djrtwo commented Oct 17, 2019 •

edited

Loading