Skip to content

l2geth: fix eth_getBlockRange#2390

Merged
mergify[bot] merged 2 commits intodevelopfrom
fix/get-block-range
May 2, 2022
Merged

l2geth: fix eth_getBlockRange#2390
mergify[bot] merged 2 commits intodevelopfrom
fix/get-block-range

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented Mar 31, 2022

Description
Fix for eth_getBlockRange when an item is not found

@changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2022

🦋 Changeset detected

Latest commit: b37354a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/l2geth Patch

Not sure what this means? Click here to learn what changesets are.

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

@tynes tynes force-pushed the fix/get-block-range branch from 0b7d338 to f895c9c Compare March 31, 2022 22:32
@mslipper mslipper added the C-protocol-critical Category: Modifies protocol-critical code label Apr 1, 2022
@mslipper
Copy link
Collaborator

mslipper commented Apr 3, 2022

Is there any way we could unit test this? And are we sure that the this new behavior doesn't break any implied contracts? Even though this is technically a bug, it's possible that the DTL or other downstream callers relied on the broken error handling.

@mergify mergify bot requested a review from Inphi April 4, 2022 18:47
@tynes
Copy link
Contributor Author

tynes commented Apr 6, 2022

Unit tests wouldn't catch a downstream service hitting this error, what would happen is the dtl would index a null transaction

@tynes tynes force-pushed the fix/get-block-range branch from f895c9c to 0450d08 Compare April 13, 2022 19:19
@tynes
Copy link
Contributor Author

tynes commented Apr 13, 2022

I've pushed a new commit that returns a custom error

@github-actions
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Apr 28, 2022
@tynes tynes removed the Stale label Apr 28, 2022
@mergify
Copy link
Contributor

mergify bot commented May 2, 2022

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

@mergify mergify bot merged commit 1bcee8f into develop May 2, 2022
@mergify mergify bot deleted the fix/get-block-range branch May 2, 2022 00:55
@mergify
Copy link
Contributor

mergify bot commented May 2, 2022

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

This was referenced May 2, 2022
@mslipper mslipper mentioned this pull request May 2, 2022
theochap added a commit that referenced this pull request Dec 10, 2025
… build task to reuse the insert task (#2399)

## Description

This PR does the following cleanups to the engine tasks:

- Use explicit error types instead of `dyn Box<...>` for the
`EngineTaskError`. The goal of this is twofold:
- Improve the composability of engine tasks (and be able to do some
control flow when an engine task emits an error)
- Allows explicit testing of the error cases (in the future when we'll
add tests for the engine tasks)
- Refactors the build task in the engine to reuse the `InsertUnsafe`
task logic. This task is now called `InsertTask` to also encapsulate the
case where we insert a derived block.

Close #2390. Progress towards #2389
theochap added a commit that referenced this pull request Jan 14, 2026
… build task to reuse the insert task (op-rs/kona#2399)

## Description

This PR does the following cleanups to the engine tasks:

- Use explicit error types instead of `dyn Box<...>` for the
`EngineTaskError`. The goal of this is twofold:
- Improve the composability of engine tasks (and be able to do some
control flow when an engine task emits an error)
- Allows explicit testing of the error cases (in the future when we'll
add tests for the engine tasks)
- Refactors the build task in the engine to reuse the `InsertUnsafe`
task logic. This task is now called `InsertTask` to also encapsulate the
case where we insert a derived block.

Close #2390. Progress towards #2389
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cannon Area: cannon C-protocol-critical Category: Modifies protocol-critical code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants