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

Make all beacon chain config values explicit and move to common directory #48

Merged

Conversation

jimmygchen
Copy link
Contributor

Prysm populate mainnet config values by default if any value is not supplied.

This behaviour does not exist on other clients (Teku, Lighthouse afaik). Making these explicit in the common beacon config will allow all other clients to easily reuse them.

Copy link
Collaborator

@roberto-bayardo roberto-bayardo left a comment

Choose a reason for hiding this comment

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

this is great thank you.

@roberto-bayardo
Copy link
Collaborator

Oh hm this is breaking the continuous build tests; looks like the prysm beacon nodes might not be coming up? Does the failing test (go run ./tests/pre-4844) pass for you locally?

@jimmygchen
Copy link
Contributor Author

Oh hm this is breaking the continuous build tests; looks like the prysm beacon nodes might not be coming up? Does the failing test (go run ./tests/pre-4844) pass for you locally?

Hi @roberto-bayardo thanks for the looking into this! I'll try running the tests locally. Not sure if PRESET_BASE config location in the file matters, but I should probably go with the convention and move it to the top of file

@jimmygchen
Copy link
Contributor Author

jimmygchen commented Nov 5, 2022

@roberto-bayardo

after moving PRESET_BASE to the top, the test runs locally now without any error.. so it seems like the order matters

EDIT: I just reverted the change and the test still passes 😅 let me take a look again on the CI logs..

2022/11/05 23:47:50 waiting for geth
2022/11/05 23:47:57 waiting for prysm beacon node
2022/11/05 23:48:00 waiting for prysm beacon node follower
2022/11/05 23:48:06 waiting for validator
2022/11/05 23:48:06 Nonce: 0
2022/11/05 23:48:06 Waiting for transaction (0xf6e4d98e2d071638d7c9d15a1ab7a70f03536d4ce39d1b8137b7e6123fda89cb) to be included...

@roberto-bayardo
Copy link
Collaborator

@jimmygchen make sure you run "make devnet-clean" or similar to force a rebuild between changes! I have made that mistake many a time.... the current scripts aren't smart enough to rebuild what's needed when you make changes to the subrepos.

@roberto-bayardo
Copy link
Collaborator

roberto-bayardo commented Nov 5, 2022

Hmm tests are working for me locally when I include your PR. Wonder if the CI config needs to be updated somehow.

@roberto-bayardo
Copy link
Collaborator

I think our timeout is just too short... maybe they started running the CIs on slower machines! I will try doubling it.

@roberto-bayardo
Copy link
Collaborator

Ahah, they seem to be running the CI on older processors without the instructions expected

  • beacon-node --accept-terms-of-use --verbosity=debug --datadir /chaindata --force-clear-db --interop-eth1data-votes --http-web3provider=http://execution-node:8545 --deposit-contract 0x8A04d14125D0FDCDc742F4A05C051De07232EDa4 --chain-config-file=/config/chain-config.yml --contract-deployment-block 0 --interop-num-validators 4 --rpc-host 0.0.0.0 --rpc-port 4000 --grpc-gateway-host 0.0.0.0 --grpc-gateway-port 3500 --enable-debug-rpc-endpoints --p2p-local-ip 0.0.0.0 --p2p-host-ip 172.18.0.5 --p2p-priv-key=/etc/prysm-priv-key --subscribe-all-subnets --enable-tracing --tracing-endpoint http://jaeger-tracing:14268/api/traces --tracing-process-name beacon-node --min-sync-peers=0 --bootstrap-node=
    Caught SIGILL in blst_cgo_init, consult /bindinds/go/README.md.

@jimmygchen I will fix this later.. feel free to merge (you can back out my timeout update & the PRESET_BASE change if you liike)

@jimmygchen
Copy link
Contributor Author

ahh nice catch. Thanks for looking into this! I'll revert the timeout changes and keep the PRESET_BASE as it is since most config files have them on the top anyways.

@roberto-bayardo roberto-bayardo merged commit 591a01b into Inphi:devnet-v3 Nov 5, 2022
@jimmygchen jimmygchen deleted the move-beacon-chain-config branch November 7, 2022 01:56
roberto-bayardo added a commit that referenced this pull request Nov 7, 2022
…tory (#48)

* Add common beacon chain config file

* Move PRESET_BASE to top of config file

* bump service start timeout from 1 to 2 min

* Revert timeout change from 2 minutes to 60 sec

Co-authored-by: Roberto Bayardo <bayardo@alum.mit.edu>
roberto-bayardo added a commit that referenced this pull request Nov 17, 2022
…ongoing development (#60)

* create devnet v3 branch including submodules + new submodule for erigon client

* Add Lodestar

* Supply chain config

* Cache all docker work before yarn build

* Build packages individually to allow them to cache

* We must also build the CLI

* Fix clean

* Update to head of Lodestar

* Fun through Capella

* Latest Lodestar

* latest prsym

* Remove nonfunctional setting

* Bump Lodestar

* update geth to latest merge w/ master (#46)

* Make all beacon chain config values explicit and move to common directory (#48)

* Add common beacon chain config file

* Move PRESET_BASE to top of config file

* bump service start timeout from 1 to 2 min

* Revert timeout change from 2 minutes to 60 sec

Co-authored-by: Roberto Bayardo <bayardo@alum.mit.edu>

* add BLST_PORTABLE to CGO_CFLAGS to fix SIGILL issue with blst (#49)

* Update lodestar submodule. Copy trusted setup

* Update devnet v3 with latest subrepos and new test from master (#51)

* Add historical sync test (#44)

Asserts that the beacon node follower can sync blobs during initial-sync.
This patch also refactors the test harness to accommodate dynamic nodes.

* update subrepos to latest

Co-authored-by: Inphi <mlaw2501@gmail.com>

* Update test harness to start either prysm or lodestar

* Fix the Prysm communication

* Fix the Lodestar wiring

* Fix up validator <-> beacon node communication

* Start switching tests to real beacon API

* fee-market tests pass against Prysm without gRPC

* Fix initial-sync test

* Switch prysm to my branch which has GetBlockJSON

* Configure Prysm to upgrade later (as though it had Capella). Also run Lodestar pre-4844 test in CI

* Fix make lodestar-up

* Remove Prysm-specific chain config. We used a shared config now.

* Point Prysm back at the devnet-v3 branch

* Update geth to latest from devnet-v3

* Put erigon back

* Remove preset base, let it take the default

* Attempt to parallelize CI

* idunno lol

* Try two jobs

* Comment failing Lodestar CI test

* Fix clean

* Use basename pwd instead

* Go back to using --project-name, but correctly supply it to docker compose down also

* upgrade subrepos to latest (#56)

* update geth & prysm subrepos to latest, pointing at eip-4844 instead of devnet-v3 (#57)

* update subrepos to latest, move back to using eip-4844 branch for development

* add 30 min timeout to some tests that sometimes hang

* fix CI, Makefile, and update Prysm & geth subrepos

Note: the new prysm & geth subrepos use a different kzg trusted setup matching, which now matches the latest consensus spec

Co-authored-by: dancoffman <coffman@coinbase.com>
Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>
Co-authored-by: Inphi <mlaw2501@gmail.com>
Co-authored-by: Daniel Coffman <dgcoffman@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants