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

fix off by one attestaton issue #627

Merged
merged 1 commit into from
Feb 14, 2019
Merged

fix off by one attestaton issue #627

merged 1 commit into from
Feb 14, 2019

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Feb 14, 2019

addresses #618

Note that this is the quick fix. There is a discussion to be had on when exactly the epoch transition should occur. An alternative would be to be at the start of the 0th slot of an epoch rather than at the end of the last slot of an epoch. I'd like to get this fix merged now for the spec release because this is a critical bug and can save any debate for over this next week.

@djrtwo
Copy link
Contributor Author

djrtwo commented Feb 14, 2019

cc: @arnetheduck

@JustinDrake JustinDrake merged commit 677efe5 into dev Feb 14, 2019
@JustinDrake
Copy link
Collaborator

I'd like to get this fix merged now

Done! :)

@djrtwo djrtwo deleted the off-by-one-attestation branch February 14, 2019 20:17
@arnetheduck
Copy link
Contributor

what a beauty! :)

hwwhww added a commit to hwwhww/trinity that referenced this pull request Feb 22, 2019
hwwhww added a commit to hwwhww/trinity that referenced this pull request Feb 22, 2019
hwwhww added a commit to ethereum/trinity that referenced this pull request Feb 24, 2019
* Update privkeys fixtures, thanks to VB

* Address ethereum/consensus-specs#672: fix up previous epoch logic around genesis

* Address ethereum/consensus-specs#627: fix off by one attestaton issue

* Fix attester tools

1. Fix `create_mock_signed_attestation`: Address ethereum/consensus-specs#671
2. Fix `test_demo.py`
	1. Use queued `attestations_map: Dict[Slot, Sequence[Attestation]]` to keep the generated attestations, and include them when reache `MIN_ATTESTATION_INCLUSION_DELAY`.
	2. Add justification assertions

* PR feedback

* PR feedback
ralexstokes added a commit to ralexstokes/trinity that referenced this pull request Apr 8, 2019
this seems to be feasible now that we are removing the 'attestation kludge'
ethereum/consensus-specs#627
ralexstokes added a commit to ralexstokes/trinity that referenced this pull request Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants