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

app: handle fee recipient #1246

Merged
merged 6 commits into from
Oct 10, 2022
Merged

app: handle fee recipient #1246

merged 6 commits into from
Oct 10, 2022

Conversation

dB2510
Copy link
Contributor

@dB2510 dB2510 commented Oct 10, 2022

Handles fee recipient address by adding a slot subscriber to scheduler which calls prepare_beacon_proposer endpoint at the start of each epoch.

category: feature
ticket: #1100

@codecov
Copy link

codecov bot commented Oct 10, 2022

Codecov Report

Base: 53.54% // Head: 53.59% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (40b2391) compared to base (698d02a).
Patch coverage: 55.14% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1246      +/-   ##
==========================================
+ Coverage   53.54%   53.59%   +0.05%     
==========================================
  Files         139      139              
  Lines       16152    16284     +132     
==========================================
+ Hits         8648     8728      +80     
- Misses       6260     6298      +38     
- Partials     1244     1258      +14     
Impacted Files Coverage Δ
app/eth2wrap/eth2wrap_gen.go 4.70% <0.00%> (-0.14%) ⬇️
cluster/definition.go 58.90% <ø> (ø)
eth2util/signing/signing.go 42.56% <ø> (ø)
testutil/beaconmock/beaconmock.go 51.37% <0.00%> (-0.97%) ⬇️
testutil/beaconmock/options.go 33.05% <0.00%> (-0.28%) ⬇️
core/fetcher/fetcher.go 65.40% <46.66%> (-2.04%) ⬇️
core/validatorapi/validatorapi.go 55.95% <53.84%> (-0.15%) ⬇️
core/validatorapi/router.go 62.97% <66.66%> (+0.09%) ⬆️
app/app.go 59.66% <76.31%> (+1.34%) ⬆️
core/qbft/qbft.go 81.54% <0.00%> (-0.43%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

app/app.go Outdated
// setFeeRecipient returns a slot subscriber for scheduler which calls prepare_beacon_proposer endpoint at start of each epoch.
func setFeeRecipient(eth2Cl eth2wrap.Client, pubkeys []eth2p0.BLSPubKey, feeRecipient string) func(ctx context.Context, slot core.Slot) error {
return func(ctx context.Context, slot core.Slot) error {
if !slot.FirstInEpoch() {
Copy link
Contributor

Choose a reason for hiding this comment

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

also do this on startup (ie the first time it is called)

if block.Version == spec.DataVersionBellatrix {
actual := fmt.Sprintf("%#x", block.Bellatrix.Body.ExecutionPayload.FeeRecipient)
if actual != f.feeRecipientAddress {
log.Warn(ctx, "Fee recipient address different than expected", nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

Proposing block with unexpected fee recipient address

@@ -650,6 +655,12 @@ func submitSyncCommitteeMessages(s eth2client.SyncCommitteeMessagesSubmitter) ha
}
}

func submitProposalPreparations() handlerFunc {
return func(context.Context, map[string]string, url.Values, []byte) (interface{}, error) {
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment why we swallow this

app/app.go Outdated
// setFeeRecipient returns a slot subscriber for scheduler which calls prepare_beacon_proposer endpoint at start of each epoch.
// TODO(dhruv): move this somewhere else once more use-cases like this becomes clear.
func setFeeRecipient(eth2Cl eth2wrap.Client, pubkeys []eth2p0.BLSPubKey, feeRecipient string) func(ctx context.Context, slot core.Slot) error {
onStartup := false
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest:

onStartup := true
...
if !onStartup && !slot.FirstInEpoch() {
  return nil
}
onStartup = false

@dB2510 dB2510 added merge when ready Indicates bulldozer bot may merge when all checks pass and removed merge when ready Indicates bulldozer bot may merge when all checks pass labels Oct 10, 2022
@dB2510 dB2510 added the merge when ready Indicates bulldozer bot may merge when all checks pass label Oct 10, 2022
@dB2510 dB2510 linked an issue Oct 10, 2022 that may be closed by this pull request
@obol-bulldozer obol-bulldozer bot merged commit eac1119 into main Oct 10, 2022
@obol-bulldozer obol-bulldozer bot deleted the dhruv/feerecipient branch October 10, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle feeRecipient gracefully
2 participants