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] - Address ENR update loop #2216

Closed
wants to merge 3 commits into from
Closed

Conversation

paulhauner
Copy link
Member

@paulhauner paulhauner commented Feb 21, 2021

Issue Addressed

Proposed Changes

Addresses a potential loop when the majority of peers indicate that we are contactable via an IPv6 address.

See sigp/discv5#62 for further rationale.

Additional Info

The alternative to this PR is to use --disable-enr-auto-update and then manually supply an --enr-address and --enr-upd-port. However, that requires the user to know their IP addresses in order for discovery to work properly. This might not be practical/achievable for some users, hence this hotfix.

@paulhauner paulhauner added the ready-for-review The code is ready for review label Feb 21, 2021
@paulhauner paulhauner changed the base branch from stable to unstable February 21, 2021 01:18
@@ -5,7 +5,7 @@ authors = ["Sigma Prime <contact@sigmaprime.io>"]
edition = "2018"

[dependencies]
discv5 = { version = "0.1.0-beta.3", features = ["libp2p"] }
discv5 = { git = "https://github.com/sigp/discv5 ", branch = "enr-loop", features = ["libp2p"] }
Copy link
Member

Choose a reason for hiding this comment

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

We should use the hash in master, so that when the branch gets removed this commit still works

Copy link
Member Author

Choose a reason for hiding this comment

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

Waiting on sigp/discv5#63 to merge then I can use the master commit.

@paulhauner
Copy link
Member Author

I've pushed an updated docker image to paulhauner/lighthouse:v1.1.2-hotfix.1.

I recommend any users that are experiencing the issue in #2215 to either compile from this branch or use the aformentioned docker image.

We will push a formal hotfix release next week once we've passed review and full pre-release testing.

@Wazzymandias
Copy link

Just pulled the new image, I think it resolved the problem, will continue monitoring to confirm. Appreciate the really fast turnaround on this!

@paulhauner
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Feb 21, 2021
## Issue Addressed

- Resolves #2215

## Proposed Changes

Addresses a potential loop when the majority of peers indicate that we are contactable via an IPv6 address.

See sigp/discv5#62 for further rationale.

## Additional Info

The alternative to this PR is to use `--disable-enr-auto-update` and then manually supply an `--enr-address` and `--enr-upd-port`. However, that requires the user to know their IP addresses in order for discovery to work properly. This might not be practical/achievable for some users, hence this hotfix.
@paulhauner
Copy link
Member Author

bors r-

Trying to batch

@bors
Copy link

bors bot commented Feb 21, 2021

Canceled.

@paulhauner
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Feb 21, 2021
## Issue Addressed

- Resolves #2215

## Proposed Changes

Addresses a potential loop when the majority of peers indicate that we are contactable via an IPv6 address.

See sigp/discv5#62 for further rationale.

## Additional Info

The alternative to this PR is to use `--disable-enr-auto-update` and then manually supply an `--enr-address` and `--enr-upd-port`. However, that requires the user to know their IP addresses in order for discovery to work properly. This might not be practical/achievable for some users, hence this hotfix.
@bors bors bot changed the title Address ENR update loop [Merged by Bors] - Address ENR update loop Feb 22, 2021
@bors bors bot closed this Feb 22, 2021
@michaelsproul michaelsproul deleted the enr-loop branch February 22, 2021 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review The code is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Beacon Node memory spikes and crashes, nonstop loop ENR address update
3 participants