Skip to content

Conversation

RodrigoVillar
Copy link
Collaborator

@RodrigoVillar RodrigoVillar commented Sep 2, 2025

Why this should be merged

As described in ava-labs/coreth#1137, database inspection of a Coreth chain database fails because InspectDatabase() expects for freezers to be used. This PR fixes this bug by adding an option to skip freezer inspection.

This PR should be followed with a downstream PR in Coreth to pass in an option to skip freezer inspection during database inspection.

How this works

Extends inspectDatabaseConfig with a skipFreezers field and adds the associated option function for it.

How this was tested

CI + ran InspectDatabase() against Coreth locally

@RodrigoVillar RodrigoVillar self-assigned this Sep 2, 2025
@RodrigoVillar RodrigoVillar marked this pull request as ready for review September 2, 2025 13:06
Copy link
Collaborator

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

Please also add a test in database.libevm_test.go. I think the simplest way is to change stubDatabase.Ancients() to return an error and then confirm the error returned by InspectDatabase() with and without the option.

@RodrigoVillar RodrigoVillar requested a review from ARR4N September 2, 2025 17:28
Copy link
Collaborator

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

LGTM

This surfaced some things I don't like in our original test setup, but that's not within the scope of this PR. I'll send you a follow-up PR soon.

@ARR4N ARR4N enabled auto-merge (squash) September 3, 2025 13:50
@ARR4N ARR4N merged commit f19cd58 into main Sep 3, 2025
12 checks passed
@ARR4N ARR4N deleted the RodrigoVillar/skip-freezer-inspection branch September 3, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants