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] - beacon_node: add --disable-deposit-contract-sync flag #3597

Closed
wants to merge 4 commits into from

Conversation

pinkiebell
Copy link
Contributor

@pinkiebell pinkiebell commented Sep 21, 2022

Overrides any previous option that enables the eth1 service.
Useful for operating a light beacon node.

@CLAassistant
Copy link

CLAassistant commented Sep 21, 2022

CLA assistant check
All committers have signed the CLA.

@paulhauner
Copy link
Member

Heyo, thanks for the contribution. I'm open to the "disable-eth1-sync" flag, however we intentionally don't provide a "disable-backfill" flag since there's no protocol-level solution to storing blocks unless we have nodes that sync them. If all nodes were to run with this flag then we'd be at risk of losing those blocks forever. I know this is not ideal and it would be great to find a protocol-level solution, however until then I think forcing users to download them is the safest route.

Would you be open to removing the backfill flag please?

@paulhauner paulhauner added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Sep 22, 2022
@pinkiebell pinkiebell changed the title Add 'light' beacon node configuration options beacon_node: add --disable-eth1-sync flag Sep 27, 2022
@pinkiebell
Copy link
Contributor Author

@paulhauner
Done ✅

@paulhauner paulhauner 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 Sep 28, 2022
@paulhauner paulhauner self-requested a review September 28, 2022 06:28
@paulhauner paulhauner self-assigned this Sep 28, 2022
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 for the PR! Just a few minor points of feedback 🙏

beacon_node/src/cli.rs Outdated Show resolved Hide resolved
beacon_node/src/config.rs Outdated Show resolved Hide resolved
@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Oct 17, 2022
Overrides any previous option that enables the eth1 service.
@pinkiebell pinkiebell changed the title beacon_node: add --disable-eth1-sync flag beacon_node: add --disable-deposit-contract-sync flag Oct 18, 2022
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 19, 2022
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!

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 19, 2022
Overrides any previous option that enables the eth1 service.
Useful for operating a `light` beacon node.

Co-authored-by: Michael Sproul <[email protected]>
@bors bors bot changed the title beacon_node: add --disable-deposit-contract-sync flag [Merged by Bors] - beacon_node: add --disable-deposit-contract-sync flag Oct 20, 2022
@bors bors bot closed this Oct 20, 2022
@pinkiebell pinkiebell deleted the light branch October 20, 2022 09:59
bors bot pushed a commit that referenced this pull request Oct 25, 2022
## Issue Addressed

NA

## Proposed Changes

Bump version to `v3.2.0`

## Additional Info

- ~~Blocked on #3597~~
- ~~Blocked on #3645~~
- ~~Blocked on #3653~~
- ~~Requires additional testing~~
bors bot pushed a commit that referenced this pull request Oct 25, 2022
## Issue Addressed

NA

## Proposed Changes

Bump version to `v3.2.0`

## Additional Info

- ~~Blocked on #3597~~
- ~~Blocked on #3645~~
- ~~Blocked on #3653~~
- ~~Requires additional testing~~
bors bot pushed a commit that referenced this pull request Oct 25, 2022
## Issue Addressed

NA

## Proposed Changes

Bump version to `v3.2.0`

## Additional Info

- ~~Blocked on #3597~~
- ~~Blocked on #3645~~
- ~~Blocked on #3653~~
- ~~Requires additional testing~~
macladson pushed a commit to macladson/lighthouse that referenced this pull request Jan 5, 2023
Overrides any previous option that enables the eth1 service.
Useful for operating a `light` beacon node.

Co-authored-by: Michael Sproul <[email protected]>
macladson pushed a commit to macladson/lighthouse that referenced this pull request Jan 5, 2023
## Issue Addressed

NA

## Proposed Changes

Bump version to `v3.2.0`

## Additional Info

- ~~Blocked on sigp#3597~~
- ~~Blocked on sigp#3645~~
- ~~Blocked on sigp#3653~~
- ~~Requires additional testing~~
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
Overrides any previous option that enables the eth1 service.
Useful for operating a `light` beacon node.

Co-authored-by: Michael Sproul <[email protected]>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

NA

## Proposed Changes

Bump version to `v3.2.0`

## Additional Info

- ~~Blocked on sigp#3597~~
- ~~Blocked on sigp#3645~~
- ~~Blocked on sigp#3653~~
- ~~Requires additional testing~~
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants