Prysm: Suspicious looking enr["attnets"] bitfields in Topaz

[jrhea]

🐞 Bug Report

Description

I am noticing anomalies when inspecting subnet subscriptions stored in enr["attnets"].

The column labeled enr_attnets represent the bits in the attnets bitfield that == TRUE. I have yet to see a bit > 4 == TRUE for a peer with a fork-digest of f071c66c.

Note: the one peer with bit 27 set has a fork-digest of 9925efd6 (Schlesi) and could potentially be a lighthouse node.

We could potentially explain why many of them are in the range of 0 to 4 bc:

1. the enr["attnets"] field is including regular committee assignments + persistent committee assignment. (I requested spec clarification for this: https://github.com/ethereum/eth2.0-specs/issues/1774)
2. 16000/128/32 ~= 4. In other words, There are only 4 to 5 committee indices per slot so only the lower subnets will be used (per @djrtwo).

That being said, I would expect that random persistent committees would show up in the bitfield and we would see some values in the [5,63] range. Since I didn't see any...I looked at the code and (might) have an explanation.

Starting at SubscribeCommitteeSubnets():

https://github.com/prysmaticlabs/prysm/blob/5636cd3ed87ac35a208d4311ef641bc97fff473e/beacon-chain/rpc/validator/attester.go#L183
-> cache.CommitteeIDs.AddAttesterCommiteeID(req.Slots[i], req.CommitteeIds[i])
https://github.com/prysmaticlabs/prysm/blob/37cba662f1077cd023c6efdb9e85f12acb49b916/beacon-chain/cache/committee_ids.go#L37

If I call: AddAttesterCommiteeID(0, 10), then I (think) that AddAttesterCommiteeID(slot,committeeIds) sets: 0101 instead of 0000 0000 0010

I might have an explanation for why the tests pass. In subnet_tests.go:
https://github.com/prysmaticlabs/prysm/blob/f880fb4ee404e9dd6a7d5de06c387c8e8ba865b7/beacon-chain/p2p/subnets_test.go#L130

we have

    cache.CommitteeIDs.AddAttesterCommiteeID(0, 10)
    testService.RefreshENR(0)
    time.Sleep(2 * time.Second)

    exists, err = s.FindPeersWithSubnet(2)
    if err != nil {
        t.Fatal(err)
    }

I believe this test passes bc earlier bit 1,2,3 were set here:

https://github.com/prysmaticlabs/prysm/blob/f880fb4ee404e9dd6a7d5de06c387c8e8ba865b7/beacon-chain/p2p/subnets_test.go#L42

and tested here:

https://github.com/prysmaticlabs/prysm/blob/f880fb4ee404e9dd6a7d5de06c387c8e8ba865b7/beacon-chain/p2p/subnets_test.go#L109

Disclaimer:

I am not a competent Go dev and I just eyeballed the code, but I thought it was worth filing an issue.

[terencechain]

Thanks for opening the issue, we'll take a look! ❤️

[nisdas]

@jrhea Thanks for opening this up, I will take a look at this today/tomorrow and hope to have a fix for this. I had some incorrect assumptions on how persistent or temporary assignments were supposed to be represented in our bitfield, which is why you see all the weird values in our subnet subscriptions. We were limiting ourselves to basically the max committtees per slot for that particular validator set.

Although while I am fixing this, I know you opened an issue here:
https://github.com/ethereum/eth2.0-specs/issues/1777

Should I still continue getting validators to subscribe from [5,63] ? Those subnets will be empty with 0 activity. It makes sense to make persistent subscriptions dynamic

[jrhea]

@nisdas ya good question about

https://github.com/ethereum/eth2.0-specs/issues/1777

It's just a proposal at this point, but I think it would be good to weigh in with an opinion on that issue. I agree that it'd make sense persistent committees membership dynamic...of u have any tweaks or suggestions to the calc or logic definitely comment too.

[nisdas]

@jrhea This should be resolved now, however the change will take a while to propagate across the network as users will have to update their nodes. But you should start seeing the expected ENR bitfield.