Skip to content

feat(conductor): include sequencer block hash #1999

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
Merged
joroshiba merged 5 commits into from
Mar 6, 2025
Merged

feat(conductor): include sequencer block hash #1999

joroshiba merged 5 commits into from
Mar 6, 2025

Conversation

@joroshiba
Copy link
Member

@joroshiba joroshiba commented Feb 25, 2025 •
edited
Loading

Summary

Adds a sequencer_block_hash to Execution API Block and ExecuteBlock shapes such that an execution can include it in their block space.

This is an addition but is backwards compatible. Old rollups can run the new binary, rollups that don't use the sequencer hash can utilize the old binary. Rollup forks which rely on the sequencer hash must utilize the new binary.

Background

We are upgrading Flame to the Cancun upgrade which includes BeaconRoot in the block hash. This is a natural time to add the sequencer block, as it is a similar concept.

Changes

  • Add new hash field to execution v1 protos and astria-core
  • Added the field to in development execution v2 protos
  • Update conductor to include the new data in ExecuteBlock requests
  • Added information to the execute API spec.

Testing

CI/CD testing has been updated to run the smoke tests with an updated version of geth that utilizes the sequencer and includes a beacon root contract in genesis.

I have done manual testing of the following:

  • old geth w/ new conductor unchanged
  • new geth w/ cancun upgrade disabled

Changelogs

Changelogs updated

Breaking Changelist

This is not a breaking change as it is a protobuf addition.

@github-actions github-actions bot added conductor pertaining to the astria-conductor crate proto pertaining to the Astria Protobuf spec labels Feb 25, 2025
@github-actions github-actions bot added the cd label Mar 5, 2025
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 5, 2025
@joroshiba joroshiba force-pushed the joroshiba/test-with-block-hash branch from 0de20da to 4542744 Compare March 5, 2025 23:00
@joroshiba joroshiba requested a review from mycodecrafting March 5, 2025 23:02
@joroshiba joroshiba marked this pull request as ready for review March 5, 2025 23:02
@joroshiba joroshiba requested a review from quasystaty1 March 5, 2025 23:02
@joroshiba joroshiba changed the title feat(conductor)!: include sequencer block hash feat(conductor): include sequencer block hash Mar 5, 2025
Fraser999
Fraser999 approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@Fraser999 Fraser999 left a comment

Choose a reason for hiding this comment

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

One small, non-blocking nitpick only.

I also wonder if it's worth documenting how the long hex string added to dev.yaml is generated? If it's obvious to others though, no need.

quasystaty1
quasystaty1 approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@quasystaty1 quasystaty1 left a comment

Choose a reason for hiding this comment

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

Infra approval. Few notes:

  • changes should be added to flame dev file as well for local deployments.
  • note that deployments will now activate Cancun by default, this is ok with me but wanted to flag here.
  • reminder to switch geth devTag back to latest once pr-75 is merged in astria-geth repo

SuperFluffy
SuperFluffy approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@SuperFluffy SuperFluffy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some nits mainly.

Don't we have changelogs for the Protos?

@joroshiba
Copy link
Member Author

joroshiba commented Mar 6, 2025

@quasystaty1 re note here:

note that deployments will now activate Cancun by default, this is ok with me but wanted to flag here.

Just wanted to note that this was done such that it only affects deployments using the dev configurations, so local deployments and smoke tests. When dependent changes are merged I will rollback the dev file changes and roll into the top level charts.

@joroshiba
Copy link
Member Author

joroshiba commented Mar 6, 2025

Don't we have changelogs for the Protos?

@SuperFluffy not currently. Can look into changing separately but not in scope here.

@joroshiba joroshiba added this pull request to the merge queue Mar 6, 2025
Merged via the queue into main with commit 46a5213 Mar 6, 2025
48 checks passed
@joroshiba joroshiba deleted the joroshiba/test-with-block-hash branch March 6, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ethanoroshiba ethanoroshiba Awaiting requested review from ethanoroshiba

4 more reviewers

@SuperFluffy SuperFluffy SuperFluffy approved these changes

@Fraser999 Fraser999 Fraser999 approved these changes

@mycodecrafting mycodecrafting mycodecrafting approved these changes

@quasystaty1 quasystaty1 quasystaty1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cd conductor pertaining to the astria-conductor crate documentation Improvements or additions to documentation override-freeze proto pertaining to the Astria Protobuf spec

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@joroshiba @Fraser999 @mycodecrafting @SuperFluffy @quasystaty1