Skip to content

Conversation

spiral-ladder
Copy link

@spiral-ladder spiral-ladder commented Jul 24, 2025

This PR contains major changes that overhaul how base branch implements state transition, which mainly is a direct lift of the TS version.

Proposed changes in this PR

No getters/setters in types

Most of what getters/setters accomplish can be accomplished similar with a pointer to the type, eg. *Type. In this PR, I propose having 'getter's of 2 variants. As an example:

    pub fn slot(self: *const BeaconStateAllForks) u64 {
        return switch (self.*) {
            inline else => |state| state.slot,
        };
    }

    pub fn slotPtr(self: *const BeaconStateAllForks) *u64 {
        return switch (self.*) {
            inline else => |state| &state.slot,
        };
    }

We use slot() for immutable access to state.slot, and slotPtr() if we want to change its value. I think realistically, using only 1 function that returns a pointer to the variable (in this case, only supporting slotPtr()) should have negligible overhead, but isn't aesthetic to deref every time we need to access the value. Having a separate *Ptr() function also implies that the value is subject to change.

Simplifying types

In the base branch, we have a separate types.zig that imports some ssz types such as ssz.primitive.Root, ssz.primitive.Epoch and so on, and reexports them. I think this is unnecessary. We can import them directly from the consensus_types module.

Breaking up utils

I'm personally not a fan of naming things utils because they usually could 100% be better named, eg. if the functions in the utils have to do with block processing, just call it block_processing.zig or at the very least block_utils.zig. Otherwise, they just end up as a dumping ground for functions that should belong in other files that we don't know how to name, and we end up with a file/directory with a random assortment of things. Another advantage of not naming things utils is that at first glance we know what code lives in a file by its filename.

@spiral-ladder spiral-ladder self-assigned this Jul 24, 2025
@spiral-ladder spiral-ladder force-pushed the bing/state-transition-fn branch 4 times, most recently from bf76cb8 to 73e0263 Compare July 28, 2025 07:45
* fix: wrong type for `ExecutionPayloadHeader` methods

* add usage to confirm
Adds support for `BlindedBeaconBlock` and `BlindedBeaconBlockBody` with
a sanity test.
feat(types): add support for blinded block variants
fix import

undo panic dedup deletion
@spiral-ladder spiral-ladder force-pushed the bing/state-transition-fn branch from 73e0263 to 24bf479 Compare July 30, 2025 06:23
@spiral-ladder spiral-ladder changed the base branch from te/naive_state_transition to te/fix_process_epoch July 30, 2025 06:23
@spiral-ladder spiral-ladder force-pushed the bing/state-transition-fn branch from 3f718af to 08c1029 Compare September 5, 2025 16:57
Copy link

github-actions bot commented Sep 7, 2025

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 9817885 Previous: null Ratio
get values - 1000 338.00 ns/op
get values - naive - 1000 688.00 ns/op
set values - 1000 322.00 ns/op
set values - naive - 1000 634.00 ns/op
get values - 1000000 849.00 ns/op
get values - naive - 1000000 1.4730 us/op
set values - 1000000 880.00 ns/op
set values - naive - 1000000 2.1510 us/op
JS - computeSyncCommitteeIndices - 16384 indices 320.18 ms/op
Zig - computeSyncCommitteeIndices - 16384 indices 3.9825 ms/op
JS - computeSyncCommitteeIndices - 250000 indices 292.22 ms/op
Zig - computeSyncCommitteeIndices - 250000 indices 16.884 ms/op
JS - computeSyncCommitteeIndices - 1000000 indices 310.14 ms/op
Zig - computeSyncCommitteeIndices - 1000000 indices 32.477 ms/op
JS - unshuffleList - 16384 indices 938.54 us/op
Zig - unshuffleList - 16384 indices 579.08 us/op
JS - unshuffleList - 250000 indices 13.844 ms/op
Zig - unshuffleList - 250000 indices 8.7613 ms/op
JS - unshuffleList - 1000000 indices 54.701 ms/op
Zig - unshuffleList - 1000000 indices 35.154 ms/op

by benchmarkbot/action

@spiral-ladder spiral-ladder force-pushed the bing/state-transition-fn branch from 76c85f5 to 38109bf Compare September 8, 2025 01:29
@spiral-ladder spiral-ladder marked this pull request as ready for review September 8, 2025 02:03
`test_utils` rely on `state_transition`, so importing both of them in
unit tests result in a circular dependency. It makes more sense to put
`test_utils` in `state_transition` and reuse it that way.
@spiral-ladder spiral-ladder merged commit 55de148 into te/naive_state_transition Sep 8, 2025
12 checks passed
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