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

[Merged by Bors] - Use only lighthouse types in the mock builder #4793

Closed

Conversation

realbigsean
Copy link
Member

Proposed Changes

  • only use LH types to avoid build issues
  • use warp instead of axum for the server to avoid importing the dep

Additional Info

  • wondering if we can move the execution_layer/test_utils to its own crate and import it as a dev dependency
  • this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
  • or maybe we can look into using reth types for the engine api if they are in their own crate

@realbigsean realbigsean added the ready-for-review The code is ready for review label Sep 28, 2023
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.

Fantastic! Should hopefully make compilation a bit lighter with those deps removed

impl<E: EthSpec> BidStuff<E> for BuilderBid<E, BlindedPayload<E>> {
fn set_fee_recipient(&mut self, fee_recipient: Address) {
match self.header.to_mut() {
BlindedPayloadRefMut::Merge(payload) => {
Copy link
Member

Choose a reason for hiding this comment

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

Wanted to rewrite this with a mapping macro but realised it isn't possible. Opened an issue on superstruct: sigp/superstruct#31

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. v4.6.0 ETA Q1 2024 dependencies Pull requests that update a dependency file and removed ready-for-review The code is ready for review labels Sep 29, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Sep 29, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@paulhauner
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Sep 29, 2023

Canceled.

@paulhauner
Copy link
Member

bors r+

@paulhauner
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Sep 29, 2023

Canceled.

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed:

@realbigsean
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed:

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Thanks Sean!

Comment on lines 598 to 601
pub fn set_mock_builder(
&mut self,
beacon_url: SensitiveUrl,
) -> impl std::future::Future<Output = ()> {
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
pub fn set_mock_builder(
&mut self,
beacon_url: SensitiveUrl,
) -> impl std::future::Future<Output = ()> {
pub async fn set_mock_builder(&mut self, beacon_url: SensitiveUrl) {

and bellow call mock_builder_server.await

Copy link
Member Author

Choose a reason for hiding this comment

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

true! updated

Copy link
Member Author

Choose a reason for hiding this comment

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

impl Future actually implicitly has a 'static lifetime, and switching to async changes this, so had to change it back

@@ -193,7 +180,7 @@ impl<E: EthSpec> MockBuilder<E> {
beacon_url: SensitiveUrl,
spec: ChainSpec,
executor: TaskExecutor,
) -> (Self, MockBuilderServer) {
) -> (Self, (SocketAddr, impl Future<Output = ()>)) {
Copy link
Member

Choose a reason for hiding this comment

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

same as above, we can just:

Suggested change
) -> (Self, (SocketAddr, impl Future<Output = ()>)) {
) -> (Self, SocketAddr) {

if we change the signature of the function to async fn and .await on server

Copy link
Member

Choose a reason for hiding this comment

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

We want to lazily evaluate the server future though, after the socket address has been returned and used by the caller.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yea, this one I think we have to leave

@realbigsean
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Build failed:

@realbigsean
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Build failed:

  • dockerfile-ubuntu
  • merge-transition-ubuntu
  • release-tests-ubuntu
  • state-transition-vectors-ubuntu

@realbigsean
Copy link
Member Author

bors retry

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Build failed:

@realbigsean
Copy link
Member Author

bors retry

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Build failed:

@realbigsean
Copy link
Member Author

bors retry

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Build failed:

@realbigsean
Copy link
Member Author

bors retry

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors
Copy link

bors bot commented Oct 3, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Use only lighthouse types in the mock builder [Merged by Bors] - Use only lighthouse types in the mock builder Oct 3, 2023
@bors bors bot closed this Oct 3, 2023
@realbigsean realbigsean deleted the lh-types-in-mock-builder branch November 21, 2023 16:15
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Proposed Changes

- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

## Additional Info

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate


Co-authored-by: realbigsean <seananderson33@gmail.com>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate

Co-authored-by: realbigsean <seananderson33@gmail.com>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
- only use LH types to avoid build issues
- use warp instead of axum for the server to avoid importing the dep

- wondering if we can move the `execution_layer/test_utils` to its own crate and import it as a dev dependency
- this would be made easier by separating out our engine API types into their own crate so we can use them in the test crate
- or maybe we can look into using reth types for the engine api if they are in their own crate

Co-authored-by: realbigsean <seananderson33@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file ready-for-merge This PR is ready to merge. v4.6.0 ETA Q1 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants