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: Spec clarification: attnet ENR field updates ethereum/consensus-specs#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():

prysm/beacon-chain/rpc/validator/attester.go

Line 183 in 5636cd3

func (vs *Server) SubscribeCommitteeSubnets(ctx context.Context, req *ethpb.CommitteeSubnetsSubscribeRequest) (*ptypes.Empty, error) {

-> cache.CommitteeIDs.AddAttesterCommiteeID(req.Slots[i], req.CommitteeIds[i])

prysm/beacon-chain/cache/committee_ids.go

Line 37 in 37cba66

func (c *committeeIDs) AddAttesterCommiteeID(slot uint64, committeeID uint64) {

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:

prysm/beacon-chain/p2p/subnets_test.go

Line 130 in f880fb4

cache.CommitteeIDs.AddAttesterCommiteeID(0, 10)

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:

prysm/beacon-chain/p2p/subnets_test.go

Line 42 in f880fb4

for i := 1; i <= 3; i++ {

and tested here:

prysm/beacon-chain/p2p/subnets_test.go

Line 109 in f880fb4

exists, err := s.FindPeersWithSubnet(1)

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:
ethereum/consensus-specs#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

ethereum/consensus-specs#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.