Skip to content

Conversation

@RodrigoVillar
Copy link
Contributor

@RodrigoVillar RodrigoVillar commented Jan 5, 2026

Why this should be merged

As part of testing Firewood's archival support, we need to test that C-Chain queries against historical states are supported.

Closes ava-labs/firewood#1499.

How this works

Introduces a state access test that, given a start ($X$) and end block ($Y$), queries an instance of Coreth by asking for the nonce of the zero address for all blocks $\alpha \in [X, Y]$. This is sufficient for testing historical queries as this succeeds only if Coreth has the matching trie for each $\alpha$.

How this was tested

CI

Need to be documented in RELEASES.md?

No

@RodrigoVillar RodrigoVillar force-pushed the rodrigo/coreth-firewood-historical-access-test branch from 9ef57dc to d701cf1 Compare January 6, 2026 21:41
Comment on lines +27 to +28
# XXX: REMOVE BEFORE MERGING
pull_request:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove before merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've went with creating a new script rather than extending ./scripts/benchmark_cchain_range.sh since the state access tests simply reads over the current state, while both the reexecution and chaos tests start with the current state and write to it. This causes common terms such as START_BLOCK and END_BLOCK to take on different meanings depending on the context they're used in (state access test vs reexecution/chaos tests) and so I went with creating a new script to make things clearer.

@RodrigoVillar RodrigoVillar added the DO NOT MERGE This PR must not be merged in its current state label Jan 7, 2026
@RodrigoVillar RodrigoVillar marked this pull request as ready for review January 7, 2026 16:25
@RodrigoVillar RodrigoVillar requested a review from a team as a code owner January 7, 2026 16:25
Copilot AI review requested due to automatic review settings January 7, 2026 16:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a test framework for verifying Firewood's ability to query historical C-Chain state by implementing a state access test that queries the nonce of the zero address across a range of blocks.

Key Changes:

  • Adds a new state access test program that validates historical state queries for archival nodes
  • Creates shell scripts to orchestrate test execution with configurable block ranges and data sources
  • Implements CI workflow for automated daily testing and on-demand execution

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/reexecute/stateaccess/main.go Core test program that queries historical nonces to verify archival state accessibility
scripts/tests.firewood_state_access.sh Test orchestration script with predefined test configurations and S3/local data source support
scripts/import_cchain_data.sh Updates import script to make block data optional while keeping state data required
Taskfile.yml Adds task definition for running the Firewood state access test
.github/workflows/firewood-state-access-test.yml GitHub Actions workflow for scheduled and manual test execution with configurable parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maru-ava
Copy link
Contributor

maru-ava commented Jan 7, 2026

This seems awfully heavyweight to validate that historical state will be accessible via firewood. Are there faster/lighter tests already that validate accessibility in a way that doesn't require access to s3 and processing 250k+ blocks?

@RodrigoVillar
Copy link
Contributor Author

This seems awfully heavyweight to validate that historical state will be accessible via firewood. Are there faster/lighter tests already that validate accessibility in a way that doesn't require access to s3 and processing 250k+ blocks?

I could reduce the block range (e.g. only query blocks 100k-250k) but to the point of this being heavyweight, could you elaborate more?

Copy link
Contributor

@alarso16 alarso16 left a comment

Choose a reason for hiding this comment

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

From the EVM perspective, everything looks good. The test setup seems ok too, but not my area of expertise

@alarso16
Copy link
Contributor

alarso16 commented Jan 7, 2026

This seems awfully heavyweight to validate that historical state will be accessible via firewood. Are there faster/lighter tests already that validate accessibility in a way that doesn't require access to s3 and processing 250k+ blocks?

I could reduce the block range (e.g. only query blocks 100k-250k) but to the point of this being heavyweight, could you elaborate more?

You can explicitly set state-history in the config, which corresponds to RevisionsInMemory. With this, you could set it to like 5, and then just reading 10 blocks is sufficient to test that historical state is available

@maru-ava
Copy link
Contributor

maru-ava commented Jan 7, 2026

I could reduce the block range (e.g. only query blocks 100k-250k) but to the point of this being heavyweight, could you elaborate more?

Why do we even need historical state for this? Unlike reexecution, where block processing involves a lot more than just firewood, this seems like something that could be trivially verified in a unit test.

@alarso16
Copy link
Contributor

alarso16 commented Jan 7, 2026

Why do we even need historical state for this? Unlike reexecution, where block processing involves a lot more than just firewood, this seems like something that could be trivially verified in a unit test.

It does seem unnecessary to be a re-execution test, you shouldn't need real state to verify this. You could just generate a few random blocks and then query to ensure that the state didn't disappear

@RodrigoVillar
Copy link
Contributor Author

Closing this PR in favor of @alarso16's suggestions.

@github-project-automation github-project-automation bot moved this to Done 🎉 in avalanchego Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE This PR must not be merged in its current state

Projects

Status: Done 🎉

Development

Successfully merging this pull request may close these issues.

test: integration test with Coreth and RootStore

4 participants