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

client: add shouldOverrideBuilder flag for getPayloadV3 #2891

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Jul 14, 2023

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #2891 (0e84706) into master (54e10a2) will increase coverage by 88.26%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
client 87.20% <100.00%> (?)
common 97.13% <ø> (?)
trie 90.08% <ø> (?)
tx 95.96% <ø> (?)
vm 78.15% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

// ethereumjs doesnot provide any transaction censoring detection (yet) to suggest
// overriding builder/mev-boost blocks
const shouldOverrideBuilder = false
return { executionPayload, blockValue: bigIntToHex(value), blobsBundle, shouldOverrideBuilder }
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can see, this does not do anything except return false? (Is this part of some spec?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see the comment on the top:

since we don't have any transaction censoring detection features,, we just return it as false (which is what is recommended to be returned if an EL doesn't has such features so the CL can choose builder/mev-boost block if it wants to)

@g11tech g11tech added the target: master Work to be done towards master branch label Jul 16, 2023
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

LGTM

@holgerd77 holgerd77 merged commit e33b0f8 into master Jul 17, 2023
41 checks passed
@holgerd77 holgerd77 deleted the override-builder branch July 17, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants