-
Notifications
You must be signed in to change notification settings - Fork 251
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.
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
Initial commit for eip-7732 on fulu #6768
base: fulu
Are you sure you want to change the base?
Conversation
Tomi-3-0
commented
Dec 18, 2024
- Execution payloads are removed from the beacon block structure.
- Slot intervals are increased to 4 to align with EIP-7732 timing changes.
- Refactored validators and consensus object pools for compatibility.
This commit introduces changes to support EIP-7732 in the Fulu fork: - Execution payloads are removed from the beacon block structure. - Slot intervals are increased to 4 to align with EIP-7732 timing changes. - Refactored validators and consensus object pools for compatibility.
operations across blob quarantine, gossip validation, sync, and validator flows
b97a1d5
to
7b144dc
Compare
|
Initially omitted adding Thanks. |
7b144dc
to
3e016d2
Compare
|
|
Why does |
That To run all the tests, |
e47ceb1
to
0e9e2c8
Compare
Causing Electra fork test failures on macos and Windows:
|
Thanks What's the best way to reproduce this error locally? |
I was able to do so by cloning your repro/branch ( The most targeted way to reproduce this locally is either
for the mainnet version or
for the minimal version.
|
Thanks I had to download the test vectors manually using the provided script: cd vendor/nim-eth2-scenarios && ./download_test_vectors.sh
Thanks I had to download the test vectors manually: cd vendor/nim-eth2-scenarios && ./download_test_vectors.sh |
Ok, that works, but |
Hey @Tomi-3-0, can you send me an invite to your repo? |
91c62a3
to
ac49cfe
Compare
Hi @shyam-patel-kira , sent an invite |
to reproduce,
should suffice. |
Since |
Well, for me, they do pick up the issue. I did:
Both |
Thank you. I will go through this again |
payload signature tests, update toSignedBlindedBeaconBlock
ac49cfe
to
960c790
Compare
|
quarantine[].addUnviable(signed_beacon_block.root) | ||
return dag.checkedReject( | ||
"BeaconBlock: mismatched execution payload timestamp") | ||
when signed_beacon_block is fulu.SignedBeaconBlock: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we discarding a signed_beacon_block
from fulu here? If a block belongs to fulu it shouldn't enter validateBeaconBlockBellatrix
, right? Or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validateBeaconBlockBellatrix
is a generic template to allow code reuse while maintaining type safety, accepting multiple SignedBeaconBlock types from Bellatrix upwards.
Since fulu blocks have had the execution_payload
removed from their beaconblocks, we need to handle them explicitly with when signed_beacon_block is fulu.SignedBeaconBlock: discard
to skip the timestamp validation that depends on the execution payload
Hey @Tomi-3-0, can we avoid force pushing commit, now even a 1-commit change PR shows a lot of commits on my end. |