[Merged by Bors] - Rename random to prev_randao #3040

paulhauner · 2022-02-24T23:28:27Z

Issue Addressed

As discussed on last-night's consensus call, the testnets next week will target the Kiln Spec v2.

Presently, we support Kiln V1. V2 is backwards compatible, except for renaming random to prev_randao in:

With this PR we'll no longer be compatible with the existing Kintsugi and Kiln testnets, however we'll be ready for the testnets next week. I raised this breaking change in the call last night, we are all keen to move forward and break things.

We now target the merge-kiln-v2 branch for interop with Geth. This required adding the --http.aauthport to the tester to avoid a port conflict at startup.

Changes to exec integration tests

There's some change in the merge-kiln-v2 version of Geth that means it can't compile on a vanilla Github runner. Bumping the go version on the runner solved this issue.

Whilst addressing this, I refactored the testing/execution_integration crate to be a binary rather than a library with tests. This means that we don't need to run the build.rs and build Geth whenever someone runs make lint or make test-release. This is nice for everyday users, but it's also nice for CI so that we can have a specific runner for these tests and we don't need to ensure all runners support everything required to build all execution clients.

More Info

~~EF tests are failing since the rename has broken some tests that reference the old field name. I have been told there will be new tests released in the coming days (25/02/22 or 26/02/22).~~

paulhauner · 2022-02-25T02:17:19Z

I've flagged this as v2.1.4 so that release is compatible with Kiln v2 testnets.

paulhauner · 2022-02-25T04:11:29Z

Blocked on a new release of EF tests:

ethereum/consensus-specs#2839

This reverts commit bb02c2f.

paulhauner · 2022-03-01T23:04:55Z

Blocked on a new release of EF tests:

ethereum/consensus-specs#2839

New tests have been released: https://github.com/ethereum/consensus-spec-tests/releases/tag/v1.1.10

I'm working getting this ready for review now!

michaelsproul

LGTM, happy to merge once CI passes

testing/execution_engine_integration/src/build_geth.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

paulhauner · 2022-03-02T05:36:19Z

Alrighty, so there were conflicts with unstable so I merged it and then wrangled some tests.

It turned out I had to do some yak shaving. I made four commits:

5313364 unstable merge commit
cd7a0f5 implemented consensus-specs/#2844.
4f7610f made changes to the tests so they don't conflict with new block validation conditions from [Merged by Bors] - Enforce Optimistic Sync Conditions & CLI Tests (v2) #3050.
f5a1dee fix a bug in proto-array.

In (2) I implemented consensus-specs/#2844. It has looser restrictions on which blocks can be imported. Since I was wrangling tests against import restrictions, it makes sense to update to the new, looser spec. (3) is updating the tests so blocks don't fail on opt sync import conditions.

In (4), I noticed one of the tests was failing. You can see from the old code that we would always add the head_block_root to invalidated_indices, causing us to incorrectly invalidate it and any descendants! 🌶️

michaelsproul · 2022-03-02T05:39:05Z

You can see from the old code that we would always add the head_block_root to invalidated_indices, causing us to incorrectly invalidate any descendants!

Interesting! This would previously only happen if latest_valid_ancestor_hash was set to None though, right? Otherwise that loop would exit on the first iteration when Some(hash) == latest_valid_ancestor_hash.

Now if we call invalidate with a block root and no latest_valid_ancestor_hash then we won't invalidate that block root, is that really the behaviour we want?

paulhauner · 2022-03-02T05:40:46Z

Interesting! This would previously only happen if latest_valid_ancestor_hash was set to None though, right? Otherwise that loop would exit on the first iteration when Some(hash) == latest_valid_ancestor_hash.

I think this is actually the source of confusion. It's

Some(hash) == latest_valid_ancestor_hash

Not:

if let Some(hash) = latest_valid_ancestor_hash

This reverts commit f5a1dee.

paulhauner · 2022-03-02T09:04:53Z

Alrighty, so the reason that we have to address these payload_invalidation tests is that the beacon-chain-tests are run for each PR but not for bors. Because of this #3050 managed to get into unstable and has caused unstable CI to break.

Because we've just discovered these tests are failing, we've also discovered an issue with the optimistic sync changes to fork choice (#2837) that we now need to fix to make CI happy again. We are yak shaving :(

As pointed out by @michaelsproul, f5a1dee is bugged. I'm working on an alternative now. I think 78ecd40 is a good solution, but I'll come back to it tomorrow.

Unfortunately, this PR is the difference between supporting Kiln v2 and not supporting it. So our Kiln v2 support is now somewhat blocked on this yak shaving adventure :(

@ethDreamer

commit 6370edd Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 16:31:18 2022 +1100 Update testing/execution_engine_integration/src/build_geth.rs Co-authored-by: Michael Sproul <micsproul@gmail.com> commit f5a1dee Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 16:24:03 2022 +1100 Fix proto-array bug commit 4f7610f Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 16:23:52 2022 +1100 Fix payload tests commit cd7a0f5 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 15:22:54 2022 +1100 Implement consensus-specs/2844 commit 5313364 Merge: d5e84a5 668115a Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 14:23:19 2022 +1100 Merge branch 'unstable' into prev-randao-rename commit d5e84a5 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 13:31:45 2022 +1100 Fix windows test errors commit 668115a Author: Akihito Nakano <sora.akatsuki@gmail.com> Date: Wed Mar 2 01:05:08 2022 +0000 Rename Eth1/Eth2 in documents (sigp#3021) ## Issue Addressed Resolves sigp#3019 ## Proposed Changes - Eth2 Eth2.0 Ethereum 2.0 -> Ethereum consensus - Eth2 network -> consensus layer - Ethereum 2.0 specification -> Ethereum proof-of-stake consensus specification - Eth2 deposit contract -> Staking deposit contract - Eth1 -> execution client ## Additional Info The description needs to be updated by someone who has permission to do. 📝 <img width="487" alt="image" src="https://user-images.githubusercontent.com/1885716/153995211-816d9561-751e-4810-abb9-83d979379783.png"> commit e34524b Author: Age Manning <Age@AgeManning.com> Date: Wed Mar 2 01:05:07 2022 +0000 Increase default target-peer count to 80 (sigp#3005) Increase the default peer count from 50 to 80 commit db2e3ae Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 11:08:40 2022 +1100 Fix yaml indenting commit a2f99e5 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 11:05:57 2022 +1100 Specify go version in github actions commit 0c5e9c6 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 11:04:15 2022 +1100 Switch EE tests to be a binary commit 7f07fa5 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 10:22:32 2022 +1100 Print output of failed make command commit fa20696 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Mar 2 10:01:00 2022 +1100 Bump spec tests version commit c2d7677 Author: Paul Hauner <paul@paulhauner.com> Date: Sat Feb 26 10:17:14 2022 +1100 Add auth port commit 540337a Author: Paul Hauner <paul@paulhauner.com> Date: Sat Feb 26 10:10:06 2022 +1100 Use `merge-kiln-v2` branch for geth commit 1a3d32e Author: Paul Hauner <paul@paulhauner.com> Date: Fri Feb 25 15:08:38 2022 +1100 Revert "Add serde "random" alias" This reverts commit bb02c2f. commit 63376bd Author: Paul Hauner <paul@paulhauner.com> Date: Fri Feb 25 14:59:26 2022 +1100 Add serde "random" alias commit 375aa32 Author: Paul Hauner <paul@paulhauner.com> Date: Fri Feb 25 10:09:38 2022 +1100 Rename random to prev_randao commit b6493d5 Author: Paul Hauner <paul@paulhauner.com> Date: Tue Mar 1 22:56:47 2022 +0000 Enforce Optimistic Sync Conditions & CLI Tests (v2) (sigp#3050) ## Description This PR adds a single, trivial commit (f5d2b27) atop sigp#2986 to resolve a tests compile error. The original author (@ethDreamer) is AFK so I'm getting this one merged ☺️ Please see sigp#2986 for more information about the other, significant changes in this PR. Co-authored-by: Mark Mackey <mark@sigmaprime.io> Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com>

paulhauner · 2022-03-02T23:26:35Z

I have a good solution for the opt sync fork choice bug now. It's across two commits and the diff is here:

f045fb8...e093de9

It's the same solution from #3040 (comment), however I added an enum to make things tidier and easy to comprehend.

michaelsproul

Awesome, looks good to go!

paulhauner · 2022-03-02T23:34:15Z

Thanks! I'll run bors now to try and catch any config mistakes.

bors r+

## Issue Addressed As discussed on last-night's consensus call, the testnets next week will target the [Kiln Spec v2](https://hackmd.io/@n0ble/kiln-spec). Presently, we support Kiln V1. V2 is backwards compatible, except for renaming `random` to `prev_randao` in: - ethereum/execution-apis#180 - ethereum/consensus-specs#2835 With this PR we'll no longer be compatible with the existing Kintsugi and Kiln testnets, however we'll be ready for the testnets next week. I raised this breaking change in the call last night, we are all keen to move forward and break things. We now target the [`merge-kiln-v2`](https://github.com/MariusVanDerWijden/go-ethereum/tree/merge-kiln-v2) branch for interop with Geth. This required adding the `--http.aauthport` to the tester to avoid a port conflict at startup. ### Changes to exec integration tests There's some change in the `merge-kiln-v2` version of Geth that means it can't compile on a vanilla Github runner. Bumping the `go` version on the runner solved this issue. Whilst addressing this, I refactored the `testing/execution_integration` crate to be a *binary* rather than a *library* with tests. This means that we don't need to run the `build.rs` and build Geth whenever someone runs `make lint` or `make test-release`. This is nice for everyday users, but it's also nice for CI so that we can have a specific runner for these tests and we don't need to ensure *all* runners support everything required to build all execution clients. ## More Info - [x] ~~EF tests are failing since the rename has broken some tests that reference the old field name. I have been told there will be new tests released in the coming days (25/02/22 or 26/02/22).~~

bors · 2022-03-03T02:10:30Z

Canceled.

paulhauner · 2022-03-03T02:10:41Z

bors retry

Looks like bors wasn't happy with the local-testnet action.

## Issue Addressed As discussed on last-night's consensus call, the testnets next week will target the [Kiln Spec v2](https://hackmd.io/@n0ble/kiln-spec). Presently, we support Kiln V1. V2 is backwards compatible, except for renaming `random` to `prev_randao` in: - ethereum/execution-apis#180 - ethereum/consensus-specs#2835 With this PR we'll no longer be compatible with the existing Kintsugi and Kiln testnets, however we'll be ready for the testnets next week. I raised this breaking change in the call last night, we are all keen to move forward and break things. We now target the [`merge-kiln-v2`](https://github.com/MariusVanDerWijden/go-ethereum/tree/merge-kiln-v2) branch for interop with Geth. This required adding the `--http.aauthport` to the tester to avoid a port conflict at startup. ### Changes to exec integration tests There's some change in the `merge-kiln-v2` version of Geth that means it can't compile on a vanilla Github runner. Bumping the `go` version on the runner solved this issue. Whilst addressing this, I refactored the `testing/execution_integration` crate to be a *binary* rather than a *library* with tests. This means that we don't need to run the `build.rs` and build Geth whenever someone runs `make lint` or `make test-release`. This is nice for everyday users, but it's also nice for CI so that we can have a specific runner for these tests and we don't need to ensure *all* runners support everything required to build all execution clients. ## More Info - [x] ~~EF tests are failing since the rename has broken some tests that reference the old field name. I have been told there will be new tests released in the coming days (25/02/22 or 26/02/22).~~

bors · 2022-03-03T04:20:23Z

Pull request successfully merged into unstable.

Build succeeded:

paulhauner added work-in-progress PR is a work-in-progress bellatrix Required to support the Bellatrix Upgrade labels Feb 24, 2022

paulhauner force-pushed the prev-randao-rename branch from 3561d36 to 7a046a2 Compare February 25, 2022 02:13

paulhauner added the v2.1.4 label Feb 25, 2022

paulhauner added blocked and removed work-in-progress PR is a work-in-progress labels Feb 25, 2022

paulhauner self-assigned this Feb 28, 2022

paulhauner added 6 commits March 2, 2022 09:59

Rename random to prev_randao

375aa32

Add serde "random" alias

63376bd

Revert "Add serde "random" alias"

1a3d32e

This reverts commit bb02c2f.

Use merge-kiln-v2 branch for geth

540337a

Add auth port

c2d7677

Bump spec tests version

fa20696

paulhauner force-pushed the prev-randao-rename branch from 69eb3c3 to fa20696 Compare March 1, 2022 23:04

paulhauner removed the blocked label Mar 1, 2022

paulhauner added 4 commits March 2, 2022 10:22

Print output of failed make command

7f07fa5

Switch EE tests to be a binary

0c5e9c6

Specify go version in github actions

a2f99e5

Fix yaml indenting

db2e3ae

paulhauner mentioned this pull request Mar 2, 2022

[Merged by Bors] - auth for engine api #3046

Closed

5 tasks

Fix windows test errors

d5e84a5

paulhauner added the ready-for-review The code is ready for review label Mar 2, 2022

paulhauner marked this pull request as ready for review March 2, 2022 02:36

michaelsproul approved these changes Mar 2, 2022

View reviewed changes

testing/execution_engine_integration/src/build_geth.rs Outdated Show resolved Hide resolved

paulhauner added 3 commits March 2, 2022 14:23

Merge branch 'unstable' into prev-randao-rename

5313364

Implement consensus-specs/2844

cd7a0f5

Fix payload tests

4f7610f

paulhauner and others added 2 commits March 2, 2022 16:24

Fix proto-array bug

f5a1dee

Update testing/execution_engine_integration/src/build_geth.rs

6370edd

Co-authored-by: Michael Sproul <micsproul@gmail.com>

paulhauner added 2 commits March 2, 2022 20:03

Revert "Fix proto-array bug"

f045fb8

This reverts commit f5a1dee.

Add invalidate_head_block

78ecd40

Add InvalidationOperation

e093de9

Update bors.toml

b517f78

michaelsproul approved these changes Mar 2, 2022

View reviewed changes

michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Mar 2, 2022

Remove local-testnet from bors

b537b39

bors bot changed the title ~~Rename random to prev_randao~~ [Merged by Bors] - Rename random to prev_randao Mar 3, 2022

bors bot closed this Mar 3, 2022

paulhauner mentioned this pull request Mar 15, 2022

[Merged by Bors] - Use windows-2019 in release CI #3090

Closed

paulhauner mentioned this pull request Apr 8, 2022

Remove HeadSafetyStatus #3152

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Rename random to prev_randao #3040

[Merged by Bors] - Rename random to prev_randao #3040

paulhauner commented Feb 24, 2022 •

edited

Loading

paulhauner commented Feb 25, 2022

paulhauner commented Feb 25, 2022 •

edited

Loading

paulhauner commented Mar 1, 2022

michaelsproul left a comment

paulhauner commented Mar 2, 2022 •

edited

Loading

michaelsproul commented Mar 2, 2022 •

edited

Loading

paulhauner commented Mar 2, 2022

paulhauner commented Mar 2, 2022

paulhauner commented Mar 2, 2022

michaelsproul left a comment

paulhauner commented Mar 2, 2022

bors bot commented Mar 3, 2022

paulhauner commented Mar 3, 2022

bors bot commented Mar 3, 2022

[Merged by Bors] - Rename random to prev_randao #3040

[Merged by Bors] - Rename random to prev_randao #3040

Conversation

paulhauner commented Feb 24, 2022 • edited Loading

Issue Addressed

Changes to exec integration tests

More Info

paulhauner commented Feb 25, 2022

paulhauner commented Feb 25, 2022 • edited Loading

paulhauner commented Mar 1, 2022

michaelsproul left a comment

Choose a reason for hiding this comment

paulhauner commented Mar 2, 2022 • edited Loading

michaelsproul commented Mar 2, 2022 • edited Loading

paulhauner commented Mar 2, 2022

paulhauner commented Mar 2, 2022

paulhauner commented Mar 2, 2022

michaelsproul left a comment

Choose a reason for hiding this comment

paulhauner commented Mar 2, 2022

bors bot commented Mar 3, 2022

paulhauner commented Mar 3, 2022

bors bot commented Mar 3, 2022

paulhauner commented Feb 24, 2022 •

edited

Loading

paulhauner commented Feb 25, 2022 •

edited

Loading

paulhauner commented Mar 2, 2022 •

edited

Loading

michaelsproul commented Mar 2, 2022 •

edited

Loading