make `execution-endpoint` required #5165

zhiqiangxu · 2024-02-01T01:09:35Z

Issue Addressed

This PR addresses this issue.

beacon_node/src/config.rs

dapplion

Linking a diff of the code with the same indentation so it's easier to review https://www.diffchecker.com/v81VrtmX/

Change looks good thanks 🙏

dapplion · 2024-06-27T15:56:30Z

@zhiqiangxu can you solve conflicts?

zhiqiangxu · 2024-06-28T01:41:01Z

@dapplion Done.

chong-he · 2024-07-30T02:13:50Z

The cli-check is failing. You can run make cli-local from the root of the repository and commit the changes

zhiqiangxu · 2024-07-30T06:01:09Z

@chong-he The real problem is the error that happened when running make nextest-release, but I can't run it locally because of this error:

chong-he · 2024-07-31T05:13:11Z

Probably need to solve the compile issue first

zhiqiangxu · 2024-09-26T09:00:57Z

Probably need to solve the compile issue first

No idea how to solve the compile issue :(

lighthouse/tests/exec.rs

macladson

Hijacking this PR a bit sorry @zhiqiangxu

I've fixed the CLI flag tests now.

macladson · 2024-10-25T17:23:57Z

lighthouse/tests/beacon_node.rs

@@ -2505,13 +2518,13 @@ fn logfile_format_flag() {
 fn sync_eth1_chain_default() {
    CommandLineTest::new()
        .run_with_zero_port()
-        .with_config(|config| assert_eq!(config.sync_eth1_chain, false));
+        .with_config(|config| assert_eq!(config.sync_eth1_chain, true));


Note that sync_eth1_chain will now effectively default to true since it is set to true when --execution_endpoint is provided (which will now be all the time).

This may very well deprecate this config value entirely, although it can still technically be set back to false using the --disable-deposit-contract-sync flag.

I think this means we can hide and deprecate the --eth1 flag.

lighthouse/tests/beacon_node.rs

michaelsproul · 2024-11-01T04:41:08Z

@mergify queue

mergify · 2024-11-01T04:41:19Z

queue

🛑 The pull request has been removed from the queue `default`

Pull request #5165 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-01T04:55:02Z

@mergify dequeue

mergify · 2024-11-01T04:55:09Z

dequeue

✅ The pull request has been removed from the queue `default`

michaelsproul · 2024-11-01T04:56:31Z

@mergify queue

mergify · 2024-11-01T04:56:56Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 16693b0

zhiqiangxu changed the base branch from stable to unstable February 1, 2024 01:27

michaelsproul reviewed Feb 1, 2024

View reviewed changes

beacon_node/src/config.rs Outdated Show resolved Hide resolved

michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. backwards-incompat Backwards-incompatible API change labels Feb 1, 2024

make execution-endpoint mandatory

3dedab2

zhiqiangxu force-pushed the make_ee_required branch from 1a3fa0c to 3dedab2 Compare February 1, 2024 07:36

use parse_required instead

baf755b

michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 2, 2024

dapplion approved these changes Feb 8, 2024

View reviewed changes

zhiqiangxu force-pushed the make_ee_required branch 6 times, most recently from b0f19f1 to a78692d Compare February 26, 2024 05:08

make test pass

486686d

zhiqiangxu force-pushed the make_ee_required branch from a78692d to 486686d Compare February 27, 2024 07:03

dapplion requested a review from michaelsproul April 2, 2024 03:08

Merge branch 'unstable' into make_ee_required

d8dd82f

fix test

bc30999

Merge branch 'unstable' into make_ee_required

bf8be11

macladson requested changes Oct 17, 2024

View reviewed changes

lighthouse/tests/exec.rs Outdated Show resolved Hide resolved

Fix cli help text

93bc19b

macladson added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Oct 17, 2024

Fix tests

d745d09

macladson reviewed Oct 25, 2024

View reviewed changes

Merge branch 'unstable' into make_ee_required

dd37102

macladson approved these changes Oct 25, 2024

View reviewed changes

macladson requested a review from dapplion October 25, 2024 18:02

macladson added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 25, 2024

zhiqiangxu commented Oct 27, 2024

View reviewed changes

lighthouse/tests/beacon_node.rs Show resolved Hide resolved

Add comment

dae79a5

michaelsproul added the v6.0.0 New major release for hierarchical state diffs label Oct 31, 2024

michaelsproul reviewed Nov 1, 2024

View reviewed changes

lighthouse/tests/beacon_node.rs Outdated Show resolved Hide resolved

Clarification

1d7694f

michaelsproul approved these changes Nov 1, 2024

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 Nov 1, 2024

mergify bot added a commit that referenced this pull request Nov 1, 2024

Merge of #5165

9f45f6d

mergify bot mentioned this pull request Nov 1, 2024

merge queue: embarking unstable (1126058) and #5165 together #6557

Closed

5 tasks

Merge remote-tracking branch 'origin/unstable' into make_ee_required

9f869a4

mergify bot merged commit 16693b0 into sigp:unstable Nov 1, 2024
30 checks passed

jimmygchen mentioned this pull request Nov 1, 2024

Make execution-endpoint required on the CLI #5159

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make `execution-endpoint` required #5165

make `execution-endpoint` required #5165

zhiqiangxu commented Feb 1, 2024

dapplion left a comment

dapplion commented Jun 27, 2024

zhiqiangxu commented Jun 28, 2024 •

edited

Loading

chong-he commented Jul 30, 2024

zhiqiangxu commented Jul 30, 2024

chong-he commented Jul 31, 2024

zhiqiangxu commented Sep 26, 2024

macladson left a comment

macladson Oct 25, 2024

michaelsproul Oct 31, 2024

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024 •

edited

Loading

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024 •

edited

Loading

make execution-endpoint required #5165

make execution-endpoint required #5165

Conversation

zhiqiangxu commented Feb 1, 2024

Issue Addressed

dapplion left a comment

Choose a reason for hiding this comment

dapplion commented Jun 27, 2024

zhiqiangxu commented Jun 28, 2024 • edited Loading

chong-he commented Jul 30, 2024

zhiqiangxu commented Jul 30, 2024

chong-he commented Jul 31, 2024

zhiqiangxu commented Sep 26, 2024

macladson left a comment

Choose a reason for hiding this comment

macladson Oct 25, 2024

Choose a reason for hiding this comment

michaelsproul Oct 31, 2024

Choose a reason for hiding this comment

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024 • edited Loading

🛑 The pull request has been removed from the queue default

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024

✅ The pull request has been removed from the queue default

michaelsproul commented Nov 1, 2024

mergify bot commented Nov 1, 2024 • edited Loading

✅ The pull request has been merged automatically

make `execution-endpoint` required #5165

make `execution-endpoint` required #5165

zhiqiangxu commented Jun 28, 2024 •

edited

Loading

mergify bot commented Nov 1, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

✅ The pull request has been removed from the queue `default`

mergify bot commented Nov 1, 2024 •

edited

Loading