Skip to content

Conversation

jtraglia
Copy link
Member

@jtraglia jtraglia mentioned this pull request Oct 30, 2024
20 tasks
Copy link
Member

@ppopth ppopth left a comment

Choose a reason for hiding this comment

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

Looks good after all

Comment on lines +145 to +147
| `DEPOSIT_REQUEST_TYPE` | `Bytes1('0x00')` |
| `WITHDRAWAL_REQUEST_TYPE` | `Bytes1('0x01')` |
| `CONSOLIDATION_REQUEST_TYPE` | `Bytes1('0x02')` |
Copy link
Contributor

Choose a reason for hiding this comment

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

For CL specs, they should be named with REQUEST_TYPE_*, but I figured out they are EIP constants... 😭

@lightclient is it too late to change?

etan-status and others added 4 commits November 21, 2024 23:58
Request hash is not considered in `compute_el_block_hash`, have to use
one of the other overloads for this to work.
Fix block hash computation for deposit transition tests
Request hash is not considered in `compute_el_block_hash`, have to use
one of the other overloads for this to work.
Copy link
Contributor

@etan-status etan-status left a comment

Choose a reason for hiding this comment

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

Cross-checked the block hashes in the test vectors against Nimbus. LGTM ✅

@jtraglia jtraglia merged commit 148ccca into ethereum:dev Nov 22, 2024
26 checks passed
@jtraglia jtraglia deleted the exclude-empty-requests branch November 22, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants