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

Attestation changes + persistent committee changes #1294

Merged
merged 32 commits into from
Jul 29, 2019
Merged

Attestation changes + persistent committee changes #1294

merged 32 commits into from
Jul 29, 2019

Conversation

vbuterin
Copy link
Contributor

@vbuterin vbuterin commented Jul 13, 2019

  • Shard slots
  • Attestation list -> single attestation
  • One-day periods
  • Committee simplification

@vbuterin vbuterin mentioned this pull request Jul 13, 2019
10 tasks
@hwwhww hwwhww added the phase1 label Jul 15, 2019
@vbuterin vbuterin changed the title Minimal attestation simplification Attestation changes + persistent committee changes Jul 19, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Jul 23, 2019

@vbuterin Should this be reviewed, tests fixed, and merged?

@protolambda
Copy link
Collaborator

If we change PLACEHOLDER, we should also change the other PLACEHOLDER in custody spec, or rename this one to something more specific.

@dankrad
Copy link
Contributor

dankrad commented Jul 23, 2019

Is there a discussion on why we want to change it to one day? Isn't that worse for light clients?

@vbuterin
Copy link
Contributor Author

Is there a discussion on why we want to change it to one day? Isn't that worse for light clients?

See the "implementation alternative" section here #1293

The shard period (I'm proposing to rename "persistent committee period" to "shard period" because the former is waaaay too long) goes down to 1 day, but the committee size drops to 128, so on net it's still a savings for light clients.

specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved

```python
class ShardBlockHeader(Container):
slot: Slot
shard: Shard
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are losing value by removing shard here by not being able to immediately recognize a shard block as being from a particular shard and instead just understanding it in the context of it's parent and the state (that must be calculated) related to the state_root.

We still have 80 bytes to play with in ShardBlockCore now that we have divided it into two 256-byte halves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see that; I'd be ok with putting it back in (though we'd have to take it back out if we decide we want to add another slot as then we would exceed 8; or we could add slot offsets for different shards 😄)

specs/core/1_shard-data-chains.md Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved
specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved
specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
vbuterin and others added 2 commits July 24, 2019 15:13
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
@@ -75,207 +91,191 @@ The following types are defined, mapping into `DomainType` (little endian):

| Name | Value |
| - | - |
| `PLACEHOLDER` | `2**32` |
| `PLACEHOLDER` | `2**3` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| `PLACEHOLDER` | `2**3` |

Copy link
Contributor

Choose a reason for hiding this comment

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

And the whole ### TODO PLACEHOLDER section.

vbuterin and others added 5 commits July 24, 2019 15:16
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
specs/core/1_shard-data-chains.md Show resolved Hide resolved
@@ -75,207 +91,191 @@ The following types are defined, mapping into `DomainType` (little endian):

| Name | Value |
| - | - |
| `PLACEHOLDER` | `2**32` |
| `PLACEHOLDER` | `2**3` |
Copy link
Contributor

Choose a reason for hiding this comment

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

And the whole ### TODO PLACEHOLDER section.

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

Needs more testing but we'll add that in iterative testing PRs

@vbuterin vbuterin merged commit de9b4f2 into dev Jul 29, 2019
lightclient pushed a commit to lightclient/eth2.0-specs that referenced this pull request Aug 7, 2019
* Minimal attestation simplification

* minor fix

* Make the tests pass

* Decrease `PLACEHOLDER`, Use `compute_epoch_of_shard_slot`

* Fix proposer signature name and use get_seed() to calculate current_shuffling_seed

* Fix linter error

* Add the WIP `test_is_valid_shard_block`

* Add `get_shard_block_attester_committee`

* Simplified committee selection

* Added some helpers and simplified

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

* Update 1_shard-data-chains.md

* Simplified switchover epochs, changed block structure, changed crosslink structure

* Update 1_shard-data-chains.md

* Moved balance dependency to proposer selection

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>

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

* Fixed shard header flattening

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

* Minor fixes

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

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

Co-Authored-By: Hsiao-Wei Wang <hwwang156@gmail.com>

* cleanup testing and lint

* return none if not active validators in persistent committee

* only allow active validators as shard proposer
@djrtwo djrtwo deleted the vitalik88 branch May 20, 2020 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants