Anchor Pre-PR: unstable and some changes from modularized-validator-store
#6966
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Update builder api for electra * Refactor mock builder to separate functionality * Return a higher payload value for builder by default * Add additional methods * Cleanup * Add a flag for always returning a max bid * Add logs for debugging * Take builder secret key as an argument * Merge branch 'unstable' into refactor-mock-builder * Change return type for submit_blinded_blocks * Merge branch 'unstable' into refactor-mock-builder * Respect gas_limit from validator registration * Revert "Respect gas_limit from validator registration" This reverts commit 1f7b4a3. * Merge branch 'unstable' into refactor-mock-builder * Remove unnecessary derive
* Add metadata v3 support to `node/identity` api.
…ted (sigp#6816) * Avoid computing columns from EL blobs if block has already been imported. * Downgrade a `warn` log to `debug` and update handling.
* Update discv5 dep * Handle yanked crates
sigp#4669 Modularize the beacon node backend to make it easier to add new database implementations
No substantial changes in v1.5.0-beta.1, this PR just updates the tests. The optimisation described in this PR is already implemented in our single-pass epoch processing: - ethereum/consensus-specs#4081
N/A Cover all error cases for decoding JsonExecutionRequests
Complements - sigp#6321 by detecting if the proposer signature is valid or not during RPC block processing. In lookup sync, if the invalid signature signature is the proposer signature, it's not deterministic on the block root. So we should only penalize the sending peer and retry. Otherwise, if it's on the body we should drop the lookup and penalize all peers that claim to have imported the block
N/A Add metrics that tell us if a duplicate message that we received was from a mesh peer or from a non mesh peer that we requested with iwant message.
N/A In sigp#6329 we changed `max_blobs_per_block` from a preset to a config value. We weren't using the right value based on fork in that PR. This is a follow up PR to use the fork dependent values. In the proces, I also updated other places where we weren't using fork dependent values from the ChainSpec. Note to reviewer: easier to go through by commit
When working on unrelated changes I noted: - An unnecessary closure left by a commit of some guy named @dapplion that can be removed - match statements that can be simplified with the new let else syntax - instead of mapping a result to ignore the Ok value, return
I was looking at sync and noticed a potential underflow and a typo, so just fixed those whilst I was in there.
Currently, we set the `chain_id` of range sync chains to `u64(hash(target_root, target_slot))`, which results in a long integer. ``` Jan 27 00:43:27.246 DEBG Batch downloaded, chain: 4223372036854775807, awaiting_batches: 0, batch_state: [p,E,E,E,E], blocks: 0, epoch: 0, service: range_sync ``` Instead, we can use `network_context.next_id()` as we do for all other sync items and get a unique sequential (not too big) integer as id. ``` Jan 27 00:43:27.246 DEBG Batch downloaded, chain: 4, awaiting_batches: 0, batch_state: [p,E,E,E,E], blocks: 0, epoch: 0, service: range_sync ``` Also, if a specific chain for the same target is retried later, it won't get the same ID so we can more clearly differentiate the logs associated with each attempt.
Addresses sigp#6706 This PR activates PeerDAS at the Fulu fork epoch instead of `EIP_7594_FORK_EPOCH`. This means we no longer support testing PeerDAS with Deneb / Electrs, as it's now part of a hard fork.
Update cargo dependencies while keeping `rust_eth_kzg` pinned to `0.5.1` due to the regression described in: - sigp#6608 The changes from that PR were not sufficient to actually pin the dependencies of `rust_eth_kzg`, because the dependencies from the workspace Cargo.toml file were not being used anywhere. To fix this, I've added them as explicit dependencies in `crypto/kzg/Cargo.toml`. With this change, `cargo update` no longer tries to update them.
https://github.com/sigp/lighthouse/actions/runs/13063781937/job/36452383133 `mdbook` ci job above is failing because the latest release now requires a newer version of glibc: > Updated the Linux pre-built binaries which requires a newer version of glibc (2.34). rust-lang/mdBook#2523 https://github.com/rust-lang/mdBook/blob/master/CHANGELOG.md Updating to latest ubuntu to fix this.
Addresses sigp#6026. Post-PeerDAS the DB expects to have data columns for the finalized block. Instead of forcing the user to submit the columns, this PR computes the columns from the blobs that we can already fetch from the checkpointz server or with the existing CLI options. Note 1: (EDIT) Pruning concern addressed Note 2: I have not tested this feature Note 3: @michaelsproul an alternative I recall is to not require the blobs / columns at this point and expect backfill to populate the finalized block
Updates libp2p to `0.55`. Will address the deprecations in a subsequent PR
Run `cargo update` to address [RUSTSEC-2025-0004](https://rustsec.org/advisories/RUSTSEC-2025-0004), a vulnerability in `openssl`. I don't think we are affected, but this PR is required for us to pass `cargo audit` and unblock CI.
Hopefully fixes sigp#6732 In our `scheduled_subscriptions`, we were setting unsubscription slot to be `current_slot + 1`. Given that we were subscribing to the subnet at `duty.slot - 1`, the unsubscription slot ended up being `duty.slot`. So we were unsubscribing to the subnet at the beginning of the duty slot which is insane. Fixes the `scheduled_subscriptions` to unsubscribe at `duty.slot + 1`.
`TODO(das)` now that PeerDAS is scheduled in a hard fork we can subscribe to its topics on the fork activation. In current stable we subscribe to PeerDAS topics as soon as the node starts if PeerDAS is scheduled. This PR adds another todo to unsubscribe to blob topics at the fork. This other PR included solution for that, but I can include it in a separate PR - https://github.com/sigp/lighthouse/pull/5899/files Include PeerDAS topics as part of Fulu fork in `fork_core_topics`.
Resolve a `TODO(das)` to use KZG batch verification in `put_rpc_custody_columns` Uses `verify_kzg_for_data_column_list_with_scoring` in all paths that send more than one column. To use batch verification and have attributability of which peer is sending a bad column. Needs to move `verify_kzg_for_data_column_list_with_scoring` into the type's module to convert to the KZG verified type.
Partially sigp#5900 Migrate the validator client cli to clap derive
This optimizes the time it takes to load the context, so that tests do not time out
There were two things I came across during some recent testing, that this PR addresses. 1 - The default port for IPv6 was set to 9090, which is confusing. I've set this to match its ipv4 counterpart (i.e 9000 and 9001). This makes more sense and is easier to firewall, for those firewalls that support both versions for a single rule. 2 - Watching the NAT status of lighthouse, I notice we only set the field to 1 once the NAT is passed. We don't give it a default 0 (false). So we only see results when its successful. On peer disconnects, i've piggy-backed a loop of the connected peers to also watch and check for NAT status updates.
N/A Previously, we were returning an empty vec of Nones if get_blobs was not supported in the EL. This results in confusing logging where we try to process the empty list of blobs and log a bunch of Unexpected errors. See ``` Feb 03 17:32:12.383 DEBG Fetching blobs from the EL, num_expected_blobs: 6, block_root: 0x7326fe2dc1cb9036c9de7a07a662c86a339085597849016eadf061b70b7815ba, service: fetch_engine_blobs, service: beacon, module: beac on_chain::fetch_blobs:84 Feb 03 17:32:12.384 DEBG Processing engine blobs, num_fetched_blobs: 0, block_root: 0x7326fe2dc1cb9036c9de7a07a662c86a339085597849016eadf061b70b7815ba, service: fetch_engine_blobs, service: beacon, module: beacon_c hain::fetch_blobs:197 Feb 03 17:32:12.384 ERRO Error fetching or processing blobs from EL, block_root: 0x7326fe2dc1cb9036c9de7a07a662c86a339085597849016eadf061b70b7815ba, error: BlobProcessingError(AvailabilityCheck(Unexpected)), module : network::network_beacon_processor:1011 ``` The error we should be getting is that getBlobs is not supported, this PR adds a new error variant and returns that.
Part of - sigp#6258 To address PeerDAS sync issues we need to make individual by_range requests within a batch retriable. We should adopt the same pattern for lookup sync where each request (block/blobs/columns) is tracked individually within a "meta" request that group them all and handles retry logic. - Building on sigp#6398 second step is to add individual request accumulators for `blocks_by_range`, `blobs_by_range`, and `data_columns_by_range`. This will allow each request to progress independently and be retried separately. Most of the logic is just piping, excuse the large diff. This PR does not change the logic of how requests are handled or retried. This will be done in a future PR changing the logic of `RangeBlockComponentsRequest`. ### Before - Sync manager receives block with `SyncRequestId::RangeBlockAndBlobs` - Insert block into `SyncNetworkContext::range_block_components_requests` - (If received stream terminators of all requests) - Return `Vec<RpcBlock>`, and insert into `range_sync` ### Now - Sync manager receives block with `SyncRequestId::RangeBlockAndBlobs` - Insert block into `SyncNetworkContext:: blocks_by_range_requests` - (If received stream terminator of this request) - Return `Vec<SignedBlock>`, and insert into `SyncNetworkContext::components_by_range_requests ` - (If received a result for all requests) - Return `Vec<RpcBlock>`, and insert into `range_sync`
Fixes sigp#5206, a low-hanging fruit.
We were using the wrong queue length for attestation work event metrics.
A temporary workaround for the failing execution tests for geth. https://github.com/sigp/lighthouse/actions/runs/13192297954 The test is broken due to the following breaking changes in geth that requires updating our tests: 1. removal of `personal` namespace in v1.14.12: See #30704 2. removal of `totalDifficulty` field from RPC in v1.14.11. See #30386. Using an older version for now (` 1.14.10`) as we need to get things merged for the upcoming release. Will create a separate issue to fix this.
Noted that there's a bit of fork boiler plate in fork context. If we list a mapping of ForkName -> fork_version in the ForkName enum we can get rid of it :) Not much, but should make the next fork a tiny tit less annoying
N/A Removed metrics that were defined but not used anywhere.
- PR sigp#6497 made obsolete some consistency checks inside the batch I forgot to remove the consumers of those errors Remove un-used batch sync error condition, which was a nested `Result<_, Result<_, E>>`
… set to false (sigp#6766) - sigp#6510 - Keep execution payload during historical backfill when `--prune-payloads false` is set - Add a field in the historical backfill debug log to indicate if execution payload is kept - Add a test to check historical blocks has execution payload when `--prune-payloads false is set - Very minor typo correction that I notice when working on this
Fix another issue with fetch-blobs, similar to: - sigp#6911 Check if the list of blobs returned is all `None`, and if so, do not proceed any further. This prevents an ugly error like: > Feb 03 17:32:12.384 ERRO Error fetching or processing blobs from EL, block_root: 0x7326fe2dc1cb9036c9de7a07a662c86a339085597849016eadf061b70b7815ba, error: BlobProcessingError(AvailabilityCheck(Unexpected)), module : network::network_beacon_processor:1011
Closes - sigp#6805 - Use a new `WorkEvent::GossipAttestationToConvert` to handle the conversion from `SingleAttestation` to `Attestation` _on_ the beacon processor (prevents a Tokio thread being blocked). - Improve the error handling for single attestations. I think previously we had no ability to reprocess single attestations for unknown blocks -- we would just error. This seemed to be the case in both gossip processing and processing of `SingleAttestation`s from the HTTP API. - Move the `SingleAttestation -> Attestation` conversion function into `beacon_chain` so that it can return the `attestation_verification::Error` type, which has well-defined error handling and peer penalties. The now-unused variants of `types::Attestation::Error` have been removed.
This PR adds an implementation to get fork_version and fork_epoch given a `ForkName`. I didn't realize that this is already implemented in the `ChainSpec` sorry - sigp#6933 Remove duplicated fork_epoch and fork_version implementation
Closes: - sigp#6818 Use `MAX_EFFECTIVE_BALANCE_ELECTRA` (2048) for attestation reward calculations involving Electra. Add a new `InteropGenesisBuilder` that tries to provide a more flexible way to build genesis states. Unfortunately due to lifetime jank, it is quite unergonomic at present. We may want to refactor this builder in future to make it easier to use.
- Re-opened PR from sigp#6869 Writing and running tests I noted that the sync RPC requests are very verbose now. `DataColumnsByRootRequestId { id: 123, requester: Custody(CustodyId { requester: CustodyRequester(SingleLookupReqId { req_id: 121, lookup_id: 101 }) }) }` Since this Id is logged rather often I believe there's value in 1. Making them more succinct for log verbosity 2. Make them a string that's easy to copy and work with elastic Write custom `Display` implementations to render Ids in a more DX format _ DataColumnsByRootRequestId with a block lookup_ ``` 123/Custody/121/Lookup/101 ``` _DataColumnsByRangeRequestId_ ``` 123/122/RangeSync/0/5492900659401505034 ``` - This one will be shorter after sigp#6868 Also made the logs format and text consistent across all methods
Update spec tests for recent v1.5.0-beta.2 release. There are no substantial changes for Electra and earlier, and the Fulu test updates to be made are tracked here: - sigp#6957 - Add `SingleAttestation` SSZ tests - Add new `deposit_with_reorg` fork choice tests - Update tag to v1.5.0-beta.2 - Ignore Fulu tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Same as last time: this is a PR for the
anchorbranch and requires no in-depth review.This merges the
unstablebranch and recent changes from #6705 intoanchor. The main motivator for this PR is to get the update tolibp2p 0.55into the branch.Ignore the failing CI and please merge without squashing!