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.

Sign up for GitHub

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

Jump to bottom

test: move withdrawal tests to sim tests #6507

Merged
merged 8 commits into from
Mar 20, 2024
Merged

test: move withdrawal tests to sim tests #6507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
511502b
Support withdrawal credentials
nazarhussain Mar 5, 2024
1734e82
Fix log level for errors
nazarhussain Mar 5, 2024
e6c927f
Fix lint warnings
nazarhussain Mar 5, 2024
765e322
Add withdrawal assertions
nazarhussain Mar 5, 2024
871cd7f
Update withdrawal assertions
nazarhussain Mar 5, 2024
f3fab85
Update the withdrawal check
nazarhussain Mar 5, 2024
094b1bb
Remove old withdrawal sim test
nazarhussain Mar 5, 2024
c69d5d5
Update code as per feedback
nazarhussain Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 0 additions & 33 deletions .github/workflows/test-sim-merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,39 +71,6 @@ jobs:
ENGINE_PORT: 8551
ETH_PORT: 8661

- name: Pull geth withdrawals
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we need to remove the whole file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, one other PR in progress then we can remove this file and a lot of utility code along it.

run: docker pull $GETH_WITHDRAWALS_IMAGE

- name: Test Lodestar <> geth withdrawals
run: yarn test:sim:withdrawals
working-directory: packages/beacon-node
env:
EL_BINARY_DIR: ${{ env.GETH_WITHDRAWALS_IMAGE }}
EL_SCRIPT_DIR: gethdocker

- name: Pull ethereumjs withdrawals
run: docker pull $ETHEREUMJS_WITHDRAWALS_IMAGE

- name: Test Lodestar <> ethereumjs withdrawals
run: yarn test:sim:withdrawals
working-directory: packages/beacon-node
env:
EL_BINARY_DIR: ${{ env.ETHEREUMJS_WITHDRAWALS_IMAGE }}
EL_SCRIPT_DIR: ethereumjsdocker

# Disable nethermind build as the withdrawal test config seems to be no
# longer available, enable after grabbing a build which has one
#
# - name: Pull nethermind withdrawals
# run: docker pull $NETHERMIND_WITHDRAWALS_IMAGE

# - name: Test Lodestar <> nethermind withdrawals
# run: yarn test:sim:withdrawals
# working-directory: packages/beacon-node
# env:
# EL_BINARY_DIR: ${{ env.NETHERMIND_WITHDRAWALS_IMAGE }}
# EL_SCRIPT_DIR: netherminddocker

# Enable the blob sims when stable images
# - name: Pull ethereumjs blobs
# run: docker pull $ETHEREUMJS_BLOBS_IMAGE
Expand Down
18 changes: 0 additions & 18 deletions docs/pages/contribution/testing/integration-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,6 @@

The following tests are found in `packages/beacon-node`

#### `test:sim:withdrawals`

This test simulates capella blocks with withdrawals. It tests lodestar against Geth and EthereumJS.

There are two ENV variables that are required to run this test:

- `EL_BINARY_DIR`: the docker image setup to handle the test case
- `EL_SCRIPT_DIR`: the script that will be used to start the EL client. All of the scripts can be found in `packages/beacon-node/test/scripts/el-interop` and the `EL_SCRIPT_DIR` is the sub-directory name in that root that should be used to run the test.

The command to run this test is:

`EL_BINARY_DIR=g11tech/geth:withdrawals EL_SCRIPT_DIR=gethdocker yarn vitest --run test/sim/withdrawal-interop.test.ts`

The images used by this test during CI are:

- `GETH_WITHDRAWALS_IMAGE: g11tech/geth:withdrawalsfeb8`
- `ETHEREUMJS_WITHDRAWALS_IMAGE: g11tech/ethereumjs:blobs-b6b63`

#### `test:sim:mergemock`

#### `yarn test:sim:blobs`
1 change: 0 additions & 1 deletion packages/beacon-node/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
"test:e2e": "LODESTAR_PRESET=minimal vitest --run --segfaultRetry 3 --config vitest.e2e.config.ts --dir test/e2e",
"test:sim": "vitest --run test/sim/**/*.test.ts",
"test:sim:mergemock": "vitest --run test/sim/mergemock.test.ts",
"test:sim:withdrawals": "vitest --run test/sim/withdrawal-interop.test.ts",
"test:sim:blobs": "vitest --run test/sim/4844-interop.test.ts",
"download-spec-tests": "node --loader=ts-node/esm test/spec/downloadTests.ts",
"test:spec:bls": "vitest --run --config vitest.spec.config.ts --dir test/spec/bls/",
Expand Down
12 changes: 6 additions & 6 deletions packages/beacon-node/src/execution/engine/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ export class ExecutionEngineHttp implements IExecutionEngine {
ForkSeq[fork] >= ForkSeq.deneb
? "engine_newPayloadV3"
: ForkSeq[fork] >= ForkSeq.capella
? "engine_newPayloadV2"
: "engine_newPayloadV1";
? "engine_newPayloadV2"
: "engine_newPayloadV1";

const serializedExecutionPayload = serializeExecutionPayload(fork, executionPayload);

Expand Down Expand Up @@ -299,8 +299,8 @@ export class ExecutionEngineHttp implements IExecutionEngine {
ForkSeq[fork] >= ForkSeq.deneb
? "engine_forkchoiceUpdatedV3"
: ForkSeq[fork] >= ForkSeq.capella
? "engine_forkchoiceUpdatedV2"
: "engine_forkchoiceUpdatedV1";
? "engine_forkchoiceUpdatedV2"
: "engine_forkchoiceUpdatedV1";
const payloadAttributesRpc = payloadAttributes ? serializePayloadAttributes(payloadAttributes) : undefined;
// If we are just fcUing and not asking execution for payload, retry is not required
// and we can move on, as the next fcU will be issued soon on the new slot
Expand Down Expand Up @@ -373,8 +373,8 @@ export class ExecutionEngineHttp implements IExecutionEngine {
ForkSeq[fork] >= ForkSeq.deneb
? "engine_getPayloadV3"
: ForkSeq[fork] >= ForkSeq.capella
? "engine_getPayloadV2"
: "engine_getPayloadV1";
? "engine_getPayloadV2"
: "engine_getPayloadV1";
const payloadResponse = await this.rpc.fetchWithRetries<
EngineApiRpcReturnTypes[typeof method],
EngineApiRpcParamTypes[typeof method]
Expand Down