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.

Sign up for GitHub

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

Jump to bottom

Finish off builder flow for Deneb #7266

Merged
merged 8 commits into from
Jun 19, 2023
Merged

Finish off builder flow for Deneb #7266

merged 8 commits into from
Jun 19, 2023

Conversation

StefanBratanov
Copy link
Contributor

@StefanBratanov StefanBratanov commented Jun 16, 2023 •
edited
Loading

PR Description

  • Resolved TODOs in ExecutionBuilderModule (basically implement getHeader and getPayload for both local, local fallback and builder + some logging)
  • Enabled disabled tests + fixed current tests for deneb building
  • Fixed a bug when setting the kzg commitments for blinded flow (tests are useful 🛩️ )
  • Modified ExecutionLayerChannelStub for Deneb
  • Fixed a bug where blob sidecars when unblinded simulatenously with block which caused the cached builder payload value not being available

Fixed Issue(s)

fixes #7170

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@StefanBratanov StefanBratanov marked this pull request as draft June 16, 2023 12:17
@StefanBratanov StefanBratanov marked this pull request as ready for review June 16, 2023 13:03
@StefanBratanov StefanBratanov force-pushed the finish_off_builder_flow_deneb branch 2 times, most recently from dc80943 to 9eed16c Compare June 16, 2023 14:49
@tbenr
Copy link
Contributor

tbenr commented Jun 16, 2023

I'm testing this in my stubbed interop and I get BuilderPayload hasn't been cached for slot N consistently.

@StefanBratanov
Copy link
Contributor Author

I'm testing this in my stubbed interop and I get BuilderPayload hasn't been cached for slot N consistently.

Interesting. Let me give it a try as well.

@StefanBratanov StefanBratanov force-pushed the finish_off_builder_flow_deneb branch from cd64fa6 to 05d3629 Compare June 19, 2023 07:54
tbenr
tbenr approved these changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

LGTM, just nits

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java Outdated Show resolved Hide resolved
...ionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionBuilderModule.java Outdated Show resolved Hide resolved
...ionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionBuilderModule.java Show resolved Hide resolved
@StefanBratanov StefanBratanov enabled auto-merge (squash) June 19, 2023 11:12
@StefanBratanov StefanBratanov merged commit b53ae0f into Consensys:master Jun 19, 2023
@StefanBratanov StefanBratanov deleted the finish_off_builder_flow_deneb branch June 19, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbenr tbenr tbenr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement updated builder apis
2 participants
@StefanBratanov @tbenr