Merged
Conversation
Maddiaa0
changed the title
Contributor
|
Nice - thank you. |
Maddiaa0
added a commit
that referenced
this pull request
Mar 10, 2025
## Overview Again like #12589 this is added after a chat with charlie where we want to remove the need for a complex script. #12566 asks the user for a private key as it needs to set two env vars to the same key. This pr introduces the notion of a fallback, such that a config variable will also check a list of fallbacks before using the default value. In this case, if `VALIDATOR_PRIVATE_KEY` is set, then `SEQ_PUBLISHER_PRIVATE_KEY`` does not need to be set, removing the need for it in the script.
Maddiaa0
commented
Mar 10, 2025
PhilWindle
approved these changes
Mar 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Triggered by a discussion with charlie over #12566 we think its
important to make sure that we have sound defaults across the stack, to reduce the amount of lifting extra
scripts have to do
In part of this we decided to look at the p2p config, which takes up a number of environment variables, in all of our
testnets, we have never run tcp or udp on seperate ports, we have also never run listen address as anything other than
0.0.0.0.
So we can unify the tcp and udp ports to be under the same port, im open to changing this in the future, but theres honestly no
real need to have them different.
Discovery change
If you are not running behind a NAT, then we can determine your public ip automatically by setting the updateEnr value in
Discv5 to true, this will determine your public ip, and update your ENR to use it.
If you are running behind a NAT you will be required to fill in your ip, but only for a single flag: --p2p.p2pIp. Furthermore
we can use the existing p2p.queryForIp option.
Some config examples:
If you are not behind a NAT, then due to the discv5 config update, the defaults should just work
If you are behind a NAT, this should be all you need to set:
If you want to change your port
If you know you want to override your public ip
Should be enough to work out your IP address automatically, and be all that a user needs to provide.
Changes
Removed
P2P_TCP_LISTEN_ADDR
P2P_UDP_LISTEN_ADDR
P2P_TCP_ANNOUNCE_ADDR
P2P_UDP_ANNOUNCE_ADDR
replaced with:
P2P_PORT - defaults to 40400
P2P_IP - defaults to working it out using disv5, otherwise set P2P_QUERY_FOR_IP if you are behind a NAT
P2P_LISTEN_ARR - defaults to 0.0.0.0, should never need to be set
K8s
In theory, these updates should mean that we can remove the setup-p2p-addresses.sh script entirely, as the nodes can query their own public ip instead of us resolving it from the cluster, but i do not want to make this change in this pr, as its something that we will need to debug, and i do not want to make such a change ahead of testnet