Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc. dependency cleanup #6810

Merged
merged 3 commits into from
Jan 16, 2025
Merged

Misc. dependency cleanup #6810

merged 3 commits into from
Jan 16, 2025

Conversation

dknopik
Copy link
Member

@dknopik dknopik commented Jan 15, 2025

Issue Addressed

Anchor depends on some parts of Lighthouse. Due to certain dependencies between crates, it pulled in far more crates than necessary, causing longer builds.

Proposed Changes

remove ensure_dir_exists: account_utils and validator_dir used the directory crate for a single function: ensure_dir_exists. This function had questionable value, as create_dir_all is a no-op if the directory exists anyway. Remove ensure_dir_exists and the two dependencies on directory, which is potentially expensive to depend on due to its dependency on clap_utils.

group UNHANDLED_ERRORs into a generic: warp_utils depends on beacon_chain to provide beacon_chain_error. This causes validator_client to depend on beacon_chain transitively, which is not nice.
Remove beacon_chain_error and similar functions and provide a generic unhandled_error instead, which behaves exactly the same.

Introduce separate health_metrics crate by Sayan: The warp_utils crate contained facilities for capturing health metrics. Anchor wants those metrics, but also wants to avoid pulling in warp, as we are axum-only, so we introduce a new crate called health_metrics. We use the opportunity to also move the logic for capturing this data from eth2 to that new crate.

Additional Info

Lighthouse also benefits from this, as the build can run more parallelized.

dknopik and others added 3 commits January 15, 2025 15:26
* separate health_metrics crate

* remove metrics from warp_utils

* move ProcessHealth::observe and SystemHealth::observe to health_metrics

* fix errors

* nitpick `Cargo.toml`s

---------

Co-authored-by: Daniel Knopik <daniel@dknopik.de>
# Conflicts:
#	Cargo.toml
@AgeManning
Copy link
Member

Does anchor depend on warp_utils? In principle we don't want anything to do with warp, but maybe there's stuff in there we needed?

Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an improvement to me

@AgeManning
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Jan 16, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 669932a

@mergify mergify bot merged commit 669932a into sigp:unstable Jan 16, 2025
30 checks passed
@dknopik
Copy link
Member Author

dknopik commented Jan 16, 2025

Does anchor depend on warp_utils? In principle we don't want anything to do with warp, but maybe there's stuff in there we needed?

Not any more after the health_metrics crate was introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants