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

initial attestation aggregation #769

Merged
merged 7 commits into from Apr 1, 2020
Merged

initial attestation aggregation #769

merged 7 commits into from Apr 1, 2020

Conversation

@tersec
Copy link
Contributor

tersec commented Feb 26, 2020

This version finalizes with make eth2_network_simulation.

It fills in the remaining part of #721 almost as much as feasible currently, though, given that https://github.com/status-im/nim-beacon-chain/blob/devel/beacon_chain/spec/crypto.nim (where the BLS signing would be from) is still basically the 0.9.x version.

Finalizes under make eth2_network_simulation and local testnet locally.

@tersec

This comment has been minimized.

Copy link
Contributor Author

tersec commented Mar 10, 2020

The epoch boundary condition, especially, is fixed. This is plausibly mergeable now. Testing this is still challenging due to the BLS dependency.


let bs = BlockSlot(blck: head, slot: slot)

let committees_per_slot = get_committee_count_at_slot(state, head.slot)

This comment has been minimized.

Copy link
@arnetheduck

arnetheduck Mar 17, 2020

Member

this looks fishy - why head.slot here? what if no blocks have been produced for an epoch or two?

This comment has been minimized.

Copy link
@tersec

tersec Mar 30, 2020

Author Contributor

Yeah, head.slot is incorrect. The idea is here is to be internally consistently living in the recent-past-slots (including committee assignments, numbers of committees, et cetera), as that's the world relative to which the attestation's being aggregated (not head, not the current justified slot/epoch, and not the current finalized slot/epoch, and not the current wall slot).

Which slot that exactly is given by the parameter slot. It probably hasn't made any difference in my local testnet/eth2_network_simulation runs, but obviously matters in a live network.

This comment has been minimized.

Copy link
@tersec

tersec Mar 30, 2020

Author Contributor
@tersec tersec force-pushed the ati branch 2 times, most recently from fbd05ed to 60992ea Mar 17, 2020
@tersec tersec force-pushed the ati branch 2 times, most recently from 5d814b6 to 043c609 Mar 30, 2020
@tersec

This comment has been minimized.

Copy link
Contributor Author

tersec commented Mar 31, 2020

This depends on some block pool changes in #812 to be merged to complete.

@tersec tersec mentioned this pull request Mar 31, 2020
tersec added 6 commits Feb 26, 2020
…ailing/following distance; document how the only-broadcast-if mechanism works better and what aggregation already happens, not otherwise sufficiently clear; use correct BlockSlot across epoch boundaries
…te broadcast; follow 0.11.x aggregate broadcast p2p interface topic
…d genesis_validators_root
@tersec tersec merged commit 6eb4f1f into devel Apr 1, 2020
6 checks passed
6 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jenkins/nim-beacon-chain/prs This commit looks good
Details
status-im.nim-beacon-chain Build #20200401.3 succeeded
Details
status-im.nim-beacon-chain (Windows 32-bit) Windows 32-bit succeeded
Details
status-im.nim-beacon-chain (Windows 64-bit) Windows 64-bit succeeded
Details
@delete-merged-branch delete-merged-branch bot deleted the ati branch Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.