Skip to content

Split the VC into crates making it more modular #6453

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

Merged
merged 30 commits into from
Nov 8, 2024

Conversation

AgeManning
Copy link
Member

Modularize the VC

The VC currently is built mostly as a single module. It self-references itself making it hard to modularize its components.

This PR splits the VC into multiple sub-crates that can be imported independently making it useful to external projects, but also making the type dependencies cleaner and the code more logically grouped.

@AgeManning
Copy link
Member Author

I modified the general config, to include some sub-configs (It was required in order to split up dependencies and avoid circular ones).

The only thing I can really foresee potentially breaking is serializing and de-serializing the validator config. I'm flattening the new inner configs, so I think it should behave the same, but I've not tested it.

@AgeManning AgeManning marked this pull request as ready for review October 21, 2024 22:02
@AgeManning AgeManning added the v6.0.0 New major release for hierarchical state diffs label Oct 28, 2024
@AgeManning AgeManning added the ready-for-review The code is ready for review label Oct 30, 2024
@michaelsproul
Copy link
Member

I reverted the cargo update changes so we can get CI to pass on this branch. There's a regression caused by one of our deps:

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Reviewed half, will come back for the rest tomorrow

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks good on the whole. This was less daunting than I expected.

I'll push some small tweaks and then we can merge.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 8, 2024
@michaelsproul
Copy link
Member

@mergify queue

Copy link

mergify bot commented Nov 8, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 8e95024

@mergify mergify bot merged commit 8e95024 into sigp:unstable Nov 8, 2024
29 checks passed
chong-he pushed a commit to chong-he/lighthouse that referenced this pull request Nov 26, 2024
* Starting to modularize the VC

* Revert changes to eth2

* More progress

* More progress

* Compiles

* Merge latest unstable and make it compile

* Fix some lints

* Tests compile

* Merge latest unstable

* Remove unnecessary deps

* Merge latest unstable

* Correct release tests

* Merge latest unstable

* Merge remote-tracking branch 'origin/unstable' into modularize-vc

* Merge branch 'unstable' into modularize-vc

* Revert unnecessary cargo lock changes

* Update validator_client/beacon_node_fallback/Cargo.toml

* Update validator_client/http_metrics/Cargo.toml

* Update validator_client/http_metrics/src/lib.rs

* Update validator_client/initialized_validators/Cargo.toml

* Update validator_client/signing_method/Cargo.toml

* Update validator_client/validator_metrics/Cargo.toml

* Update validator_client/validator_services/Cargo.toml

* Update validator_client/validator_store/Cargo.toml

* Update validator_client/validator_store/src/lib.rs

* Merge remote-tracking branch 'origin/unstable' into modularize-vc

* Fix format string

* Rename doppelganger trait

* Don't drop the tempdir

* Cargo fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v6.0.0 New major release for hierarchical state diffs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants