Skip to content

Conversation

yuroitaki
Copy link

@yuroitaki yuroitaki commented Feb 19, 2023

Issue Addressed

Which issue # does this PR address?
#3669

Proposed Changes

Please list or describe the changes introduced by this PR.

  • A new API to fetch fork choice data, as specified here
  • A new integration test to test the new API

Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.

  • extra_data field specified in the beacon-API spec is not implemented, please let me know if I should instead.

@CLAassistant
Copy link

CLAassistant commented Feb 19, 2023

CLA assistant check
All committers have signed the CLA.

@michaelsproul
Copy link
Member

Thanks @yuroitaki! It looks like there's a rustfmt failure which you can fix with cargo fmt --all

@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Feb 19, 2023
@yuroitaki
Copy link
Author

Thanks @yuroitaki! It looks like there's a rustfmt failure which you can fix with cargo fmt --all

Oops I ran cargo fmt -p http_api hence missed it, it should be fixed now!

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 20, 2023
@michaelsproul
Copy link
Member

Thanks! Will try to give this a proper review after the current release cycle ends. We're pushing hard to release v3.5.0 with Capella support right now.

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.

This looks really nice, very cleanly implemented.

If you fix up the merge conflict I think we might be able to merge this for v4.0.0/v4.1.0.

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. v4.1.0 Post-Capella minor release and removed ready-for-review The code is ready for review labels Mar 14, 2023
@michaelsproul
Copy link
Member

Nominating for v4.1.0 as the v4.0.0 deadline is very soon and we have a lot of other stuff to get in

@yuroitaki
Copy link
Author

This looks really nice, very cleanly implemented.

If you fix up the merge conflict I think we might be able to merge this for v4.0.0/v4.1.0.

Hi @michaelsproul sorry for the delay, I've fixed the merge conflict and it should be ready now!

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Mar 26, 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.

Thanks! Will merge with one stylistic suggestion applied

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Mar 28, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 29, 2023
## Issue Addressed

Which issue # does this PR address?
#3669

## Proposed Changes

Please list or describe the changes introduced by this PR.
- A new API to fetch fork choice data, as specified [here](ethereum/beacon-APIs#232)
- A new integration test to test the new API

## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.

- `extra_data` field specified in the beacon-API spec is not implemented, please let me know if I should instead.

Co-authored-by: Michael Sproul <[email protected]>
@bors bors bot changed the title Add debug fork choice api [Merged by Bors] - Add debug fork choice api Mar 29, 2023
@bors bors bot closed this Mar 29, 2023
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
## Issue Addressed

Which issue # does this PR address?
sigp#3669

## Proposed Changes

Please list or describe the changes introduced by this PR.
- A new API to fetch fork choice data, as specified [here](ethereum/beacon-APIs#232)
- A new integration test to test the new API

## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.

- `extra_data` field specified in the beacon-API spec is not implemented, please let me know if I should instead.

Co-authored-by: Michael Sproul <[email protected]>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
Which issue # does this PR address?
sigp#3669

Please list or describe the changes introduced by this PR.
- A new API to fetch fork choice data, as specified [here](ethereum/beacon-APIs#232)
- A new integration test to test the new API

Please provide any additional information. For example, future considerations
or information useful for reviewers.

- `extra_data` field specified in the beacon-API spec is not implemented, please let me know if I should instead.

Co-authored-by: Michael Sproul <[email protected]>
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. v4.1.0 Post-Capella minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants