Skip to content

bedrock-goerli genesis from commit 69ee68#2978

Merged
mergify[bot] merged 2 commits intodevelopfrom
bedrock-goerli-69ee68-genesis
Jul 11, 2022
Merged

bedrock-goerli genesis from commit 69ee68#2978
mergify[bot] merged 2 commits intodevelopfrom
bedrock-goerli-69ee68-genesis

Conversation

@optimisticben
Copy link
Contributor

Description
New bedrock-goerli deployment from images built with commit #2961

This removed the OptimismMintableTokenFactoryProxy, should I put that back, or does it need to be redeployed?

@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2022

⚠️ No Changeset found

Latest commit: 1bb8ee7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Jul 11, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@github-actions github-actions bot added 2-reviewers A-pkg-contracts-bedrock Area: packages/contracts-bedrock labels Jul 11, 2022
@tynes
Copy link
Contributor

tynes commented Jul 11, 2022

This removed the OptimismMintableTokenFactoryProxy, should I put that back, or does it need to be redeployed?

This contract was renamed so its all good

@mergify
Copy link
Contributor

mergify bot commented Jul 11, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Jul 11, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 7b054b2 into develop Jul 11, 2022
@mergify mergify bot deleted the bedrock-goerli-69ee68-genesis branch July 11, 2022 20:22
@mergify mergify bot removed the on-merge-train label Jul 11, 2022
theochap pushed a commit that referenced this pull request Dec 10, 2025
#3018)

# Description
The main goal of this change is to allow engine processing to continue
after block building has been initiated but before it has been sealed.

This is accomplished by:
- Updating the `EngineActor` to
    - Make the `BuildTask` only initiate block building
    - Add a new `SealTask` to seal and insert the initiated block
- Updating the `SequencerActor` loop to 
    - Seal the last block and start the next block
    - Dynamically set the loop interval to account for sealing duration

Importantly the `SequencerActor`'s `build_block` function no longer
starts and completes block building in a single call but rather seals
the last block if there was one, and starts the next. This means that
the time spent waiting for the next tick will be _during block
building_.

This also changes the `EngineTask` pattern within the `SealTask` such
that if there was a caller (i.e. there exists a `Sender` to use to relay
results), the task may not be retried if there is a "temporary" task
error. While this diverges from existing patterns, it is a meaningful
step toward #3021, which aims to pull logic out of the engine and into
the appropriate actor. This gives the Sequencer more insight into and
control over block building and timing.

## Considerations
This PR creates a number of possible race conditions. One race condition
that did not previously exist but has been introduced and mitigated is
`Build` (FCU) -> `Reset` -> `Seal`.

A worthwhile exercise for a reviewer is to consider the other possible
race conditions introduced and evaluate the feasibility of them
occurring and impact if they happen.

## Testing
While this PR is ready to review and passes existing e2e and acceptance
tests, a byproduct of breaking apart the `BuildTask` is a number of
possible race conditions. This PR aimed to address a number of them
(e.g. detecting if the unsafe head changed between `Build` and `Seal`
operations and retrying), but a worthwhile exercise will be to:
1. List all possible `EngineActor` task race conditions that could
result from this change
1. Define expected behavior for each of those conditions
1. Confirm via unit tests that behavior matches expected behavior.

There is also value in unit testing the logic in the different functions
within `SequencerActor` and the affected `EngineActor` tasks.

Unfortunately the act of making the `SequencerActor`, `EngineActor`, and
`EngineTask`s unit testable is a considerable amount of work. A
different PR will be created, branching off of this PR to attempt that.
This PR _can_ be merged as is, as it meets current standards, but
another reasonable option is to wait until `kona` is more unit testable
and unit tests exist to demonstrate that the changes work as intended.

## Other 

Fixes #2978
Relates to #2848, #3021
theochap pushed a commit that referenced this pull request Jan 14, 2026
op-rs/kona#3018)

# Description
The main goal of this change is to allow engine processing to continue
after block building has been initiated but before it has been sealed.

This is accomplished by:
- Updating the `EngineActor` to
    - Make the `BuildTask` only initiate block building
    - Add a new `SealTask` to seal and insert the initiated block
- Updating the `SequencerActor` loop to 
    - Seal the last block and start the next block
    - Dynamically set the loop interval to account for sealing duration

Importantly the `SequencerActor`'s `build_block` function no longer
starts and completes block building in a single call but rather seals
the last block if there was one, and starts the next. This means that
the time spent waiting for the next tick will be _during block
building_.

This also changes the `EngineTask` pattern within the `SealTask` such
that if there was a caller (i.e. there exists a `Sender` to use to relay
results), the task may not be retried if there is a "temporary" task
error. While this diverges from existing patterns, it is a meaningful
step toward #3021, which aims to pull logic out of the engine and into
the appropriate actor. This gives the Sequencer more insight into and
control over block building and timing.

## Considerations
This PR creates a number of possible race conditions. One race condition
that did not previously exist but has been introduced and mitigated is
`Build` (FCU) -> `Reset` -> `Seal`.

A worthwhile exercise for a reviewer is to consider the other possible
race conditions introduced and evaluate the feasibility of them
occurring and impact if they happen.

## Testing
While this PR is ready to review and passes existing e2e and acceptance
tests, a byproduct of breaking apart the `BuildTask` is a number of
possible race conditions. This PR aimed to address a number of them
(e.g. detecting if the unsafe head changed between `Build` and `Seal`
operations and retrying), but a worthwhile exercise will be to:
1. List all possible `EngineActor` task race conditions that could
result from this change
1. Define expected behavior for each of those conditions
1. Confirm via unit tests that behavior matches expected behavior.

There is also value in unit testing the logic in the different functions
within `SequencerActor` and the affected `EngineActor` tasks.

Unfortunately the act of making the `SequencerActor`, `EngineActor`, and
`EngineTask`s unit testable is a considerable amount of work. A
different PR will be created, branching off of this PR to attempt that.
This PR _can_ be merged as is, as it meets current standards, but
another reasonable option is to wait until `kona` is more unit testable
and unit tests exist to demonstrate that the changes work as intended.

## Other 

Fixes #2978
Relates to #2848, #3021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-pkg-contracts-bedrock Area: packages/contracts-bedrock

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants