Skip to content

Conversation

powerslider
Copy link
Contributor

@powerslider powerslider commented Oct 3, 2025

Why this should be merged

Check #4386

How this works

  • Move plugin/evm/customrawdb in coreth to vms/evm/sync in avalanchego as part of state sync code migration effort.
  • Replace geth-style zero/nil read returns with explicit errors across customrawdb.
  • Add exported sentinel errors:
    • ErrEntryNotFound, ErrInvalidData, ErrStateSchemeConflict, ErrInvalidArgument
  • Update accessors to return errors consistently.
  • Align tests with new errors and function signatures.
  • Add more tests to cover all exported functionality.

How this was tested

Existing UT that come with the migrated code plus newly added UT to cover the entire package API.

Need to be documented in RELEASES.md?

no

resolves #4386

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

- Move `plugin/evm/customrawdb` in `coreth` to `vms/evm/sync` in
  `avalanchego` as part of state sync code migration effort.

resolves #4386

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

- Add tests:
  - Time markers (offline_pruning, populate_missing_tries): missing marker and bad-encoding cases.
  - Pruning disabled flag: presence toggling.
  - Acceptor tip: invalid length error and write/read behavior (table-driven: none, single_write, overwrite).
  - State sync: sync root read/write, code-to-fetch iterate/delete, segments/storage tries iterate/unpack/clear, sync performed latest (empty/increasing/unsorted).
  - Snapshot: block hash read/write/delete and account iterator key-length filtering.
  - ParseStateSchemeExt: Firewood on empty DB, conflict on existing scheme, passthrough for "hash".

- Refactor/behavioral tightening (internal-only):
  - Add unexported sentinels: errMarkerNotFound, errMarkerInvalid, errAcceptorTipInvalid.
  - Normalize time marker reads: use `Has()` for presence and return `errMarkerNotFound` when absent/empty. Wrap decode errors with `errMarkerInvalid`.
  - Wrap acceptor tip invalid-length with `errAcceptorTipInvalid`.
  - Tests assert with `require.ErrorIs` for known sentinels.

- Schema organization:
  - Group all package-level globals in a single var block in schema_ext.go
  - Add clear section headers to improve readability
@powerslider
Copy link
Contributor Author

powerslider commented Oct 10, 2025

@joshua-kim I added lots of new tests that enable having the following coverage for the package:

coverage: 86.7% of statements
ok  	github.com/ava-labs/avalanchego/vms/evm/sync/customrawdb	0.213s	coverage: 86.7% of statements

I fixed most of the minor comments and will get back on the rest of your points once I have more historical context from the team.

… add sentinel errors, fix tests

- Replace geth-style zero/nil read returns with explicit errors across customrawdb.
- Add exported sentinel errors:
  - ErrEntryNotFound, ErrInvalidData, ErrStateSchemeConflict, ErrInvalidArgument
- Update accessors to return errors consistently.
- Align tests with new errors and function signatures.
- Convert cases to table-driven, snake_case names.
- Add no-op clear tests for segments/storage tries.
- Add invalid-length/invalid-encoding tests (snapshot block hash, time markers).
- Add iterator error checks after loops.
… add sentinel errors, fix tests

- Replace geth-style zero/nil read returns with explicit errors across customrawdb.
- Add exported sentinel errors:
  - ErrEntryNotFound, ErrInvalidData, ErrStateSchemeConflict, ErrInvalidArgument
- Update accessors to return errors consistently.
- Align tests with new errors and function signatures.
- Convert cases to table-driven, snake_case names.
- Add no-op clear tests for segments/storage tries.
- Add invalid-length/invalid-encoding tests (snapshot block hash, time markers).
- Add iterator error checks after loops.
@powerslider
Copy link
Contributor Author

@joshua-kim all comments should be fixed. Discussed with the team to omit geth api style and to remove the _ext suffixes. Please check these changes.

…sibilities

- Rename scheme.go -> db.go (state scheme parsing, shared package items).
- Rename state_sync.go -> sync_progress.go (state sync progress + iterators).
- Merge snapshot helpers into markers.go (all node/chain metadata in one place).
- De-flake and simplify tests:
  - Make time-marker writes deterministic by passing a fixed timestamp in tests
  - Simplify ReadChainConfig tests, remove mutate case and rely on dedicated invalid JSON test.
  - Use require.ErrorIs consistently for error assertions.
@joshua-kim joshua-kim enabled auto-merge October 20, 2025 16:35
@joshua-kim joshua-kim requested a review from DracoLi October 20, 2025 16:35
@joshua-kim joshua-kim disabled auto-merge October 20, 2025 16:35
"github.com/stretchr/testify/require"
)

func TestParseStateScheme(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No action required, but I'd probably slightly prefer this as a test vector... Admittedly this might be more succinct as is.

@StephenButtolph StephenButtolph added this pull request to the merge queue Oct 20, 2025
Merged via the queue into master with commit f369e5d Oct 20, 2025
35 checks passed
@StephenButtolph StephenButtolph deleted the powerslider/4386-migrate-customrawdb-coreth branch October 20, 2025 23:50
@github-project-automation github-project-automation bot moved this from In Progress 🏗️ to Done 🎉 in avalanchego Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done 🎉

Development

Successfully merging this pull request may close these issues.

Migrate customrawdb package from coreth to avalanchego

5 participants