-
Notifications
You must be signed in to change notification settings - Fork 957
Update chainspec p2p and etc #8601
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
base: unstable
Are you sure you want to change the base?
Conversation
|
You'll need to rebase on unstable to get CI to pass |
|
I'll be OOO until Jan 12 but someone might be able to review |
|
Some required checks have failed. Could you please take a look @0xmrree? 🙏 |
chong-he
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested the beacon API output and the fields added in this PR are now displayed correctly as the output for mainnet.
Some comments below. Some CIs are failing too, so would be great if you can fix them, thanks.
common/eth2_network_config/built_in_network_configs/chiado/config.yaml
Outdated
Show resolved
Hide resolved
f8b6209 to
618615c
Compare
|
@chong-he addressed the above feedback, ran cargo fmt --all, reran cargo nextest run -p types, and re tested api. the ci failures should be gone now that i have added default values for the added config fields. also the code is much simpler as i am not longer changing any values in the config anymore. |
chong-he
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking much better, just a bit of minor comments and something that I ain't sure about - will need a dev look into this
Thanks!
common/eth2_network_config/built_in_network_configs/chiado/config.yaml
Outdated
Show resolved
Hide resolved
87f79c9 to
d79505b
Compare
|
addressed feedback and ready for another review |
This now looks great to me, the beacon API query for As this involves spec change, we will wait for a final review from the team in case I miss anything. |
Issue Addressed
Missing values in /eth/v1/config/spec #8571
- there will be follow up PR for the re org props
Proposed Changes
to
/eth/v1/config/spec2. Had to change the minimal config for UPDATE_TIMEOUT to get currents tests to pass. This is ok given UPDATE_TIMEOUT is not used in lighthouse as this config for light client spec from altair
3. ATTESTATION_SUBNET_PREFIX_BITS is now dynamically calculated and shimmed into the /eth/v1/config/spec output as advised by @michaelsproul
Testing
./target/release/lighthouse bn --network sepolia ...and ranwhich returned
{ "EPOCHS_PER_SUBNET_SUBSCRIPTION": "256", "ATTESTATION_SUBNET_COUNT": "64", "ATTESTATION_SUBNET_EXTRA_BITS": "0", "ATTESTATION_SUBNET_PREFIX_BITS": "6", "UPDATE_TIMEOUT": "8192", "DOMAIN_BLS_TO_EXECUTION_CHANGE": "0x0a000000" }