Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Fix attestation, previous epoch logic and builder tools #300

Merged
merged 6 commits into from
Feb 24, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Feb 22, 2019

What was wrong?

To fix #294, also need to fix other issues to make test_demo.py work.

How was it fixed?

  1. Address fix up previous epoch logic around genesis consensus-specs#672: fix up previous epoch logic around genesis
  2. Address fix off by one attestaton issue consensus-specs#627: fix off by one attestaton issue
  3. Fix create_mock_signed_attestation: Address validator guide bug fixes consensus-specs#671
  4. Fix test_demo.py
    1. Use map attestations_map: Dict[Slot, Sequence[Attestation]] to keep the generated attestations, and include them when reache MIN_ATTESTATION_INCLUSION_DELAY.
    2. Add justification assertions

Cute Animal Picture

albino_kangaroo
(Foto: Heike Katthagen)

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
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.

Logic looks good on the spec side. haven't dug deep into the tests written

Rationales:
1. Making the privkeys be small integers to make multiplying easier for tests.
2. Using ``2**i`` instead of ``i``:
The combinations of validators would not lead to unique pubkeys.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be ...would lead to unique pubkeys?

@hwwhww hwwhww merged commit 033ae08 into ethereum:master Feb 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test state.finalized_epoch in test_demo.py
4 participants