Skip to content

Conversation

@powerslider
Copy link
Contributor

@powerslider powerslider commented Nov 27, 2025

Why this should be merged

Check #4569

How this works

  • Delete graft/coreth/plugin/evm/customrawdb.
  • Switch dependent code to using github.com/ava-labs/avalanchego/vms/evm/sync/customrawdb import instead.
  • Fix build and test failures.

How this was tested

existing UT

Need to be documented?

no

Need to update RELEASES.md?

no

resolves #4569

Signed-off-by: Tsvetan Dimitrov ([email protected])

Handle database.ErrNotFound returned by ReadSyncRoot when there's no
previous sync root. This is expected for fresh databases and should be
treated as an empty hash (no previous sync) rather than an error.
Handle database.ErrNotFound from ReadAcceptorTip as expected case
(empty hash) instead of error. This fixes VM initialization failures
on fresh databases.
Update TestWipe to accept database.ErrNotFound as valid result
after wipe, since ReadSnapshotBlockHash returns ErrNotFound when
the marker doesn't exist.
- Move block number override fix before NewEVMBlockContext creation
so GetHashFn can correctly resolve blockhash(n) when block number
is overridden to n+1. This fixes TestTracingWithOverrides for
Firewood scheme.

- The bug was probably a pre-existing code issue where the header modification
happened after creating the block context. Since GetHashFn creates
a closure that captures the header reference, modifying the header
afterward left the closure with stale values. This bug was exposed
by the recent avalanchego update (3b08d61),
which likely changed behavior that made the test fail specifically
for Firewood scheme, while HashScheme continued to pass.
- Extract sentinel errors in journal.go for better error checking
and maintainability.
- Refactor loadSnapshot to use switch statements
for clearer control flow when validating snapshot block hash and root.
- Improve TestWipe validation logic:
   - Use switch statements for consistent error handling patterns.
   - Add context to error messages ("before wipe" / "after wipe").
@powerslider powerslider force-pushed the powerslider/1302-delete-customrawdb-package branch 4 times, most recently from 96abb67 to 831d8f3 Compare November 28, 2025 14:56
@powerslider powerslider force-pushed the powerslider/1302-delete-customrawdb-package branch from 831d8f3 to 98a81dc Compare November 28, 2025 14:57
Copy link
Contributor

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

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

just some cleanup from this PR. otherwise lgtm

@powerslider powerslider force-pushed the powerslider/1302-delete-customrawdb-package branch from cbd8271 to b77d79e Compare December 1, 2025 11:45
@powerslider powerslider force-pushed the powerslider/1302-delete-customrawdb-package branch from b77d79e to 17f9aa3 Compare December 1, 2025 11:46
Copy link
Contributor

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +107 to 113
switch {
case errors.Is(err, database.ErrNotFound): // Expected: marker was deleted.
case err != nil:
t.Errorf("unexpected error reading snapshot block hash after wipe: %v", err)
case blockHash != (common.Hash{}):
t.Errorf("snapshot block hash marker remained after wipe: %#x", blockHash)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't check the blockhash if the error is database.ErrNotFound. Is that expected? Just looks like a bug at first glance

Copy link
Contributor

Choose a reason for hiding this comment

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

In either case, this definitely seems like it's a symptom of racy code. Will you add a comment describing why the error may or may not be nil?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Delete customrawdb package after migration to avalanchego

5 participants