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

add BLST_PORTABLE to CGO_CFLAGS to fix SIGILL issue with blst #49

Merged
merged 1 commit into from
Nov 7, 2022

Conversation

roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Nov 5, 2022

Fixes issue seen in #48

@roberto-bayardo roberto-bayardo requested a review from Inphi November 5, 2022 20:44
Copy link
Owner

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

thanks

@Inphi
Copy link
Owner

Inphi commented Nov 5, 2022

one qq; is docker meant to directly consume the .env fille? I couldn't find any documentation on how workflows use it

@roberto-bayardo
Copy link
Collaborator Author

https://docs.docker.com/compose/environment-variables/

This is where I saw it mentioned.

@Inphi
Copy link
Owner

Inphi commented Nov 6, 2022

i could be wrong, but i think the working directory of ci/docker is at the project root. The .env won't be picked up unless you move it to the root.

@roberto-bayardo
Copy link
Collaborator Author

Ok let me see if the test will fail if I comment it out.

@roberto-bayardo roberto-bayardo force-pushed the sigill-fix branch 2 times, most recently from 50b80a2 to 804a156 Compare November 6, 2022 02:20
@roberto-bayardo
Copy link
Collaborator Author

roberto-bayardo commented Nov 6, 2022

Yeah test is passing regardless, I guess we're back on a more modern CPU for our builds! I moved it into the ci.yml file via an "env:" section which I suppose is better anyway (more explicit).

@Inphi
Copy link
Owner

Inphi commented Nov 6, 2022

I dont think this will work either because docker doesnt use the env of the caller/parent process for runs.

I'd update the go docker script to provide the env file.

@roberto-bayardo
Copy link
Collaborator Author

roberto-bayardo commented Nov 6, 2022

Ugg good point. I didn't want to put it in the dockerfile since I was hoping to scope it only to the CI, but perhaps that's the simplest solution. In fact it's already in the dockerfile only commented out!

@Inphi
Copy link
Owner

Inphi commented Nov 6, 2022

yeah updating the Dockerfile sounds fine by me.

@roberto-bayardo
Copy link
Collaborator Author

@Inphi ptal I believe this should work while isolating it to the CI

Copy link
Owner

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

LGTM

@roberto-bayardo roberto-bayardo merged commit 04aa67d into devnet-v3 Nov 7, 2022
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