During custody backfill sync if a peer fails to serve columns for a batch don't penalize them more than once per batch
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
#8314 left a few ugly potentially panicking location behind - all of them believed to be unreachable, but this PR fixes them regardless for good hygiene.
Update to `ethereum_ssz 0.10.4` for two new helpers: `not_inplace` and `clone_zeroed`.
Remove remaining `expect` and `todo!` in favour of these helpers and one new fallible (but practically infallible) method.
Co-Authored-By: Daniel Knopik <daniel@dknopik.de>
In Gloas, beacon blocks are imported into fork choice immediately - the payload envelope and data columns arrive
separately. KZG commitments moved from the column sidecar into the execution payload bid, so the existing
`DataAvailabilityChecker` (which assumes block and data are coupled) can't be used for Gloas.
* Introduced `PendingPayloadCache` to keep track of payload and data columns per block root.
* Added gossip column verification
* Added support for Gloas data column reconstruction
* Payload envelope verification simplified: removed `MaybeAvailableEnvelope`, `ExecutedEnvelope`, `EnvelopeImportData`
Not yet implemented (tracked with TODOs):
- Proper lookup sync for Gloas columns arriving before blocks
- Partial column merging for Gloas
- Moving `load_gloas_payload_bid` disk reads off the async runtime
- Backfill/range sync for Gloas
Based on @eserilev's PR and work in progress. See also #9202 for verification.
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
Co-Authored-By: Eitan Seri- Levi <eserilev@gmail.com>
Co-Authored-By: Daniel Knopik <daniel@dknopik.de>
Co-Authored-By: Daniel Knopik <107140945+dknopik@users.noreply.github.com>
Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com>
Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
We got in a little bit of trouble in devnet-3. After gossip verifying an envelope and before importing it, we got the following error
```
May 07 08:04:24.383 WARN Execution payload envelope rejected reason: "EnvelopeProcessingError(WithdrawalsRootMismatch { state: 0x852d38aaecc9f4e2e309919f74020c7bbcf050fea4a20edf3304f171e44ee9d5, payload:
```
The envelope had already passed gossip verification checks and was correctly propagated to the network, we should not penalize peers for doing this. This caused our node to isolate itself from the rest of the network. This PR removes peer penalties for any envelope that passes gossip validation
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
We have a legacy `TestRandom` trait which generates random types for testing and fuzzing.
This function overlaps with `arbitrary` which is used very commonly in the ecosystem.
Remove `TestRandom` and generate random type instances using `Arbitrary`.
Co-Authored-By: Mac L <mjladson@pm.me>
Co-Authored-By: Michael Sproul <michael@sigmaprime.io>
Allow for the vc to submit its proposer preferences to the network
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
Payloads from the reprocess queue should be gossiped after import if they are still timely. In devnets this happens frequently since there are many cases where the envelope arrives before the block
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
Store gossip-verified `PayloadAttestationMessage`s in the operation pool and pack them into the block body at during block production.
Built on top of #9145.
Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
N/A
Adds lints for rust 1.95. Mostly cosmetic.
1. .zip(a.into_iter()) -> .zip(a) . Also a few more places where into_iter is not required
2. replace sort_by with sort_by_key
3. move if statements inside match block.
4. use checked_div instead of if statements. I think this is debatable in terms of being better, happy to remove it if others also feel its unnecessary
Co-Authored-By: Pawan Dhananjay <pawandhananjay@gmail.com>
Closes#8949
Implements peer penalties and REJECT/IGNORE message propagation for `SignedExecutionPayloadEnvelope` gossip handling, completing follow-up work from #8806.
Feedback on the error classification would be appreciated.
### Key Implementation Details
- Maps all 15 `EnvelopeError` variants to REJECT/IGNORE based on [Gloas p2p spec](https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/p2p-interface.md#execution_payload)
- Follows `ExecutionPayloadError` handling pattern from block gossip (`penalize_peer()` method)
- Uses explicit variant matching (rather than catch-all `_`) for type safety
- Applies `LowToleranceError` penalty for protocol violations (invalid signatures, mismatches, etc.)
- Ignores without penalty for spec-defined cases (unknown block root, prior to finalization) and internal errors
Co-Authored-By: 0u-Y <yyw1000@naver.com>
Co-Authored-By: Eitan Seri-Levi <eserilev@gmail.com>
Gossip verify and cache bids and proposer preferences. This PR also ensures we subscribe to new fork topics one epoch early instead of two slots early. This is required for proposer preferences.
Co-Authored-By: Eitan Seri- Levi <eserilev@gmail.com>
#9077
Where possible replaces all instances of `validator_monitor::timestamp_now` with `chain.slot_clock.now_duration().unwrap_or_default()`.
Where chain/slot_clock is not available, instead replace it with a convenience function `slot_clock::timestamp_now`.
Remove the `validator_monitor::timestamp_now` function.
Co-Authored-By: Mac L <mjladson@pm.me>
Serves envelope by range and by root requests. Added PayloadEnvelopeStreamer so that we dont need to alter upstream code when we introduce blinded payload envelopes.
Co-Authored-By: Eitan Seri- Levi <eserilev@gmail.com>
Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>
Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com>
Add a queue that allows us to reprocess an envelope when it arrives over gossip references a unknown block root. When the block is finally imported, we immediately reprocess the queued envelope.
Note that we don't trigger a block lookup sync. Incoming attestations for this block root will already trigger a lookup for us. I think thats good enough
Co-Authored-By: Eitan Seri- Levi <eserilev@gmail.com>
Which issue # does this PR address?
None
All of these are performing a check, and adding a batch, or creating a new lookup, or a new query, etc..
Hence all of these limits would be off by one.
Example:
```rust
// BACKFILL_BATCH_BUFFER_SIZE = 5
if self.batches.iter().filter(...).count() >= BACKFILL_BATCH_BUFFER_SIZE {
return None; // ← REJECT
}
// ... later adds batch via Entry::Vacant(entry).insert(...)
```
Without the `>` being changed to a `>=` , we would allow 6. The same idea applies to all changes proposed.
Co-Authored-By: Antoine James <antoine@ethereum.org>
Co-Authored-By: Jimmy Chen <jimmy@sigmaprime.io>
Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
N/A
Another find by @gitToki. Sort the preferred_ids in descending order as originally intended from the comment in the function.
Co-Authored-By: Pawan Dhananjay <pawandhananjay@gmail.com>
- Step 0 of the tree-sync roadmap https://github.com/sigp/lighthouse/issues/7678
Current lookup sync tests are written in an explicit way that assume how the internals of lookup sync work. For example the test would do:
- Emit unknown block parent message
- Expect block request for X
- Respond with successful block request
- Expect block processing request for X
- Response with successful processing request
- etc..
This is unnecessarily verbose. And it will requires a complete re-write when something changes in the internals of lookup sync (has happened a few times, mostly for deneb and fulu).
What we really want to assert is:
- WHEN: we receive an unknown block parent message
- THEN: Lookup sync can sync that block
- ASSERT: Without penalizing peers, without unnecessary retries
Keep all existing tests and add new cases but written in the new style described above. The logic to serve and respond to request is in this function `fn simulate` 2288a3aeb1/beacon_node/network/src/sync/tests/lookups.rs (L301)
- It controls peer behavior based on a `CompleteStrategy` where you can set for example "respond to BlocksByRoot requests with empty"
- It actually runs beacon processor messages running their clousures. Now sync tests actually import blocks, increasing the test coverage to the interaction of sync and the da_checker.
- To achieve the above the tests create real blocks with the test harness. To make the tests as fast as before, I disabled crypto with `TestConfig`
Along the way I found a couple bugs, which I documented on the diff.
Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com>
Add error rates metrics on unstable to benchmark against tree-sync. In my branch there are frequent errors but mostly connections errors as the node is still finding it set of stable peers.
These metrics are very useful and unstable can benefit from them ahead of tree-sync
Add three new metrics:
- sync_rpc_requests_success_total: Total count of sync RPC requests successes
- sync_rpc_requests_error_total: Total count of sync RPC requests errors
- sync_rpc_request_duration_sec: Time to complete a successful sync RPC requesst
Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com>
N/A
Fixes the issue where we were setting block observed timings for blocks that were potentially gossip invalid.
Thanks @gitToki for the find
Co-Authored-By: Pawan Dhananjay <pawandhananjay@gmail.com>
Co-Authored-By: Michael Sproul <michaelsproul@users.noreply.github.com>
