https://github.com/ethereum/consensus-specs/pull/3360 effectively
extends the valid lifetime of an attestation/aggregate to 2 epochs -
this means that an aggregate that was published at the beginning of a
slot now is valid per the gossip rules up to 2 epochs later.
Then net effect of the above change is that peers are allowed to
republish old aggregates and attestations and libp2p will not stop the
spread with the settings we recommend - instead the messages will have
to be stopped with the "attestation cover rule" or similar, even though
they have been observed already.
Significant amounts of this kind of spam have been observed on the
aggregate channel in particular leading to a 5x increase in aggregate
traffic as some clients republish these old messages in spite of the
"attestation cover rule" which should have stopped them - this simple
change will provide an additional layer of protection against such bugs.
A couple gossip validation rules are only specced out for single
un-aggregated attestations, but are also checked by implementations
for aggregates. This adds a copy of the missing gossip validation rules
to the aggregated attestation docs.
Update gossip validation rules to use the highest `finalized_checkpoint`
across _all_ branches (`store.finalized_checkpoint`), instead of the one
on the currently selected branch (`state.finalized_checkpoint`) when
deciding whether to ignore a block / blob because they are already
finalized.
* Moved configuration into network preset instead of constants.
Now that `MAX_CHUNK_SIZE` and `GOSSIP_MAX_SIZE` are in configuration, we no longer need separate constants to represent them in the spec when they change in Bellatrix.
I've changed the usage, and put the values into the presets, but I'm not sure if I've updated the descriptions in the best way...
This is following on from the work in #3375 where a number of constants got moved into configuration, so we no longer need these constants to be separately represented, they can simply be updated in presets.
* Update presets/minimal/bellatrix.yaml
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* Update presets/mainnet/bellatrix.yaml
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* Moved preset items into the correct section and updated TOC.
It looked like the items listed in configuration about the max size and chunk size were no longer needed since we're updating preset values now and the preset changes seem to only be listed in the changes at the top.
* review feedback
* hopefully correct this time! Moved the 2 fields from configs into presets completely as suggested.
* WIP - changing back to being in config and updating the phase 0 value... I think this should be close but want to see what's outstanding.
* fix intellij's formatting of table.
* more fixes
---------
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* deprecate `BeaconBlocksByRange.step`
The `step` parameter has not seen much implementation in real life
clients which instead opt to request variations on a few epochs at a
time (instead of interleaving single blocks, entire epochs are
interleaved).
At the same time, supporting `step` on the server side brings several
complications: more complex bounds checking logic, more complex loading
of blocks from linear storage (which presumably stores all blocks and
not just certain increments).
This PR suggests that we deprecate the whole idea. Backwards
compatibility is kept by simply responding with a single block when
`step > 0` - this is allowed by the spec and should thus be handled
gracefully by requesting clients already, should there exist any that
use larger step counts.
Removing `step` now allows simplifying the EL-CL protocol for serving
execution data from the EL to avoid double storage.
* Update specs/phase0/p2p-interface.md
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
* Ignore subset aggregates
When aggregates are propagated through the network, it is often the case
that a better aggregate has already been seen - in particular, this
happens when an aggregator has not been able to include itself in the
mesh and therefore publishes an aggregate with only its own
attestations.
This new ignore rule allows dropping all aggregates that are
(non-strict) subsets of aggregates that have already been seen on the
network. In particular, it does not mandate dropping aggregates where a
union of previous aggregates would cause it to become a subset).
The logic for allowing this is based on the premise that any aggregate
that has already been seen by a peer will also have been seen by its
neighbours - a subset aggregate (strict or not) brings no new value to
the aggregation algorithm, except in the extreme edge case where you
could combine several such sparse aggregates into a single, more dense
"combined" aggregate and thus use less block space.
Further, as a small benefit, computing the `hash_tree_root` of the full
aggregate is generally not done -however, `hash_tree_root(data)` is
already done for other purposes as this is used as index in the beacon
API.
* add subset ignore rule to sync contributions as well
* typo
When nodes are syncing but have not yet reached the canonical `head`,
they cannot determine whether nodes they are connected to serve a valid
history or are making bogus claims in their `Status` advertisement.
Thus, the best course of action that a client can take is to vote for
its "current" best synced head, regardless of whether it's connected to
peers that claim to have other heads or not.
However, in the p2p spec, we penalize such peers with a `REJECT` - this
should be an `IGNORE` instead because this vote is correct per the spec,
albeit "late" according to the validating clients' view of the chain.
The spec reserves the libp2p error code range `[3, 127]` for future use
but actually defines error code `3` as `ResourceUnavailable`. This patch
updates the reserved range to `[4, 127]`.