Skip to content

Add historical sync test#44

Merged
Inphi merged 6 commits intomasterfrom
inphi/initial-sync
Oct 28, 2022
Merged

Add historical sync test#44
Inphi merged 6 commits intomasterfrom
inphi/initial-sync

Conversation

@Inphi
Copy link
Copy Markdown
Owner

@Inphi Inphi commented Oct 27, 2022

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

@Inphi Inphi force-pushed the inphi/initial-sync branch from 573e777 to a59cf14 Compare October 27, 2022 01:53
@Inphi Inphi requested a review from roberto-bayardo October 27, 2022 01:58
@Inphi Inphi marked this pull request as ready for review October 27, 2022 01:58
}

func AssertBlobsEquals(a, b types.Blobs) {
// redundant for nice for debugging
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this comment is from a copy/paste of previous code but is this check really redundant? Seems without the length check you would return true instead of false if there were extra blobs at the end of "b" that were not in "a".

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

that's right. I'll remove the confusing comment.

var Env *TestEnvironment

func InitE2ETest() {
if err := RestartDevnet(); err != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we still need the RestartDevnet() function at all now that you remove this call? Seems to be the only call site.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and then you could also get rid of StartDevnet() too I think.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

You're right. Will remove.

}
if err := ctx.Err(); err != nil {
return err
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is it worth sleeping for say 1 second here?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

yes. Will do.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

that might have been the issue in CI! I have another workflow running the 1 second patch that's succeeding!

Inphi and others added 5 commits October 27, 2022 19:21
Asserts that the beacon node follower can sync blobs during initial-sync.
This patch also refactors the test harness to accommodate dynamic nodes.
Co-authored-by: Roberto Bayardo <bayardo@alum.mit.edu>
@Inphi Inphi force-pushed the inphi/initial-sync branch from cafdabc to 636d8ff Compare October 27, 2022 23:24
@Inphi Inphi merged commit 6c6c56d into master Oct 28, 2022
@Inphi Inphi deleted the inphi/initial-sync branch October 28, 2022 05:07
roberto-bayardo added a commit that referenced this pull request Nov 8, 2022
* 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>
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