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.

Sign up for GitHub

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

Jump to bottom

Executable fork choice #1185

Merged
merged 33 commits into from
Jun 21, 2019
Merged

Executable fork choice #1185

merged 33 commits into from
Jun 21, 2019

Conversation

CarlBeek
Copy link
Contributor

This is a new PR which supersedes the old executable fork choice PR #1068. This was done because dev is so far ahead of the branch that #1068 was forked from so it was easier to start again.

Special thanks to @hwwhww and @vbuterin for the help tracking down a stupid bug.

@CarlBeek CarlBeek added scope:fork-choice milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet labels Jun 16, 2019
@CarlBeek CarlBeek added this to the Phase 0 Frozen milestone Jun 16, 2019
@CarlBeek CarlBeek mentioned this pull request Jun 16, 2019
Closed
@protolambda
Copy link
Contributor

protolambda commented Jun 16, 2019 •
edited
Loading

There's no good reason to make Store and other non-consensus data a SSZ object (Container). The typing is changing, and enforces proper ssz-types as elements. So this would break, and introducing python-dict support into ssz doesn't seem like the right solution. Instead just declare it as class Store: or class Store(object) (new python class style), and it'll work just fine.

CarlBeek and others added 18 commits June 17, 2019 10:48
@CarlBeek
Moves fork-choice objects away from SSZ
2ea6ced
@CarlBeek
Kick the CI cache
bb36660
@CarlBeek
Kick the other CI cache
c31a426
@CarlBeek
Merge conflicts on dev
31a4700
@CarlBeek
Trail upgrade CI to python 3.7
d5d35b1
@CarlBeek
Actually resolve some merge conflicts
28a3e54
@CarlBeek
Remove circle ci repo caching
eb9fc7b
@CarlBeek
Python 3.7 -> 3.6 again
dfdd283
@CarlBeek
Maybe venv cache name collision
686273e
@CarlBeek
Removes defaults from Store to try get CI to pass
3bcddf5
@CarlBeek
Removes defaults from Store to try get CI to pass
d804cb3
@CarlBeek
Test removing all the Bytes objects
7a71919
@hwwhww
Add dataclasses==0.6 to setup.py
dc01218
@hwwhww
kick deposit contract cache
04c7c43
@CarlBeek
Unbreak things again
61a5141
@CarlBeek
Merge dev and resolve conflicts (please let the CI tests pass with th…
3418c35 
…e new dev)
@CarlBeek
Workaround for python3.6
241fe34
@CarlBeek
Merge branch 'dev' into executable_fork_choice
e0d814d 
* dev:
  Increase historical length and cleanups (#1196)
@djrtwo
Copy link
Contributor

djrtwo commented Jun 20, 2019

hold up on my review before merge

hwwhww
hwwhww reviewed Jun 20, 2019
View reviewed changes
specs/core/0_fork-choice.md Outdated Show resolved Hide resolved
specs/core/0_fork-choice.md Outdated Show resolved Hide resolved
@djrtwo
Copy link
Contributor

djrtwo commented Jun 20, 2019

This needs a little bit of work wrt issues in #768.
I'm working on a PR to this PR :)

@djrtwo djrtwo mentioned this pull request Jun 20, 2019
Merged
djrtwo
djrtwo reviewed Jun 20, 2019
View reviewed changes
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.

Some minor comments. The bulk of the substance of my review is found in #1198

specs/core/0_fork-choice.md Outdated Show resolved Hide resolved
specs/core/0_fork-choice.md Show resolved Hide resolved
specs/core/0_fork-choice.md Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/test/test_fork_choice.py Outdated Show resolved Hide resolved
@protolambda protolambda mentioned this pull request Jun 21, 2019
Closed
11 tasks
djrtwo
djrtwo approved these changes Jun 21, 2019
View reviewed changes
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.

Agreed on merging this PR before #1198 to divide out the restructuring from modifications of handling state.

@djrtwo djrtwo merged commit 0031eaa into dev Jun 21, 2019
@hwwhww hwwhww mentioned this pull request Apr 20, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwwhww hwwhww hwwhww left review comments

@djrtwo djrtwo djrtwo approved these changes

@JustinDrake JustinDrake JustinDrake approved these changes

Assignees
No one assigned
Labels
milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet scope:fork-choice
Projects
None yet
Milestone
Phase 0 Frozen
Development

Successfully merging this pull request may close these issues.
5 participants
@CarlBeek @protolambda @djrtwo @JustinDrake @hwwhww