Attestation aggregation: remove proper set duplicates

[farazdagi]

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

• Deduplicate aggregated attestations that contain some attestations which are nothing more but proper sets of other attestations (thus do not have any new info).
• Such proper set attestations can be removed, but only in situations where we do not plan to do any more aggregation, since if we do -- some profitable (ones that produce profitable aggregation) intermediary attestations, even if they are proper sets, might be removed. So, this deduplication must take place as late in the process as possible, hence it is implemented in proposer.

Which issues(s) does this PR fix?

Part of #7871

Other notes for review

• Extracted from Attestation aggregation: optimizations and benchmarks #7938
• In attestations/kv/aggregated.go we make sure that we do not add redundant bits (see attestations/kv/seen_bits.go). However, while our method is good enough to make sure that non-overlapping attestations all have some unseen bits present, after the aggregation situation can change (so, we still need to make sure we deduplicate before proposing).

Here is a sample case:

• Suppose we have the following two atts in database:

1001.0001
1100.0000

So, seen bits are: 1101.0001.

Now, another aggregate comes in: 0100.0010. It will not be marked as seen as it introduces new info at 0000.00X0 position. So, pool will be updated to:

1001.0001
1100.0000
0100.0010

On aggregation, it will become:

1101.0011 (1001.0001 + 0100.0010)
1100.0000

And, clearly, 1100.0000 is redundant as it is a proper subset of the first aggregate, thus needs to be deduplicated.

[prestonvanloon]

Any reason this test is removed?

[farazdagi]

It is not removed, it is moved (proposer_utils_test.go file was added, and this test, correctly, was moved there).

[farazdagi]

Here is test's new location: https://github.com/prysmaticlabs/prysm/pull/8063/files#diff-8b5612101d61e04a5d66fe163e80b50752d2c1ab84e319a65a2405571543c02fR14

[terencechain]

Are you missing a break after this line?

[farazdagi]

No, definitely not.

We fixate on atts[i] and can remove as many atts[j] w/o breaking (I am swapping the current element for the last and decrease the len -- common idiom). I decrement j so that swapped element is checked too.

The break in the second else if is because when we need to remove atts[i] (and we fixate on it), we swap the last and the current element, and need to break, and check that last element's index once again.

[terencechain]

Makes sense!