Bound min size of dynamic processor queues

[dapplion]

Issue Addressed

A discord user notes that in networks of < SLOTS_PER_EPOCH the beacon node did not process attestations.

• Attestation queues have max size 0 if num validators is < slots_per_epoch #6463

It's due to integer division here

lighthouse/beacon_node/beacon_processor/src/lib.rs

Lines 149 to 152 in 35ac9f4

	// Capacity for a full slot's worth of attestations if subscribed to all subnets
	attestation_queue: active_validator_count / slots_per_epoch,
	// Capacity for a full slot's worth of attestations if subscribed to all subnets
	unknown_block_attestation_queue: active_validator_count / slots_per_epoch,

Proposed Changes

Min against a const

Closes #6463

[michaelsproul]

I feel like this could easily regress with the beacon processor refactors. Wanna add a test?

[chong-he]

Not sure if we should use std::cmp::max here? Happy to stand corrected.

[chong-he]

I may be wrong here, I don't understand why it is std::cmp::min. From my understanding, it takes the minimum value of the two in comparison. I thought it should be std::cmp::max.

Because if it is min, then when active_validator_count / slots_per_epoch is, say 16, then the queue would be still 0? But if we use max, then for small validator count, it will return MIN_QUEUE_LEN which is 128.

[michaelsproul]

you're correct!

[dapplion]

ups, good catch

[chong-he]

Same as above

[dapplion]

I feel like this could easily regress with the beacon processor refactors. Wanna add a test?

To not conflict with the refactor, what API should the test use? i.e.

let manager = spawn_manager(event_rx, work_journal_tx, BeaconProcessorQueueLengths::empty());
event_rx.send(Work::GossipAttestation {});
// assert work_journal_tx recv GossipAttestation work

[michaelsproul]

I think it doesn't matter too much if the test conflicts, as long as it exists and can be translated over.

I would even just test initialising the Lengths struct in isolation so that it's minimally entangled with the rest of the beacon processor

[michaelsproul]

I pushed a test. This is ready to go.

[michaelsproul]

@mergify queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 244a460