Skip to content

fix: address comments on naive_state_transition branch #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 16 commits into from
Oct 2, 2025
Merged

fix: address comments on naive_state_transition branch #49

merged 16 commits into from
Oct 2, 2025

Conversation

twoeths
Copy link
Collaborator

@twoeths twoeths commented Oct 1, 2025

Motivation

Description

  • merge main to resolve conflict

@twoeths twoeths force-pushed the te/fix_naive_state_transition_oct_01 branch from a2216de to 4acd42b Compare October 1, 2025 03:03
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2025 •
edited
Loading

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 967df02 Previous: null Ratio
JS - unshuffleList - 16384 indices 937.56 us/op
Zig - unshuffleList - 16384 indices 600.67 us/op
JS - unshuffleList - 250000 indices 14.029 ms/op
Zig - unshuffleList - 250000 indices 9.4519 ms/op
JS - unshuffleList - 1000000 indices 54.465 ms/op
Zig - unshuffleList - 1000000 indices 37.025 ms/op
JS - computeSyncCommitteeIndices - 16384 indices 350.63 ms/op
Zig - computeSyncCommitteeIndices - 16384 indices 3.9960 ms/op
JS - computeSyncCommitteeIndices - 250000 indices 333.29 ms/op
Zig - computeSyncCommitteeIndices - 250000 indices 16.984 ms/op
JS - computeSyncCommitteeIndices - 1000000 indices 359.82 ms/op
Zig - computeSyncCommitteeIndices - 1000000 indices 30.646 ms/op
get values - 1000 351.00 ns/op
get values - naive - 1000 534.00 ns/op
set values - 1000 355.00 ns/op
set values - naive - 1000 604.00 ns/op
get values - 1000000 858.00 ns/op
get values - naive - 1000000 1.4670 us/op
set values - 1000000 833.00 ns/op
set values - naive - 1000000 2.1280 us/op

by benchmarkbot/action

ensi321
ensi321 reviewed Oct 1, 2025
View reviewed changes
zbuild.zon
.build_options = .{
.preset = .{
.default = "mainnet",
.type = "string",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's missing constants module related change here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh I forgot updating that, and CI only scanned through "zig build" command now
which command should I run on CIs @ensi321 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I forgot updating that, and CI only scanned through "zig build" command now which command should I run on CIs @ensi321 ?

I think it needs to run zbuild sync and then check if build.zig has changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@twoeths This PR will add CI to make sure build.zig is up to date - #52

@twoeths twoeths marked this pull request as ready for review October 2, 2025 07:14
@twoeths twoeths merged commit e53cad4 into te/naive_state_transition Oct 2, 2025
wemeetagain pushed a commit that referenced this pull request Oct 6, 2025
@twoeths @spiral-ladder
feat: naive state transition (#17)
97ba329 
* feat: implement EpochContext

* feat: implement reference couting strategy

* feat: CachedBeaconStateAllForks

* feat: electra and capella utils

* feat: implement phase0 processPendingAttestations()

* feat: epoch transition cache

* feat: processJustificationAndFinalization

* feat: processInactivityUpdates

* feat: processRegistryUpdates

* feat: processSlashings

* feat: processRewardsAndPenalties

* feat: processEth1DataReset

* fix: blst compilation error

* fix: update hashtree-z

* fix: skip darwin-x64 target due to hashtree

* feat: processPendingDeposits()

* feat: processPendingConsolidations()

* feat: processEffectiveBalanceUpdates()

* feat: processEpoch()

* feat: afterProcessEpoch()

* feat: processSyncCommitteeUpdates()

* fix: inline switch prongs for BeaconStateAllForks

* feat: model BeaconBlock

* feat: fork aware hashTreeRoot()

* feat: processBlockHeader

* feat: processWithdrawals()

* feat: convert ExecutionPayload to ExecutionPayloadHeader

* feat: processExecutionPayload()

* feat: fastAggregateVerify()

* feat: randao SignatureSet

* refactor: move beacon_state and beacon_block and execution_payload to types folder

* feat: model IndexedAttestation

* feat: indexed_attestation signature set

* feat: AttesterSlashings

* fix: implement fork in fork.zig

* feat: bls_to_execution_change signature set

* feat: proposer signature set

* refactor: move Fork definition to types folder

* feat: proposer_slashings signature set

* feat: voluntary_exits signature set

* feat: process_randao

* feat: process_eth1_data

* feat: slash_validator

* feat: process_proposer_slashing

* feat: attestation utils

* feat: is_valid_indexed_attestation

* feat: process_attester_slashing

* feat: getIndexedAttestation() and getAttestingIndices() in EpochCache

* feat: process_attestation_phase0

* feat: process_attestation_altair

* feat: process_attestations

* fix: generic validateAttestation

* chore: improve processs_attester_slashing

* fix: remove fork on process_attestation flow

* fix: remove fork in all state_transition functions

* feat: process_deposit

* feat: process_voluntary_exit

* feat: process_bls_to_execution_change

* feat: process_deposit_request

* feat: process_withdrawal_request

* feat: process_consolidation_request

* feat: process_operations

* feat: getAttestationsSignatureSets()

* feat: process_sync_committee

* feat: process_blob_kzg_commitments

* feat: process_block

* feat: process_slot

* fix: change ChainConfig property inside BeaconConfig

* fix: ForkSeq import path

* feat: params and config modules

* fix: add test folder

* fix: state_transition module in its own folder

* feat: add network chain configs

* fix: model EpochCache as reference count in CachedBeaconstateAllForks

* fix: create EpochCache from a beacon state

* feat: separate unit tests and int tests

* fix: SyncCommitteeCache

* fix: int test createCachedBeaconState

* fix: generalize committee_indices

* fix: support u64 proposer index

* refactor: ReferenceCount

* fix: generalize innerShuffleList()

* fix: processPendingDeposits

* feat: implement TestCachedBeaconStateAllForks

* fix: unit test beforeProcessEpoch (#27)

* fix: stack allocation EpochTransitionCache

* fix: pass by reference in process_epoch() functions

* fix: rewards and penalties as output params

* fix: unit test process_justification_and_finalization

* fix: reuse reards and penalties from EpochTransisionCache

* fix: process_epoch_fn compilation errors

* fix: PubkeyIndexMap to support both u32 and u64

* fix: processPendingDeposits compilation errors

* chore(types): dedup panic statements (#29)

* fix: processPendingConsolidations compilation errors

* fix: processEffectiveBalanceUpdates compilation errors

* fix: processSlashingsReset compilation errors

* fix: processHistoricalSummariesUpdate compilation errors

* fix: processHistoricalRootsUpdate compilation errors

* fix: processParticipationRecordUpdates compilation errors

* fix: processParticipationFlagUpdates compiltation errors

* fix: processSyncCommitteeUpdates compilation errors

* feat: implement and consume getters in EpochCache

* fix: wrong type for `ExecutionPayloadHeader` methods (#31)

* fix: wrong type for `ExecutionPayloadHeader` methods

* add usage to confirm

* feat(types): add support for blinded block variants

Adds support for `BlindedBeaconBlock` and `BlindedBeaconBlockBody` with
a sanity test.

* fix(types): correctly type methods in `ExecutionPayloadHeader`

* fix(types): remove `isExecutionType` from `BlindedBeaconBlockBody`

* feat: testProcessRewardsAndPenalties()

* chore: test processInactivityUpdates()

* chore: test processRegistryUpdates()

* chore: test processSlashings()

* fix: missing try in processEpoch()

* fix: testProcessEpochFn to handle no-err functions

* fix: unit test processPendingDeposits()

* fix: unit test processPendingConsolidations()

* chore: unit test processEffectiveBalanceUpdates()

* chore: unit test processSlashingsReset()

* chore: unit test processRandaoMixesReset()

* chore: unit test processHistoricalSummariesUpdate()

* fix: unit test processParticipationFlagUpdates()

* chore: processSyncCommitteeUpdates unit tests

* fix: append* instead of add*

* fix: unit test the main processEpoch() function

* fix: correct method name rotateEpochParticipations()

* fix: processSyncCommitteeUpdates next_sync_committee_pubkeys_slices[i]

* fix: wrong type for `ExecutionPayloadHeader` methods (#31)

* fix: wrong type for `ExecutionPayloadHeader` methods

* add usage to confirm

* feat(types): add support for blinded block variants

Adds support for `BlindedBeaconBlock` and `BlindedBeaconBlockBody` with
a sanity test.

* fix(types): correctly type methods in `ExecutionPayloadHeader`

* fix(types): remove `isExecutionType` from `BlindedBeaconBlockBody`

* main stf

fix import

undo panic dedup deletion

* main stf skeleton

* fix: inline else in beacon_state.zig

* fix: use imported type in beacon_state.zig

* chore: add comment for ReferenceCount.get() usage

* chore: avoid optional where possible

* chore: avoid double negative in process_rewards_and_penalties

* chore: avoid optional in process_slashings

* fix: expicit params for getTestProcessFn()

* fix: avoid negative terms in getTestProcessFn() util

* fix: TestRunner and TestOpt

* main stf skeleton

* more compilation fixes

* process attestation fixes

* processAttestation

* processDeposit

* more QoL tweaks

* processBlsToExecutionChanges

* fix: verifyMerkleBranch

* processVoluntaryExits

* processDepositRequest

* processWithdrawalRequest

* processConsolidationRequest

* processSyncCommittee

* processSyncCommittee pt 2

* processBlobKzgCommitments

* unnecessary compileLog; test runs

* rename unblinded -> regular for clearer names

* more simplification with regular vs blinded

* move SignedBlock out

* test: processBlockHeader

* test: processWithdrawals

* test: processExecutionPayload

* test:processRandao

* test: processEth1Data

* test: process operations

* test: process sync committee and blob kzg

* clean imports

* remove print

* ergonomic pending withdrawals

* zbuild sync

* fix utils

* fix build

* test: skip stf test for now

* uncomment beforeProcessEpoch and processEpoch

* slot, fork, latestBlockHeader api changes

* feat(beacon_state): more un-OOPing of API

* feat(beacon_state): more unOOP

* feat(beacon_state): finish simplifying beacon_state

* feat(beacon_block): unoops

* feat(beacon_block): fin

* feat(signed_block): clean up

* feat(signed_block): move to types/

* fix imports

* feat: revamp types

this still feels a little wrong, there are only that many reusable types
from `consensus_types` that I think the cleaner way might just be to use
them directly instead of reimports.

* remove ValidatorIndices

* feat(process_block): ProcessBlockOpts

* remove unnecessary TODO for blockToHeader

* missed out removing appendPendingDeposit

* chore: remove TODOs

Some TODOs are either already completed, or have low-hanging fruit
changes that are implemented in this commit.

* fix(bls): blst test

* fix: compilation issues

* chore: nuke primitives.zig

we should just directly import and use ssz types where possible instead of needlessly
importing and reexporting them. This provides clarity of what the
underlying type is, because we also have some types native only to state
transition (such as the beacon blocks)

* chore: remove script

* feat(stf): implement isMergeTransitionComplete

* use clone

* comments

* feat(process_epoch): use toBoolArray for justification bits

* fix(epoch_transition_cache): use initCapacity

* style changes

* fix(validator): snake_case instead of camelCase

* chore: style

* slight stylistic change for readability

* fix: typo: acrive -> active

* fix(epoch_participation): prefer `clearRetainingCapacity` over `deinit`

For 2 reasons:

1. we're about to reuse the same `ArrayList` again, so there's no reason
   to `deinit`
2. we can assume (at the very least) the capacity of the 2 arraylists are the same because `resize` was called in `beforeProcessEpoch`.

* feat(processBlock): handle expected_withdrawals_result deinit

* remove unused allocator params

* fix(proposer_slashing): verify_signature as bool instead of ?bool

* feat(slash_validator): use switch/case instead of if/else to set slashing penalty quotient

* feat(slash_validator): switch/case for all fork dependent values

* fix(processBlsToExecutionChange): verify_signature as bool instead of ?bool

* style

* fix(processSyncAggregate): verify_signature as bool instead of ?bool

* update zbuild

* build: update zbuild

* fix: deinit expected withdrawal result

* fix(beacon_state): properly implement rotateEpochParticipation

* feat(beacon_state): fully impl clone

* feat(stf): upgrade state

* chore: move options into params

* fix upgrade state

* chore(test_utils): move test_utils into state_transition module

`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.

* test: test process attestations

* chore: remove test/utils

* update .gitignore

* chore: clean up `BlockExternalData`

* fix: missing ExecutinoPayloadStatus enum

* chore: remove upgrade_state.zig

* remove allocator from processWithdrawals

* rework processWithdrawals a bit

* fix: address comments on naive_state_transition branch (#49)

* feat: merge main

* fix: use @TagName() in ForkSeq

* fix: remove ForkSeq.isXXX()

* fix: handle integer underflow of epoch - 1

* fix: verify signature in isValidIndexedAttestation()

* fix: handle addPubkey() where index=len

* fix: use @TagName() in Preset

* feat: make constants module

* fix: move ForkSeq from params to config

* fix: move preset from consensus_types to params

* fix: params should not import consensus_types

* fix: rename params to preset

* fix: CI steps

* fix: unify blst dependencies

* feat: precompute all domains

* fix: remove unnecessary CI trigger

---------

Co-authored-by: Tuyen Nguyen <[email protected]>

* fix: CI to run all tests specified in build.zig

* fix: clean up TODOs

* remove usage of ?bool in preference for plain bool

* fix: simplify Preset method

* fix: generate build.zig from zbuild

* fix: zig 0.14.1

* fix: comment out zbuild sync job

* fix: fix zig 0.14.1 after merge

* fix: switch back to zig 0.14.0

---------

Co-authored-by: Tuyen Nguyen <[email protected]>
Co-authored-by: bing <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ensi321 ensi321 ensi321 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@twoeths @ensi321 @spiral-ladder