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

attesting_indices.len == 1 Per bits check above [AssertionError] #1782

Closed
stefantalpalaru opened this issue Sep 30, 2020 · 2 comments
Closed
Labels
bug Something isn't working ❗ high priority Important feature for NBC

Comments

@stefantalpalaru
Copy link
Contributor

INF 2020-09-30 15:16:05.171+02:00 Attestation resolved                       topics="attpool" tid=8258 file=attestation_pool.nim:187 attestation="(aggregation_bits: 0b00000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"a1a04117\")" validations=44
 peers: 79 ❯ finalized: b80aa8dd:234 ❯ head: e1f9fe09:236:27 ❯ time: 236:27 (7579) ❯ sync: synced                                                   :  Traceback (most recent call last, using override)
/mnt/sde1/storage/nim-beacon-chain-clean/beacon_chain/mainchain_monitor.nim(548) main
/mnt/sde1/storage/nim-beacon-chain-clean/beacon_chain/mainchain_monitor.nim(541) NimMain
/mnt/sde1/storage/nim-beacon-chain-clean/beacon_chain/beacon_node.nim(1226) main
/mnt/sde1/storage/nim-beacon-chain-clean/beacon_chain/beacon_node.nim(937) start
/mnt/sde1/storage/nim-beacon-chain-clean/vendor/nim-chronicles/chronicles.nim(329) run
/mnt/sde1/storage/nim-beacon-chain-clean/vendor/nimbus-build-system/vendor/Nim/lib/system/excpt.nim(489) reraiseException
/mnt/sde1/storage/nim-beacon-chain-clean/vendor/nimbus-build-system/vendor/Nim/lib/system/excpt.nim(407) reportUnhandledError
/mnt/sde1/storage/nim-beacon-chain-clean/vendor/nimbus-build-system/vendor/Nim/lib/system/excpt.nim(358) reportUnhandledErrorAux
Error: unhandled exception: /mnt/sde1/storage/nim-beacon-chain-clean/beacon_chain/attestation_aggregation.nim(229, 12) `attesting_indices.len == 1` Per bits check above [AssertionError]
make: *** [Makefile:314: spadina] Error 1

Failing assertion: https://github.com/status-im/nim-beacon-chain/blob/7eaaab908cf249f981d8d36efd83663e79e5105e/beacon_chain/attestation_aggregation.nim#L229

Kim had the same crash at the same time, probably with the same attestation.

@kdeme
Copy link
Contributor

kdeme commented Sep 30, 2020

Indeed:

DBG 2020-09-30 15:16:05.087+02:00 Attestation received                       tid=13097 file=eth2_processor.nim:304 delay=4s87ms617us907ns wallSlot=7579 attestation="(aggregation_bits: 0b00000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"a7c3023a\")" committeeIndex=27
DBG 2020-09-30 15:16:05.088+02:00 Attestation received                       tid=13097 file=eth2_processor.nim:304 delay=4s88ms920us904ns wallSlot=7579 attestation="(aggregation_bits: 0b00000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"8e8bce68\")" committeeIndex=27
INF 2020-09-30 15:16:05.090+02:00 Attestation resolved                       topics="attpool" tid=13097 file=attestation_pool.nim:187 attestation="(aggregation_bits: 0b00000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"a7c3023a\")" validations=36
INF 2020-09-30 15:16:05.090+02:00 Attestation resolved                       topics="attpool" tid=13097 file=attestation_pool.nim:187 attestation="(aggregation_bits: 0b00000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"8e8bce68\")" validations=37
DBG 2020-09-30 15:16:05.091+02:00 Attestation received                       tid=13097 file=eth2_processor.nim:304 delay=4s91ms45us274ns wallSlot=7579 attestation="(aggregation_bits: 0b000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000, data: (slot: 7579, index: 0, beacon_block_root: \"e1f9fe09\", source: (epoch: 235, root: \"dc408683\"), target: (epoch: 236, root: \"e89a4a35\")), signature: \"98a138fb\")" committeeIndex=27
DBG 2020-09-30 15:16:05.091+02:00 exiting pubsub read loop                   topics="pubsubpeer" tid=13097 file=pubsubpeer.nim:138 conn=16*1uNJwK:5f7484c58cf3ad2f4df881d9 peer=16*1uNJwK closed=true
 peers: 79 ❯ finalized: b80aa8dd:234 ❯ head: e1f9fe09:236:27 ❯ time: 236:27 (7579) ❯ sync: synced                                                                                                                                                                                  :  Traceback (most recent call last, using override)
Error: unhandled exception: /home/deme/repos/nim-beacon-chain/beacon_chain/attestation_aggregation.nim(229, 12) `attesting_indices.len == 1` Per bits check above [AssertionError]

@tersec
Copy link
Contributor

tersec commented Oct 1, 2020

It's an artifact of a mismatch between different ways of checking for the number of validators an attestation represents.

The "bits check above" is
https://github.com/status-im/nim-beacon-chain/blob/7eaaab908cf249f981d8d36efd83663e79e5105e/beacon_chain/attestation_aggregation.nim#L184-L187

The important part of which is:
https://github.com/status-im/nim-beacon-chain/blob/7eaaab908cf249f981d8d36efd83663e79e5105e/beacon_chain/attestation_aggregation.nim#L133-L140

That is, it's basing it off attestation.aggregation_bits, as specified by https://github.com/ethereum/eth2.0-specs/blob/v0.12.3/specs/phase0/p2p-interface.md#beacon_attestation_subnet_id as explained in the comments.

However, the get_attesting_indices call here amounts to:
https://github.com/status-im/nim-beacon-chain/blob/78ceeed8042d6b70d9c1aae09688d43ff79db6e2/beacon_chain/spec/beaconstate.nim#L461-L478

If bits.len != committee.len, then get_attesting_indices will return an empty set, and under TRACE logging note an inconsistency.

It's thus possible for an incorrect attestation to find itself at that line of code, exploiting the distinction between the (specified) method of checking for a non-aggregated attestation and how get_attesting_indices() also works, via https://github.com/ethereum/eth2.0-specs/blob/v0.12.3/specs/phase0/beacon-chain.md#get_attesting_indices:

    committee = get_beacon_committee(state, data.slot, data.index)
    return set(index for i, index in enumerate(committee) if bits[i])

The reason this hasn't shown up until recently is that #1755 adds this assertion. It required the intersection of erroneous attestation creation and the existence of that assertion, from a few days ago, to cause this crash.

It's correct to check for that condition, but it can't assert unless attesting_indices.len > 1, which genuinely should not happen, regardless of the discussion above.

However, because
https://github.com/status-im/nim-beacon-chain/blob/7eaaab908cf249f981d8d36efd83663e79e5105e/beacon_chain/attestation_aggregation.nim#L229-L230

It's still an error condition, so the empty attesting indices set should be detected, simply not as an assertion.

Fix coming shortly.

@mratsim mratsim added ❗ high priority Important feature for NBC bug Something isn't working labels Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ❗ high priority Important feature for NBC
Projects
None yet
Development

No branches or pull requests

4 participants