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

shard_committees_at_slots missing when processing previous epoch attestations #352

Closed
arnetheduck opened this issue Dec 21, 2018 · 4 comments
Labels
general:bug Something isn't working

Comments

@arnetheduck
Copy link
Contributor

The scenario is the following:

At slot 6, we included an attestation for slot 1. We advance through time all the way to slot 127. The next slot will be an epoch update. We start the slot processing by increasing state.slot to 128. Then we proceed through the functions until we pick out previous_epoch_attestations = [a for a in state.latest_attestations if state.slot - 2 * EPOCH_LENGTH <= a.slot < state.slot - EPOCH_LENGTH] - our attestation from slot 1 fits this criteria (0 <= 1 < 64).

We call get_attestation_participants which calls get_shard_committees_at_slot(state, 1) which computes:
earliest_slot_in_array = state.slot - (state.slot % EPOCH_LENGTH) - EPOCH_LENGTH or 128 - 0 - 64 = 64.

subsequently, assert earliest_slot_in_array <= slot < earliest_slot_in_array + EPOCH_LENGTH * 2 fires.

related to #348, #92, #323

c8efdc7

@djrtwo
Copy link
Contributor

djrtwo commented Dec 21, 2018

I think using the previous slot in the calculations fixes this. Note as is (and with the fix) this helper can only safely be called in the context of the state transition.

prev_slot = state.slot - 1
earliest_slot_in_array = prev_slot - (prev_slot % EPOCH_LENGTH) - EPOCH_LENGTH

@arnetheduck
Copy link
Contributor Author

I'd love to, but that causes another error with get_beacon_proposer_index(state, state.slot) - at the epoch boundary there's no longer coverage for the proposer!

@djrtwo
Copy link
Contributor

djrtwo commented Dec 21, 2018

yeah, yeah, subtle quirks in shuffling and helpers.
on it!

@JustinDrake
Copy link
Contributor

Is this addressed @djrtwo?

@hwwhww hwwhww added this to the January Release milestone Jan 28, 2019
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Jan 30, 2019
…atest_status_change_slot; switch Fork.fork_slot to Fork.epoch; begin removing Validator.randao_layers; now that ethereum/consensus-specs#352 is fixed, test out for more slots; get process_deposit consistent with current spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants