Skip to content

Conversation

advaita-saha
Copy link
Contributor

Full fix for #3661

@advaita-saha
Copy link
Contributor Author

@kdeme Can you look at the portal tests ?

@tersec
Copy link
Contributor

tersec commented Sep 13, 2025

/home/runner/work/nimbus-eth1/nimbus-eth1/portal/tests/beacon_network_tests/test_beacon_content.nim(190, 1) template/generic instantiation of `suite` from here
/home/runner/work/nimbus-eth1/nimbus-eth1/portal/tests/beacon_network_tests/test_beacon_content.nim(197, 5) Error: the field 'bellatrix' is not accessible.

is due to:

Because

suite "Beacon Content Keys and Values":
# TODO: These tests are less useful now and should instead be altered to
# use the consensus test vectors to simply test if encoding / decoding works
# fine for the different forks.
const forkDigests = ForkDigests(
phase0: ForkDigest([0'u8, 0, 0, 1]),
altair: ForkDigest([0'u8, 0, 0, 2]),
bellatrix: ForkDigest([0'u8, 0, 0, 3]),
capella: ForkDigest([0'u8, 0, 0, 4]),
deneb: ForkDigest([0'u8, 0, 0, 5]),
electra: ForkDigest([0'u8, 0, 0, 6]), # TODO fulu: ForkDigest([0'u8, 0, 0, 7]),
)
won't work anymore.

That change is intentional, and while it's of course technically possible to revert it, maintaining the design it's based on into new forks doesn't make sense. I already haven't for Gloas.

let
metadata = getMetadataForNetwork("mainnet")
genesisState =
try:
template genesisData(): auto =
metadata.genesis.bakedBytes
newClone(
readSszForkedHashedBeaconState(
metadata.cfg, genesisData.toOpenArray(genesisData.low, genesisData.high)
)
)
except CatchableError as err:
raiseAssert "Invalid baked-in state: " & err.msg
genesis_validators_root = getStateField(genesisState[], genesis_validators_root)
forkDigests = newClone ForkDigests.init(metadata.cfg, genesis_validators_root)
shows another approach, which remains viable, and if the exact contents of forkDigests is really that unimportant, the configuration information provided to ForkDigests.init can be done on the basis of convenience as well, e.g., something like ForkDigests.init(getMetadataForNetwork("mainnet"), default(Hash32)) should work fine.

@advaita-saha
Copy link
Contributor Author

exact contents of forkDigests is really that unimportant

Then the fix is easy

@advaita-saha advaita-saha requested a review from kdeme September 13, 2025 10:27
@tersec tersec merged commit 9f75e08 into master Sep 13, 2025
20 checks passed
@tersec tersec deleted the update-nimbus-eth2 branch September 13, 2025 15:51
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.

3 participants