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 requires suggested-fee-recipient flag when always-prepare-payload is set #6079

Merged
merged 9 commits into from
Jul 18, 2024

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented Jul 12, 2024

Currently, when the flag --always-prepare-payload is set and a fee recipient is not set in the beacon node, the beacon node will print the log:

CRIT Fee recipient unknown proposer_index: 368409, msg: the suggested_fee_recipient was unknown during block production. a junk address was used, rewards were lost! check the --suggested-fee-recipient flag and VC configuration., service: exec

which can be noisy and not desirable. This PR changes it so that the flag --suggested-fee-recipient must always be set when --always-prepare-payload is set. The documentation is also updated to reflect this change.

Thanks to the discussion raised by @just_in_time and @michaelsproul in Discord

@michaelsproul michaelsproul added backwards-incompat Backwards-incompatible API change UX-and-logs v5.3.0 Q3 2024 release with database changes! labels Jul 12, 2024
@chong-he chong-he added the work-in-progress PR is a work-in-progress label Jul 12, 2024
@michaelsproul
Copy link
Member

Based on the test failure it seems like suggested-fee-recipient requires execution-endpoint, so you need to set that flag as well

@chong-he
Copy link
Member Author

chong-he commented Jul 15, 2024

Based on the test failure it seems like suggested-fee-recipient requires execution-endpoint, so you need to set that flag as well

Thank you!
Added in 5ed5de4 and 1375779 and looks good now

@chong-he chong-he added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Jul 15, 2024
@realbigsean
Copy link
Member

@mergify queue

Copy link

mergify bot commented Jul 15, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

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
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Jul 18, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

Copy link

mergify bot commented Jul 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3859c9b

mergify bot added a commit that referenced this pull request Jul 18, 2024
@mergify mergify bot merged commit 3859c9b into sigp:unstable Jul 18, 2024
29 checks passed
@chong-he chong-he deleted the always-prepare-payload branch July 18, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change ready-for-review The code is ready for review UX-and-logs v5.3.0 Q3 2024 release with database changes!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants