Skip to content

Conversation

@Inspector-Butters
Copy link
Contributor

What type of PR is this?
Feature

What does this PR do? Why is it needed?
This PR integrates the state diff path into the State() function from db/kv, which allows reading of states using the state diff db, when the EnableStateDiff flag is enabled.

Notes for reviewers:
Files kv/state_diff_test.go and config/features/config.go only contain renamings:

  • kv/state_diff_test.go: rename setDefaultExponents() to setDefaultStateDiffExponents() to be less vague.
  • config/features/config.go: rename enableStateDiff to EnableStateDiff to make it public.

// If state diff is enabled, we get the state from the state-diff db.
if features.Get().EnableStateDiff {
st, err := s.getStateUsingStateDiff(ctx, blockRoot)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

If there error is a state not found error, could it fallback to the original implementation? I am thinking about folks that enabled state diff on a fully or partially synced node and they may have some states in the other bucket.

Copy link
Contributor

Choose a reason for hiding this comment

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

we won't have coexisting dbs. If the node reaches this point it can't use the previous impl.

Copy link
Contributor

@potuz potuz left a comment

Choose a reason for hiding this comment

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

LGTM, added a request for a change in tests.

err = db.saveStateByDiff(context.Background(), st1)
require.NoError(t, err)

slot := primitives.Slot(math.PowerOf2(5))
Copy link
Contributor

Choose a reason for hiding this comment

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

The chaindiff is trivial for this slot, better to do something like 2^exponents[len(exponents)-2] + 2^exponents[len(exponents)-1] to have some non-trivial diffs, you´ll also need to save the anchor at the penultimate level.

@potuz potuz added this pull request to the merge queue Nov 24, 2025
github-merge-queue bot pushed a commit that referenced this pull request Nov 24, 2025
**What type of PR is this?**
Feature

**What does this PR do? Why is it needed?**
This PR integrates the state diff path into the `State()` function from
`db/kv`, which allows reading of states using the state diff db, when
the `EnableStateDiff` flag is enabled.

**Notes for reviewers:**
Files `kv/state_diff_test.go` and `config/features/config.go` only
contain renamings:
- `kv/state_diff_test.go`: rename `setDefaultExponents()` to
`setDefaultStateDiffExponents()` to be less vague.
- `config/features/config.go`: rename `enableStateDiff` to
`EnableStateDiff` to make it public.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 24, 2025
@Inspector-Butters Inspector-Butters added this pull request to the merge queue Nov 24, 2025
Merged via the queue into develop with commit 11bb854 Nov 24, 2025
18 checks passed
@Inspector-Butters Inspector-Butters deleted the read-state-diff branch November 24, 2025 13:07
github-merge-queue bot pushed a commit that referenced this pull request Nov 24, 2025
**Review after #16033 is merged**

**What type of PR is this?**
Other

**What does this PR do? Why is it needed?**
This PR refactors the code to find the corresponding slot of a given
block root using state summary or the block itself, into its own
function `SlotByBlockRoot(ctx, blockroot)`.

Note that there exists a function `slotByBlockRoot(ctx context.Context,
tx *bolt.Tx, blockRoot []byte)` immediately below the new function. Also
note that this function has two drawbacks, which led to creation of the
new function:
- the old function requires a boltdb tx, which is not necessarily
available to the caller.
- the old function does NOT make use of the state summary cache. 

edit: 
- the old function also uses the state bucket to retrieve the state and
it's slot. this is not something we want in the state diff feature,
since there is no state bucket.
terencechain pushed a commit that referenced this pull request Nov 24, 2025
**Review after #16033 is merged**

**What type of PR is this?**
Other

**What does this PR do? Why is it needed?**
This PR refactors the code to find the corresponding slot of a given
block root using state summary or the block itself, into its own
function `SlotByBlockRoot(ctx, blockroot)`.

Note that there exists a function `slotByBlockRoot(ctx context.Context,
tx *bolt.Tx, blockRoot []byte)` immediately below the new function. Also
note that this function has two drawbacks, which led to creation of the
new function:
- the old function requires a boltdb tx, which is not necessarily
available to the caller.
- the old function does NOT make use of the state summary cache. 

edit: 
- the old function also uses the state bucket to retrieve the state and
it's slot. this is not something we want in the state diff feature,
since there is no state bucket.
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.

4 participants